[vidhuUC]

What

src/agent/codex/cli/start-cli.sh: put the bundled node runtime (engine/runtime/node/bin) and ~/.npm-global/bin on PATH before the guards; wait up to 120s (SUTANDO_CODEX_WAIT_TIMEOUT) for an in-flight codex install instead of exiting permanently; report codex login status exit-127 as a missing-interpreter fault instead of 'not authenticated'.

Why (all three observed on fresh-Mac VMs, 2026-08-31)

1. A fresh Mac has no system node; codex is a node script — without the bundled runtime on PATH every codex invocation dies env: node: No such file or directory (exit 127).
2. Onboarding installs codex ~60s after the core's first launch (observed: engine unpacked 21:11:25, codex landed 21:12:32) — the command -v codex guard exits a tmux window that never recovers; the setup wizard shows [exited].
3. The auth guard turned that 127 into "Codex CLI is not authenticated… Run 'codex login'" with a valid auth.json on disk.

Live round trip

Before: guard exited permanently on the timeline above; wizard stuck at [exited] until an app relaunch.
After (same release DMG, fix patched into the bundle): 23:13 engine / 23:18 codex on a fresh VM — core up clean, full onboarding completed hands-off; reproduced on a second fresh VM.

Draft notes: branch is based on this fork's older main (token lacks workflow scope to sync the fork) — will rebase onto current main before ready; comments to be condensed to the house 2-line style.

🤖 Generated with Claude Code

https://claude.ai/code/session_0192oQr1bbiZ3CHAAJRqrFHc

[CLAassistant]

All committers have signed the CLA.

[github-actions]

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

[yixuan-ag2]

Approving. The reasoning in the comments is doing real work here — particularly "APPEND, never
prepend: an operator (or a test harness) who put a codex or node earlier on PATH must keep winning".
That is the decision most PATH fixes get wrong silently, and stating it in the code means the next
person cannot reverse it by accident.

The exit-127 note is the other half and is the sharper one: without a reachable node, codex dies
env: node: No such file or directory and the auth probe downstream reports "not authenticated".
A wrong diagnosis that sends someone to codex login forever is worse than a hard failure, and the
comment names it so the next reader does not re-derive it.

One minor, non-blocking: CODEX_NODE_BIN="${CODEX_NODE_BIN:-$(cd "$REPO/.." … && pwd)/runtime/node/bin}"
— if that cd fails the substitution is empty and the value becomes the absolute path
/runtime/node/bin. Harmless in practice because [ -d "$_d" ] then skips it, but the failure mode
is a plausible-looking absolute path rather than an obviously-empty one, which is the shape that
survives a code read. A [ -n "$REPO" ] guard or a :? would make the impossible case loud.

The install-race wait is well-scoped: bounded at 120s, only entered when codex is genuinely absent,
and it reports how long it waited rather than silently succeeding.

[john-the-dev]

APPROVE — reviewed at 003f09fdd. Three changes, one coherent story (a fresh Mac cannot start codex), and the live round trip on two fresh VMs is the right evidence for a startup path.

What I checked rather than assumed

The 120s wait cannot hang a non-codex host. That was my main worry — the previous behaviour was fail-fast and this replaces it with wait-then-fail, which would be bad if anything invoked the script speculatively. It doesn't: src/agent/start-cli.sh is the dispatcher and resolves src/agent/$runtime/cli/start-cli.sh, so this file only runs when the resolved runtime is codex. Blast radius is exactly the hosts the wait is for.

A malformed SUTANDO_CODEX_WAIT_TIMEOUT degrades safely — I expected it to crash and was wrong. set -euo pipefail is on at line 3, so I assumed [ "$w" -lt "abc" ] would kill the script. Measured:

SUTANDO_CODEX_WAIT_TIMEOUT=10    -> loop runs, exit 0
SUTANDO_CODEX_WAIT_TIMEOUT=abc   -> "[: abc: integer expected", loop exits immediately, exit 0
SUTANDO_CODEX_WAIT_TIMEOUT=      -> ${:-120} gives 120, loop runs, exit 0

set -e does not fire on a command in a while condition, so a typo degrades to the old immediate 127 plus one stderr line. Worth knowing rather than fixing.

$REPO is bound before use — line 5, well above the new CODEX_NODE_BIN derivation.

The 127 discrimination is sound. command -v codex succeeds (the file exists) and then executing it fails 127 because the #!/usr/bin/env node shebang cannot resolve its interpreter — the shell reports 127 for an unresolvable interpreter exactly as for an unresolvable command. So the new branch catches the real case, and || _codex_auth_rc=$? is the correct way to keep set -e from eating the probe. The old message sent people to codex login with a valid auth.json on disk, which is the worst kind of wrong: actionable and useless.

