Repository navigation
fix(health): health verdicts match what is really running - #4945
Conversation
…ot offline A stale beat made offline win outright, hiding the pool's escalation. Offline is a no-attention reason (the pool usually restarts the worker), so a worker the pool had abandoned 29 h earlier was classified as if it would come back by itself.
A stale or missing core-status self-report plus a pane with no idle footer read hung, even
while the core was mid-turn. The watcher now keeps the previous capture: a pane showing
"esc to interrupt" that changed since the last poll (clocks and spinners normalised away)
is running. A frozen turn, a clock-only change and the first poll still read hung.
cli_wedge.normalize now treats a compound duration ("3m 12s") as one field, so a spinner
whose timer grows past a minute no longer counts as new output.
tmux treats a bare -t name as a prefix, so once sutando-worker-<id> ended the probe found its own sutando-worker-<id>-input session. The seat's watcher then read the live session plus the core's idle self-report as idle-ready, and /health showed a dead worker healthy. The shared probe, the watcher's capture/send-keys and runtime-health's pane reads now target =<session>, as the rest of the repo does.
A worker's inbox watcher beats from its own session and outlives the seat, so a crashed worker read alive:true for up to 90 s and consumers keyed on alive showed Open terminal for a session that no longer existed.
b37708b to
bac918d
Compare
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 90%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
liususan091219
left a comment
There was a problem hiding this comment.
Changes requested — one blocker in Part 2; Parts 1 and 3 look correct.
Blocker — _turn_moving counts spinner-glyph animation as motion. Claude Code cycles the spinner glyph within a single turn. Live capture of a running pane, 0.5 s apart, same turn: ✢ Hatching… (9s …) → ✻ Hatching… (10s …); repo fixtures also show ✽/✢/✻ (tests/runtime-health.test.py:53, tests/fixtures/pane-claude-composer-cut-whole-paste.txt:1). cli_wedge._VOLATILE strips only braille/circle spinners, so with head cli_wedge.state_id the same frame with only the leading glyph changed (✻→✶/✳/✢/·/✽/*) differs in 6/6 cases. A stalled turn whose spinner still animates therefore reads running, which disables hung detection for exactly that case. test_only_the_spinner_clock_moving_is_still_hung passes only because both fixtures use ✻. Please normalize the spinner-line leader and add a test whose frames differ only by glyph. Also consider adding seconds to the <dur> units (Retrying in 5 seconds vs 4 seconds also reads as motion).
Non-blocking:
src/health_snapshot.py_alive: the docs say "a currentcrashedverdict", but_supervisor_sourcehas no freshness gate. Acrashedfile left by an input-watch that is no longer running, next to a fresh beat after a restart, givesalive: falseindefinitely (and hides Open terminal for a live session). Suggest gating onage_s.- Same prefix-match class still at head:
src/health-check.py:14672/14678(with the core dead,-t sutando-coreprefix-matchessutando-core-watchercreated bystart-cli.sh:59, so/schedule-cronsis typed into the watcher pane),src/agent/claude/cli/session-launch.sh:78,scripts/sutando-whoami.sh:43.tmux_probe.has_sessionis the shared owner; please route these through it or file a follow-up. - Part 2's before/after uses synthetic panes. The captured working panes in-repo (
tests/runtime-health.test.py:49-53) contain noesc to interrupt. Can you confirmframe_workingfires on a real pane from the current CLI?
Verified at bac918d: CI green, license/cla success, 100% diff coverage, tests call the production functions (no source-text asserts), no hardcoded host paths in added lines. Not verified: I did not run the suites locally, and did not check src/pool_remedy.py for bare -t targets.
— Sutando (susans-MacBook-Pro)
bassilkhilo-ag2
left a comment
There was a problem hiding this comment.
Reviewed at bac918d. The prefix-match bug is real and the fix is right almost everywhere, but one target breaks a live signal.
Blocker: src/runtime-health.py:508, _pane_text() now always returns an empty string.
- It uses
capture-pane -p -t "=sutando-core", with no:windowpart. - tmux 3.5a refuses that target. On a private socket,
capture-pane -t =sutando-coregivescan't find pane: =sutando-core(rc=1), while=sutando-core:and=sutando-core:0both return 0. - I ran
_pane_text()from each revision against a real pane showing a login marker. Main givespane_len=56 needs_login=True; this PR givespane_len=0 needs_login=False. - Impact:
needs_login(_pane_text())inderive()(:594) can never be true for the core again. That feedsscripts/sutando-config.sh:641(the desktop's authenticated/health reading) and the watcher'sneeds_logininput. Markers that only runtime-health's list matches, such as "invalid api key" and "run claude login", are lost. - Fix: use
f"={SESSION}:0", the shape the repo already uses atcore-input-watch.py:488,pool_supervise.py:148andpool_wedge_cards.py:72. - Why CI stayed green:
tests/runtime-health.test.py:338only checksisinstance(rh._pane_text(), str), and""passes that, so 100% diff coverage hides the regression. Please add a real-tmux test that_pane_text()returns the pane's contents. - Under lesson 15, the live evidence covered the seat and supervisor paths but not the core's runtime-health pane read.
Leave list-windows -t =sutando-core (:270) as it is. It takes a session target and works (rc=0 on the same socket).
Verified
- All 4 commits and the full diff reviewed. It's based on current main (0c03031), not stacked on #4944.
- Prefix bug reproduced on tmux 3.5a: a bare
has-session -t sutando-worker-abcreturns 0 while onlysutando-worker-abc-inputexists; the exact form returns 1. - The touched suites pass at the head: health-snapshot (48), core-input-watch (121), cli-wedge (102), tmux-probe (30), runtime-health.
- The new tests catch the bugs: run against main's
src/, health-snapshot fails 3, core-input-watch 4 plus 5 errors, cli-wedge 1, tmux-probe 2. - Adjacent suites pass (core-health-verdict, pool-wedge-frame-change, pool-supervision-wedge-rung and others), and none left processes running.
review-checks.sh --diffpasses.- CI is green except "PowerShell + Windows runtime", which skipped. The touched suites run in CI.
- The compound-duration regex can't join lines, because
normalizeworks one line at a time.
Nits (non-blocking)
health_snapshot.py:231:_alivetreats anycrashedsupervisor verdict as current, but_supervisor_sourcenever checks the file's age. The docs call it "a currentcrashedverdict". A file left atcrashedby a dead watcher would pinalive:falseover a fresh beat.reasonhas the same blindness, so this isn't a new failure class, but the docs overstate it._turn_moving: any frame change with "esc to interrupt" counts as motion, so a CLI looping on changing output within one turn readsrunning. That's acceptable given cli_wedge's retry-loop detection, but worth a line in the PR body.
Not verified: the desktop and cloud-pod before/after outputs in the PR body, whether the CI runners have tmux (i.e. whether the two real-tmux tests ran or skipped there), and the rebase against #4944.
tmux refuses capture-pane against a bare =session, so the pane read always came back empty and needs_login could never fire for the core. A real-tmux test now reads the pane.
…tions are not motion The CLI cycles the leading glyph of its spinner line within one turn, so a stalled turn whose glyph still animated read as moving; "Retrying in 5 seconds" did the same.
The supervisor file is written on change only, so a crashed verdict left by a watcher that has since died pinned alive:false over a running core. A fresh beat written after it that recorded a live core pane now supersedes it; workers already drop a previous run's verdict.
…nd whoami A bare -t prefix-matches a longer session: with the core gone, the /schedule-crons nudge found sutando-core-watcher and typed into its pane, and the launcher and whoami read the core as running.
The working marker sits in the footer ("· esc to interrupt ·"), and the spinner line's
glyph cycles within the turn; both are now pinned against a real capture.
|
Thanks to you both. Everything below is pushed: the head is now
Re-requesting review from you both. |
bassilkhilo-ag2
left a comment
There was a problem hiding this comment.
Approving at 1cfe4ee. My blocker is fixed, with a real test.
Blocker fixed:
src/runtime-health.py:508now reads=sutando-core:0.- The new test 6b starts a real core session plus a
-watchersession on a scratch socket. It checks that_pane_text()returns the core pane's own text (not the watcher's) and that the text feedsneeds_login. - The old
isinstance(str)check is replaced with a real assertion:== ""when no session exists.
The rest of the delta (bac918d..1cfe4ee) also checks out:
- Exact targets in three more places: the cron nudge (
has-session -t =session,send-keys -t =session:),session-launch.shandsutando-whoami.sh.health-check-recover-cronpasses. - Stale
crashednit addressed:_core_seen_sincedrops acrashedsupervisor verdict only when a fresh beat written after it recorded a live core pane (pid != heartbeat_pid). There are tests for all three cases: dropped, kept when the beat saw no pane, and kept when the verdict is newer than the beat. - cli_wedge:
- Spelled-out durations ("5 seconds", "2 minutes 1 second") now normalize to one field.
- The spinner line's cycling glyph is ignored only before a capitalized word followed by an ellipsis, so a markdown
*bullet is still content. There's a test for that. - A real Claude Code 2.1.286 mid-turn pane is pinned as a fixture.
Ran locally on 1cfe4ee (clean env, tmux 3.5a): runtime-health (green, including the real-tmux case), health-snapshot, cli-wedge, core-input-watch and health-check-recover-cron all pass. No leftover tmux processes.
CI: 28 checks pass, 1 skipped.
liususan091219
left a comment
There was a problem hiding this comment.
Re-reviewed at 1cfe4ee (incremental from bac918d).
Blocker resolved. I loaded src/cli_wedge.py from both revisions and ran the real state_id on 7 frames that differ only by spinner glyph: 7 distinct ids at bac918d, 1 at head. The real-fixture glyph swap gives a different id at bac918d and the same id at head. "Retrying in 5 seconds" vs "4 seconds" was unequal and is now equal. The new tests call production normalize/state_id/compose_state and fail on bac918d (2 in cli-wedge, 7 in core-input-watch). The markdown-bullet test is a useful guard.
Non-blocking items, all addressed:
_core_seen_sinceonly dropscrashedwhen a later fresh beat recorded a core pid.core_heartbeatsets that pid only for a pane whose argv names the session, so this is better than the age gate I suggested.- The exact
=sessiontargets in the cron nudge,session-launch.shandsutando-whoami.share covered by a real-tmux test. - The real 2.1.286 pane fixture confirms
frame_workingfires.
Nit (no action needed): the glyph fold matches a single capitalised word only. ✻ Thinking hard… is not folded, and a * Loading... bullet is.
Not verified: a repo-wide sweep for other bare -t targets.
— Sutando (susans-MacBook-Pro)
sonichi
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES at exact head 1cfe4eebe. The two approvals here are real — both reviewers re-read this head and their earlier blockers are resolved — and the three parts of the fix hold under mutation (below). One thing neither caught:
Blocker — src/runtime-health.py:508 reads the wrong window on a host that numbers tmux windows from 1. _pane_text() now targets f"={SESSION}:0". The launchers start tmux without -f, so the user's ~/.tmux.conf applies, and with set -g base-index 1 the core window is :1; :0 then fails, _pane_text() returns "", and needs_login can never fire for the core — the very outcome Bassil's blocker was about, reintroduced for those hosts. Main's bare -t sutando-core read the right pane there. Measured in my read-through on a private socket with a core pane printing "Please run /login" and a sutando-core-watcher decoy, loading each revision's runtime-health.py in turn:
## tmux conf: /dev/null core windows: 0
main.py 'Please run /login' needs_login= True
pr.py 'Please run /login' needs_login= True
## tmux conf: base-index 1 core windows: 1
main.py 'Please run /login' needs_login= True
pr.py '' needs_login= False
The repo already owns this policy: src/cli_wedge.py:443 core_target() resolves =<session>:<lowest window index> "whatever base-index says — a fixed :0 assumes base-index 0", and health-check.py:814 already routes through it (on the base-index-1 socket it returned =sutando-core:1). So _pane_text copies targeting policy that has an owner, which is the duplicated-policy defect CLAUDE.md names; please route it through cli_wedge.core_target + capture_pane, and give test 6b a base-index-1 arm (it runs only with default numbering, so it cannot see this).
Nits, not gating:
src/core-input-watch.py:488,498carry the same:0assumption; it predates this PR (main already had{session}:0, this PR adds=), but one consequence is that_turn_movinggets no pane on a base-index-1 host, so Part 2 silently does nothing there. Same owner would fix both.cli_wedge.normalizechanged andstate/cli-wedge/window.jsonlpersistsstate_idhashes (:693,:805), so the first window after an upgrade mixes old and new hashes and counts one spurious novel state; from the code, only that window's confidence is affected (_classify_run's idle verdict comes fromraw_static). Read, not run.- For a Codex core,
core_pid()falls back to any pane pid in the exact session, so_core_seen_since(health_snapshot.py:116) treats "session exists" as "core seen"; looks right (the watcher won't writecrashedwhile the session exists). Read, not run.
Run at this head in a clean env (env -u TMUX -u TMUX_PANE -u SUTANDO_*): cli-wedge 105/105, core-input-watch 123/123, health-snapshot 51/51, tmux-probe 30/30, runtime-health PASS (6b ran on tmux 3.6b), health-check-recover-cron OK. (With a live worker's SUTANDO_* in the env, 3 cli-wedge tests fail identically on main — an environment leak in those tests, not this PR.) git diff origin/main...pr4945 | bash scripts/review-checks.sh → PASS; no added comment over 2 lines; nothing new on by default, no new config. Mutation controls, each reverted after: tmux_probe.has_session with a bare name → 2 tmux-probe failures + test_a_dead_seat_whose_input_session_lives_on_is_crashed; _turn_moving forced False → 3 TestMovingTurnIsNotHung failures; the not-answering exception removed → test_a_dead_worker_the_pool_gave_up_on_reads_not_answering_not_offline fails; _pane_text back to ={SESSION} → both real-tmux _pane_text checks fail. Not verified: the Part 1/Part 2 bugs on main (desktop, cloud pod), how common base-index 1 is across installs, the Codex path and the window.jsonl transition (read only), whether CI runners have tmux, how this rebases onto #4944 (the two conflict in src/health_snapshot.py).
sonichi
left a comment
There was a problem hiding this comment.
Reviewed at 1cfe4eebe, merge-base 0c03031b. Both standing blockers are fixed. One residual of liususan's blocker is still live; the fix is one character.
Blocker
src/cli_wedge.py:104 the spinner-glyph fold misses every hyphenated spinner verb, so a frozen turn with one of them still reads running. The lookahead is (?=[A-Z][A-Za-z]+(?:…|\.\.\.)), which stops at the hyphen. I extracted the spinner vocabulary the installed CLI ships (strings .../claude-code/bin/claude.exe, 176 gerunds) and ran each through normalize with 7 glyphs:
verbs: 176 | hyphenated: 5 ['Dilly-dallying','Fiddle-faddling','Razzle-dazzling','Sock-hopping','Topsy-turvying']
glyph-only change STILL reads as different for: 5 (the same five)
'✢ Re-ticulating… (3s)' -> '✢ Re-ticulating<dots> (<dur>)' # not folded
'✢ Hatching… (3s)' -> '<spin> Hatching<dots> (<dur>)' # folded
So about 1 turn in 35 keeps the exact hole #4945 set out to close. Add - to the class and pin one hyphenated verb in TestMovingTurnIsNotHung.
Nits
- The verb itself rotates mid-turn and is not folded. Live capture, one turn:
✢ Catapulting…then✢ Marinating…for 39 samples. A frame pair straddling a re-roll readsrunning. I only saw the re-roll at a content boundary, so I cannot say a frozen turn re-rolls. src/health_snapshot.py:285,350the worker side of liususan's_alivenit is still open._core_seen_sinceguards the core only; a worker'scrashedfile is gated by_worker_started_at, which returnsNonewhen its records are unreadable, and then no gate applies. Probe:_alive({"value":"fresh"}, {"opinion":{"reason":"crashed"}}) -> False. The PR body's "workers already drop a previous incarnation's verdict" holds only when that record parses.sincecan now benullon the offline path. Withescalated: trueandfirst_detected_atabsent,_verdictreturnsreason: not-answering, since: null, where main returned the beat mtime.- Same prefix-match class still at head, not covered by any guard:
src/watch-tasks-stream.sh:979(send-keys -t "$TMUX_SESSION"),src/agent/codex/cli/task-notifier.sh:248,263(-t "$SESSION:0"). Six call sites now hand-build=;tmux_probe.has_sessionis the shared owner and nothing stops the seventh regressing. - The new
_pane_texttmux test creates both sessions, so it also passes against main; a watcher-only case would pin the prefix hazard.
Existing reviewers
- bassilkhilo,
_pane_textempty: addressed (8fdd19f9). - liususan, glyph cycle: mostly addressed (
20d615ab), see blocker. - liususan #2, prefix class elsewhere: partly (
cd497d29); see nit 4. - liususan #3, real pane: addressed (
1cfe4eeb). _alivefreshness: core yes (99d76222), workers no (nit 2).
What I verified
tmux 3.5a, private socket, watcher only:
has-session -t sutando-core rc=0 (prefix match, the bug)
has-session -t =sutando-core rc=1
send-keys -t "=sutando-core:" rc=1 (core absent, nudge correctly skipped)
with core present: core pane 2 hits of /schedule-crons, watcher pane 0
Suites at head, all OK: health-snapshot 51, cli-wedge 105, tmux-probe 30, core-input-watch 123, runtime-health, recover-cron. Head tests against main's src/ fail 3/2/5/4+12E/5, so they discriminate. review-checks.sh --diff PASS. gh pr checks: all pass, Windows skipping. No hardcoded host paths in added lines.
What I did not verify
The desktop and cloud-pod before/after in the PR body; whether CI runners have tmux; the rebase onto #4944. No live core, bridge or worker was restarted.
john-the-dev
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES at 1cfe4eebe (merge-base 0c03031b, 16 files, +403/-30). The other two parts of the PR do what they claim, and the new crashed rules only make verdicts less green. But two changes can make a real failure read healthy. Both have a repro below, both have a small fix, and both independently confirm @sonichi's standing blockers at this head.
Blocking
1. src/runtime-health.py:508: _pane_text() returns "" on a host with tmux base-index 1, so a logged-out core can no longer read needs_login. The launcher (session-launch.sh:359) starts tmux without -f, so the user's ~/.tmux.conf applies, and base-index 1 is a common setting. Main's bare -t sutando-core resolves the session's current pane. The PR's ={SESSION}:0 names a window that does not exist on such a host. My repro used a private socket, a conf with only set -g base-index 1, and one session printing Please run /login. It loaded src/runtime-health.py from each revision:
windows: 1
0c03031b 'Please run /login' needs_login= True
pr4945 '' needs_login= False
This regresses main: the failure signal that should page the owner goes silent. The repo already owns this policy in cli_wedge.core_target() (src/cli_wedge.py:443), which resolves =<session>:<lowest window index>, and health-check.py already routes through it. Please call it here and add a base-index-1 case to tests/runtime-health.test.py. The current 6b test also passes against main's src/ (I ran it: PASS — runtime-health green with the source reverted), so it does not pin this.
2. src/cli_wedge.py:105: the spinner-glyph fold skips hyphenated spinner verbs, so a frozen turn using one still reads running (healthy) instead of hung. The lookahead [A-Z][A-Za-z]+ stops at the -. Repro against the real state_id at head, comparing two frames that differ only in glyph, elapsed time and token count:
Hatching same id
Dilly-dallying DIFFERENT id (reads as motion)
_turn_moving (core-input-watch.py) then returns running for a stalled turn. @sonichi counted 5 of the CLI's 176 gerunds as hyphenated. Fix: [A-Za-z-]+, and pin one hyphenated verb in the moving-turn tests.
Not blocking
src/core-input-watch.py:488,498(={session}:0) carry the same:0assumption. Main had{session}:0, so this is not a regression, but on a base-index-1 host_turn_movinggets no pane and Part 2 silently does nothing. The samecore_target()fixes both._core_seen_since(health_snapshot.py:116). I checked that it cannot clear a real crash: the beat records the core pid only through exact-t ={sess}targets (core_heartbeat.py:343,410), and only a verdict older than that beat is dropped.- The
offline→not-answeringchange and_alive(..., supervisor)both stayabnormal/False, so neither one moves anything toward green.
Ran at head (clean env: env -u TMUX -u TMUX_PANE -u SUTANDO_*)
cli-wedge 105/105; core-input-watch 123/123; health-snapshot 51/51; tmux-probe 30/30
runtime-health PASS (real-tmux 6b ran); health-check-recover-cron OK
/bin/bash tests/sutando-whoami.test.sh PASS (bash 3.2)
gh pr diff 4945 | scripts/review-checks.sh --diff at head: PASS. gh pr checks: 28 pass, 1 skipping.
Revert-control
I reverted src/ and scripts/ to 0c03031b and kept the tests:
- cli-wedge: failures=3
- core-input-watch: failures=4, errors=12
- health-snapshot: failures=5
- tmux-probe: failures=2
- health-check-recover-cron: rc=1
- runtime-health: still PASS. This is why blocker 1 went uncaught.
Overlap with #4944
The test merge conflicts textually in two lines of snapshot() (session vs _alive's new argument). With both kept, and #4952 merged on top, every touched suite passes (see my #4944 review).
Not verified
- The desktop and cloud-pod before/after in the body.
- Whether CI runners have tmux, i.e. whether the real-tmux cases ran there.
- Whether a frozen turn ever re-rolls its spinner verb.
…r base-index says A fixed :0 names no window on a host whose ~/.tmux.conf sets base-index 1, so the core pane read empty and needs_login could never fire. Both now go through cli_wedge.core_target, the shared owner health-check already uses.
Five of the CLI's spinner gerunds carry a hyphen (Dilly-dallying, Topsy-turvying, ...), and the fold stopped at it, so a frozen turn with one of them still read as motion.
…is known; not-answering keeps a since Without a readable incarnation the verdict cannot be shown to be this run's, so alive stays the beat's while the condition still reads crashed. A given-up worker with no recorded first detection takes the beat's since instead of null.
…actly A bare -t prefix-matches <session>-watcher once the core is gone, and a fixed :0 misses a base-index-1 host. A guard scans src/ and scripts/ so new bare targets fail; the launchers and attach helpers still carrying them are counted as a follow-up.
|
@sonichi @john-the-dev, thanks. All three round-2 reviews are addressed at Blocker:
Blocker: hyphenated spinner verbs (sonichi review 2, john-the-dev 2) —
sonichi review 2, nits
sonichi review 1, other nits: john-the-dev, "Not blocking": the The touched suites pass in a clean env. The launcher/notifier suites match the baseline. |
Addressed at 85e258e: b8eaf75 widens the glyph-fold lookahead to [A-Z][A-Za-z-]+ and pins it with test_a_hyphenated_spinner_verb_cycles_its_glyph_without_a_new_state (Dilly-dallying, Re-ticulating, Topsy-turvying); 268c74c routes _pane_text through cli_wedge.core_target, closing the base-index-1 item too. Verified by reading the head over the API, not by re-running the suites, so this is a dismissal rather than an approval.
sonichi
left a comment
There was a problem hiding this comment.
APPROVE at exact head 85e258efc — my 1cfe4eebe blocker is fixed and pinned, and the nits are gone.
Blocker (base-index-1 _pane_text) → fixed in 268c74c2c. src/runtime-health.py:508-511 now reads cli_wedge.core_target(sock, SESSION) + cli_wedge.capture_pane(...), the owner of that policy. Test 6c pins it. Revert control, head tests against 1cfe4eebe's src/runtime-health.py:
FAIL real _pane_text reads the core on a base-index-1 host
FAIL real _pane_text feeds needs_login on a base-index-1 host
FAIL — 2 failing
and against origin/main's (the bare -t sutando-core, which did read the core on such hosts): 6c passes, and only the new exact-target case fails (real _pane_text never reads the watcher when the core is gone), so the suite now discriminates both the :0 regression and the prefix-match bug.
Nits: src/core-input-watch.py:487,493 route through core_target too; no :0 target remains in core-input-watch.py or src/agent/codex/cli/task-notifier.sh.
Ran at head, clean env (env -u SUTANDO_* …): runtime-health PASS (6b/6c on tmux 3.6b), core-input-watch 126/126, cli-wedge 106/106, tmux-session-targets-are-exact 2/2, health-snapshot 53/53, switch-model 33/33, git diff origin/main...HEAD | scripts/review-checks.sh PASS. CI at head: 27 success, 1 skipped.
Two red results I attribute to this host, not the PR — with controls:
tests/codex-core-launcher.test.py::test_launches_codex_and_managed_task_notifier("managed core-input monitor did not start") fails identically onorigin/main, at head, and with1cfe4eebe's notifier under head's tests. Cause:start-cli.sh:254skips the monitor whenpgrep -f "core-input-watch\.py .*--socket ${TMUX_SOCKET}"matches, the test's launcher defaults to/tmp/sutando-tmux.sock, and this host runs live monitors on that socket. Three more launcher cases failed only under load (full suite concurrent with another run) and pass 4/4 in isolation.switch-model.test.sh3 FAILED with a live worker'sSUTANDO_*in the env, on main and at head; 33/33 once scrubbed.
Not verified: the desktop/cloud-pod before/after in the body; the earlier approvals (Bassil, Susan) are at 1cfe4eebe, so the branch rule still needs one at this head; john-the-dev's 1cfe4eebe block is on the glyph fold, which b8eaf7526 addresses and pins — his to lift.
john-the-dev
left a comment
There was a problem hiding this comment.
APPROVE at 85e258efcdb029ba83af7771cdd819961c8e4f19. This lifts my CHANGES_REQUESTED from 1cfe4eebe. Both of my blockers are fixed.
Blocker 1: tmux base-index (fixed in 268c74c2).
runtime-health.py:_pane_textandcore-input-watch.py'scaptureandsend_keysnow resolve their target throughcli_wedge.core_target(). That returns=<session>:<lowest window index>fromlist-windows, andNonewhen there's no reading. It replaces the hardcoded:0.send_keysrefuses to type when no target resolves, so a missing reading never falls through to a prefix-matched sibling.tmux-session-targets-are-exact.test.pypins the targets.
Blocker 2: hyphenated spinner verb (fixed in b8eaf752). The fold at cli_wedge.py:105 now accepts [A-Z][A-Za-z-]+, and the moving-turn tests pin Dilly-dallying.
What I ran at this head (macOS):
cli-wedge,core-input-watch,runtime-health,tmux-session-targets-are-exactandhealth-snapshot: all OK.- Revert controls:
- Only the
cli_wedge.py:105regex put back to[A-Za-z]+:cli-wedgefails 3. - Only
src/runtime-health.pyreverted to1cfe4eebe:runtime-healthfails 2. - So both fixes are actually under test.
- Only the
codex-core-launcher.test.py: 8 failures, all in notifier and launch tests. The identical 8 also fail at the merge-base, so this is my machine, not the PR. That file exercisestask-notifier.sh, which this push changed. For the notifier's newcore_targetpath I'm relying on CI, which shows 28 pass and 1 skipped.
Non-blocking:
- The shell copies in
task-notifier.sh:core_target()andwatch-tasks-stream.sh:_tmux_wakere-implement the same lowest-window rule ascli_wedge.core_target. Today that's onelist-windows | sort -n | head -1, so drift risk is small. If the rule ever changes (for example, a named core window), those are the two copies to update. cc24ae7b(health_snapshot: alive only for the current incarnation; not-answering keeps asince) is outside my blockers. I read it, andhealth-snapshot.test.pypasses. I didn't review it beyond that.
|
@cla-assistant check |
#4303)" (#5166) * Revert "feat(agy-core): Slice 2 — task-file-injection notifier for agy (Antigravity CLI) (#4303)" This reverts commit 4a72ddf. #4303 was green on its own head but was not re-run against #4976 and #4945, which had landed on main; together they turned four notifier and tmux tests red on main. The forward fix (#5161) is still in review, so main is restored first; agy Slice 2 re-lands with that fix once it is approved. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(agy): retire the task notifier's leftover watcher session on rerun The reverted launcher no longer knows about the "<session>-watcher" tmux session #4303 started, and it returns early once the core session exists, so an update left the old notifier running. Every run of start-cli.sh now kills an exact-name leftover watcher on its own socket before that guard. Regressions run a real tmux server on an isolated socket. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
… tmux targets #4303 merged after #4976 and #4945 without being re-run against them: - #4976 makes watch-tasks-stream.sh clear SUTANDO_INSTANCE_ID on the core's canonical inbox, so the agy notifier's watcher stamps watch-tasks-stream.pid while the notifier polled the per-instance name it forced. The launch receipt never appeared ("did not report ready within 12s"). The notifier and start-cli.sh now run the watcher with no instance id, as the Codex core does. - #4945's scan forbids bare `-t "$SESSION"` targets; the notifier's six capture-pane/send-keys calls now use the exact "=$SESSION:" form. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… tmux targets #4303 merged after #4976 and #4945 without being re-run against them: - #4976 makes watch-tasks-stream.sh clear SUTANDO_INSTANCE_ID on the core's canonical inbox, so the agy notifier's watcher stamps watch-tasks-stream.pid while the notifier polled the per-instance name it forced. The launch receipt never appeared ("did not report ready within 12s"). The notifier and start-cli.sh now run the watcher with no instance id, as the Codex core does. - #4945's scan forbids bare `-t "$SESSION"` targets; the notifier's six capture-pane/send-keys calls now use the exact "=$SESSION:" form. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Health verdicts that match what is really running. Three fixes found by testing HealthStatus live on the desktop, consolidated so they're reviewed once:
not-answering, notoffline;running, nothung;crashedwithalive: false, not its core'sidle.It absorbs #4948 and #4953. Merge order: the HealthStatus PR (#4944) → this PR → #4952. After #4944 lands, this branch is rebased; the only overlap is the
alive/sessionlines insrc/health_snapshot.pyanddocs/health-snapshot.md.Part 1: given up → not-answering (was #4945)
What
src/health_snapshot.py_verdict: when a worker's beat is stale (offline) and a fresh pool sample says the pool gave up on it (escalated→not-answering), the reason isnot-answeringwith the pool'ssince. Everything else about "offline wins" is unchanged: motion staysunknown, and a dead agent's old supervisor verdict is still ignored.Why
Consumers treat
offlineas a no-attention reason, because the pool normally restarts a dead worker within minutes. Butofflinewon outright, so a worker the pool had abandoned looked the same as one about to come back. Found testing HealthStatus on a real desktop workspace: worker40659240…had been dead for 29 h.state/pool-supervision.jsonhadescalated: true, consecutive: 351for it, yet/healthsaidoffline.Before / after
python3 src/health_snapshot.py --workspace <desktop workspace> --agent 40659240fd884f63bcd19fa684b451f1Before (origin/main):
After (this branch):
Checks
python3 tests/health-snapshot.test.py: 46 tests OK. New:test_a_dead_worker_the_pool_gave_up_on_reads_not_answering_not_offline.ruff check: pass.scripts/review-checks.sh --diff <(git diff origin/main...HEAD): PASS.skills/release/scripts/docs_audit.py: PASS.docs/health-snapshot.md: the combining rule now documents the exception.Not a live path (read-only snapshot), so the before/after above is the evidence.
Part 2: mid-turn → running, not hung (was #4948)
What
src/core-input-watch.py:compose_statemaps runtime-healthunknown(a live session whosecore-status.jsonsaysrunningbut is >90 s stale, or is missing) tohung. Today the only way out is a pane that positively shows the idle footer.cli_wedge.frame_working, "esc to interrupt") and has changed since the last poll, it readsrunning/ "actively processing".cli_wedge.state_id(normalised), so clocks, token counters and spinners don't count as change.hung:derive(), and all prompts.src/cli_wedge.py:normalizenow treats a compound duration (3m 12s,1h 3m 12s) as one<dur>field. Before,12s→<dur>but3m 12s→<dur> <dur>, so a spinner whose timer passed a minute counted as new output. That contradictsnormalize's own contract ("frames differing only in clocks, counters, durations or spinners compare equal").Why
This is the real case, on a cloud Sutando pod.
state/core-status.jsonwas{"status":"running","step":"Assessing system for autonomous loop tick","ts":1790669508}. That's about 19 h old: it carried over on the workspace volume from before the pod was reprovisioned, and the loop never setidle.state/core-supervisor.jsonread{"state":"hung","detail":"core alive but stalled (status stale, no recognized prompt)"}./health(docs/health-snapshot.md), so a working core would show as abnormal.This PR is the Sutando half. The AG2 Space backend also resets
core-status.jsontoidleat pod boot, since a freshly started core has no turn in flight.Before / after
compose_state(pane, "unknown", True, prev_pane=…)on three captured panes (script: load the module at each revision, same inputs):In these panes, "working" has a new tool call above the spinner line. "Only the clock ticked" is the same frame with the spinner at
3m 12s · ↓ 4.8k tokensinstead of12s · ↓ 1.2k tokens.Checks
python3 tests/core-input-watch.test.py: 119 OK. NewTestMovingTurnIsNotHungcovers:main()run that carries the capture between polls (hung, then running)python3 tests/cli-wedge.test.py: OK. Newtest_a_compound_duration_is_one_field.tests/core-input-watch*.test.pytests/*wedge*.test.pytests/health-snapshot.test.pysrc/core-input-watch.pyandsrc/cli_wedge.pyis covered.ruff check,scripts/review-checks.sh --diff,skills/release/scripts/docs_audit.py: PASS.No live restart of any core, desktop engine, bridge or worker. The supervisor's input is the pane capture and the status file, both exercised above.
Part of the Agent Health work (HealthStatus).
Part 3: dead seat → crashed, alive false (was #4953)
What
These now target the tmux session exactly (
-t =<session>), which is the form the rest of the repo already uses:src/tmux_probe.has_sessioncaptureandsend_keysinsrc/core-input-watch.pyruntime-health's two pane reads (list-windowsandcapture-pane)has_sessionleaves a target that already starts with=alone, becausestart-cli.sh's relay loop passes=<session>itself.Why
A worker was killed with Ctrl+C in its pane, and its session
sutando-worker-<id>ended./healththen showed that worker healthy while the desktop's Open terminal said it "isn't running".Cause: tmux treats a bare
-t nameas a prefix. Once the seat ended, the probe forsutando-worker-<id>matched the seat's own input-watch session,sutando-worker-<id>-input. That made the seat's watcher:idleself-report fromderive();idle-ready.state/watchers/<id>.aliveis beaten by the separate-watchersession, so it stayed fresh too.health_snapshottherefore read a dead worker as green until the pool's ladder caught up.Fixing the target makes the watcher see the absent session within one poll (3 s) and write
crashed, before the per-seat loop, which checks every 30 s, stops it.health_snapshotneeds no change:crashedalready maps to abnormal, and the pool ladder (offline→not-answering) still takes over later.Also sent to the wrong pane before this fix:
send_keyscould type an auto-answer into the-inputpane.Before / after
Setup: a private tmux socket and a throwaway workspace.
core-status.jsonisidlein the resolved workspace, which is the state on Mark's Mac.sutando-worker-40659240…runs, plus the realpool_remedy.input_watch_commandloop.-watchersession does.health_snapshot.py --agent <id>.Before (origin/main):
After (this branch):
With no core self-report (e.g. CI), main wrote
hungfor the dead seat instead ofcrashed, for the same reason.Tests
tests/tmux-probe.test.py:=core;=is not prefixed again;seat-inputsession does not makeseatread present.tests/core-input-watch.test.py:<seat>-inputalive,--oncewritescrashed;=<seat>:0.tests/health-snapshot.test.py: a fresh inbox beat plus acrashedseat file readsabnormal · crashed.runtime-health,core-health-verdict,core-heartbeat-refused-client,start-cli-relay-loop-dies-with-its-repo,pool-input-watch-per-seatandhealth-snapshot.ruff,scripts/review-checks.sh --diff,docs_audit.py.Not live
This is a probe-targeting change on the supervisor path, so no bridge restart. The real-tmux tests and the before/after above are the round trip. No real core, worker, app engine or tmux socket was touched.
Second commit (940a9ee): a crashed session is not alive
Verified live on the desktop test build: after the seat died, the worker read
abnormal · crashedwithin about 10 s. But it still readalive: truefor up to 90 s, because its inbox watcher (the-watchersession) beats from its own session and outlives the seat. Consumers that keyed onaliveoffered Open terminal for a session that was gone.health_snapshot._alivenow returnsfalsewhen the current supervisor verdict iscrashed. This applies to the core as well, since its heartbeat writer is also a separate process.Tests:
test_a_seat_the_watcher_saw_end_is_crashed_while_the_inbox_beat_is_freshnow expectsalive: false.test_a_core_the_supervisor_saw_crash_is_not_alive_while_its_beat_is_fresh.docs/health-snapshot.md: thealivefield and combining rule 5 are updated.Review follow-ups (head 1cfe4ee)
Trade-off in Part 2. A CLI that loops on changing output within one turn reads
running, nothung. That case is cli_wedge's: its retry-loop detection classifies repeated retries across the sample window.Changed behaviour, before (bac918d) vs after (1cfe4ee). Each run uses a private tmux socket with a
sutando-corepane showing "Please run /login" and asutando-core-watchersession, run from each revision's source:runtime-health._pane_textnow reads={SESSION}:0. tmux refusescapture-paneagainst a bare=session, so before this change the core's pane always read as empty./schedule-cronsnudge,session-launch.shandsutando-whoami.shuse exact targets. Before, with the core gone, the nudge typed intosutando-core-watcher.cli_wedge.normalizefolds the spinner line's cycling leading glyph and spelled-out durations ("5 seconds").crashedcore verdict is dropped when a fresh beat written after it recorded a live core pane, i.e. itspidis the core's, notheartbeat_pid. Workers already drop a previous incarnation's verdict.Real pane.
tests/fixtures/pane-claude-turn-in-flight.txtwas captured mid-turn from Claude Code 2.1.286, on a private tmux socket runningclaudein an empty directory:⏵⏵ auto mode on (shift+tab to cycle) · esc to interrupt · ← 2 agents.✢ Transmogrifying… (1s · ↓ 113 tokens · thinking); within the same turn the glyph was seen as·too.frame_workingfires on that capture. The new test pins it, along with the glyph cycle (still hung) and real output growth (running).Review follow-ups (round 2, head
85e258efc)base-index 1host (268c74c2c).runtime-health._pane_text(), pluscore-input-watch'scaptureandsend_keys, now go through the shared ownercli_wedge.core_target()(=<session>:<lowest window index>) andcli_wedge.capture_pane. That's the pathhealth-checkalready uses; a fixed:0named no window there.needs_loginfires_pane_text()returns""and never reads the watchercore-input-watch.captureruntime-health.py:core-input-watch.test.py -k base_indexagainst1cfe4eebe'score-input-watch.py:FAILED (failures=1).b8eaf7526). The glyph fold's word class now allows-.tests/cli-wedge.test.py(Dilly-dallying, Re-ticulating, Topsy-turvying) and inTestMovingTurnIsNotHung.1cfe4eebe'scli_wedge.py: cli-wedgefailures=3, core-input-watchfailures=6.crashed→alive(cc24ae7b0). A worker'scrashedverdict setsalive: falseonly when its current incarnation is readable. Without that, it can't be shown to be this run's, soalivestays the beat's while the condition still readscrashed.test_a_crashed_verdict_of_unknown_incarnation_cannot_override_a_fresh_beat.sinceonnot-answering(cc24ae7b0). When the pool didn't recordfirst_detected_at, it falls back to the offline opinion'ssince(the beat mtime) instead ofnull.test_a_given_up_worker_without_a_first_detection_takes_the_beats_since.85e258efc).watch-tasks-stream.sh_tmux_wakenow targets=$TMUX_SESSION:<lowest window>.codex/cli/task-notifier.shgains acore_targetthat every capture and send-keys goes through. With no core window it refuses to type.tests/tmux-session-targets-are-exact.test.py, scanssrc/andscripts/for bare-tsession targets.FOLLOW_UP, and that count may only fall.origin/mainthe scan finds 43; on this head, 27, all inFOLLOW_UP.agent/{claude,codex,agy}/cli/start-cli.sh), the attach helpers (src/Sutando/main.swift,runtime-api/instance_registry.py,runtime-cli/sutando-runtime.py),scripts/tmux-send-line.sh,src/tmux-status.ts, and theTARGET/NUDGE_TARGETdefaults in the two notifier scripts.codex-core-launcherandswitch-model's fake tmux now answerlist-windowswith one window. Without it,sutando-config.sh runtime→ runtime-health no longer captured, and switch-model's capture count shifted.✢ Catapulting…→✢ Marinating…). It isn't folded, because it can't be told apart from real change, so a frame pair straddling a re-roll readsrunning.normalizechanged, andstate/cli-wedge/window.jsonlkeepsstate_idhashes. So the first window after an upgrade mixes old and new hashes, and counts one spurious novel state. Only that window's confidence is affected.Checks, in a clean env (
TMUX,TMUX_PANEandSUTANDO_*unset):ruff,review-checks.sh --diffanddocs_audit: PASS