fix: harden Codex auth and backend task startup#521
Draft
robinbraemer wants to merge 6 commits into
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fix Firstmate's Codex CLI launch auth boundary so every Firstmate-launched Codex crewmate, scout, and secondmate preserves HOME=/Users/robin and explicitly unsets ambient CODEX_HOME at the Codex process boundary. The fix must be one-owner in fm-spawn's Codex template, leave raw launch-command escape hatches and all non-Codex harnesses unchanged, and preserve model, effort, autonomy, notify/turn-end, quoting, and secondmate behavior. Add regression coverage for hostile ambient CODEX_HOME across ship, scout, batch, and secondmate launches plus a safe fake-process smoke. Do not modify no-mistakes, Orca, CodexBar, Pi/router credentials, token files, active servers, or healthy sessions. Keep operator guidance generic with no product-specific auth or session paths.
What Changed
CODEX_HOMEfor Codex ship, scout, batch, and secondmate launches while preserving existing launch options and raw-command escape hatches.Risk Assessment
✅ Low: Captain, the change is narrowly scoped to the Codex process boundary, preserves existing launch behavior, and correctly resolves both prior findings without introducing new material risk.
Testing
The supplied full-suite baseline was successful; fresh focused spawn and secondmate suites also passed. An end-to-end fake-process smoke exercised ship, scout, two batch children, and secondmate launches, proving each preserved HOME=/Users/robin, unset hostile CODEX_HOME, and retained expected options and notification behavior, while raw-command and Claude controls remained unchanged. Evidence was retained, temporary fixtures were removed, and the worktree is clean.
Evidence: Codex auth-boundary end-to-end smoke transcript
PASS: 5 Codex child processes preserved HOME=/Users/robin and received CODEX_HOME unset; raw and Claude controls preserved the ambient variable unchanged.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
.agents/skills/harness-adapters/SKILL.md:156- Shared operator guidance hard-codesHOME=/Users/robinand adds Orca/Pi credential rules. This reusable skill should instead say to preserve the invoking user's existingHOMEwhile unsettingCODEX_HOME, without unrelated product-specific authentication guidance..agents/skills/harness-adapters/SKILL.md:157- The new rule requires the auth boundary for resume/recovery, but the concrete instruction at line 153 remains barecodex resume <session-id>. With hostile ambientCODEX_HOME, that command uses the wrong auth/session state; make the command explicitly boundary-safe, such asenv -u CODEX_HOME codex resume <session-id>.🔧 Fix: Captain, generalize Codex auth-boundary guidance
✅ Re-checked - no issues remain.
🔧 **Test** - 1 issue found → auto-fixed (3) ✅
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: Captain: fix bootstrap fixture and Zellij creation races
1 error still open:
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: Captain: stabilize Herdr and Zellij startup races
1 error still open:
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: Captain: stabilize real-Herdr fixture shell readiness
✅ Re-checked - no issues remain.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"Pre-run baseline supplied by the runner:command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"bash tests/fm-spawn-dispatch-profile.test.shbash tests/fm-secondmate-harness.test.shset -o pipefail; ./.tmp-codex-auth-boundary-smoke.sh | tee /var/folders/1y/cjgf53nj31n_dxsspqnjfjvc0000gn/T/no-mistakes-evidence/01KXDB531RZJT9VR5GAA6Z7JMB/codex-auth-boundary-smoke.txttest -s "$artifact"; test ! -e /var/folders/1y/cjgf53nj31n_dxsspqnjfjvc0000gn/T/no-mistakes-evidence/01KXDB531RZJT9VR5GAA6Z7JMB/.codex-auth-smoke-work; sed -n '1,240p' "$artifact"git status --short; git diff --stat; git diff --cached --stat; git rev-parse HEAD⏭️ **Document** - skipped
bin/fm-spawn.sh:836- Captain, production Herdr spawn sendstreehouse getbefore proving the new pane shell is ready; fixing this runtime race requires executable changes outside the documentation-only scope.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.