APPEND, never prepend is the right call and the comment says why. An operator or test harness with its own node/codex earlier on PATH keeps winning.

Non-blocking

• If cd "$REPO/.." ever fails, CODEX_NODE_BIN becomes the literal /runtime/node/bin. Harmless in the loop ([ -d ] skips it) but it would be printed verbatim in the 127 message, pointing at a path that was never real. A ${...:-<unknown>} guard on the message would keep the diagnostic honest.
• Your draft note says the comments need condensing to the house 2-line style. Worth knowing CI will not catch it either way: scripts/review-checks-prose-cap.py has DEFAULT_EXTS = (".py",) and SUPPORTED_EXTS = (".py", ".pyi"), so .sh is unscanned. That makes this a house-style choice rather than a gate — and for what it's worth, these particular comments earn their length by recording observed timings and the reason for a non-obvious ordering.
• The other draft note (rebase onto current main) still stands.

Nothing here blocks. The diagnosis is specific, the fix matches it, and the failure it prevents is one an onboarding user cannot recover from without relaunching the app.

[TustinOC]

Reviewed at 75061696. Approve. All three faults are real and the fixes are the right shape — particularly separating exit-127 from "not authenticated", which is the difference between a user fixing the problem and a user running codex login at a machine that has no node.

I verified the load-bearing claim on a real install rather than taking the path on faith. The derivation is $REPO/.. + /runtime/node/bin, and on this host (a normal app install, engine at <app-support>/engine, this script at <app-support>/engine/sutando):

/Users/…/space.ag2.app/engine/runtime/node/bin
  node
  npm

So the "one level up from $REPO" comment is accurate and the bundled runtime is where you say it is.

A concern I had and then disproved — recording it so the next reviewer doesn't re-raise it. set -euo pipefail is active at line 3, and the wait loop ends its body with command -v codex >/dev/null 2>&1 && break, which looks like the classic set -e trap. It isn't: a failing command in an AND-list is exempt, and I confirmed empirically that the loop runs to completion and exits 0 when the binary never appears. The _codex_waited=$((…)) arithmetic is likewise safe as an assignment. Both fine.

One real, minor thing: that path assignment can exit the script silently

CODEX_NODE_BIN="${CODEX_NODE_BIN:-$(cd "$REPO/.." 2>/dev/null && pwd)/runtime/node/bin}"

An assignment takes the exit status of its command substitution, so if $REPO/.. is ever unreachable, cd fails, the assignment returns 1, and set -e kills the script there — before any of the friendly diagnostics this PR adds. Measured, with a control:

parent unreachable:  prints "before", then exit=1        <- silent death
parent exists:       resolved: '/tmp/runtime/node/bin'   exit=0

Low likelihood — $REPO's parent normally exists — but the failure mode is a silent exit with no message, inside a change whose whole purpose is replacing silent and misleading failures with loud accurate ones. … 2>/dev/null || true or a dirname-based derivation removes it. Non-blocking.

Related nit in the same spot: on that path the later diagnostic prints looked in $CODEX_NODE_BIN, which would render as /runtime/node/bin (absolute, at filesystem root) if the substitution ever produced empty — misleading at exactly the moment someone is reading it to debug.

An observation that supports your design choice

CODEX_INSTALL_BIN defaults to ~/.npm-global/bin, and that is not universal — on this host codex resolves to ~/.local/bin/codex, and ~/.npm-global/bin does not exist at all. That is not a defect, and it is worth stating why: the loop [ -d "$_d" ] || continue and appends only, so a host whose codex lives elsewhere is simply unaffected rather than broken. The APPEND-never-prepend rule earns its comment here too — an operator with their own node earlier on PATH keeps winning. Both choices hold up on a host that does not match the assumption, which is the real test.

Two things to save you a pass (same as I noted on #3653)

• prose-cap never scans this file. REVIEW.md's checks block sets prose_exts: ['.py'], so a .sh diff is out of scope. Your draft note about condensing comments to house style is a human-review matter, not a gate — and the green CI is not evidence either way.
• You are 3 commits behind origin/main, not the older-main drift the note implies. Small rebase.

|| _codex_auth_rc=$? with the comment explaining why is exactly right, and worth keeping verbatim — it is the kind of line someone "simplifies" back into a bug later.

[github-actions]

@cla-assistant check