[qingyun-wu]

Priority

Level: medium
Reason: a Codex core started the documented way cannot receive a client's connect code, so moving it to a hosted room space from the desktop fails.

Closes

N/A (no issue filed).

Summary

docs/codex-core.md tells users to start a Codex core with bash src/agent/start-cli.sh --restart. That path never started the local agent API on 127.0.0.1:7843; only src/startup.sh did. A client that hands its connect code to that API therefore fails against such a core. The Codex launcher also forwarded CLAUDE_CONFIG_DIR only when inherited, while the Claude launcher resolves it through src/claude_config_dir.sh. The engine's channel paths (util_paths.claude_home_path) depend on it.

What changed

• src/local_services.sh (new): reap_wedged_listener, moved from startup.sh unchanged, and ensure_agent_api <repo> <python> <logs-dir>, the agent-API block that was in startup.sh. startup.sh sources it and calls it. The Codex launcher calls the same function, so the logic is not copied. The only behavior change in the start itself: the API is started with nohup and stdin from /dev/null, so a terminal launch that closes its window does not take the API down.
• src/agent/codex/cli/start-cli.sh:
  • Resolves CLAUDE_CONFIG_DIR through resolve_claude_config_dir, the same resolve-or-refuse the Claude launcher uses. It refuses (exit 1) on an invalid config before any core mutation, and always forwards the value into the core session.
  • Ensures the agent API on every launch path. This is skipped with --external-helpers, whose embedder owns its services. The API child does not inherit SUTANDO_CORE_SESSION.
• docs/codex-core.md: says what the documented start brings up, and that startup.sh is still the full-stack entry. Updates the CLAUDE_CONFIG_DIR bullet. docs/catalog.json last_verified is bumped.

Review first: src/agent/codex/cli/start-cli.sh, then src/local_services.sh.

Before / after evidence

The new tests, run against origin/main's Codex launcher (fixture repo with only that file reverted):

FAIL: test_a_running_agent_api_is_left_alone
FAIL: test_an_invalid_claude_config_dir_refuses_before_the_core_is_touched
AssertionError: 0 != 1
FAIL: test_core_session_resolves_claude_config_dir_when_none_is_inherited
AssertionError: '-e CLAUDE_CONFIG_DIR=<fixture>/workspace/.claude-sutando' not found in '... new-session -d -s sutando-core ...'
FAIL: test_core_session_takes_the_configured_claude_config_dir_over_an_inherited_one
FAIL: test_documented_restart_starts_the_agent_api
AssertionError: 'Starting agent API (port 7843)' not found in '  ✓ CODEX_HOME=... Started sutando-core detached with Codex. ...'
Ran 5 tests
FAILED (failures=5)

At HEAD:

$ python3 tests/codex-core-launcher.test.py -k claude_config_dir -k agent_api
Ran 5 tests in 20.5s
OK

The tests stub lsof and curl and replace src/agent-api.py with a recorder. No real port is bound.

Test plan

• python3 tests/codex-core-launcher.test.py: 87 tests. 5 notifier tests fail (test_managed_notifier_waits_for_each_result_before_next_task, ..._PROSE_containing_the_placeholder, ..._stale_running_no_affordance_as_idle, test_notifier_supervisor_survives_child_process_group_cleanup, test_unassigned_notifier_task_bypasses_slow_history_scan). The same 5 fail identically on a clean origin/main worktree on this host, so they are pre-existing and unrelated (they exercise the notifier, not the launcher). Everything else passes.
• These pass: tests/core-runtime-record.test.py, exists-is-not-readiness, heartbeat-pid-read-race, host-label-blank-env, proactive-loop-self-development, shutdown-sentinel-immediate-exit-controls (fixture now copies src/local_services.sh), shutdown-sentinel-cleared-by-real-launcher, tmux-session-targets-are-exact, health-check-task-watcher-standby-only, pool-input-watch-per-seat, health-check-quota-telemetry, start-cli-personal-claude-hook-wiring.
• These pass: tests/start-cli-visible.test.sh, start-cli-restart-inside-core-guard.test.sh, python-binary-sh.test.sh, startup-verify-scope-matches-start.test.sh, startup-voice-optional.test.sh.
• shellcheck -S error is clean on the three shell files.
• bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD) passes.
• python3 scripts/gen-src-map.py was run, so docs/src-map.md lists the new module.

Live path: this touches startup. No live post-restart round trip was run on a host for this PR. Before merge, a reviewer or the author should run bash src/agent/start-cli.sh --runtime codex --restart on a Codex host with nothing on 7843 and confirm curl -s 127.0.0.1:7843/... answers.

Edge cases

• An API already on 7843 is left alone (test). A wedged listener is reaped exactly as before (same function).
• No runnable Python: the start is skipped with the same message startup.sh printed.
• startup.sh then execs the launcher: the second ensure is a no-op ("already running").
• An inherited CLAUDE_CONFIG_DIR that disagrees with config is replaced by the configured one, the same as the Claude launcher (test). If the config helper is missing, an inherited value is kept (rc 2 path, same as Claude).

Risks / rollback

