fix(codex): start sidecars in requested cwd#309
fix(codex): start sidecars in requested cwd#309danshapiro wants to merge 1 commit intoexact-durable-session-contractfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1af9510bb4
ℹ️ 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".
| '--listen', | ||
| wsUrl, | ||
| ], { | ||
| ...(this.cwd ? { cwd: this.cwd } : {}), |
There was a problem hiding this comment.
Handle spawn errors when applying requested sidecar cwd
Passing cwd into spawn allows startup to fail with ENOENT when the requested directory is missing (for example, a stale/deleted project path), and this runtime still has no child.on('error') handler. In Node, that emits an unhandled error event and crashes the process instead of surfacing a normal launch failure/retry; this became reachable with the new cwd threading in this commit.
Useful? React with 👍 / 👎.
Summary
Root cause
The sidecar-owned Codex app-server was always spawned in Freshell's own cwd instead of the requested launch directory. That caused Codex sessions to boot against the wrong project context, which in turn surfaced the trust warning and incorrect startup directory behavior.
Testing
npm run test:vitest -- --config vitest.server.config.ts test/unit/server/coding-cli/codex-app-server/runtime.test.ts test/unit/server/coding-cli/codex-app-server/launch-planner.test.ts test/integration/server/codex-session-flow.test.ts test/server/ws-terminal-create-reuse-running-codex.test.ts