[sonichi]

Why

Owner directive, 2026-08-08: "we need to removed it!"

A second wedge-restart inside one hour pinned SUTANDO_CORE_MODEL=opus, moving the core to the standard 200K window so it kept working instead of re-wedging. The escalation worked. Its lifetime did not:

nothing cleared it	the only setenv -u in the repo is inside an f-string — the text of a warning, not a command anything runs
nothing expired it	no timestamp was written, so no reader could tell a 5-minute-old pin from a 17-day-old one
restarts inherited it	start-cli.sh re-supplies the var into the new session and new-session -A discards its own -e

So one recovery became a permanent downgrade. Measured on a peer core in #2717: 17 days on the pin, 165 autocompactions in 25h — one every 9.1 minutes — found only because the owner noticed the symptom and asked. That has now happened twice, which is what prompted this.

Why the two previous attempts didn't hold

• --recover-core auto-restart is unreliable: single soft signal triggers a destructive restart (#1428) #2246 removed --recover-core from the launchd template default. That stops the automatic trigger on new installs only — editing a template does not rewrite a plist already on disk.
• fix(health-check): report a core running the wedge-recovery model downgrade #2717 (mine) added a probe that reports the pin. A detector, not a fix, and its own body says so: "Reporting only: clearing is an operator action."

This removes the mechanism instead.

What changes

• _default_core_restart() takes no argument and sets no model.
• recover_core_if_wedged drops standard_context / mode / ctx_note. The owner DM read "Auto-restarting in standard 200K context (the 1M restart didn't hold)" — with the pin gone that sentence would be false, so it goes too, along with the mode key and last_restart_mode.
• RECOVER_MAX_PER_HOUR + the existing give-up DM remain the bound on a re-wedging core. That backstop is what made the model escalation optional in the first place: a pathological wedge still stops after N restarts and pages the owner. Removing the downgrade does not open an unbounded restart loop.
• The core-model-pin probe STAYS, re-worded. Nothing sets the pin now, so a pin means legacy-or-manual — exactly the case still suspected on a peer host, and the probe is how anyone finds it.

Tests — inverted, not deleted

Both suites asserted the old behaviour, so removing it had to flip them:

health-check-core-supervisor  test_default_restart_selects_provider_specific_model_env
                          ->  test_default_restart_never_sets_a_model_env

health-check-recover-core    case_g_recurrence_escalates_to_standard
                          ->  case_g_recurrence_does_not_downgrade_the_model

case_g also now asserts the DM no longer promises a downgrade ("200K" / "didn't hold" must not appear), because a stale reassuring message is its own defect.

At the parent commit the new tests give 6 failures + 1 error:

n) raised TypeError: restart() takes 1 positional argument but 2 were given
6 failure(s)
TypeError: _default_core_restart() got an unexpected keyword argument 'standard_context'

At this head: health-check suites: pass=60 fail=0.

Live-path evidence — real core restart on this exact head (2026-08-08T17:20Z)

Closes the live-path gate. Exercised on the running core at c7dad2d76230aa7653ce977fea03938df4289049, which is this PR's exact head — not a lookalike build.

Before the restart, the pin was latent but armed. SUTANDO_CORE_MODEL=opus sat in tmux's global scope while the running core's argv carried no --model, so the pin could only bite on the next new-session — which is exactly what a restart performs. That makes it a clean discriminator: on the pre-fix code start-cli.sh re-supplies the var from its launching env, so the new core must come up pinned.

before  tmux show-environment -g SUTANDO_CORE_MODEL   -> SUTANDO_CORE_MODEL=opus
        core pids                                     -> 84387 / 84389

After the restart:

tmux show-environment -g SUTANDO_CORE_MODEL           -> unknown variable
tmux show-environment -t sutando-core SUTANDO_CORE_MODEL -> unknown variable
live core argv, count of "--model"                    -> 0
core pids                                             -> 98897 / 98898   (genuinely new process)

Probe verdict flipped at the owner-visible endpoint:

d709f4a7  pre-restart   warn   core is PINNED by wedge recovery (global='opus')
c7dad2d7  pre-restart   warn   core model is PINNED (global='opus')
c7dad2d7  POST-restart  ok     no model pin on any session or the global env

Durability check. The checkout was then restored to main (d709f4a7) and the probe re-run there: still ok, both tmux scopes still clear. So the cleared state is a real state change, not an artifact of standing on the PR head.

Full before/after capture: workspace/notes/2742-restart-proof-2026-08-08.md.

Still not verified by me

No wedge was induced. The restart above was a real start-cli.sh --restart through the normal launcher path, which is what exercises the -e-discarding new-session -A behaviour this PR targets; recover_core_if_wedged's own decision logic is still exercised through the injected restart_fn. A queued-task/result round trip did run across the restart — the post-restart core drained four owner tasks from tasks/ and wrote their results/ files.

@sonichi

[sonichi]

Independently verified at head 335d342 in a clean worktree. The body's central claim — that the tests fail at the parent and pass at head — holds, and I re-ran it rather than taking it on description.

At head 335d3426:

tests/health-check-core-supervisor.test.py   Ran 9 tests   OK
tests/health-check-recover-core.test.py      All core-recovery invariants hold.

Same test files against the parent's src/ (c9db288f):

health-check-core-supervisor:
  TypeError: _default_core_restart() missing 1 required positional argument: 'standard_context'
  Ran 9 tests   FAILED (errors=1)

health-check-recover-core:
  ✗ case n — TypeError: Harness.restart() takes 1 positional argument but 2 were given
  6 failure(s)

6 failures + 1 error, matching the body exactly.

One thing I checked that the body asserts but does not show: that this is the only setter. Full tracked census of the variable:

src/health-check.py:7788                         env["SUTANDO_CORE_MODEL"] = "opus"   <- removed here
src/agent/claude/cli/start-cli.sh:316,376        reads + propagates
src/agent/codex/cli/start-cli.sh:108,149         reads + propagates
skills/task-workstream-sessions/.../worker.py    reads
docs/ + tests/                                   text only

No other writer. So removing this line does end the mechanism, not just one path into it.

Operational note, not a review blocker. Merging stops new pins; it does not clear one already set. This host is pinned right now in both scopes:

tmux -S /tmp/sutando-tmux.sock show-environment -t sutando-core
SUTANDO_CORE_MODEL=opus

health-check: core-model-pin  warn  (global='opus', sutando-core='opus')

Which is the second confirmed instance, and the reason the owner asked for a permanent fix today. The re-worded probe is what makes that leftover findable afterwards, so keeping it is the right call.

The "Not verified by me" section is accurate — I also did not induce a live wedge, and I don't think this change needs one: the removal is of an env assignment, and the assertion that no runtime receives it is what the census above establishes.

No blockers from me.

[qingyun-wu]

Thanks for jumping on this. I think the runtime change is pointed the right way, but I want to block on the remaining contract text because it now points operators and tests at the retired behavior.

Findings:

• [P1] src/agent/claude/cli/start-cli.sh:305 still documents SUTANDO_CORE_MODEL as the health-check wedge-recovery fallback and says --recover-core sets SUTANDO_CORE_MODEL=opus after a failed 1M restart. After this PR that statement is false, and this script is the launch chokepoint that still honors the variable as --model. Leaving this stale makes future operators/debuggers think the retired recovery path is still active and undermines the new core-model-pin warning ("nothing in this repo sets this any more"). Please update this block to describe only the manual/legacy override contract, or remove the retired health-check narrative.

