fix(F-868): status reports the backend this shell would use, not the first record; liveness ladder + adoption walk extracted to backend_liveness - #104
Merged
Conversation
…first record `~/.stealth-mcp/server.json` holds one entry per display context and nothing prunes a dead one. On a machine recording a dead `win-session-2` (7169), a dead `headless` (19222) and a healthy `win-session-1` (52554) serving 56 proxies, `stealth-chrome-devtools status` run from that same session-1 shell printed `backend : not running` and `pid : 89892` — the pid of a process gone for hours. `singleton._probe_backend_status` selected its record with `backend_registry.first_backend`, which under schema v2 is dict insertion order and says in its own docstring that it carries no preference. Discovery had been walking `adoption_candidates` — the one home for "which backend would THIS client use" — all along, which is why every proxy in that shell was correctly on 52554. The probe now walks that same list and reports the first candidate that answers (wedged over down when none does). No second selection policy: it still only says which of the offered candidates is up. `status`, `doctor`'s summary lines, `stop` and `kill-orphans`'s live-backend guard all already consumed this ONE function. The CLI status block now selects once and passes `(status, port)` down, so its lines cannot describe different backends: `_format_backend_status` is pure formatting, `_recorded_backend_pid(port)` and `_doctor_port_occupant_line(port)` read the entry on that port via `backend_on_port` instead of each making its own `first_backend` read. Two independent selections deleted, none added. `status` gained one `other records:` line naming the display contexts the summary is not about, and only when the record holds more than one. RED first: 7 tests, the CLI ones reproducing the observed output byte-for-byte. Stale records are still pruned by nobody — the argument for leaving that alone is in the finding. Finding: audit/stage2/finding_F868_cli_status_reports_a_dead_record.md
…ned on
Review follow-up on the F-868 branch. Three product changes, all of them
consequences of the same selection bug the first commit fixed.
`singleton._probe_port(port)` is now THE one home for the socket ->
`initialize` -> down/wedged/responsive ladder. It was four lines copied into
`cli._probe_recorded_backend` under a comment justifying the copy with two
claims: that `_probe_backend_status` "reads the FIRST recorded backend" and so
could not answer per-entry, and that singleton.py sat at its budget. The first
stopped being true in the previous commit; the second was never a reason to
have two answers to one question. The CLI form is now a thin adapter keeping
only the one word the ladder cannot reach ("no port recorded").
`restart_backend` reports `_probe_port(port)` for the port it spawned on. It
took `status` from the record-wide adoption walk while `pid` came from
`backend_on_port(state, port)`, so on a multi-context record a responsive
SIBLING reported "responsive" beside the pid of a backend that had just come up
wedged - two processes in one return, contradicting the docstring's own promise
that a wedged restart must be visible. Pinned RED-first: the new test fails
against the old reporter with ('responsive', 4242) == ('wedged', 4242), and
asserts both halves so it cannot pass by the sibling merely being invisible.
`_other_records_note` compares on display context, not port: `backends_in`
stamps a context on every entry, whereas a hand-edited non-int port reads as
None and would have matched a None reported port - hiding itself in exactly the
"nothing is running" case that needs it. Its label is `others :`, aligned
with every other label in the block, and the RUNBOOK sample is now byte-identical
to what ships.
Also: `stop`'s narrowing to adoptable contexts is pinned (green both sides by
construction - it exists so the behaviour change is not reverted by accident,
and the finding says so rather than claiming a RED); the `_v2` record builder
moved to tests/fakes.py as `v2_record`, where the two test modules that had
copies now import it along with the existing `pretend_display_context`; and the
finding's RED claim is corrected to 6 of 7, the single-entry no-note case being
green on both sides.
singleton.py lands at 999/1000 LOC. Two lines were paid for honestly - the
F-856 paragraph in `_same_identity_backend_ready` was retelling what
`scheduling_lag.FairWindow` is THE one home for and now points at it - but one
line of headroom is the most fragile thing here. The next change to that file
needs the `backend_liveness` extraction (probes handed in as arguments, the way
backend_watchdog already does), not more prose-trimming; recorded in CLAUDE.md
and finding section 6.
…leaf Pure move, no behaviour change. singleton.py came out of the F-868 fix at 999 of its 1000-LOC default - a gate that passes and a file nobody can edit. embedded/backend_liveness.py is now THE one home for "is the backend on this port alive, and which recorded backend would THIS client be served by": probe_port (the socket -> initialize -> down/wedged/responsive ladder) and probe_recorded (the adoption-order walk). Both docstrings moved verbatim. A leaf on backend_watchdog's proven pattern: the two liveness primitives arrive as ARGUMENTS (is_healthy / http_ready) and the record arrives as a PATH, so the module never imports singleton and never decides WHICH record either - the adoption ORDER stays backend_registry's policy, merely consumed here. Importing the leaf alone does not pull singleton into sys.modules. singleton keeps thin _probe_port / _probe_backend_status wrappers that bind OUR probes, OUR record path and OUR display context. That is the point, not a hop to delete: the suite patches singleton._server_is_healthy, singleton._backend_http_ready and singleton._probe_port by name, and a wrapper resolving those module globals at CALL time is what keeps every existing monkeypatch.setattr(singleton, ...) reaching this code. probe_recorded takes the per-port probe as a parameter for the same reason, so the walk asks singleton._probe_port rather than its own module-level function. A direct import would bind at import time and silently stop seeing such a patch. singleton.py 999 -> 985; the leaf is 91 lines; no cap ratcheted up. Also folds in the review nit: _cmd_restart's trailing comment still described a "none" verdict, which _probe_port cannot return - that word belonged to the record-wide walk restart stopped using. The branch is unchanged (it is the "down" arm); only the comment is corrected.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect (F-868)
stealth-chrome-devtools statusprintedbackend: not running / pid 89892from the FIRSTserver.jsonrecord (a dead 2.1.1 backend on port 7169, contextwin-session-2) while this shell's own context had a healthy 2.1.5 backend on 52554 (pid 53836) serving 56 proxies. The pid/log lines and the doctor port line were two further independent selections, which is how they could disagree with the status line rather than merely inherit its error.Root cause:
singleton._probe_backend_statuspicked its record withbackend_registry.first_backend(dict insertion order, "carries no preference") while discovery already walkedbackend_registry.adoption_candidates, the one home for "which backend would THIS client use". Full write-up:audit/stage2/finding_F868_cli_status_reports_a_dead_record.md.The fix (two commits, reviewed separately)
c6d27d0—_probe_backend_statuswalksadoption_candidatesfor the shell's own display context; one_probe_port(port)ladder serves the walk,restart_backend(which now reports the SPAWNED port's own liveness instead of a responsive sibling's) and the CLI;statusnames the other records (others: 2 backends recorded (win-session-2, headless) — run doctor for each one's state);stopis per-display-context and pinned as such. Fixture helpers (v2_record,pretend_display_context) live intests/fakes.py.df0152f— pure move: the ladder and the walk go to a new leafembedded/backend_liveness.py(probes, record path and per-port probe arrive as arguments, asbackend_watchdogdoes; the leaf never importssingleton), with thin wrappers left onsingletonso every existing patch target still lands.singleton.py999 → 985.Evidence
('responsive', 4242) == ('wedged', 4242)); verified by the reviewer against main's source.backend: running (responsive) on port 52554 / pid 53836.Not fixed here (named)
stopon a record whose only entries are foreign proven contexts now says "not running" instead of stopping the first one; deliberate, stated, pinned.