Repository navigation
Give Codex a working sandbox for workspace tasks on Windows - #135
Conversation
Codex sandboxes commands on Windows only when a Windows sandbox is configured. Without one, workspace-write resolved to read-only, so Papilot refused every workspace task with "Codex could not apply workspace permissions", and Codex's default sandbox could not start a command. After it starts, Papilot reads Codex's effective config. On Windows without windows.sandbox set, its threads use the unelevated sandbox, which needs no administrator setup: on a Windows 11 machine it let a command write in the workspace and denied a write to the home folder. A sandbox the user configured is kept. The real Codex permission tests now run on Windows too.
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 3836af0985
ℹ️ 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".
| const effective = await this.request('config/read', {}).catch(() => null); | ||
| if (generation !== this.generation) throw new Error('Codex is not connected.'); |
There was a problem hiding this comment.
On Windows, if the app-server exits while config/read is pending, this catch swallows the disconnection error. The exit handler clears child and ready, but does not change generation, so initialization then sets ready = true despite having no child. Subsequent start() calls return immediately, meaning Papilot's Reconnect action cannot recover. I reproduced this with a stand-in app-server that answers initialize and exits upon config/read. Only suppress configuration-API errors, or verify that the launch's child is still connected before marking it ready.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 13fda68: only an error Codex answers (RpcResponseError) is ignored; a disconnect fails the launch, and it checks the child is still connected before marking ready. A stand-in app-server that exits on config/read now makes start() reject with ready false.
Suppressing every config/read error also swallowed the disconnect, so the launch marked Codex ready without a child and Reconnect could not recover. Only an error Codex answers is ignored now, and the launch checks that its child is still connected.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
On Windows, Codex now reads its config when it starts. The fake server in the RPC test ignored that request, so the launch waited for its 60 s timeout; like Codex, it now answers unknown methods with an error.
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Codex's Windows sandbox can hold the folder for a moment after Codex closes, which failed one Windows CI run with EPERM during cleanup.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
What and why
On Windows, every Codex task with Workspace access failed to start with "Codex could not apply workspace permissions." Found when #131 put the real Codex permission tests on Windows CI.
Codex sandboxes commands on Windows only when a Windows sandbox is configured (
[windows] sandbox). Without one,workspace-writeresolves toreadOnly, and Papilot correctly refuses to run a workspace task with the wrong policy. Checked on CI and on a Windows 11 machine:windows.sandboxcodex sandbox(workspace-write)readOnlytimed out … connecting runner pipe-in)unelevatedworkspaceWriteelevatedworkspaceWriteFix (
src/server/codex.ts): after Codex starts, Papilot reads its effective config withconfig/read. On Windows withoutwindows.sandboxset, everythread/startandthread/resumecarrieswindows.sandbox = "unelevated", which needs no administrator setup. A sandbox the user configured is kept; other systems are unchanged; ifconfig/readis unavailable, nothing changes.Tests: the six real Codex permission tests are no longer skipped on Windows, and a new test checks, with the real app-server, that threads get the unelevated sandbox on Windows, keep a user-configured
elevatedone, and are unchanged on Linux.How it was checked
tests/codex-permissions.test.tson macOS (8 passed) and on a Windows 11 machine (8 passed, Codex 0.164 from the dev dependency)codex sandboxwithwindows.sandbox="unelevated"on that machine: inside write succeeds, write to%USERPROFILE%deniednpm run typecheck