Skip to content

fix(codex): settle unfinished web searches when turns end - #32177

Open
Sidreyas wants to merge 1 commit into
pingdotgg:mainfrom
Sidreyas:sidreyas/fix-22248-settle-unfinished-searches
Open

Sidreyas wants to merge 1 commit into
pingdotgg:mainfrom
Sidreyas:sidreyas/fix-22248-settle-unfinished-searches

Conversation

@Sidreyas

@Sidreyas Sidreyas commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #22248.

If Codex emits item/started(webSearch) but no corresponding item/completed, a terminal turn leaves the search item and its execution node stored as running, without a completion timestamp. A deliberately omitted native notification reproduces this on the current branch base; the original October 3 incident's missing frames still leave its cause unknown.

Change

Track unfinished searches in their native turn context and remove them when their own completion arrives. At turn termination, close the remaining search items and nodes using cancelled for an otherwise completed turn, or the turn's failed/interrupted status. Preserve their identity, query, start time, and ordinal, and leave completed searches unchanged. A completed turn does not imply that an unobserved search succeeded.

Scope and approval

This is a focused fix for an inconsistent provider lifecycle state, submitted under the small, obvious bug-fix exception. It extends the existing terminal cleanup and test table, with no settings, model-facing text, client UI, or T3 wire-contract changes. Web, desktop, mobile, and remote connections use the same server projections; only the Codex adapter changes. Historical stored rows are not migrated or repaired.

Verification

Windows, Node.js 26.12.1, checkout-local locked dependencies, source base df13391fb1b321b810cfab0e721ca858c65b931f.

  • Fail first: production adapter verified identical to the base. From apps/server, vp test run src/orchestration-v2/Adapters/CodexAdapterV2.test.ts -t 'settles unfinished': 4 search cases failed, 3 existing compaction cases passed. Searches emitted only [running] instead of [running, cancelled/failed/interrupted].
  • Fixed focused run: the same command passed all 7 cases, including failed Stop acknowledgement and completed-search preservation.
  • Complete selected tests: from apps/server, vp test run --passWithNoTests src/orchestration-v2/Adapters/CodexAdapterV2.test.ts src/provider/AgentContextBudget.test.ts: 151 passed, 2 files. The unchanged owning-package config avoids the previously reproduced root-workspace Vitest startup stall in this Windows checkout.

The mandatory vp run check:changed --concurrency 1 was attempted and stopped after hanging in its pre-check Windows junction scan, tracked by #32023. The wrapper did not pass. Every selected step was derived from scripts/check-changed.ts and .github/scripts/ci-changes.cjs for this two-file diff, then run individually with identical scope/settings:

Selected check Result
Format check and lint, both changed files Passed
Both Knip tasks, exact repository arguments Both passed
Server GraphQL generation, --check Passed
Typechecks: t3, @t3tools/evals, @t3tools/scripts 3/3 passed
Adapter neighbor and context-budget cross-file tests 151 passed

No unused-patch, React compiler, doc-reader, or Node test step was selected. No cache or test configuration was altered, no check was weakened, and no repo-wide test suite was run.

Exact equivalent commands from the repository root
vp fmt --check --no-error-on-unmatched-pattern apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts
vp lint --no-error-on-unmatched-pattern apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts
vp exec knip --include files,dependencies --no-config-hints
vp exec knip --workspace apps/server --workspace apps/desktop --workspace apps/web --workspace packages/client-runtime --workspace packages/contracts --workspace packages/effect-acp --workspace packages/effect-codex-app-server --workspace packages/shared --workspace packages/ssh --workspace packages/tailscale --workspace packages/provider-core --workspace packages/provider-acp --workspace packages/provider-acp-registry --workspace packages/provider-testing --workspace packages/provider-pi --workspace packages/provider-muse --workspace packages/provider-opencode --workspace packages/provider-cursor --workspace packages/provider-fx --workspace packages/provider-grok --workspace packages/provider-kiro --workspace packages/secret-provider-core --workspace packages/secret-provider-definitions --workspace packages/secret-provider-onepassword --workspace packages/source-control-core --workspace packages/source-control-definitions --workspace packages/source-control-azure-devops --workspace packages/source-control-bitbucket --workspace packages/source-control-forgejo --workspace packages/source-control-gitcafe --workspace packages/source-control-github --workspace packages/source-control-gitlab --workspace packages/source-control-testing --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints
vp run --filter ./apps/server generate --check
vp run --cache --parallel --concurrency-limit 1 --filter @t3tools/evals --filter @t3tools/scripts --filter t3 typecheck

The selected Vitest command and owning-package directory are given above.

