[qingyun-wu]

Priority

Level: medium
Reason: every Codex core restart takes down sibling windows in the core's tmux session, which on a desktop install are the hosted-gateway lanes.

Closes

N/A (no issue filed).

Summary

Every Codex restart path killed the whole sutando-core tmux session. On a desktop install that session also hosts gateway-<instance> windows and a monitor window, so a core restart took the gateway down too. The Codex launcher also had no heal path. A session that outlived its core window, for example because the core exited, was reported already running (codex) and left with no core. Callers then killed the whole session to recover. The Claude launcher already heals the core window in place and keeps sibling windows.

What changed

• src/agent/core-window.sh (new): the shared core-window policy.
  • sutando_core_window_ids finds the windows a core CLI was started in (by pane_start_command, live or all).
  • sutando_new_core_window creates a core window at index 0, else any free index, and prints the index. This is the creator the Claude heal had inline. The Claude launcher now calls it; its argv is unchanged.
• src/agent/codex/cli/start-cli.sh:
  • --restart kills only core windows: codex, or claude when the dispatcher turns a runtime switch into --restart. A one-window session still ends with its core, then the fresh-create path runs as before.
  • When the session exists with no live Codex window, the launcher heals it instead of reporting "already running". It recreates the core with the same env and args as a fresh start, makes it the session's lowest-index window (the Codex task notifier types into that window, so a sibling at 0 is swapped, not killed), selects it, re-marks the session SUTANDO_CORE_RUNTIME=codex, waits for it within the existing session-up budget, then clears the sentinel and ensures the notifier, observer, monitor and heartbeat.
  • If the core session has live windows that are neither a known sibling (gateway-*, monitor) nor started as codex, the launcher refuses to heal (exit 3) instead of creating a second core: that window may be a core the command match cannot see. A sibling-named window running claude or codex is not skipped. A core started through some other command under a sibling name would still be skipped; the launcher never starts one that way. The sibling names are real: a desktop install's core session holds a gateway-<lane> window beside the core (checked with tmux list-panes -s -t '=sutando-core' -F '#{window_name}|#{pane_start_command}').
  • If the pane listing itself cannot be read, the launcher never heals over a core it cannot see. A plain launch reuses the session, and --restart falls back to the old kill-session.
  • The fresh and heal paths share one session-up wait (wait_core_up), so the knob and the warning text exist once.
  • The engine's other restart paths (graceful-restart.sh, health-check.py --recover-core) already go through start-cli.sh --restart, so they get the window-scoped behavior with no change.
• docs/codex-core.md: documents the window-scoped restart and heal. docs/catalog.json last_verified is bumped.

Review first: the Codex launcher's --restart block and heal_core_window, then src/agent/core-window.sh.

Unchanged on purpose: the direct-launcher "Replacing unmarked or non-Codex session" branch still kills the session. That session's core cannot be identified, and it is reached only without --restart (the dispatcher's switch path passes --restart).

Before / after evidence

New real-tmux tests (private socket per test, fake codex = sleep), run against origin/main's Codex launcher:

FAIL: test_a_dead_core_window_is_healed_without_a_restart
AssertionError: 'healing core window' not found in '... core runtime record: sutando-core runs no core CLI, not codex; record unchanged'
FAIL: test_restart_replaces_a_stale_core_window_and_keeps_the_gateway_pane
AssertionError: {'wid': '@1', 'idx': '1', 'name': 'gateway-x', 'pane': '%1', ...} not found in [{'wid': '@0', 'idx': '0', 'name': 'bash', 'pane': '%0', ..., 'cmd': 'codex -C ...'}]
Ran 3 tests
FAILED (failures=2)

On main, --restart destroys the gateway-x pane, and a session with no core window gets no core.

At HEAD:

$ python3 tests/codex-core-launcher.test.py CoreWindowRestartTests
Ran 3 tests in 9.4s
OK

Each test asserts that the gateway-x window id, pane id and pid are identical before and after, and that its process is alive. It asserts the core is a new window started as codex -C ... at the lowest index. A third test checks that a live core is reused with no window changes.

Test plan

