fix(daemon): replace deprecated --full-auto with --sandbox workspace-write#449
fix(daemon): replace deprecated --full-auto with --sandbox workspace-write#449nmsn wants to merge 3 commits intonexu-io:mainfrom
Conversation
Setting overflow-x: auto implicitly sets overflow-y to auto per CSS spec. With a fixed height of 44px, this causes a vertical scrollbar when content overflows by even 1px. Explicitly set overflow-y: hidden.
…write Codex CLI v0.128.0 deprecated the --full-auto flag. Replace it with --sandbox workspace-write in the agent argv construction, tests, spec, and all README locale variants.
|
Hi @nmsn! 🎉 Thanks for making open-design better! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36934768ca
ℹ️ 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".
| 'exec', | ||
| '--json', | ||
| '--skip-git-repo-check', | ||
| '--full-auto', | ||
| '--sandbox', 'workspace-write', |
There was a problem hiding this comment.
Fix Codex args prefix assertion length
Update the Codex argv assertion to account for the extra workspace-write token: this test now compares args.slice(0, 8) against a 9-element expected array, so it will always fail even when buildArgs is correct. Because this regression is in agents.test.ts, it can block CI and hide real adapter regressions until the slice bound is corrected (e.g., include 9 elements or assert the prefix differently).
Useful? React with 👍 / 👎.
lefarcen
left a comment
There was a problem hiding this comment.
Hi @nmsn! The core Codex CLI deprecation fix looks solid — you've correctly replaced --full-auto with --sandbox workspace-write across all the right places (daemon, tests, spec, 9 README variants). Nice thorough coverage! 🎯
I have two minor suggestions (non-blocking):
P2 — Scope drift: The apps/web/src/index.css change (adding overflow-y: hidden) is unrelated to the Codex CLI flag update. Could you move that to a separate PR? Keeping each PR focused on one logical change makes reviews faster and git history cleaner.
P3 — Backward compatibility note: For users on older Codex CLI versions (<0.128) that don't have --sandbox, the daemon will fail to spawn the agent. Consider adding a brief note in the spec or README about the minimum Codex CLI version requirement (v0.128+). Not a blocker, but would help users troubleshoot if they hit this.
Overall this fixes the issue reported in #438. Once the index.css change is removed, this is ready to merge from my side.
| border-bottom: 1px solid var(--border); | ||
| background: var(--bg-panel); | ||
| overflow-x: auto; | ||
| overflow-y: hidden; |
There was a problem hiding this comment.
P2 This CSS change (overflow-y: hidden) is unrelated to the Codex CLI flag deprecation. Please remove it from this PR and submit it separately if it addresses a different issue.
| - `--json`: structured event output | ||
| - `--skip-git-repo-check`: allows running in a temporary working directory | ||
| - `--full-auto`: non-interactive automatic execution | ||
| - `--sandbox workspace-write`: non-interactive automatic execution with workspace write access (replaces deprecated `--full-auto`) |
There was a problem hiding this comment.
P3 Consider mentioning the minimum Codex CLI version requirement here (v0.128+). Users on older versions will get a failure when the daemon tries to spawn the agent with the new --sandbox flag. Something like:
"(requires Codex CLI v0.128+; older versions do not support --sandbox and will fail with an unrecognized option error)"
Not a blocker, but would help users troubleshoot.
mrcfps
left a comment
There was a problem hiding this comment.
@nmsn thanks for updating the Codex CLI flag path and keeping the docs/tests aligned. I found one blocking test assertion issue that should be quick to fix before merge. 🙂
Generated by Looper 0.5.2 · runner=reviewer · agent=opencode| 'exec', | ||
| '--json', | ||
| '--skip-git-repo-check', | ||
| '--full-auto', | ||
| '--sandbox', 'workspace-write', |
There was a problem hiding this comment.
This expected prefix now has nine elements because --sandbox and workspace-write are two argv tokens, but the assertion still compares it against args.slice(0, 8). That means the actual value ends at --disable while the expected array also includes plugins, so this regression test will fail even when buildArgs is correct and keeps the daemon CI red. Please update the slice bound to include all nine expected entries (for example args.slice(0, 9)) or switch this to an assertion pattern that checks the prefix length from the expected array.
- Fix test slice bound from 8 to 9 (--sandbox + workspace-write are two tokens, replacing the single --full-auto token) - Remove unrelated CSS change (overflow-y: hidden) from this PR - Add Codex CLI v0.128+ version requirement note to spec
lefarcen
left a comment
There was a problem hiding this comment.
Nice work @nmsn! Both issues are resolved:
✅ mrcfps's test assertion fix — args.slice(0, 9) now correctly matches the 9-element expected array
✅ Scope drift removed — the unrelated index.css change has been moved out
The core Codex CLI deprecation fix is solid and ready to ship. My P3 backward compat note was optional — it would be nice to have but won't block merge.
Looks good to me; deferring final approval to a maintainer.
mrcfps
left a comment
There was a problem hiding this comment.
@nmsn I reviewed the Codex CLI adapter update, the daemon regression test adjustment, and the README/spec documentation changes for the new --sandbox workspace-write argv shape. The prior slice-bound and unrelated CSS feedback are addressed in the current head; I did not find any remaining actionable issues in the changed ranges. Thanks for keeping the runtime docs and test coverage aligned with the daemon behavior — this is ready from my side. 🙂
Summary
--full-autoflag with--sandbox workspace-writein Codex CLI agent argv construction.Background
Codex CLI v0.128.0 deprecated
--full-auto. Selecting thegpt-5-codexmodel causes the agent to exit with code 1 because the newer CLI version no longer accepts this flag.Closes #438
Changes
apps/daemon/src/agents.ts--full-auto→--sandbox workspace-writeapps/daemon/tests/agents.test.tsspecs/current/runtime-adapter.mdREADME*.md(×9)Test plan
pnpm --filter @open-design/daemon test— agents test should passgpt-5-codexmodel, trigger a task, verify it no longer exits with code 1