Integrated application verification: the production CLI and authenticated MCP/HTTP called the real server, orchestrator, persistence, Codex adapter, client and protocol decoder. The external Codex boundary was a deterministic JSONL subprocess; its model id was fixture metadata and made no model/API requests. Baseline and fixed phases each used a fresh disposable home, provider home, workspace and SQLite database. For Stop, the driver awaited the persisted running search, issued t3_thread_interrupt over MCP, observed the native turn/interrupt request, and compared the completed search's stored payload byte-for-byte before and after.

Commands used by the external driver (temporary paths and ids elided):

node <fixture-dir>/run.mjs baseline
node <fixture-dir>/run.mjs fixed
# Each phase starts the production CLI entry point through a small signal-handling launcher:
node <fixture-dir>/server.mjs serve --base-dir <isolated-home> --mode web --host 127.0.0.1 --port 59548 --no-browser
node apps/server/src/bin.ts project add --base-dir <isolated-home> <workspace> --title "Codex search fixture"
node apps/server/src/bin.ts thread start <workspace> E2E_<case> --provider codex-search-e2e --model gpt-5.4 --title "<phase> <case>" --wait --json --base-dir <isolated-home>
# STOP: authenticated MCP t3_thread_interrupt {threadId: <test-thread>}

Sanitized runtime transcript, read from persisted run/item/node projections:

baseline OMIT:    run=completed   search=running      node=running      completedAt=null
baseline CONTROL: run=completed   both searches and nodes=completed
fixed OMIT:       run=completed   search=cancelled    node=cancelled    completedAt=set
fixed FAILED:     run=failed      search=failed       node=failed       completedAt=set
fixed INTERRUPTED:run=interrupted search=interrupted  node=interrupted  completedAt=set
fixed MCP STOP:   run=interrupted search=interrupted  node=interrupted  completedAt=set
fixed CONTROL:    run=completed   both searches and nodes=completed
Every pre-completed search remains completed; its full stored payload is unchanged across MCP Stop.
All assertions passed; all owned server processes stopped.

Limitations: no live Codex CLI/model, naturally occurring missing notification, client UI, or historical-data repair was tested or claimed. The fixture establishes the application behavior for the specified protocol sequence; it does not identify the cause of the historical report. Raw server logs and disposable authentication material are excluded from this evidence.

Implemented with OpenAI GPT-6 via the Codex harness.

Track started searches until their completion and close remaining search items and execution nodes with a non-success terminal status. Preserve completed searches.

Implemented with OpenAI GPT-6 via the Codex harness.
Copilot AI balanced review requested due to automatic review settings November 6, 2026 20:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Nov 6, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 1177e1d

Macroscope's review found this PR approvable — Unfinished Codex web searches now receive a terminal status when their turn ends instead of remaining marked as running. The change is confined to search-status reporting and does not alter external search execution or product defaults.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The Codex adapter now tracks running web-search items and settles any still running when a turn ends. Tests cover the projected statuses for completed, failed, interrupted, and stopped turns.

Changes: Web-search lifecycle

Layer / File(s) Summary
Track and project web-search status
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts
Turn contexts track running web searches. Start and completion events project the corresponding lifecycle status, and projections use the supplied status and completion time.
Finalize unfinished searches
apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts, apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
When a turn ends, remaining searches receive cancelled, failed, or interrupted status based on the turn outcome. Tests check these outcomes and terminal status.

Estimated code review effort: 2 (Simple) | ~15 minutes

Severity of issue fixed: Medium

Suggested reviewers: luke2x, juliusmarminge

Merge Risk: Minimal · up to 1177e

The web-search settlement change has no identified merge-blocking issue. The reported pre-check hang still limits validation, but it is not attributed to this change.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly describes the Codex web-search settlement change.
Description check Passed The description covers the problem, change, scope, and verification. It reports that the mandatory check wrapper did not pass and states the test limitations.
Linked Issues check Passed [#22248] The Codex adapter tracks unfinished searches and settles their items and nodes at turn termination. It marks an unobserved search cancelled after a completed turn, or failed or interrupted wh…
Out of Scope Changes check Passed [#22248] The changes are limited to Codex search lifecycle handling and its tests. Both support the issue objective.
Full details: Linked Issues check

Explanation

[#22248] The Codex adapter tracks unfinished searches and settles their items and nodes at turn termination. It marks an unobserved search cancelled after a completed turn, or failed or interrupted when the turn has that status. Completed searches remain unchanged. Automated tests cover these outcomes and Stop.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between bdb8d5d and 1177e1d.

Files selected for processing: 2

  • apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.test.ts
  • apps/server/src/orchestration-v2/Adapters/CodexAdapterV2.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codex web search items can remain running inside completed runs

2 participants