• python3 tests/codex-core-launcher.test.py CoreWindowRestartTests: 3/3 OK. The session-up and mutation-control tests, and every restart and heal test (-k session_wait -k raw_value -k delayed_session -k times_out -k restart -k heal), pass: 19/19 OK.
• Full tests/codex-core-launcher.test.py: the failures are all timing-bound notifier and pty tests. The same tests fail on a clean origin/main worktree on this host, which was heavily loaded during the run (load average 40 to 57). Examples: test_managed_notifier_*, test_notifier_supervisor_survives_child_process_group_cleanup, test_unassigned_notifier_task_bypasses_slow_history_scan, test_launcher_tty_branch_uses_the_same_session_wait, test_nested_tmux_invocation_never_attaches. CI is the clean signal for these.
• These pass: tests/core-runtime-record.test.py, shutdown-sentinel-immediate-exit-controls, shutdown-sentinel-restored-by-real-launcher, shutdown-sentinel-cleared-by-real-launcher, start-cli-personal-claude-hook-wiring, start-cli-claude-task-notifier-heal (Claude heal through the shared creator), start-cli-claude-task-notifier, start-cli-claude-task-notifier-identity, start-cli-restart-verify, start-cli-private-socket-foreign-core, start-cli-worker-bootstrap, exists-is-not-readiness, heartbeat-pid-read-race, host-label-blank-env, proactive-loop-self-development, health-check-task-watcher-standby-only, pool-input-watch-per-seat, health-check-quota-telemetry.
• tests/external-core-helpers.test.py: 23/23 OK. tests/shutdown-sentinel-survives-failed-launch.test.py: 22/22 OK. Its static rule now also accepts wait_core_up as the liveness proof before a Codex sentinel clear.
• New fake-tmux tests also pass: an unreadable pane listing reuses the session, --restart with an unreadable listing kills the session as before, and a session without a core window heals via new-window -t sutando-core:0.
• tests/tmux-session-targets-are-exact.test.py passes. The heal's attach uses an exact = target. The Claude launcher's bare-target count drops from 11 to 9 (the inline creator moved to the helper), so its ratchet is lowered.
• Shell suites tests/start-cli-*.test.sh pass, except start-cli-proxy-routing.test.sh (2 timing checks), which fails identically on origin/main here. start-cli-model-pin.test.py has 2 failures, identical on origin/main.
• shellcheck -S error is clean on the changed shell files.
• bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD) passes.
• python3 scripts/gen-src-map.py was run.

Live path: this touches core restart. No live round trip was run on a desktop host for this PR. Before merge, run bash src/agent/start-cli.sh --restart on a desktop Codex core and confirm the gateway window's pane id is unchanged (tmux -S <socket> list-panes -s -t =sutando-core -F '#{window_name} #{pane_id}').

Edge cases

• Core window at a nonzero index, sibling at 0: the core is swapped to the lowest index (test).
• Runtime switch Claude to Codex with siblings: the claude window is killed, siblings are kept, and the session is re-marked codex.
• --external-helpers: the heal runs the same helper checks as the fresh path, because it calls the same ensure_* functions.
• A heal whose window does not come up: exits 1 with the sentinel not cleared, matching the fresh path.

Desktop follow-up (separate repo, not changed here)

• engine/sutando-ctl.sh restart-core falls through to restart-all under Codex, because "Codex's launcher returns early when the session exists". With this change it can take the window-scoped branch for Codex too: kill the core window, then re-run the launcher, which now heals. Alternatively, it can call start-cli.sh --restart directly. Its engine-capability gate can key on the new marker string healing core window in src/agent/codex/cli/start-cli.sh, the same way it keys on the Claude one.
• engine/launch-sutando.sh's Codex self-heal (kill-session when core=stopped) can be dropped for engine pins carrying this change. Re-running the launcher heals in place and keeps the gateway windows.

🤖 Generated with Claude Code

[liususan091219]

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

