Repository navigation
Worktrees and a Codex worker: accepted, and nothing could commit - #757
jorgemanrubia wants to merge 2 commits into
Conversation
basecamp connect setup --worktrees --worker codex was accepted and produced workers that can never commit. Codex's sandbox is workspace-write: it writes inside the working directory and nowhere else. A worktree's git data lives outside that directory, so every commit fails and a task that edits anything ends with its worktree kept. The check is on the resulting file rather than on the flags typed, because the two settings are separate flags on separate runs: turn worktrees on today, name the worker tomorrow, and a refusal that reads one invocation refuses nothing. Validate also runs on Load, so a file edited by hand is refused where it is read instead of quietly producing workers that cannot commit.
There was a problem hiding this comment.
🟡 Changes recommended
Existing invalid profiles cannot be repaired through setup, rejection occurs after avoidable network work, and shipped guidance still recommends the prohibited configuration.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds validation preventing Codex workers from using git worktrees because their sandbox cannot write worktree metadata.
Changes:
- Rejects incompatible Codex/worktree configurations.
- Tests direct, incremental, hand-edited, and valid configurations.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
internal/connector/setup/file.go |
Adds cross-field configuration validation. |
internal/connector/setup/file_test.go |
Covers invalid and supported worker/worktree combinations. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Codex's sandbox is workspace-write: it writes inside the working | ||
| // directory and nowhere else. A worktree's git data lives outside that | ||
| // directory, so a Codex worker in a worktree cannot commit, and a task | ||
| // that edits anything ends with its worktree kept. The combination is |
The refusal was a trap. `connect setup` reads connect.json before it applies a change, so refusing the file at Load left an operator already in that state unable to leave it: the command that repairs the file would not read it, and the remedy the error named could not be run. The pairing is now its own error, and setup loads through it — LoadToRepair tolerates that one state and nothing else. What setup applies still has to validate, so this widens what can be repaired rather than what can be written: a run that changes something else is refused exactly as before. Apply refuses the pairing too, so setup rejects the flags before spending token, identity and trust checks on a configuration already known to be impossible. That is what Apply's contract promises. Shipped guidance recommended the combination in three places: the skill's example command, its worktrees prose, and `connect worktrees` help. All three now say it is refused and name the two flags that repair a profile already set up that way.
github-actions
Bot
added
commands
There was a problem hiding this comment.
🟡 Changes recommended
Repair loading can mask additional validation failures, and the connector-specific skill remains outdated.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 2
- Review effort level: Balanced
| if err := f.workerCanCommit(); err != nil { | ||
| return err | ||
| } |
| basecamp connect -P agent # Run the connector in the foreground: hear the agent's events, admit what a trusted person asks, and hand the work to a local coding agent that replies as the agent | ||
| basecamp connect -P agent --project <id> --shadow # Narrow it to one project, and watch without acting: an isolated state directory, nothing dispatched and nothing posted | ||
| basecamp connect setup -P agent --worker codex --worktrees # Run workers with Codex instead of Claude Code, and give each task its own git worktree | ||
| basecamp connect setup -P agent --worker codex # Run workers with Codex instead of Claude Code (not with --worktrees: a Codex worker cannot commit in one) |
|
🤖 Closing: the connector is dropping routes-to-directories and worktrees entirely. It will run where it is started, and the agent decides whether a task needs a clone or a directory — sandboxing is being built separately and is where containment belongs. The constraint this removes never constrained anything. A worker started in a routed directory can already write anywhere the account can: Codex is confined by Codex's own sandbox, Claude Code by policy. So starting it in the connector's own directory loses no containment — it drops a claim that was not true. That takes this pull request's subject with it. Not a judgement on the work: the refusal was correct, and the repair path it grew was the right answer to a trap I had built, and the reasoning is on the card rather than lost here. Tracked in The connector runs where it is started: no routes to directories, no worktrees. |
basecamp connect setup --worktrees --worker codexwas accepted, and produced workers that could never commit. Nothing joined the two settings, so an operator got a connector that looked configured and quietly failed at the last step of every task that edited anything — plus a retained worktree per task.Originally tracked in Worktrees plus a Codex worker: accepted, and cannot commit.
Codex's sandbox is
workspace-write: it writes inside the working directory and nowhere else. A worktree's git data lives outside that directory. The Codex driver's own package comment has said so since it was written; nothing acted on it.The check is on the resulting file rather than on the flags typed. That is the whole point of where it sits. The two settings are separate flags on separate runs — turn worktrees on today, name the worker tomorrow — so a refusal that reads one invocation refuses nothing and gives false assurance.
Validatealso runs onLoad, so a hand-editedconnect.jsonis refused where it is read instead of producing workers that cannot commit.Reachable four ways, refused in all four, and each case proven to fail without the guard: both settings together, either one added to the other later, and a file edited by hand. Worktrees with a worker that can commit stay accepted, including when the file names no worker and the default applies.