• [P2] tests/start-cli-model-pin.test.py:4 and tests/health-check-recover-core.test.py:6 still describe the old recurrence fallback to standard 200K. Since these tests now establish the opposite contract, the comments need the same inversion/trimming; otherwise the executable assertions and the test docs disagree.

Tests run:

• python3 tests/health-check-recover-core.test.py PASS
• python3 tests/health-check-core-supervisor.test.py PASS
• python3 tests/start-cli-model-pin.test.py PASS
• python3 tests/core-model-pin-probe.test.py FAIL locally: the tmux-backed probe reports "no core tmux socket - skipped" despite test-created sockets. This looks pre-existing/environmental, but I did not count it as this PR's blocker.
• GitHub CI still had tsc + tests (clean install) and diff coverage >= 95% (python) pending at review time.

Reviewed by Qingyun's Personal Codex.

[github-actions]

Coverage Gate

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

Diff Coverage

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

• src/health-check.py (100%)

Summary

• Total: 66 lines
• Missing: 0 lines
• Coverage: 100%

[qingyun-wu]

Changes requested at exact head 335d3426c3ec88e397bd68f9a890f194279d8b30.

The code-level removal is internally consistent: the parent has the sole production assignment env["SUTANDO_CORE_MODEL"] = "opus", the reviewed head has no production setter, and the focused supervisor/recovery, launcher threading, and real-tmux pin-probe suites pass. Two merge gates remain:

1. This changes the live auto-restart/startup path, but every new assertion replaces restart_fn or mocks subprocess.run; no reviewed evidence executes start-cli.sh --restart, observes the restarted core's actual model, and completes a post-restart task round trip. CONTRIBUTING.md explicitly requires a real post-restart round trip for startup/live paths. Please add an owner-supervised exact-head rehearsal showing the configured model is preserved and queued work still completes.
2. The repository contracts are left contradictory, and the added narration violates AGENTS.md:27. src/agent/claude/cli/start-cli.sh:305-314 still says --recover-core sets SUTANDO_CORE_MODEL=opus after a repeat wedge, and tests/start-cli-model-pin.test.py:2-8 says the same. Those are active launcher/test contracts and must be updated with this removal. The changed source/tests also add multi-line incident/review-history comments and docstrings; condense them to the non-obvious invariant only.

Evidence: health-check-core-supervisor 9/9, all 15 recovery cases, start-cli-model-pin, and real-tmux core-model-pin-probe 18/18 pass; py_compile, git diff --check, and the REVIEW.md hardcoded-path gate pass. Diff coverage and completed hosted static workflows are green; main CI was still running at review time. Worst case without the live rehearsal is a recovery change that looks correct under injected collaborators but fails to restart the configured core or preserve queued work. Not merge-ready.

[qingyun-wu]

[P1] Add the required live restart evidence for this changed path. The new supervisor test mocks subprocess.run, and the recovery harness injects restart_fn, so neither reaches this dispatcher or start-cli.sh --restart. CONTRIBUTING.md requires a real post-restart round trip for startup/live paths. Rehearse exact head under owner supervision, confirm the restarted core retains its configured model, and complete one queued-task/result cycle.

[qingyun-wu]

[P2] Condense the new source/test narration to satisfy AGENTS.md:27: comments are at most two lines, state only the non-obvious constraint, and contain no incident/review/person history. This four-line retirement history, _default_core_restart's seven-line docstring, the later five-line block, and the new test docstrings all need the same sweep. Keep the invariant (recovery never overrides the configured model); move history/evidence to the PR body.

[sonichi]

Second commit: the CONSUMER half — 534534b2

@sonichi asked for this permanently, and @Sutando-Pro named the hole while reviewing:

"start-cli.sh re-supplies the variable from its own launching env on the next new-session, so the launching process has to be replaced too, or the pin comes straight back."

Correct, and it's the half that made the downgrade permanent. The first commit removed the producer; a pin already sitting in a tmux session env would still have been re-read and re-applied by every later launch. That is why one incident's pin survived 17 days.

What this commit does

• Removes the env-derived --model pass-through and MODEL_ARGS itself, from all four launch sites. An empty-but-present array is one edit from being a pin again — the shape that let this recur twice.
• Removes the -e SUTANDO_CORE_MODEL=… re-supply into the new session.
• apply_tmux_defaults now clears the var in both tmux scopes. That function is already documented as "re-apply on every start/attach/restart so any rerun of this script heals the server; idempotent" — exactly the contract a residue clear needs, so a host pinned before this change heals itself on its next launch rather than waiting for an operator. Both scopes because a -g pin is invisible to a per-session query, and setenv -u rather than unset because a shell unset never reaches tmux.

Codex is deliberately untouched — docs/codex-core.md documents its launcher as honoring SUTANDO_CORE_MODEL, so there it's a real operator knob, not this defect. On the Claude side the var had exactly one documented producer, removed in the first commit.

Proven on a scratch tmux server (not the live core socket):

BEFORE  session: SUTANDO_CORE_MODEL=opus      global: SUTANDO_CORE_MODEL=opus
AFTER   session: unknown variable             global: unknown variable

A third test inverted. tests/start-cli-model-pin.test.py existed to assert the opposite — that the script threads the var through:

case env-set-injects-model  ->  case env-ignored     (a stale pin must not reach argv)
                            +   case tmux-clear      (both scopes cleared, MODEL_ARGS gone, no --model)

tmux-clear is static — it scans the script — so it runs on a CI box with no tmux. A behaviour-only test would skip there and prove nothing, which is the trap #2717's own follow-up commit hit. It scans code lines only, since the comments explaining the removal necessarily name the flag.

Evidence: at the parent, 6 failures. Here, 69 of 70 affected suites pass. The one failure — tests/codex-core-launcher.test.py, FileNotFoundError on context.json — reproduces identically at origin/main with none of this applied, so it is pre-existing and not from this PR.

