Repository navigation
Add local execution with per-turn write policies - #5501
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsBlocking findings (1)
Advisory findings (4)
ℹ️ 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 759b4ec016
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 759b4ec016
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Changes requested at 759b4ec.
Two independently confirmed compatibility regressions need correction. Existing inline threads already cover both, so I am not duplicating them:
- Preserve moved implicit remote workspaces on durable resume: the exact receipt comparison includes cwd and roots even without strict writes, unlike the portable restore contract. Keep explicit write/network policy validation while preserving legacy implicit workspace relocation.
- Preserve explicit host MCP credential grants: credential filtering runs before the location check, so adding local execution alters a separately configured host MCP process. Limit executor filtering to environment-located servers.
Please add focused regression coverage for both paths. Reviewed native root admission, network preservation, restore ordering, startup redaction, retained denial records, and layer dependencies. Hosted lint/docs passed; Linux/macOS native CI remains running. The inherited-shell-environment advisory needs stronger boundary evidence before I would treat it as a separate blocker.
Static review only; no candidate code or tests run. Leave unmerged as requested.
9b1faae to
c516553
Compare
759b4ec to
7ceb5da
Compare
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Changes requested at 7ceb5da.
The two previously identified compatibility regressions remain: implicit remote workspace relocation on durable resume and explicit credential grants for host MCP processes. Exact blob comparison confirms both affected production files are unchanged. Please address these existing threads; no duplicate inline findings added. The new public receipt projection correctly omits the internal permission profile without changing retained native policy checks; no new regression identified.
Static review only; no candidate code or tests run. Fresh hosted CI remains pending. Leave unmerged as requested.
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 7ceb5da33f
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
c516553 to
f36abef
Compare
7ceb5da to
99f4b1c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99f4b1c837
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The previous executor blockers are addressed: implicit remote workspace relocation preserves the other authority checks, and explicitly granted host MCP credentials are retained. Reviewed the additional local sandbox restrictions; no further actionable finding in this layer.
Static review only; no candidate code or tests executed. Hosted CI: pending: native-linux / Native package (ubuntu-24.04), native-linux / Native package (ubuntu-24.04), native-macos / Native package (macos-latest), native-macos / Native package (macos-latest); no failed checks observed.
99f4b1c to
82035ad
Compare
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed the updated executor-policy handling. Compaction preserves the canonical applied roots and checks replacement authority before MCP startup or source retirement. Model-only reopening permits only recognized implicit remote receipts; malformed and unknown authority fail closed. No actionable finding in these changes.
Static review only; no candidate code or tests executed. Hosted CI: pending: native-linux / Native package (ubuntu-24.04), native-macos / Native package (macos-latest); no failed checks observed.
|
@codex review |
1770bd7 to
d2fb57c
Compare
954eb25 to
a5ae33d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5ae33d0cf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The shared physical write-root and storage protections are sound: filesystem identities cover alternate paths, checklist aliases and hard links are checked, and private runtime symlinks fail closed. Disjoint storage and write grants remain supported. The executor startup fixture also removes the parent-selected port race. Reconciliation behavior is unchanged in the actual old/new blob diff.
Coordinator and independent static reviews of actual old/new file contents found no actionable issue. Hosted CI on this new head remains pending; prior-head results do not qualify it. No candidate code or local tests executed.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-approved this refreshed commit after verifying its complete Git tree is identical to the previously reviewed commit. No source, test, or metadata changes. Existing feedback was rechecked; no new actionable finding. Coordinator and independent static review. Hosted native checks are running for the new commit; no candidate code or local tests executed.
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed 949bea6. The Linux long-path helper scan safely falls back to component-wise directory opens, rejects descendant symlinks, preserves distinct mount views, and keeps the directory descriptor alive through entry inspection. The patch and manifest propagation are consistent with the previously reviewed stack. No new actionable findings. Hosted CI remains in progress; this was static review only, with no candidate code or local tests executed.
Review stack: #5504 (CI) > #5499 (turn controls) > #5500 (receipts) > #5501 (executor) > #5502 (recovery) > #5503 (example) > #5495 (docs) > #5509 (automatic compaction during cancellation). Combined reference: #5494.
Requirements ·
REQ732for executor configuration and startup,REQ733for owned cancellation,REQ734for execution evidence, andREQ736for isolation.Summary
A coding worker needs to run commands under a bounded write policy and know when its processes have stopped. This layer adds local execution on supported Linux and macOS hosts. The first commit enforces the policy in Core. The second exposes it through the SDK and tests the public interface.
When a result callback finishes during cancellation, the stopped turn cannot send an ordinary HTTP or WebSocket follow-up to the model.
The worker example uses these guarantees for artifact acceptance (
REQ780) and workspace isolation (REQ781). Managed login and billing (REQ731) are excluded.The native build applies the Core patches in the order listed in
packages/codex/patches/series.json. The Linux shutdown regression exercises the resulting branch that joins all managed local commands.On macOS, shutdown waits for the starting wrapper to report its kernel process identity or for its control channel to finish. It can then resume a paused wrapper. Success requires both the drain acknowledgment and the wrapper's exit within the shutdown deadline.
A session saved before its first model request can reopen without a sampling receipt. After sampling, an explicit network policy or strict write grant needs recorded evidence. The SDK checks the owner of surviving modern receipt fields before allowing a legacy reopen. Older implicit remote sessions can reopen when every retained Core context matches the original default read-only or single-workspace policy. If all modern receipt fields were erased, a strict session that recorded the same Core context as an older implicit session can be indistinguishable from it. The caller can reopen that history only under a compatible implicit policy.
A writable root can expose private session history in either direction. It can contain a relocated sessions directory, or it can sit inside one and receive the next rollout. On Linux and macOS, Core compares directory identities and their ancestors so alternate spellings reach the same check. Disjoint storage relocations remain supported. On open, resume, and policy replacement, the SDK also checks the selected rollout file and the files directly inside its private checklist and budget directories. The SDK refuses symbolic links inside its temporary runtime home.
Test plan
In an unprivileged Linux guest, the installed Python wheel passed 12 storage checks, including direct and relocated SQLite aliases, writable-root replacement and reopen. Sixteen installed Linux durable receipt cases passed, including corrupted identities, strict policies, and compatible legacy reopening. All 165 Rust binding tests passed. Five shared control-channel tests passed after the queued-identity regression failed on the previous source. They exercised the socket protocol with a simulated failed signal on Linux.
The installed Linux wheel passed 16 cancellation and lifecycle cases. Five unprivileged Linux guest scenarios passed, including pre-launch cancellation, policy restore and two-worker recovery. The wrapper passed service-delegation, terminal, filesystem and network tests on macOS 15 and 26. macOS 15 run · macOS 26 run · Installed Python package run
An installed Linux wheel passed all 12 session-storage tests, including refusing a compressed rollout before writing to it and reopening a disjoint relocated rollout. The Rust compressed-rollout test also passed.
A hosted child-approval test read the command output file while the command was still running. The fixture now polls the approved process until it exits successfully. A forced delay reproduced the failure before the change. Both delayed policy cases and all three ordinary controls passed with the installed wheel.
A sandbox command corrupted a saved checklist through a hard link before the fix. The checklist and budget correction passed seven installed Linux cases. Separately, the old native wheel created a real rollout inside a writable subdirectory on a case-insensitive volume. The final wheel passed all four overlap cases on case-insensitive ext4 and five controls on case-sensitive ext4, including resuming a disjoint rollout.
Issue number
None linked. Owning requirements database IDs are listed above.
Checks
AGENTS.md, including.agents/skills/code-change-verification/scripts/run.shwhen requiredAGENTS.md