Fix worktree-bound Codex app-server startup#568
Conversation
Fence root signaling before the sole destructive reap, retry transient wait failures, and keep TERM-to-KILL cleanup bounded. Align default launch cwd, fully retire failed controller state, and canonicalize primary repository promotion with deterministic regression coverage.
Persistent kernel wait failures now retry with exponential backoff from 10 ms to a 1 s ceiling instead of a fixed 10 ms loop, while the observer retains sole destructive-reap ownership until a terminal result.
Five teardown routes each maintained their own checklist of tab-scoped task cancellations and controller-runtime field clears; new controller-scoped resources had to be added to every list. The shared mechanical core now lives in cancelCodexTabScopedControllerTasks and clearCodexControllerRuntimeState while semantic decisions (reconnect marking, queue abandonment, interaction settlement, shutdown sequencing, tracking waits) stay caller-owned.
Both termination origins duplicated the first-claim protocol (same- generation task reuse, terminal-cause claim, task publication) with small drift-prone differences. installTransportTermination now owns the protocol; origins supply only their validation guard, post-claim capture, and completion strategy.
The controller now retains CodexRuntimeWorkspacePaths as one property so the launch/execution roles cannot drift through partial initialization. The observer's signaling flag is named for what it guards (rootSignalingClosed) since wait-failure retries reopen it before any reap. Both view-model initializers derive their session projection closures from one factory.
The post-claim capture now flows through a generic Sendable return value instead of a mutable unchecked box, so the compiler enforces the capture-before-publication handoff.
kevent NOTE_EXIT delivery proved non-contractual under saturation (a 64-live/80-early-exit harness stranded one child in roughly a quarter of runs). Every registration now keeps a 500 ms non-destructive fallback probe that routes a missed notification through the same token-guarded sole-reap path; the timer is cancelled at completion.
There was a problem hiding this comment.
Pull request overview
This PR fixes worktree-bound Codex Agent Mode startup by separating the Codex app-server process launch directory from the session’s execution directory (thread/turn/sandbox), while tightening worktree repository selection and improving early-exit diagnostics (bounded stderr + structured exit status) with a single destructive child reaper.
Changes:
- Introduces
CodexRuntimeWorkspacePaths(launch vs execution) and updates Codex controller/coordinator/view model plumbing to key controller reuse on the full path pair. - Refactors Codex app-server transport lifecycle to preserve typed early-exit evidence (
processExited) with bounded stderr capture and observer-owned reaping/teardown. - Adjusts worktree selector repository discovery/defaulting to the declared primary workspace root and adds broad regression coverage.
Maintainer-guidance check
- User impact and invariant: Worktree-bound Codex sessions must start reliably; app-server launch cwd must be stable (logical root) while execution remains bound to the selected worktree.
- Root-cause confidence: confirmed (launch/execution cwd coupling was the reported failure axis; PR cleanly splits them and adds regression coverage).
- Authority: Codex CLI/app-server process launch configuration + Agent Mode worktree binding projection are the sources of truth.
- State-safety risks: medium (provider/controller replacement + teardown correctness); mitigated by typed failure evidence, sole-reaper enforcement, and added lifecycle tests.
- Scale and observability risks: moderate (new per-child observation registry); bounded probes + nonblocking publication queue help, but correctness/compile safety is critical.
- Recommended scope: now (fits acceptance criteria for #554).
- Validation boundary: Agent Mode worktree binding projection + Codex app-server spawn/termination lifecycle (tests added across both).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/RepoPromptTests/MCP/ToolCatalogSnapshotTests.swift | Updates tool catalog snapshot hashes after schema changes. |
| Tests/RepoPromptTests/MCP/AgentRunWorktreeStartTests.swift | Adds regression tests for Codex workspace path projection and worktree selector scoping; adjusts Codex controller fakes. |
| Tests/RepoPromptTests/Helpers/CodexSessionControllerTestDefaults.swift | Adds passive-stub defaults to simplify controller test doubles. |
| Tests/RepoPromptTests/AI/ProcessTerminationExitStatusTests.swift | Expands tests around sole-reaper semantics, teardown ordering, and retry behavior. |
| Tests/RepoPromptTests/AI/CodexNativeSessionControllerTurnDispatchTests.swift | Updates controller construction to use workspacePaths pair. |
| Tests/RepoPromptTests/AgentMode/Codex/CodexNativeSessionControllerGoalConfigTests.swift | Updates config field names and adds tests for launch directory updates and worktree path separation semantics. |
| Tests/RepoPromptTests/AgentMode/Codex/CodexNativeSessionControllerEventRecoveryTests.swift | Updates controller construction to use workspacePaths. |
| Tests/RepoPromptTests/AgentMode/Codex/CodexMCPBootstrapReadinessTests.swift | Updates controller construction/config to use launch directory vs runtime workspace paths. |
| Tests/RepoPromptTests/AgentMode/Codex/CodexFallbackFIFOTests.swift | Updates controller construction to use workspacePaths. |
| Tests/RepoPromptTests/AgentMode/Codex/CodexAppServerClientProcessExitTests.swift | New, comprehensive tests for early-exit evidence, precedence rules, and lifecycle/termination races. |
| Sources/RepoPrompt/Infrastructure/VCS/GitService.swift | Updates to the new beforeReap callback name/semantics for child reaping. |
| Sources/RepoPrompt/Infrastructure/Process/ProcessTermination.swift | Adds process-wide child observation registry + observer-oriented termination APIs; updates reaping API surface. |
| Sources/RepoPrompt/Infrastructure/Process/CLIProcessConfiguration.swift | Centralizes default working directory resolution. |
| Sources/RepoPrompt/Infrastructure/Process/ChildProcessExitObserver.swift | New sole-reaper observer abstraction with bounded retries and signaling window closure. |
| Sources/RepoPrompt/Infrastructure/MCP/WindowTools/MCPAgentControlToolProvider.swift | Clarifies worktree_repo_root default behavior in tool schema. |
| Sources/RepoPrompt/Infrastructure/MCP/Agent/AgentMCPStartWorktreeCoordinator.swift | Scopes worktree resolution to the selected repo; defaults discovery to primary workspace root deterministically. |
| Sources/RepoPrompt/Infrastructure/AI/Providers/Codex/CodexCLIProvider.swift | Improves surfaced error message for typed processExited failures. |
| Sources/RepoPrompt/Infrastructure/AI/Providers/Codex/AppServer/CodexProcessStderrCapture.swift | New bounded stderr tail capture used for typed exit evidence. |
| Sources/RepoPrompt/Infrastructure/AI/Providers/Codex/AppServer/CodexNativeSessionController.swift | Uses runtime launch/execution pair; routes launch cwd to client and execution cwd to thread/turn/sandbox. |
| Sources/RepoPrompt/Infrastructure/AI/Providers/Codex/AppServer/CodexAppServerClient.swift | Adds typed exit evidence, observer-owned termination, and transport-termination arbitration; separates launch directory config. |
| Sources/RepoPrompt/Features/Diagnostics/AgentMode/Stress/AgentChatStressHarness.swift | Updates Codex controller construction to use workspacePaths. |
| Sources/RepoPrompt/Features/AgentMode/ViewModels/AgentModeViewModel+TabSession.swift | Replaces scalar Codex workspace path tracking with launch/execution pair tracking. |
| Sources/RepoPrompt/Features/AgentMode/ViewModels/AgentModeViewModel.swift | Adds shared workspace projection providers and exposes runtime workspace paths for tests. |
| Sources/RepoPrompt/Features/AgentMode/Services/AgentWorktreeRuntimeWorkspaceResolver.swift | Adds Codex-specific projection that validates launch root and keeps execution bound to worktree. |
| Sources/RepoPrompt/Features/AgentMode/Runtime/Codex/CodexRuntimeWorkspacePaths.swift | New type + error for Codex launch/execution directory modeling. |
| Sources/RepoPrompt/Features/AgentMode/Runtime/Codex/CodexAgentModeCoordinator.swift | Keys controller replacement on workspace path pair and consolidates teardown paths for new controller-scoped state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Baseline v1.0.29 packaged failureBoth 
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bfaec2b9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ee-app-server-start # Conflicts: # Sources/RepoPrompt/Infrastructure/AI/Providers/Codex/AppServer/CodexNativeSessionController.swift # Tests/RepoPromptTests/MCP/ToolCatalogSnapshotTests.swift
|
@mplibunao thank you always for your amazing PRs. We're actually going to be bundling the app server and work on general reliability that will hopefully solve most issues with codex involving self-installed clis (like PATH stuff). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 33002fbdfa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@baron Bundling the app-server sounds like a great direction — that should take care of the self-installed CLI/PATH class nicely, and I'm totally fine if this PR ends up covered or superseded by that work. One data point in case it's useful for the bundling design: in the worktree-bound failures here, binary resolution was actually fine (the same binary starts cleanly from the same directory when run manually) — the trigger was launching the app-server with the worktree as its cwd, where repo-local tooling reacts to the directory. Something to keep in mind for wherever the bundled server gets launched from. Meanwhile I'll keep the branch synced and green in case it's still useful. |
|
@mplibunao you make a good point I think this PR merits a separate look to see if we should incorporate it before bundling as it addresses a very specific deficiency. |
Summary
Fixes #554
Validation
AgentRunWorktreeStartTests: 47/47CodexNativeSessionControllerGoalConfigTests: 19/19CodexNativeSessionControllerTurnDispatchTests: 9/9f65e925d,dirty: false, with deep strict codesign verificationPackaged evidence
Using the same
wizyemm-infraworkspace, linked worktree, selector target, and Codex model:worktree_idand@branch:demo/rpce-554-worktree-start-evidencewithCodex native start failed: Codex app-server process is not running./Users/mp/Projects/wizy/wizyemm/wizyemm-infrademo/rpce-554-worktree-start-evidenceworktree and made no editsThe accepted baseline failure screenshot is attached in a PR comment. macOS Screen Recording authorization did not survive the ad-hoc signature change, so no patched screenshot is claimed; the patched result is backed by RepoPrompt session results plus the independent process-cwd observations.
Update 2026-07-22
main(post-Fix Codex app-server tool override policy #603) into the branch: the launch/execution directory split is unchanged; the experimental-steering plumbing removed by Fix Codex app-server tool override policy #603 is gone from this branch too; MCP tool-catalog snapshot hashes refreshed for the merged catalog.app shard 4failure (CodemapAutomaticSelectionColdTests.testColdAutomaticSelectionNeverPlansSameNamedDefinitionFromAnotherRoot) no longer reproduces after the CodeMap core rework onmain— the suite passes 8/8 post-merge.AgentRunWorktreeStartTests47/47,CodexNativeSessionControllerGoalConfigTests19/19,ToolCatalogSnapshotTests5/5, SwiftFormat + strict SwiftLint clean.agent_runstart targeting a repo with cwd-sensitive tooling (mise.toml+ repo-local.codexhooks) fails withCodex app-server process is not running— persisted session records show the failure lands ~250 ms after run start, before any Codex thread exists. After including this branch and rebuilding, the identical probe passes; unbound starts and worktree starts on a repo without such tooling pass in both builds as controls.