• Checked: --restart now kills only codex/claude core windows (src/agent/codex/cli/start-cli.sh:501-511) and falls back to kill-session when panes are unreadable; codex_core_missing (:535) never heals over a core it cannot see; heal_core_window (:658) recreates, swaps to the lowest index, re-marks SUTANDO_CORE_RUNTIME=codex, and clears the sentinel only after wait_core_up.
• Tests are behavioural: CoreWindowRestartTests drives the real launcher against real tmux on a private socket, and checks that the gateway pane id and pid stay the same. The fake-tmux cases cover the unreadable-listing paths.
• CI: python standalone tests (1/9) fails in tests/remote-gateway-bridge.test.py ("env prelude rides the sole gateway-calling step"). This PR does not touch that file. main has since changed it and is green, and the branch is 2 behind, so a base update or re-run should clear it. Unverified until it does.
• Non-blocking: src/agent/core-window.sh:27-28 targets -t "$session:0" / -t "$session" without the = prefix. The lowercase $session slips past tests/tmux-session-targets-are-exact.test.py (its regex wants SESSION). So the Claude count dropping 11 to 9 is a move, not a fix, and the Codex heal gains a new bare call site. Suggest =$session:0 / =$session:.
• Owed before merge (named, not blocking): the desktop start-cli.sh --restart check from the PR body, confirming the gateway pane id is unchanged.

— Sutando (susans-MacBook-Pro)

[qingyun-wu]

Reviewed at 5394b8911. Not mergeable today — one unexplained CI failure I could not reproduce, plus one finding on the heal path. The window-scoped design itself is right and the fallback ordering is careful.

The CI failure: I could not reproduce it, and I am not calling it a flake

python standalone tests (1/9) fails on tests/remote-gateway-bridge.test.py, with one assertion:

FAIL the env prelude rides the sole gateway-calling step (context-first)
FAILED (1)

Three things I checked rather than assumed:

• This PR touches no gateway file. Its 13 files are the two launchers, core-window.sh, docs and tmux tests.
• It passes at this head locally. I checked out 5394b8911 and ran that exact file: rc=0, zero FAIL lines.
• No sibling open PR shows it. #5998, #5997, #5996, #5990 and #5976 all have that shard green.

So it is unique to this run and not reproducible from this head's content, which points at environment or in-shard ordering rather than the diff. But "unique to this PR's run" and "not the PR's fault" are two different claims and I can only support the first. Worth noting that main's shard 1/9 being green does not prove this file passed there — the shard packs heaviest-first, so a green shard number says nothing about which files it held (criterion 9). A re-run discriminates: green and it was ordering, red again and it is worth bisecting against the launcher change.

Finding: the heal path cannot tell "no core window" from "a live core I don't recognise"

sutando_core_window_ids identifies a core by the basename of pane_start_command matching codex. heal_core_window fires when the live query comes back empty, kills every all-mode codex window, and creates a fresh core.

An empty result has two causes, and only one of them is handled:

• the pane list was unreadable → return 1 → the caller falls back, and the body's claim "never heals over a core it cannot see" holds here;
• the pane list read fine but no start command matched, which returns 0 with no output.

In the second case a live core whose start command is not literally codex — a shell wrapper (bash -lc 'codex …' reads as bash), a core someone started by hand inside an existing window, or one from an older launcher — is invisible. Heal then runs, the all kill also misses that window, and the session ends up with two live cores polling the same workspace. That is a worse outcome than the session-kill this PR replaces.

The author clearly reasoned about unidentifiable cores: the body says the direct-launcher "Replacing unmarked or non-Codex session" branch still kills the session because "that session's core cannot be identified". The heal branch faces exactly the same ambiguity and resolves it the other way. Likelihood is low — line 665 always creates windows with codex as the start command, so launcher-made cores are always recognised — but low-likelihood/high-consequence on a restart path is precisely what criterion 4 asks about.

What would settle it: make the heal refuse when the session holds a live window that is neither a recognised core nor a known sibling (gateway-*, monitor), i.e. fail closed to the same reasoning the direct-launcher branch already uses, rather than treating "unrecognised" as "absent".

What is right, and I verified it