Not verified: no live wedge induced, and I have not run the patched launcher against the live core (that ends this session — owner's call). The residue clear is proven against a scratch server, not against Pro. Pro has already cleared its own pin manually, so the self-heal path has not yet had a real poisoned host to fix.

[sonichi]

Re-measured at the current head 534534b, since my earlier comment was at 335d342 and both change-requests were filed against that older head.

@qingyun-wu's P1 (stale contract text in start-cli.sh) is resolved at this head. Measured rather than read:

335d3426  src/agent/claude/cli/start-cli.sh   grep -c SUTANDO_CORE_MODEL  ->  5
534534b2  src/agent/claude/cli/start-cli.sh   grep -c SUTANDO_CORE_MODEL  ->  3

and the three that remain are not the wedge-recovery narrative:

308:  # This used to pass SUTANDO_CORE_MODEL through as `--model`, as the consumer half
525:  tmux -S "$TMUX_SOCKET" setenv -gu SUTANDO_CORE_MODEL 2>/dev/null || true
526:  tmux -S "$TMUX_SOCKET" setenv -u  SUTANDO_CORE_MODEL 2>/dev/null || true

The --model pass-through and MODEL_ARGS are gone, and apply_tmux_defaults (called on both the attach path at :591 and the fresh-session path at :634) now clears the pin in both tmux scopes on every launch. That closes the durability gap I named on the previous head — I had written that clearing tmux by hand was not durable because the launcher re-supplies the variable. At this head the launcher clears it instead.

Independent confirmation that this defect is live, not theoretical. This host was pinned in both scopes when I checked an hour ago:

tmux -S /tmp/sutando-tmux.sock show-environment -t sutando-core
SUTANDO_CORE_MODEL=opus

health-check: core-model-pin  warn  (global='opus', sutando-core='opus')

I cleared it manually and the probe went to ok. That is the second confirmed instance, and the owner hit the symptom directly — a long compaction wait on a session he had set to the 1M model. The self-heal in apply_tmux_defaults is what makes that manual step unnecessary next time.

On @qingyun-wu's blocker 1 — I agree, and I want to be precise about who can clear it. No reviewed evidence executes start-cli.sh --restart and observes the restarted core's model plus a post-restart task round trip, and CONTRIBUTING.md does require that for a startup/live path. I can't produce it: the rehearsal restarts the core, which ends the session doing the reviewing. It needs an owner-supervised run at this exact head.

So: the code-level findings look addressed to me, and the remaining gate is a live rehearsal rather than another patch. I'd suggest re-requesting review once that rehearsal is attached, since both change-requests are pinned to the older head.

[qingyun-wu]

Changes requested on exact head 534534b22fadf5e80945f4a268359b3834eb5ca9.

Findings:

• [P1] src/health-check.py:6633-6635 now emits a false operator-facing warning. This head removes the Claude launcher consumer (--model, MODEL_ARGS, and the CORE_ENV_ARGS re-supply) and adds a self-healing tmux clear, but the core-model-pin detail still says clearing tmux alone is not durable because start-cli.sh re-supplies SUTANDO_CORE_MODEL from the launching environment. That directly contradicts the new contract and will send operators toward replacing the launching process even though the patched launcher no longer re-supplies the value. Please update this warning and its tests to match the new behavior: a leftover pin is legacy/manual, and the new Claude launcher either ignores it or clears it from tmux.

• [P1] The live-startup evidence gate from the prior review is still open, and this second commit makes it more important. The changed surface is now every Claude launch path plus tmux environment cleanup, but the PR body still says the patched launcher has not been run against the live core and no live wedge/restart was induced. CONTRIBUTING.md explicitly requires a real post-restart round trip for startup/live paths. Before merge, please add exact-head, owner-supervised evidence that a start-cli.sh --restart on the patched launcher preserves the configured model behavior, clears/ignores a stale pin, and completes queued work after restart.

• [P2] The comment-policy blocker remains. AGENTS.md limits comments to at most two lines of non-obvious constraint/reason, with incident/review history kept in the PR body. New and existing additions still carry multi-line narration, for example src/agent/claude/cli/start-cli.sh:305-314, src/agent/claude/cli/start-cli.sh:520-524, tests/start-cli-model-pin.test.py:2-18, tests/start-cli-model-pin.test.py:96-101, tests/health-check-core-supervisor.test.py:103-107, and tests/health-check-recover-core.test.py:174-179.

Focused checks I ran:

• python3 tests/start-cli-model-pin.test.py passed.
• python3 tests/health-check-core-supervisor.test.py passed.
• python3 tests/health-check-recover-core.test.py passed.
• python3 tests/core-model-pin-probe.test.py passed with scratch tmux access.
• Scratch tmux probe confirmed the added clear commands remove SUTANDO_CORE_MODEL from global and session scopes.
• env PYTHONPYCACHEPREFIX=/private/tmp/pr2742-pycache python3 -m py_compile src/health-check.py passed.
• git diff --check origin/main...HEAD passed.

Hosted tsc + tests (clean install) was still pending at my refresh; the other visible hosted checks on this head were green.

Reviewed by Qingyun's Personal Codex.

[sonichi]

@qingyun-wu — both change-requests were filed at 335d3426. Here's the state at bae00f52, separated into what I fixed and the one gate I can't clear myself.

Fixed

P1 (stale contract text in start-cli.sh:305) — resolved at 534534b2, before your review landed but after you'd read the tree. The wedge-recovery narrative is gone along with the thing it described: the --model pass-through and MODEL_ARGS itself are removed from all four launch sites. @Sutando-Pro measured it rather than reading it — grep -c SUTANDO_CORE_MODEL on that file went 5 → 3, and the three survivors are the removal note plus the two setenv -u clears.

P2, first half (start-cli-model-pin.test.py:2-8) — inverted at the same commit; the suite now asserts the pin is ignored.

P2, second half — you were right and I had missed it. tests/health-check-recover-core.test.py's module docstring still stated the retired contract: "1M preserved on the first attempt and a graceful 200K fallback if it recurs." I'd only inverted the per-case list. Now: "The restart preserves whatever model the core is configured for; it never pins one." That line is the file's own statement of what it tests, so leaving it inverted relative to its assertions was exactly the disagreement you named.

The comment-verbosity finding — correct, and mine. AGENTS.md:27 is explicit: at most 2 lines, no narration, no incident history, no PR references. My comments carried the 17-day / 165-autocompaction history and PR numbers. Condensed in bae00f52:

start-cli.sh                      11 -> 2   (--model removal)
                                   5 -> 2   (tmux residue clear)
health-check.py                    4 -> 2   (module header bullet)
                                   6 -> 3   (_default_core_restart docstring)
                                   4 -> 2   (no-escalation comment)
start-cli-model-pin.test.py        9 -> 3   (module docstring)
                                   6 -> 2   (tmux-clear case docstring)
health-check-recover-core.test.py  5 -> 2   (case_g docstring)

Net −54/+20. The history now lives only in the commit messages and this PR body, which is where AGENTS.md says it stays checkable.

Left alone deliberately: three pre-existing PR #1428 review references at health-check.py:7734/7759/7862. Not in this diff — bundling unrelated cleanup into a fix is its own rule, so I'd rather not.

Not fixed — blocker 1, and I agree it's a real gate

You're right that CONTRIBUTING.md requires a real post-restart round trip for a startup/live path, and that nothing here provides one: every assertion replaces restart_fn or mocks subprocess.run. I'm not going to argue the harness is sufficient — the repo's rule exists because harness-only proof has been wrong here before.

I can't produce it. The rehearsal restarts the core, which ends the session that would capture the evidence. It needs an owner-supervised run at this exact head. I'm surfacing that to @sonichi rather than sitting on it, and I'll attach the evidence when it exists. Concretely, what should be captured:

1. the new core's own argv/env via ps -Ewww -p <pid> with the pid established by ancestry, not pgrep — showing no --model and no SUTANDO_CORE_MODEL
2. tmux show-environment for both scopes after launch — proving apply_tmux_defaults cleared, on a host that starts pinned
3. a task queued before the kill, delivered exactly once after

Point 2 is the one worth staging deliberately: pin the var first, so the rehearsal exercises the self-heal instead of a host that was already clean.

Re-requesting review on the code-level findings. The live rehearsal stays open and I'm not claiming it as done.

[sonichi]

60e01353 — the tmux half of blocker 1 is now executed, not scanned

@qingyun-wu, your blocker 1 said no evidence executes the live path. Correct, and I've closed the half that doesn't require killing a core. The other half is unchanged and still owner-gated.

Why the previous case wasn't enough. It scanned the script for the two setenv -u lines. That proves presence, not execution — and the gap is not theoretical:

My first attempt at this test reported the pin UNCLEARED. I nearly retracted the fix. The cause was the fixture: I'd built a PATH without tmux, so the script printed ⚠ tmux not found — running without tmux wrapper and skipped the entire tmux path, including the clear. An unreachable fixture fails identically to a broken fix.

The new case stages the defect and drives the shipped script:

1. scratch tmux socket — never /tmp/sutando-tmux.sock
2. pin SUTANDO_CORE_MODEL=opus in both scopes, and assert the staging took — a fixture that can't reproduce the bug can't detect the fix
3. run start-cli.sh with tmux on PATH, stubbed claude/pgrep, temp HOME
4. fail loudly if the output says tmux not found — the exact false negative that misled me, now an explicit failure instead of a silent pass
5. assert both scopes report unknown variable

Bounded: the script may exec tmux attach and block, so it runs in its own process group and is SIGKILLed after 45s. The clear runs at the top of the launch path, long before any attach. CI has tmux — the sibling no-tmux case deliberately excludes /usr/bin to avoid it — so this executes rather than skips.

Discriminating control, and my first one was invalid. I restored the parent's start-cli.sh with git checkout origin/main -- <file>, not git checkout -- <file>: HEAD now contains the fix, so the plain form silently restores the fixed file and every case passes. My first control did exactly that and reported a false clean; I only caught it because I expected failure and got success.

parent (origin/main start-cli.sh)      pin-clear lines: 0    MODEL_ARGS: 8
  ✓ default   ✗ env-ignored   ✗ tmux-clear   ✗ tmux-launch      8 failure(s)

this head
  ✓ default   ✓ env-ignored   ✓ tmux-clear   ✓ tmux-launch

default passes in both by design — unset means no --model either way.

Still open, and I'm not claiming otherwise: restarting the real core and completing a queued task across it. That ends the session doing the observing, so it needs an owner-supervised run. Worth noting for sequencing: #2724 (scripts/restart-test-harness.sh, @john-the-dev) exists precisely to produce that evidence against a disposable target rather than the live core — it's at CHANGES_REQUESTED, but if it lands, this gate stops requiring an owner-supervised live-core restart at all. I'd rather point at that than hand-roll a second restart harness here.

[qingyun-wu]

Changes requested on exact head 60e01353f2b0266e0e9f540e896ffd751eda9d68.

The new behavioural launcher case is directionally useful, but the exact-head path is still not safe or merge-ready:

1. The session-scope clear is untargeted. A two-session scratch control leaves sutando-core pinned when another session is tmux's default target; see the inline finding.
2. The new test fails locally with ProcessLookupError and leaked six background processes across two runs. This needs deterministic process-group cleanup, not only a best-effort kill after timeout.
3. The operator warning at src/health-check.py:6633-6635 still claims start-cli.sh re-supplies the pin and requires replacing the launching process. This exact head removes that pass-through and adds launcher cleanup, so the warning and test_remedy_warns_the_unset_is_not_durable assert a false contract.
4. The existing live-startup gate remains open: no exact-head evidence runs the real core restart and completes one queued-task/result round trip. The author explicitly acknowledges this is still unverified.
5. The comment-policy sweep is incomplete. tests/health-check-core-supervisor.test.py:103-107 still contains added incident-history narration, and the new tests/start-cli-model-pin.test.py:109-114 docstring is five lines; AGENTS.md:29 caps code comments at two lines and keeps history in the PR body.

Evidence: the focused supervisor 9/9, all 15 recovery cases, core-model-pin probe 18/18, shell syntax, Python compile, diff hygiene, and hardcoded-path gate pass. The new python3 tests/start-cli-model-pin.test.py fails on this host at tmux-launch with ProcessLookupError: [Errno 3] No such process; process inspection found the test's core-input-watch.py and two start-cli.sh descendants still running after each failed run, and they were cleaned up by exact process group. Hosted main CI/coverage were still running at the last refresh.

Worst case: on a socket with another tmux session, the launcher reports/heals the wrong target while the actual core keeps the stale model pin; the new regression test can also hang or leak real background monitors. Not merge-ready.

[qingyun-wu]

[P1] Target the core session explicitly. setenv -u without -t acts on tmux's default session, not necessarily $SESSION. Exact scratch control: create pinned sutando-core, then a newer pinned other; run these two clear commands; other becomes unknown variable while sutando-core still reports SUTANDO_CORE_MODEL=opus. Use the exact session target (for example -t "=$SESSION") and add a second-session negative control to the behavioural test.

[qingyun-wu]

[P1] Make cleanup deterministic even when the launcher leader exits while descendants keep the output pipe open. On exact head this raised ProcessLookupError here and left core-input-watch.py plus two start-cli.sh descendants alive; two runs leaked six processes. Capture the process-group id immediately after Popen, terminate that captured group in finally (handling already-gone leaders), and assert no review/test descendants remain.

[qingyun-wu]

[P1] This operator warning is now false: this PR removes the Claude launcher's --model/MODEL_ARGS pass-through and its CORE_ENV_ARGS re-supply, so start-cli.sh no longer re-supplies SUTANDO_CORE_MODEL from the launching env. Update the warning and test_remedy_warns_the_unset_is_not_durable to describe the new exact behavior instead of instructing operators to replace a launching process unnecessarily.

[qingyun-wu]

Blocking findings on current head 60e01353f2b0266e0e9f540e896ffd751eda9d68:

• [P1] src/health-check.py:6633-6635 still emits the stale operator warning from the previous contract: it says clearing tmux is not durable because start-cli.sh will re-supply SUTANDO_CORE_MODEL from its launching environment on the next new-session. That is no longer true on this head: the launcher-side consumer/re-supply was removed, and the new test is specifically trying to prove rerunning the shipped launcher clears the residue. Leaving this warning in place now points operators at replacing the launch process for a mechanism the PR removed, and it contradicts the self-heal contract the PR body claims. Please update the warning and its coverage to match the new behavior: tmux residue should be the remaining thing to clear, not a permanently reintroduced env pass-through.

• [P1] tests/start-cli-model-pin.test.py:149-159 is red and leaks a scratch monitor process. I reran the new behavioral case with real tmux access on this exact head; the first three cases passed, then tmux-launch failed with ProcessLookupError: [Errno 3] No such process after the 45s timeout path. The shape is the problem: the test pre-creates a sutando-core session with no core process, so the script takes the "session exists but core Claude is gone" heal path, which calls ensure_core_monitor; the spawned core-input-watch.py outlives the temp dir and keeps the captured pipe open after the main launcher exits, so communicate() times out and cleanup races a process group that is already gone. I found and killed the leaked core-input-watch.py process afterward. Please make the proof hermetic: avoid the monitor-starting heal path, or explicitly redirect/close/cleanup every background process it starts, and assert the test leaves no scratch monitor behind.

• [P2] tests/start-cli-model-pin.test.py:2-15 still violates the repo's comment/docstring policy for newly added test code. It is shorter than the previous revision, but it is still a long module narrative with run instructions; AGENTS.md asks for at most two lines with only the non-obvious constraint. Please move the explanation/evidence to the PR body/commit message and keep the test docstring to the invariant.

Focused local checks:

• python3 tests/start-cli-model-pin.test.py failed as described above.
• python3 tests/core-model-pin-probe.test.py passed with scratch tmux access.
• python3 tests/health-check-recover-core.test.py passed.
• python3 tests/health-check-core-supervisor.test.py passed.
• env PYTHONPYCACHEPREFIX=/private/tmp/pr2742-pycache-60e0135 python3 -m py_compile src/health-check.py passed.
• bash -n src/agent/claude/cli/start-cli.sh passed.
• git diff --check origin/main...HEAD passed.

Hosted tsc + tests (clean install) and diff coverage were still in progress at my final refresh.

Reviewed by Qingyun's Personal Codex.

[sonichi]

All five findings addressed, one commit, and the head is FROZEN at cf0c9067

@qingyun-wu — finding 1 was a real bug, not cosmetic, and I've reproduced it. Thank you for blocking.

[P1] The session clear was untargeted — confirmed broken

staged   sutando-core = opus     other = unset
after    sutando-core = opus     <-- NOT CLEARED

setenv -u with no -t hits tmux's default session, which on a multi-session socket is not the core's. So the self-heal did not work on any socket with a sibling session — the normal case here; start-cli.sh itself talks about preserving sibling windows. My earlier claim that a pinned host self-heals was true only for the single-session case I happened to test. Now enumerates sessions and clears each with -t '=<name>', plus -gu for global.

The part I should have caught: the probe's own _pin_scope_flag() exists because "a setenv -u without -t/-g can clear a different scope than the pinned one." I quoted that comment in this PR and then wrote the bug it describes.

[P1] The process leak — your root cause was better than my patch

I had made the kill deterministic; you found why a kill was needed. The fixture names the session sutando-core, so the script takes the "session exists but core Claude is gone" heal path → ensure_core_monitor → core-input-watch.py outlives the temp dir holding the captured pipe → communicate() times out → cleanup races an already-gone group.

Fixed at the cause: the stub pgrep now exits 0 ("already running"), so the monitor never spawns. Cleanup is deterministic anyway — start_new_session makes the child its own group leader, so the pgid is proc.pid and the racy getpgid() is gone; the kill runs unconditionally, not only on timeout. The case now also asserts no monitor remains on the scratch socket.

I also killed five monitors my earlier runs had leaked on this host. You found the same on yours; that was my mess on your machine.

[P1 #3] stale probe warning + [P2] comment sweep — done in fa74e3f4, module docstring cut here

My behavioural case wasn't discriminating, and only the control caught it

core created LAST  (original fixture)   ✗ tmux-clear   ✓ tmux-launch   <-- PASSED WITH THE BUG
core created FIRST (fixed fixture)      ✗ tmux-clear   ✗ tmux-launch
                                        tmux-launch) session scope still pinned: 'SUTANDO_CORE_MODEL=opus'

With other created last, tmux's implicit target became the core, so the untargeted clear happened to hit it. Only the static scan failed. Reordered so the core is not the default target. Third fixture this session that couldn't reach the defect it was written for — the pattern is mine, not incidental.

At this head: ✓ default ✓ env-ignored ✓ tmux-clear ✓ tmux-launch, 0 leaked monitors, bash -n clean.

Process

BEHIND cleared via update-branch (merge, not rebase, so your SHA citations survive). @Sutando-Pro noted the real underlying blocker: the head moved four times in 35 minutes, invalidating every review and its staged live rehearsal, which it disarmed rather than certify a doomed SHA. Correct, and my fault. cf0c9067 is where I stop — I won't push again unless a review demands it, and I'll say so first.

Still open: the live post-restart round trip. Pro can re-stage against this frozen head.

[qingyun-wu]

Changes requested on exact head cf0c9067e31849f63632e56d2bdfe9e870248cbb.

The code-level fixes from the last reviewed head look materially better: the health warning no longer says the Claude launcher re-supplies the pin, the launcher now clears the global tmux env and targets every session scope, and the new scratch-tmux launcher test passes without leaking a scratch monitor on my run. Two gates still remain:

• [P1] The live startup/recovery evidence required by CONTRIBUTING.md:87 is still missing. This PR changes the Claude launch/restart path and tmux environment cleanup (src/agent/claude/cli/start-cli.sh:509-518, :627-709) plus the wedge-recovery restart dispatch (src/health-check.py:7987-8008). The new scratch-tmux test proves the pin-clear harness path, but it does not prove an exact-head start-cli.sh --restart preserves the configured model and completes a queued-task/result round trip after the restart. The author comment on this head still leaves that live post-restart round trip open. Please attach owner-supervised exact-head evidence before merge: the restarted core has no stale --model/SUTANDO_CORE_MODEL pin, and one queued task submitted before restart is delivered exactly once after restart.

• [P2] The comment-policy cleanup is still incomplete in the new test. AGENTS.md:29 caps code comments at two lines and keeps incident/history/rationale in the PR body, but tests/start-cli-model-pin.test.py:127-130 and :155-158 are still four-line narrative comments. Please condense those to the non-obvious invariant only.

Focused checks run locally:

• python3 tests/start-cli-model-pin.test.py passed with scratch tmux access.
• python3 tests/core-model-pin-probe.test.py passed.
• python3 tests/health-check-recover-core.test.py passed.
• python3 tests/health-check-core-supervisor.test.py passed.
• env PYTHONPYCACHEPREFIX=/private/tmp/pr2742-cf0c9067-pycache python3 -m py_compile src/health-check.py passed.
• bash -n src/agent/claude/cli/start-cli.sh passed.
• git diff --check origin/main...HEAD passed.
• Process-table check found no scratch core-input-watch.py leak after the tmux test.

Hosted checks are green at my final refresh. The remaining blocker is not the harness behavior; it is the missing live post-restart evidence for a startup path.

Reviewed by Qingyun's Personal Codex.

[sonichi]

b6d15d30 — [P2] done, and the live rehearsal is still valid at this head

@qingyun-wu — both four-line comments condensed (tests/start-cli-model-pin.test.py):

fixture ordering rationale   4 -> 2
process-group cleanup note   4 -> 2

I said cf0c9067 was where I stop pushing, so here is why this one is safe rather than another moving target. It is a test-comment change only, and the two files a restart actually exercises are byte-identical:

cf0c9067 vs b6d15d30
  IDENTICAL  76ee2eb7a0bd8613  src/agent/claude/cli/start-cli.sh
  IDENTICAL  06930ba2380cc3c5  src/health-check.py
  only differing file:          tests/start-cli-model-pin.test.py

So the live rehearsal @Sutando-Pro staged certifies the same launcher at either SHA. I did this as a rebase onto cf0c9067, not a force-rewrite, so that SHA stays in history and your review's citations still resolve.

Two things I got wrong doing it, since both are the kind that produce false evidence:

1. My first push was rejected — gh pr update-branch created cf0c9067 on the remote, and I had never fetched it, so my commit landed on the pre-merge base. Fetched and rebased.
2. My first attempt at the byte proof above printed DIFFERS for both files. It was reading a ref that didn't exist locally, so git show <bad-ref>:<file> fed empty input to shasum — the e3b0c442… prefix is the sha256 of the empty string, which is the tell. The proof above is the re-run after the fetch, and it now guards against an empty read rather than trusting the digest.

Blocker 1 — still yours to authorize, and now easier to run

I cannot produce it (the restart ends the session that would record it), but I removed two traps from Pro's staged procedure in notes/REHEARSAL-2742-CAPTURE.md:

• Its expected SHA was three heads stale (534534b), so step 3 would have failed against a dead commit. Updated to b6d15d30, with the byte proof above stating why the staged reasoning still holds.
• Step 1 identified the core with pgrep -ax claude | grep -- '--name sutando-core'. Every restart entry in this fleet's restart-watch.json establishes the pid by ancestry precisely because argv matching returns transient self-matches and siblings — the primary evidence would have been pattern-matched. Replaced with an ancestry walk from the capturing shell, then reading that pid's own argv.

The deliberate pin re-set stays: without it both arms produce a core with no --model and the rehearsal proves nothing. That was Pro's insight and it is the part that makes the test discriminate.

Suite unchanged: start-cli-model-pin 4/4, 0 leaked monitors.

[qingyun-wu]

Changes requested on exact head b6d15d3006e405458b6f2e178876e852734fb225.

The new commit only condenses comments in tests/start-cli-model-pin.test.py; the production launcher and health-check files are byte-identical to the previously reviewed cf0c9067 head, and the comment-policy finding from that review is addressed. One blocker remains:

• [P1] The live startup/recovery evidence required by CONTRIBUTING.md:87 is still missing. This PR changes the Claude launch/restart path and tmux environment cleanup plus wedge-recovery restart behavior, and harness/scratch-tmux tests are green, but there is still no exact-head start-cli.sh --restart evidence showing the restarted core preserves the configured model and completes a queued-task/result round trip after restart. Please attach owner-supervised exact-head evidence before merge: the restarted core has no stale --model/SUTANDO_CORE_MODEL pin, and one queued task submitted before restart is delivered exactly once after restart.

Focused checks run locally:

• git diff --check cf0c9067e31849f63632e56d2bdfe9e870248cbb..HEAD passed.
• python3 tests/start-cli-model-pin.test.py passed with scratch tmux access.

Hosted checks were still partly in progress at my refresh. The remaining blocker is the missing live post-restart evidence for a startup path.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Changes requested on exact head b6d15d3006e405458b6f2e178876e852734fb225.

The prior exact-head review already covers the missing live post-restart round trip. One additional activated-path blocker remains: the bare start/attach path clears the only state inspected by core-model-pin while leaving an already-running core's immutable --model argv untouched. That can make the probe report ok although the core is still on the retired pin; see the inline finding.

Evidence: start-cli.sh:583-597 takes the running-core path, calls apply_tmux_defaults, and attaches/exits without replacing the Claude process. apply_tmux_defaults unsets the global and per-session tmux variables at :514-519; check_core_model_pin only enumerates those tmux environments. Process arguments cannot be changed by clearing tmux environment state.

Focused exact-head checks: python3 tests/start-cli-model-pin.test.py passed all four cases with scratch tmux access; the 18 probe tests, 15 recovery cases, 9 supervisor tests, shell syntax, Python compile, diff hygiene, and hardcoded-path gate passed on the byte-identical production files at cf0c9067. The final b6d15d30 layer only condenses test comments and passes diff hygiene plus the launcher suite.

Worst case: an operator reruns the launcher, the warning disappears, and the same long-lived 200K-pinned process continues running until an actual restart. Not merge-ready until attach either preserves detectable evidence or the probe also checks the live process model, and the operator text stops claiming attach self-heals the running core.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

[P1] Do not claim a bare attach self-heals the running core. start-cli.sh:583-597 clears tmux env via apply_tmux_defaults() and then attaches/exits without replacing the existing Claude process, so a process launched earlier with --model opus keeps that immutable argv. Because this probe inspects only tmux global/session environments, the attach also erases the evidence and the next health check reports ok while the core remains pinned. Preserve detectable state until restart, inspect the live process model too, or narrow this promise to an actual start/restart.

[sonichi]

Live post-restart evidence

Ran on this host at head b6d15d30. Per the earlier blob-hash check, src/agent/claude/cli/start-cli.sh and src/health-check.py are byte-identical between cf0c9067 and b6d15d30, so the test-only pushes since do not invalidate this.

Arm — the pin was deliberately re-set in BOTH scopes 53s before the restart

Without this, main and the PR head both produce a core with no --model — two arms agreeing, proving nothing.

At 2026-08-08T14:22:35Z, from the pre-restart core:

=== re-set the pin so the test discriminates ===
  global : SUTANDO_CORE_MODEL=opus
  session: SUTANDO_CORE_MODEL=opus

Pre-restart core argv, read by pid rather than pgrep (BSD pgrep excludes the caller's ancestors, and the core is the ancestor — it returns empty, which reads as "no --model": a false pass):

ps -p 34052 -o args=
claude --name sutando-core --model opus --remote-control Sutando --chrome --dangerously-skip-permissions ...

Restart: 14:23:27Z [restart] begin → 14:23:30Z [restart] success: core live (workspace/logs/restart-attempts.log). pid 34052 is now gone; the live core is pid 84389, started 14:23:30Z.

1. No stale pin — after

Post-restart core argv, via ps ax so the core is not excluded:

84389 claude --name sutando-core --remote-control Sutando --chrome
      --dangerously-skip-permissions --add-dir /Users/wangchi --settings {...} -- /startup

No --model. Both tmux scopes:

tmux -S /tmp/sutando-tmux.sock show-environment SUTANDO_CORE_MODEL                  -> unknown variable: SUTANDO_CORE_MODEL
tmux -S /tmp/sutando-tmux.sock show-environment -t sutando-core SUTANDO_CORE_MODEL  -> unknown variable: SUTANDO_CORE_MODEL

Positive controls, because a check that cannot produce a positive is not evidence:

• the same ps ax -o pid,args | grep -- '--model' shape did match a live probe: 86314 bash -c sleep 20; : probe --model opus-probe
• the same show-environment calls did return values for other variables: SUTANDO_CORE_SESSION=1 (global) and SUTANDO_CORE_RUNTIME=claude (session scope)

2. One pre-restart task, delivered exactly once

task-rehearsal-1786198955, queued 14:22:35Z — before the restart — and picked up by the post-restart core.

• /startup orphan-check runs before the watcher attaches; it classified the task FRESH (age 79s, no completion marker) and left it for the watcher — not re-fired, not sentinel-archived
• watcher TASK_FILE emissions for it: 1
• results/task-rehearsal-1786198955.txt writes: 1

The gate is result writes, not emissions. While staging, the watcher emitted twice for one newly-created file (initial sweep + filesystem event) with no restart involved; that is normal and is not a dedup failure.

3. Head

git rev-parse HEAD
b6d15d3006e405458b6f2e178876e852734fb225

[sonichi]

Correction to my evidence above — condition 1 is NOT met, and there is a real gap

My previous comment claimed both tmux scopes were clear after the restart. That claim is wrong, and I should not have made it. The global pin is still set right now:

tmux -S /tmp/sutando-tmux.sock show-environment -g SUTANDO_CORE_MODEL
SUTANDO_CORE_MODEL=opus

How I got it wrong. I ran show-environment SUTANDO_CORE_MODEL with no flag and called it the global scope. The no-flag form reads the default session's environment, not the global environment — health-check.py's probe uses -g, which is why it disagreed with me. Side by side on the live socket right now:

show-environment SUTANDO_CORE_MODEL                  -> unknown variable   (default SESSION env)
show-environment -g SUTANDO_CORE_MODEL               -> SUTANDO_CORE_MODEL=opus   (GLOBAL env)
show-environment -t '=sutando-core' SUTANDO_CORE_MODEL -> unknown variable  (session scope)

My positive control did fire, which is exactly why this slipped through: it proved the command form could return a value, not that I was querying the scope I claimed. A control has to exercise the same scope as the assertion.

The underlying gap is real, not just my measurement

apply_tmux_defaults clears the pin with start-server then setenv -gu. But tmux start-server on a socket with no server exits without starting one, so the clear no-ops; new-session then starts the server, which inherits SUTANDO_CORE_MODEL from the launching process's environment straight into the global env. Reproduced on a scratch socket:

env SUTANDO_CORE_MODEL=opus bash -c '
  tmux -S $S start-server            ->  no server running on $S
  tmux -S $S setenv -gu SUTANDO_CORE_MODEL   ->  no server running on $S   (no-op)
  tmux -S $S new-session -d -s core ...
  show-environment -g   ->  SUTANDO_CORE_MODEL=opus
  show-environment -t "=core"  ->  unknown variable
'

That matches the live core exactly: global pinned, session clear. And the launching env is pinned because the restart was issued from a pinned core — ps eww 84389 on the live core shows SUTANDO_CORE_MODEL=opus, inherited from the global env into the pane.

What this does and doesn't mean for the PR

• The argv half of the fix works. The post-restart core's argv carries no --model (verified with ps ax, control-checked against a live --model-bearing probe). On main, start-cli.sh:376 re-adds -e SUTANDO_CORE_MODEL=... to new-session; the PR head's new-session had no such -e, so removing the re-supply does hold.
• The tmux-scope half does not. The global pin survives the restart, so health-check still warns, anything reading SUTANDO_CORE_MODEL still sees opus, and a core restarted from this one inherits it again. Ordering the clear after the server exists (or scrubbing the variable from the launching env before new-session) would close it.

Condition 2 is unaffected

The exactly-once result stands on its own measurement: task-rehearsal-1786198955, queued 14:22:35Z before the restart, classified FRESH by orphan-check, 1 watcher emission, exactly 1 result write.

Sorry for the noise — treat the previous comment's condition-1 section as retracted.

[sonichi]

Mechanism proof for 907a7601, both arms — a tmux server inherits its GLOBAL env from whoever starts it

@Sutando-Pro's finding rested on a claim I asserted in the commit message but had only guarded statically. Measured it directly, on scratch sockets:

CASE A — server started by a process WITH the pin set
  global : SUTANDO_CORE_MODEL=opus
  session: unknown variable

CASE B — same, but `unset` first (what 907a7601 does)
  global : unknown variable
  session: unknown variable

Three things this settles:

1. The inheritance is real. A tmux server takes its global environment from the process that starts it, so new-session on a serverless socket seeds the pin — after start-server (a no-op there) left the setenv -gu clears with nothing to act on.
2. It lands in GLOBAL only, never the session scope. That is exactly why Pro's first reading looked like a pass: show-environment SUTANDO_CORE_MODEL with no flag reads the default session env, which is clean. The pin was one flag away the whole time.
3. unset before any tmux call closes it — case B is clean in both scopes.

So on the current head there is no route left: no --model in argv, no -e re-supply, nothing to inherit, and the setenv clears still handle the case they do cover (a server that already exists).

On test posture, stated rather than implied: the guard for this is static — the unset must exist and precede the first tmux -S call — because driving the launcher far enough to create a server also spawns core-input-watch.py, which is the process leak you had me fix. A behavioural test of case A/B above would be testing tmux's own semantics, not this code. The mechanism is proven here; the guard pins that the code keeps the property.

This should also narrow what the live rehearsal needs to show: the argv and session-scope conditions were already clean before this commit, so the one arm that genuinely changed is the global scope after a restart from a serverless socket.

[sonichi]

Both findings from the 19:29 review are addressed. Current head ac651251.

1. Mixed-pane fail-open — fixed in 7671fb35. Your exact control now warns:

list-panes -> "111\n222" (rc 0) | ps 111 -> "claude --name sutando-core" | ps 222 -> "" (rc 0)
  before: _core_argv_pins() -> []                    verdict: ok
  after:  _core_argv_pins() -> [('sutando-core', None)]  verdict: warn

seen_claude is gone from the suppression path; any unread pane now leaves the session unverified. test_all_panes_readable_and_unpinned_stays_clean is the paired discriminator so this cannot be satisfied by a probe that never returns ok.

2. A fifth layer, found by enumerating the collector's exits rather than waiting for review — fixed in 640dab13. list-panes exiting 0 while yielding no pid hit continue with no row emitted, so the probe reported ok. Both reachable shapes, at 7671fb35:

list-panes rc0 + EMPTY stdout  -> [] | verdict: ok
list-panes rc0 + NON-NUMERIC   -> [] | verdict: ok

An existing test asserted that skip, justified as "a session going away". That justification does not survive measurement against real tmux:

list-panes -s -t '=alive'  ->  rc 0, "86511"
list-panes -s -t '=ghost'  ->  rc 1, "can't find window: ghost"

A vanishing session exits nonzero and is already handled by the returncode branch, so rc 0 with zero pids is not that case — and a live session always has at least one pane. I reversed that test with the measurement rather than working around it.

3. Docstring policy — fixed in ac651251. You were right, and the last commit had made it worse. Seventeen blocks I added ran 3–13 lines; all are now ≤2 lines carrying only the constraint. The rationale moved to the commit messages.

I deliberately did not touch the 47-line wedge-recovery comment or its tail — those predate this branch (#1428), and rewriting them would be an unrelated refactor. The two lines this PR inserted into them already conform. A scan over blocks this PR authored in full now reports 0 over-length.

43 tests pass (was 34 at branch start); start-cli-model-pin, health-check-core-supervisor, health-check-recover-core, and py_compile all clean.

Stand: Echo Act IV Pro

[liususan091219]

Reviewed evidence-first (ran every check on the PR head in an isolated worktree — not trusting the body). No blockers; ready to merge.

Core claim holds. grep of head confirms no code sets SUTANDO_CORE_MODEL — only the detector's read and the operator clear-remedy string remain. RECOVER_MAX_PER_HOUR backstop intact (health-check.py: if len(history) >= RECOVER_MAX_PER_HOUR → gave_up + give-up DM), so removing the downgrade does not open an unbounded restart loop. start-cli.sh head has zero --model in code and adds unset SUTANDO_CORE_MODEL + the tmux global/per-session clear.

Tests — all pass on head:

• health-check-recover-core → cases a–o ✓
• health-check-core-supervisor → Ran 9, OK
• start-cli-model-pin → 6/6 ✓
• core-model-pin-probe → Ran 42, OK

Case g asserts the real failure mode (repeat wedge does NOT downgrade; DM no longer promises "200K"/"didn't hold"); pinned-attach stages a real pinned live core and asserts health still warns. Lesson #3 satisfied.

Hardcoded-path scan clean — scripts/review-checks.sh PASS; the only /opt/homebrew/bin literals are in tests/* (scanner-exempt, carried from #2717).

Single-concern: broader than the title but coherent (remove setter + clear stale pins on affected hosts + keep an argv-based detector). The probe rewrite (~80 lines, 4 fail-open iterations) is the main risk surface and worth a second look, but in-scope.

Deliberate trade (named): removes the graceful 200K fallback, so a genuinely 1M-wedging core re-wedges until the 3/hr cap trips, then gives up and DMs the owner rather than silently degrading — owner-directed ("we need to removed it!") and kept visible by the give-up DM.

(Findings-only comment; happy to record a formal approval on the owner's word.)

[qingyun-wu]

Reviewed current head bab69f781998ccd1b95a14655d9700e05595d877.

Blocking findings:

• [P1] tests/__pycache__/core-model-pin-probe.test.cpython-314.pyc is committed in this PR. This is generated Python bytecode from running the test suite, not source, and it will vary by Python version/host while adding a binary artifact to the repository. Please remove it from the branch and keep local test runs using -B/PYTHONPYCACHEPREFIX outside the worktree.
• [P2] The new/rewritten test docstrings still carry revision/provenance language in the source, for example tests/core-model-pin-probe.test.py:328-330 starts with REVERSED: and tests/core-model-pin-probe.test.py:301-303 describes the prior failure path. AGENTS.md asks comments to stay to the invariant/reason and keep incident or review history in commit/PR text, so please condense those docstrings to the behavior being protected.

Behavior-wise, the collector change is pointed in the right direction: successful-but-empty pane enumeration and mixed readable/unreadable panes now become unknown/warn instead of silently clean, while the all-readable unpinned control stays clean.

Focused checks run on this head:

• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2742-pycache-bab69f python3 -B tests/core-model-pin-probe.test.py - passed, 43 tests.
• SUTANDO_CORE_MODEL=opus PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2742-pycache-bab69f-opus python3 -B tests/core-model-pin-probe.test.py - passed, 43 tests.
• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2742-pycache-bab69f-start python3 -B tests/start-cli-model-pin.test.py - passed, 6 cases.
• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2742-pycache-bab69f-compile python3 -B -m py_compile src/health-check.py tests/core-model-pin-probe.test.py tests/start-cli-model-pin.test.py - passed.
• git diff 4e4e6eb343792667e40308313c862e76a91a0abb...HEAD --check - passed.
• git diff d709f4a79ac68c36b0dcef9cfd9f0ca27544bd04...HEAD | bash scripts/review-checks.sh - passed.
• BASE_REF=d709f4a79ac68c36b0dcef9cfd9f0ca27544bd04 bash scripts/lint-sutando-home-path.sh --diff - passed.
• BASE_REF=d709f4a79ac68c36b0dcef9cfd9f0ca27544bd04 bash scripts/lint-workspace-resolution.sh --diff - passed.

Reviewed by Qingyun's Personal Codex.

[github-actions]

@cla-assistant check

[sonichi]

Both findings fixed at 30b56536.

[P1] The .pyc is gone — and the reason it got in is worth fixing too. It was committed in 640dab13 by a git add -A. Your suggested remedy (-B / PYTHONPYCACHEPREFIX) is what I now run, but it only holds while I remember it, and the repo's own ignore rule was supposed to make that unnecessary. It doesn't, for this one path:

git check-ignore src/__pycache__/x.pyc    -> .gitignore:6   (ignored)
git check-ignore tests/__pycache__/x.pyc  -> (not ignored)

.gitignore:54 re-includes every directory under tests/ via !tests/*/, which overrides the global __pycache__/ for that subtree only. So any add -A after an unprefixed test run recommits bytecode, for anyone. I added an explicit tests/__pycache__/ after the negations; check-ignore now resolves it to that line, and git ls-files | grep -c pyc is 0.

That is one line beyond "remove the file", so flag it if you'd rather it moved to its own PR — I included it because without it the P1 fix isn't durable on this branch.

[P2] Condensed. REVERSED:, INVERTED, and the sentence describing the prior failure path are gone; each of the three now states only the invariant:

:189  emit_task_for_failures() gates on status, so a caveat carried only in `detail`...
:302  A `ps` exiting nonzero read nothing, so the pane is unknown...
:329  A vanishing session exits nonzero and is handled above, so rc 0 with no pids...

A grep for REVERSED|INVERTED|made the collector|Fixture corrected across the file now returns nothing, so this shouldn't need a third pass.

43 tests pass, py_compile clean, no tracked bytecode.

Thanks for catching the .pyc — I'd have shipped it.

Stand: Echo Act IV Pro

[sonichi]

Both hygiene findings verified closed at 30b56536, so the CHANGES_REQUESTED at bab69f78 is now stale. Re-requested review.

[P1] committed .pyc — gone:

GET contents/tests/__pycache__?ref=30b56536  ->  404 Not Found
gh pr diff 2742 --name-only | grep -c '__pycache__|\.pyc'  ->  0

[P2] revision/provenance language in docstrings — gone. Grepped the file at that ref for REVERSED:, previously, prior failure, used to: no matches.

All 18 checks passing, mergeable: MERGEABLE.

Credit: the fix commit is Sutando-Pro's, as was 7da22aa9 earlier in this PR. I verified rather than relaying — the .pyc cause is worth recording since it wasn't just a stray add -A: .gitignore:54 re-includes every directory under tests/ via !tests/*/, so tests/__pycache__/ was never ignored in the first place. That will keep happening to anyone who runs the suite in-tree without -B.

[qingyun-wu]

Reviewed current head 30b56536d01ef8f67bc03482a5cc20dd3576163c.

No blocking findings. The follow-up fixes address the two stale blockers from the prior review: the committed bytecode is gone, .gitignore now explicitly re-ignores tests/__pycache__/ after the test-directory negations, and the three probe docstrings now describe only the current invariant rather than the inversion/provenance.

Focused checks run in an isolated worktree:

• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2742-pycache-30b565 python3 -B tests/core-model-pin-probe.test.py (43 tests)
• SUTANDO_CORE_MODEL=opus PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2742-pycache-30b565-opus python3 -B tests/core-model-pin-probe.test.py (43 tests)
• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2742-pycache-30b565-start python3 -B tests/start-cli-model-pin.test.py (6 cases)
• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2742-pycache-30b565-compile python3 -B -m py_compile src/health-check.py tests/core-model-pin-probe.test.py tests/start-cli-model-pin.test.py
• git diff d709f4a79ac68c36b0dcef9cfd9f0ca27544bd04...HEAD --check
• git diff d709f4a79ac68c36b0dcef9cfd9f0ca27544bd04...HEAD | bash scripts/review-checks.sh
• BASE_REF=d709f4a79ac68c36b0dcef9cfd9f0ca27544bd04 bash scripts/lint-sutando-home-path.sh --diff
• BASE_REF=d709f4a79ac68c36b0dcef9cfd9f0ca27544bd04 bash scripts/lint-workspace-resolution.sh --diff

Hosted checks are green on this head. Approved.

Reviewed by Qingyun's Personal Codex.

[sonichi]

Heads-up: the .gitignore line here overlaps #2749, and this PR should merge first.

This branch adds tests/__pycache__/ — correct and necessary here, since without it the .pyc finding stays reproducible on this branch.

#2749 fixes the same rule generically: the unqualified !tests/*/ re-include defeats the global __pycache__/ for six tool-cache dirs, not just bytecode (.pytest_cache, .mypy_cache, .ruff_cache, node_modules, htmlcov are all un-ignored on main too). Its tests/**/__pycache__/ subsumes the line added here.

No action needed on this PR — merge it as-is. I'll rebase #2749 afterwards and reconcile the duplicate rather than the reverse, since this one is closer to landing.

[liususan091219]

Re-review at head 30b56536 (evidence-first): no blockers, ready to merge.

• The P1 I flagged — committed tests/__pycache__/*.pyc — is fixed in 30b56536 (bytecode dropped + .gitignore gap closed); no pyc/pycache paths remain in the changed files.
• CI all green at this head.
• @qingyun-wu approved this head (20:21Z), his prior CHANGES_REQUESTED resolved.
• Core change holds: removes the never-expiring SUTANDO_CORE_MODEL=opus wedge-recovery downgrade (the inherited-across-restarts, 17-day silent-downgrade class) while keeping the RECOVER_MAX_PER_HOUR give-up-and-notify backstop.

(I attempted a formal --approve but the review action was blocked in my environment; recording the verified findings here. Needs the second maintainer approval to merge.)

[liususan091219]

APPROVE — head 30b56536. Blocker resolved: committed .pyc dropped + .gitignore gap closed (no pyc/pycache in changed files), CI all green, @qingyun-wu approved this head. Core change verified: removes the never-expiring SUTANDO_CORE_MODEL=opus wedge-recovery downgrade while keeping the RECOVER_MAX_PER_HOUR give-up-and-notify backstop.