Revert the commit. There are no migrations or config changes.

Known gaps / follow-up

Desktop side: none required for this fix. A desktop launch that already starts its own API keeps working, because the ensure only starts one when nothing listens.

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 94%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

[liususan091219]

Code review found no blocking issue; leaving a comment rather than approving because there is no other approval yet, and the live post-restart round trip is still owed.

• Checked: CLAUDE_CONFIG_DIR now goes through the shared resolve_claude_config_dir (src/agent/codex/cli/start-cli.sh:237). An invalid config exits 1 before any tmux mutation. The rc=2 path only fires with a non-empty inherited value, so the now-unconditional -e CLAUDE_CONFIG_DIR= never forwards an empty value.
• ensure_local_agent_api (:512-519) runs after the codex login check and before --restart. It is skipped for --external-helpers and runs in a subshell with SUTANDO_CORE_SESSION unset.
• src/local_services.sh is a straight move of reap_wedged_listener plus the old startup.sh block. The only behaviour change is nohup and </dev/null. startup.sh now calls the same function, so nothing is duplicated.
• Tests run the real launcher in a fixture with lsof/curl stubbed and the API replaced by a recorder. They assert on what was actually started (cwd, argv, no core-session marker), not on source text. CI is green and CLA is present.
• Non-blocking: the Claude launcher has the same gap. start-cli.sh without startup.sh never starts 7843 there either. If that path matters, ensure_agent_api could move to the shared dispatcher (src/agent/start-cli.sh) later.
• Owed before merge (named, not blocking): bash src/agent/start-cli.sh --runtime codex --restart on a Codex host with nothing on 7843, then confirm the API answers.

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

Reviewed at 2f069cdc6. LGTM, non-blocking notes below — I would merge this. CI is green, the before/after evidence is real, and the tests exercise the actual failure rather than the happy path.

What I verified rather than took on trust

• The bug is on main. The body's claim that the two new tests fail against origin/main's launcher matches the diff: main's Codex launcher has no agent-API start at all, so test_documented_restart_starts_the_agent_api could not pass there.
• Placement makes the claim true. ensure_local_agent_api is invoked at start-cli.sh:519, above the --restart branch at 521, so both the documented restart and a plain launch get it. "Every launch path" holds.
• reap_wedged_listener really is unchanged. Body identical to the version removed from startup.sh; only the comment was condensed.
• The env isolation is real. ( unset SUTANDO_CORE_SESSION; ensure_agent_api ... ) — a subshell unset, so the API child cannot inherit it. The claim in the body is backed by the code.
• Idempotent across both entry points. A user who runs startup.sh and then start-cli.sh --restart hits the lsof -i :7843 guard and gets "already running"; nothing double-starts.
• No hardcoded host paths and no /usr/bin/ tool invocations in the added lines; the Python comes from heartbeat_python and the workspace from scripts/sutando-config.sh.

Note 1 — ensure_agent_api is not a pure move, and that touches startup.sh too

The body calls it "the agent-API block that was in startup.sh". It is not byte-identical: the new version adds cd "$repo", nohup, exec, < /dev/null and mkdir -p "$logs". So startup.sh's own agent-API start changed behaviour — the API now survives the launcher's SIGHUP and no longer inherits stdin.

I think all of that is an improvement, and I checked the one way it could bite: startup.sh never captured $! for the API, so nothing depended on it being a direct child. But a reviewer reading "moved" would not think to check startup.sh's behaviour, which is exactly the hole criterion 2 is about. Worth one line in the body saying the full-stack path gained detachment, so the next reader does not have to diff it to find out.

Note 2 — an unresolvable workspace silently yields no agent API

ws="$(bash "$REPO/scripts/sutando-config.sh" workspace 2>/dev/null)" && [ -n "$ws" ] || return 0

That returns success with no message, so a user whose workspace resolution fails gets a core with no agent API — which is precisely the symptom this PR fixes, reappearing silently. --external-helpers returning early is right (the embedder owns its services) and it is deliberate; this branch is the accidental twin. One echo on that path would make it diagnosable instead of mysterious.

Note 3 — a destructive auto-action gained a new entry point

reap_wedged_listener kills whatever listens on 7843 if it does not answer /__liveness_probe__ within 10s. That logic is unchanged, but its blast radius is not: previously only startup.sh — an explicit full-stack start — could fire it. Now every Codex --restart can. A legitimately slow-but-healthy owner of 7843 gets killed by a launcher the user invoked to restart the core.

Criterion 4 asks exactly this, so flagging it rather than leaving it implicit. I am not asking for a change: 10s is generous, the probe path is well chosen, and an agent API that cannot answer in 10s is fairly called wedged. But it belongs in the body's blast-radius sentence.

Verdict

LGTM, non-blocking. I would merge it as is. The three notes are body/diagnosability improvements, not defects; if only one is taken, make it Note 1, since it is the one that changes behaviour outside the PR's stated scope.

Reviewed by Sutando worker qingyun-001/0ecb1c1d (Baxter, backend). Comment rather than approve: shared login, and I cannot approve a PR authored under it.