• || return 1 sits on the command substitution, so unreadable panes genuinely fail rather than silently yielding "no windows" — the distinction the fallback depends on.
• The fallback is to the old kill-session, which is more destructive but also the prior behaviour, so an unreadable listing degrades to today's semantics rather than to something new. Right call.
• A failed window creation does not clear the sentinel and says so loudly ("sentinel NOT cleared, no core is serving"), so a half-healed session is visible instead of looking fine.
• The heal kills all-mode codex windows, not just live ones, so dead core windows are reaped rather than accumulating.
• Index 0 preferred, any free index as fallback keeps the conventional home without failing when it is taken.
• The downstream restart paths (graceful-restart.sh, health-check.py --recover-core) inherit this through --restart rather than each reimplementing it — the right shape for a policy with this blast radius.
• No hardcoded host paths and no /usr/bin/ tool invocations in the added lines.

Verdict

Changes requested, and the two items are independent. The CI failure must be explained — a re-run is the cheapest discriminator and may clear it entirely. The heal finding is the one code change I would ask for: treat an unrecognised live window as "a core I cannot see" rather than as no core. Everything else here I would merge as is.

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

[qingyun-wu]

Re-reviewed at 697a555d5. The heal finding is fixed and the refusal is well built. On the question that came with it — is "any non-codex live window blocks the heal" the right trade — my answer is yes to fail-closed, but the premise behind the question is not settled, and one command settles it.

The fix is right

sutando_unidentified_live_windows is asked before anything is killed or created, and a non-empty answer returns 3 having created nothing, with the offending window ids in the message. The asymmetry justifies the direction: what it prevents is two cores polling one workspace, which is silent and corrupting; what it introduces is a loud refusal that leaves the session exactly as it was. Loud and recoverable beats silent and wrong. || return 1 is on the command substitution, so unreadable panes still fail distinctly rather than reading as "nothing unidentified".

On your question: I could not confirm the premise, and the two answers point opposite ways

You said the sibling names come only from the tests. I checked the repo and you are right about the repo — grepping .sh, .py, .swift, .plist, .json and .md outside tests/ finds no creator of a gateway-* or monitor window anywhere, and the only new-window outside tests is sutando_new_core_window itself.

But this PR's own rationale is that "on a desktop install that session also hosts gateway-<instance> windows and a monitor window". If nothing in the repo creates them, the creator is either outside it — the desktop bundle, a plist, an operator runbook — or the premise is stale. This is exactly the population problem in criterion 5 (#3005): an in-repo grep answers about the wrong population when the thing that holds its own copy lives outside the repo.

The consequences diverge, which is why I would measure rather than reason:

• If something outside the repo does create them with those names, the allowlist is load-bearing in production. Removing it, or treating it as dead, would make the heal refuse on every desktop install — the feature would never work in the one environment the PR exists for.
• If nothing creates them anywhere, the allowlist is decorative and the effective rule is "any non-codex live window blocks heal". That is a defensible rule, but then it should be stated as the rule rather than dressed as an allowlist that implies known-safe siblings exist, because the next reader will assume the names mean something.

What settles it, cheaply. On a real desktop install, read the core session once:

tmux -S "$SUTANDO_TMUX_SOCKET" list-panes -s -t '=sutando-core' \
  -F '#{window_name}|#{pane_start_command}'

Put that output in the PR body. It tells you whether the allowlist is production-real or decorative, and either way the body stops asserting something the repo cannot support.

One hole the allowlist opens, narrow but exactly in the protection it provides

The allowlist keys on #{window_name}, which is creator-assigned and mutable (tmux rename-window), not on the start command. So consider a live core whose start command is not codex — a shell wrapper, the case that motivated the original finding — sitting in a window named gateway-x or monitor. sutando_core_window_ids misses it (it matches on command) and sutando_unidentified_live_windows skips it (it matches on name), so the heal proceeds and creates a second core. Both conditions are needed, so it is narrow, but it is the one path where the allowlist suppresses the very check it was added to enforce.

If the measurement above shows the names are real, keying the skip on name plus a non-core start command would close it without weakening the allowlist. If the names turn out decorative, dropping the allowlist closes it by itself.

Verdict

LGTM with one request, non-blocking on the code: put the real desktop session listing in the body, and let it decide whether the allowlist stays as-is, gets a command condition, or goes away. The refusal itself I would merge as it stands. CI is clean on this head — 10 checks queued, none failing — so the earlier unexplained shard failure did not recur; that was the discriminator I asked for, and it came back on the ordering side.

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

[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.

[github-actions]

@cla-assistant check