diff --git a/CHANGELOG.md b/CHANGELOG.md index dd81c56..1a787a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,39 @@ # Changelog +## Unreleased + +### Fixed — `status` reported a dead sibling's record as "the backend" (F-868) + +On a machine whose `server.json` recorded three display contexts — two dead +(`win-session-2` on 7169, `headless` on 19222) and one healthy backend serving 56 +proxies (`win-session-1` on 52554) — `stealth-chrome-devtools status`, run from that +same session-1 shell, printed `backend : not running` and `pid : 89892`, the pid of a +process that had been gone for hours. `singleton._probe_backend_status` selected the +record with `backend_registry.first_backend`, which under schema v2 is dict insertion +order and carries no preference of its own, while discovery had been walking +`adoption_candidates` — the one home for "which backend would THIS client use" — all +along. The probe now walks that same list and reports the first candidate that answers +(wedged over down when none does), which fixes `status`, `doctor`'s summary lines, +`stop` and `kill-orphans`'s live-backend guard at once, since all four already consumed +it. The CLI status block now also selects once and passes the answer down: the pid and +log lines read the entry on the port just reported (`backend_on_port`) instead of making +their own `first_backend` read, and `doctor`'s port-occupant line takes the same port — +two independent record selections deleted rather than a third added. `status` gained one +`others :` line naming the display contexts it is NOT speaking about when the record +holds more than one, so a summary over a multi-context record no longer reads as "this is +all there is". + +Two things the same selection bug was hiding are fixed with it. The socket→`initialize` +ladder is now `singleton._probe_port`, one home with three callers, instead of four lines +copied into `cli._probe_recorded_backend` under a comment justifying the copy with a claim +about `_probe_backend_status` that this release makes false. And `restart` now reports +that ladder's verdict for **the port it spawned on**: it took its `status` from the +record-wide walk while its `pid` came from the spawned port, so a responsive sibling could +report "responsive" beside the pid of a backend that had just come up wedged — both halves +of one return describing two processes. + +Stale records are still pruned by nobody; see `audit/stage2/finding_F868_cli_status_reports_a_dead_record.md` §6. + ## 2.1.5 ### Fixed — the backend escapes the MCP client's Job Object (F-867) diff --git a/CLAUDE.md b/CLAUDE.md index 0fbf3b6..5c1f45d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,7 +47,7 @@ Package root: `src/stealth_chrome_devtools_mcp/`. Two console scripts (`pyprojec |---|---| | `server.py` | thin entrypoint — loads `embedded/server.py` as `__main__` via `runpy` (`main()` shim); its stdio branch is also **THE one place the PROXY process bootstraps its own observability** (`configure_logging("proxy")` + `_start_proxy_error_reporting`, F-827) — in the branch, never at the top of `main()`, or the runpy path would double-init | | `__main__.py` | `python -m stealth_chrome_devtools_mcp` → `server.main()` | -| `cli.py` | the `stealth-chrome-devtools` ops CLI verbs (`status`/`doctor`/`stop`/`restart`/`cleanup`/`kill-orphans`/`serve`) | +| `cli.py` | the `stealth-chrome-devtools` ops CLI verbs (`status`/`doctor`/`stop`/`restart`/`cleanup`/`kill-orphans`/`serve`). The status block **selects the backend ONCE** (F-868): `_probe_backend_status()` is called by the command, and its `(status, port)` is passed down to every line — `_format_backend_status` (now pure formatting, no I/O), `_recorded_backend_pid(port)` and `_doctor_port_occupant_line(port)` (both read the entry on THAT port via `backend_on_port`, never `first_backend`), and `_other_records_note(port)`, the `others :` line that names the display contexts the summary is *not* about (compared on display context, never on port — a hand-edited non-int port reads as `None` and would hide itself). Two independent record reads were deleted to get here; do not re-add one — a per-line read is how a live backend's status came to sit above a dead sibling's pid. `_probe_recorded_backend` is a thin adapter over `singleton._probe_port`, not a second ladder | | `settings.py` | **the one env home** — pydantic `Settings` + `get_settings()`; every `STEALTH_MCP_*` knob is a typed field here | | `observability.py` | Sentry error shipping — hardcoded DSN, on by default, never raises (no-op under `STEALTH_MCP_NO_ERROR_REPORTING`); **the one PII scrubber** — `_scrub_event` (Sentry's `before_send`); **the one non-exception report** — `capture_lifecycle` (F-827: proxy transitions that are decisions, not crashes) | @@ -57,9 +57,10 @@ Package root: `src/stealth_chrome_devtools_mcp/`. Two console scripts (`pyprojec | File | Owns | |---|---| | `server.py` | the real MCP server — **ZERO tool bodies**; all 94 live in `tool_sections/` (plan_SERVERSPLIT, complete at slice 12). What this file owns, and nothing else: the per-execution FastMCP app (`mcp`/`registry`), `app_lifespan`, the four `@mcp.resource` handlers, the xpool-safe gate, `build_arg_parser`, the `__main__` block, and the **binding loop** that drives registration from THIS file's module body, once per execution of it (so the canonical import, the bare-name spec load and the runpy `__main__` load each get a full 94-tool app — a section module that decorated itself would register into the first execution only). **No migration alias block**: every singleton, knob and guard this file drives is read as `rt.` against `tool_runtime`, the one patchable home, exactly as a tool body does. The lone import left is `clone_storage`, which this file never touches — it is the positive delegation handle `tests/test_clone_storage.py`'s F-201 negative-surface pin needs. At 523 LOC it is governed by the 1000-LOC default; its `GRANDFATHER` row is deleted, not merely satisfied | -| `singleton.py` | **backend lifecycle + the stdio proxy** — liveness probes (`_backend_http_ready`, `_probe_backend_status`), port selection (`_select_backend_port`, `DEFAULT_PORT`), the one identity+readiness reuse gate (`_same_identity_backend_ready`, `_source_fingerprint`, `REUSE_PATIENCE_SECONDS` — which spends its patience through `scheduling_lag.FairWindow`, F-856), cold-start lock (`_start_backend_holding_lock`), `run_stdio_proxy`. The `server.json` record moved to `backend_registry.py` (path names re-exported here for legacy callers); the proxy's *recovery from a dead backend* moved to `proxy_selfheal.py`; the watchdog LOOP moved to `backend_watchdog.py` — `_watch_backend_liveness` stays here as the wiring that knows which probes are ours; and **the spawn itself moved to `backend_launch.py` (F-867)** — `_start_server_process` still owns WHAT to start (`_server_process_cmd`, `_backend_interpreter`'s F-866 base-interpreter choice, the child env, F-830's boot-log roll) and what to RECORD afterwards (`PORT_FILE`, `_write_server_state`), and makes exactly ONE call, `backend_launch.spawn(cmd, child_env, boot_log) -> Launched(pid, rung)`; it no longer imports `subprocess` | +| `singleton.py` | **backend lifecycle + the stdio proxy** — the liveness PRIMITIVES (`_backend_http_ready`, `_server_is_healthy`) and the two thin bindings that hand them to `backend_liveness` (`_probe_port`, `_probe_backend_status` — see that row for the policy; they are wrappers on purpose, because the suite patches these names and a wrapper resolves them at CALL time). `status`, `doctor`, `stop` and `kill-orphans`'s live-backend guard all consume `_probe_backend_status`, so record selection is fixed in one place; `restart_backend` deliberately does NOT — it reports `_probe_port` for the port it spawned on, because the adoption walk answers "is there a backend for me" and a responsive SIBLING must never speak for a restart's own fresh backend. Port selection (`_select_backend_port`, `DEFAULT_PORT`), the one identity+readiness reuse gate (`_same_identity_backend_ready`, `_source_fingerprint`, `REUSE_PATIENCE_SECONDS` — which spends its patience through `scheduling_lag.FairWindow`, F-856), cold-start lock (`_start_backend_holding_lock`), `run_stdio_proxy`. The `server.json` record moved to `backend_registry.py` (path names re-exported here for legacy callers); the proxy's *recovery from a dead backend* moved to `proxy_selfheal.py`; the watchdog LOOP moved to `backend_watchdog.py` — `_watch_backend_liveness` stays here as the wiring that knows which probes are ours; and **the spawn itself moved to `backend_launch.py` (F-867)** — `_start_server_process` still owns WHAT to start (`_server_process_cmd`, `_backend_interpreter`'s F-866 base-interpreter choice, the child env, F-830's boot-log roll) and what to RECORD afterwards (`PORT_FILE`, `_write_server_state`), and makes exactly ONE call, `backend_launch.spawn(cmd, child_env, boot_log) -> Launched(pid, rung)`; it no longer imports `subprocess` | | `backend_launch.py` | **THE one home for "spawn the backend where no MCP client's Job Object can reach it"** (F-867) — `spawn`, `Launched(pid, rung)` and nothing else in the tree may create a backend. POSIX is one rung (`posix`, `start_new_session=True`, unchanged). Windows climbs three, and the rung that served is logged through the ONE `_log_rung` line on `stealth.proxy` so a post-mortem can read it: (1) `breakaway` — `CREATE_BREAKAWAY_FROM_JOB`, accepted ONLY when `_proven_in_a_job` says the new process is in NO job, because under a nested job chain the flag leaves the innermost job only and a "successful" breakaway can still sit in the client's; `ERROR_ACCESS_DENIED` (what the SDK's job returns) drops a rung while any other `OSError` propagates. A PARTIAL escape is never discarded blindly — rung 2's viability (`_scheduler_plan`) is decided FIRST, and when there is no scheduler rung the child is KEPT as the `breakaway-partial` rung at WARNING, because out of one job beats out of none; conversely, breakaway once proven PERMITTED is re-asked on rung 3. (2) `scheduler` — a one-shot Task Scheduler task through `desktop_launch._schtasks` (the F-810 seam, not a second home) running `pythonw.exe` (no console, so no window can flash) on the BASE interpreter, never a venv's `Scripts\pythonw.exe`, which is F-866's redirector and would rebuild the kill-on-close job; the stdlib-only `_LAUNCHER_SCRIPT` reads argv + the ENTIRE child env + the boot-log path from a JSON spec in `~/.stealth-mcp/backend-launch/` (the command line carries two paths only because **`schtasks` stores at most 253 chars of `/TR` and truncates beyond that with exit 0** — measured, not F-810's documented "~261"; the silent truncation costs a Last-Result-2 task, the whole 20 s pid deadline and a drop to `plain`, which is why the per-attempt token is 12 hex chars, not 32), re-opens `backend-boot.log` itself and gives the backend stdout AND stderr (F-303 survives the hand-off), and returns the SERVING pid through an `os.replace`d pid file. **Gated on `_same_session_as_console`**: taken only when the spawner is already in the logged-on console session, so the backend lands where it would have anyway and the display context `singleton` records stays true (F-808 — the tool still never PICKS a session). Task and scratch files deleted in a `finally`, and the orphan sweep that runs before a scheduler spawn is a FILESYSTEM sweep, not a `/Query`: any `.json` spec (token = 12 hex) older than 2× the pid deadline gets its `stealth-mcp-backend-` task deleted BY NAME and its files removed. Deliberately not "delete tasks whose spec is absent" — `_cleanup` deletes the task first, so a killed spawner leaves task AND spec, which is the only orphan class there is and the one that predicate missed. A live sibling's spec is younger and untouched; a clean dir costs zero `schtasks` calls. (3) `plain` — today's `DETACHED_PROCESS \| CREATE_NEW_PROCESS_GROUP` (plus the breakaway bit when rung 1 proved it permitted), for a runner with no console session; its line names F-867 and says the backend is inside this client's job. A leaf: `backend_registry` for the state dir, `desktop_launch` lazily for the ONE `schtasks` seam, the ONE pid-file reader and the ONE task teardown, so none of them gets a second home. Never raises for a rung's own failure — only a genuine `Popen` error reaches the caller | | `backend_watchdog.py` | **THE one home for the proxy's mid-session liveness watchdog** — the SLOW witness (`watch_liveness`): F-820's strikes plus the confirmation phase. A leaf: both probes arrive as arguments, so it never imports `singleton` and the dead-vs-busy policy stays single-homed in the reuse gate | +| `backend_liveness.py` | **THE one home for "is the backend on this port alive, and which recorded backend would THIS client be served by"** (F-868) — `probe_port`, the socket→`initialize`→`down`/`wedged`/`responsive` ladder (never copy those four lines again; `cli._probe_recorded_backend` is an adapter that adds only the one word the ladder cannot reach, `"no port recorded"`), and `probe_recorded`, the ADOPTION-order walk that reports the first candidate which answers — wedged over down when none does, `("none", None)` when no adoptable entry names a port at all — never `first_backend`'s "whichever entry the record lists first". A leaf on `backend_watchdog`'s pattern: the two primitives arrive as ARGUMENTS (`is_healthy`/`http_ready`) and the record as a PATH, so it never imports `singleton` and the order itself stays `backend_registry`'s policy. `probe_recorded` takes the per-port probe as an argument too, so `singleton._probe_port` — the name the suite patches — is what the walk actually asks | | `scheduling_lag.py` | **THE one home for "was this process scheduled fairly, and what does a time budget owe it when it was not"** (F-856) — `FairWindow`, whose budget is charged in fair seconds (elapsed ÷ the lag its own naps measured), plus `MAX_STRETCH`, `REPORT_FACTOR` and the `proxy: patience extended under starvation` lifecycle report. It never decides alive-or-dead: only "has this window been spent", so `proxy_selfheal`'s ONE heal path is untouched. A leaf; the `_now`/`_wait` module functions are its single timing seam | | `session_hygiene.py` | **THE one home for "this MCP session was abandoned by its client — reap it"** (F-862) — `HygienicSessionManager`, the MCP streamable-HTTP session manager plus a sweep: a session with NO standing GET event stream (a live proxy always holds one) and no request for `ABANDONED_AFTER_SECONDS` is terminated through the transport's own `terminate()`, so a liveness probe whose DELETE was lost or a proxy that died no longer costs the backend ~0.11 MB forever. `install()` binds the class to the name FastMCP constructs by module attribute — the one seam, called from `server.py`'s http branch as `rt.session_hygiene.install()` BEFORE `mcp.run()`. A leaf: imports no other embedded module | | `serve_startup.py` | **THE one home for "startup work that must not delay the backend's first serve"** (F-856) — `after_serving`, which runs one idempotent, best-effort startup job on a daemon thread. Its docstring carries the safety argument for reaping orphans CONCURRENTLY with serving. Deliberately not a general background-task runner: `clone_storage.spawn_background_sweep` keeps its own asyncio task, dedupe and trigger-time root capture | diff --git a/RUNBOOK.md b/RUNBOOK.md index f9c2a57..86fd00a 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -51,8 +51,20 @@ browser-session cap : 20.0 GB [STEALTH_MCP_BROWSER_SESSION_STORAGE_CAP_GB] - **`backend`** is the real liveness state (`singleton._probe_backend_status`): `responsive` = answers a real MCP `initialize`; `wedged` = socket open but not answering (→ `restart`); `down` = recorded but nothing there; "not running" = no - recorded backend. The port shown is the **chosen** port, which may differ from - `19222` if that was taken (see "Port already in use" below). + recorded backend this shell could adopt. The port shown is the **chosen** port, which + may differ from `19222` if that was taken (see "Port already in use" below). +- **Which backend is it about?** The one *this shell* would be served by — the same + adoption order discovery uses (F-868), not whichever entry `server.json` lists first. + `pid` and `log` name that same backend, so the four lines can never describe different + processes. `server.json` can hold one entry per display context, and dead ones are + never pruned, so an `others` line appears when there are others: + + ``` + others : 2 backends recorded (win-session-2, headless) — run `doctor` for each one's state + ``` + + `doctor`'s `contexts :` block probes every recorded backend on its own port; that is + the place to look when you want all of them rather than yours. - **`browser-session root`** and **`browser-session cap`** are about **disk** — the directory holding named browser-session profiles/clones and the cap that trims idle ones. They are named "browser-session" deliberately: this cap trims *named diff --git a/audit/stage2/finding_F868_cli_status_reports_a_dead_record.md b/audit/stage2/finding_F868_cli_status_reports_a_dead_record.md new file mode 100644 index 0000000..7d8be32 --- /dev/null +++ b/audit/stage2/finding_F868_cli_status_reports_a_dead_record.md @@ -0,0 +1,284 @@ +# F-868 — `status` reports the FIRST recorded backend, so an operator with a healthy backend is told "not running" and handed a dead pid + +**Status:** FIXED in this PR (product defect; reproduced hermetically from the exact +observed record, root cause confirmed in source) +**Opened by:** the 2026-09-14 ~23:20 local ops-CLI observation on the maintainer's +workstation (PyPI 2.1.5, Windows 11, `~/.stealth-mcp/server.json` schema 2, three entries) +**Source at:** `main` = `267bac8` +**Severity:** MEDIUM. No data loss and no backend is harmed, but it is a truthfulness +defect in the ONE surface an operator consults to decide whether the product is broken — +and the answer it gives ("not running") is the one that invites a `restart`, a +`kill-orphans`, or a bug report, in the exact state where nothing is wrong. + +--- + +## 1. What was observed + +`~/.stealth-mcp/server.json`, schema 2, three entries, in this recorded order: + +| context | port | version | pid | actual state at 23:20 | +|---|---|---|---|---| +| `win-session-2` | 7169 | 2.1.1 | 89892 | pid DEAD, nothing listening | +| `headless` | 19222 | 2.1.3 | 67720 | pid DEAD, nothing listening | +| `win-session-1` | 52554 | 2.1.5 | 53836 | pid ALIVE, listening on 127.0.0.1:52554, healthy, serving 56 proxies | + +The shell running the CLI was in Windows session 1 — the active console session — so +`display_context.display_context()` answers `win-session-1` for it, and +`singleton._find_running_server` adopts and proxies to 52554 from that same shell. + +`stealth-chrome-devtools status` printed: + +``` +backend : not running +pid : 89892 +log : C:\Users\amind\.stealth-mcp\logs\backend-89892.log +version : 2.1.5 +``` + +Three separate untruths in four lines: the backend this shell would be served by was +responsive; the pid named a process that had not existed for hours; and the log path +pointed at a dead backend's file instead of the live one's. + +**Reproduced hermetically** (`tests/test_cli_status_wedged.py::TestCliStatusReportsTheBackendThisClientWouldUse`, +the record above verbatim, both liveness primitives stubbed so no socket is opened): +RED output before the fix was byte-identical to the observation — +`'backend : not running\npid : 89892\n…'`. + +## 2. Root cause + +**One function picked the record, and it picked by position rather than by relevance:** +`singleton._probe_backend_status` (`src/stealth_chrome_devtools_mcp/embedded/singleton.py:160` +at `267bac8`) read + +```python +entry = backend_registry.first_backend(_read_server_state()) # singleton.py:171 +``` + +`first_backend` (`embedded/backend_registry.py:135`) is honest about what it is and says +so in its own docstring: *"'First' preserves the pre-v2 single-backend behaviour exactly… +It carries no preference of its own — a caller that wants the backend most likely to be +usable asks `window_capable_first`."* Under schema v2 "first" is dict insertion order, +i.e. **whichever context happened to record itself earliest and has not been superseded +by port since** — here `win-session-2`, a backend from 2.1.1 that had been dead for two +releases. The probe therefore reported that entry's port (7169) as `down`, and +`cli._format_backend_status` maps `down` and `none` alike to `"not running"`. + +**Why the own-context record was not preferred.** The ordering that answers "which +backend would THIS client actually use" already exists and is already the one home for +that policy: `backend_registry.adoption_candidates(path, own_context)` +(`backend_registry.py:178`), asymmetric by design — a client that can PROVE it has a +desktop adopts only its own context's entry plus `UNVERIFIED` ones; a client that cannot +adopts anything, window-capable first. `singleton._find_running_server` +(`singleton.py:231`) walks exactly that list at `singleton.py:242`, which is why the +proxies in that same shell were all correctly on 52554. `_probe_backend_status` simply +never consulted it. For `own_context == "win-session-1"` the candidate list is a single +entry — the live one — so the right answer was one call away. + +**The pid line was a SECOND, independent selection**, which is why it could disagree with +the status line rather than merely inherit its error: `cli._recorded_backend_pid` +(`cli.py:144`) did its own `backend_registry.first_backend(singleton._read_server_state())` +at `cli.py:154`. `cli._doctor_port_occupant_line` (`cli.py:274`) made a third at +`cli.py:280`. Three reads of "the backend", none of them agreeing by construction. +(`singleton.stop_backend:607` and `restart_backend:660` had already learned this lesson +and read `backend_on_port(…, port)` — "the entry recorded ON THIS PORT, not merely the +first" — so the pattern the CLI needed was already in the tree.) + +## 3. What is and is not affected + +**Affected — all through the ONE shared `_probe_backend_status`:** + +| verb | consumed at | symptom on the observed record | +|---|---|---| +| `status` | `cli.py:128` (via `_format_backend_status`) | "not running" + dead pid + dead log path | +| `doctor`'s `backend :` / `pid :` / `log :` / `port :` summary lines | `cli.py:381`+ | same four lines, same wrongness | +| `stop` | `singleton.py:600` | targets `win-session-2`'s record: terminates nothing, reports "already stopped", and FORGETS the dead sibling's entry while the live own-context backend keeps running. Not destructive, but it is not the backend the operator asked to stop | +| `kill-orphans` | `cli.py:502` (gate), `cli.py:504` (the pid it names) | the "a backend is running — use restart, or pass `--force`" guard reads `down` where the true answer is `responsive`, so the verb runs its reaper beside a live backend and then prints "reaped any browsers left over from a dead backend" while that backend is serving. The live backend's OWN browsers are still spared — `_recover_orphaned_processes` re-checks recorded ownership through `_owner_backend_alive` (`process_cleanup.py:319`, the F-808 fix) and only `--force` bypasses that — so this is a defeated outer gate, not a fleet kill. The residual is real but narrower: anything recorded under an owner that is NOT alive is reaped, and the operator is told the backend is dead | + +**Not affected:** + +- **Discovery / the proxies.** `_find_running_server` already walks + `adoption_candidates`; every one of the 56 proxies was correctly on 52554. This was + never a routing bug — only a reporting one. +- **`doctor`'s `contexts :` block.** `_doctor_backend_lines` (`cli.py:193`) walks + `window_capable_first` over the WHOLE record and probes each entry on its own port + (`cli.py:231`), so it listed all three backends with per-port liveness correctly. The + information was on screen four lines below the lie. +- **`restart`.** It selects its own port through `_select_backend_port` / + `own_or_first_port` (own-context first) and terminates exactly the port it selected, so + it never aimed at the sibling. Only its post-restart REPORT came through + `_probe_backend_status`. +- **The record itself.** No writer is wrong; `record_backend`'s supersede-by-port rule is + working as designed. What is wrong is only who READS it for a single-value answer. + +**Stale records are pruned by nobody.** A dead backend's entry leaves `server.json` in +exactly two ways: `stop_backend` → `forget_backend` for the context it stopped +(`singleton.py:617`), and `record_backend`'s supersede-by-port when some LATER backend +claims the same port (`backend_registry.py:381-384`). A backend that is killed, crashes, +or whose desktop logs out leaves its entry behind forever — nothing sweeps on pid +liveness, and `2.1.1` / `2.1.3` entries surviving under a `2.1.5` install is the proof. +That is deliberate to a point (a record is a never-raise cache, and "pid gone" is not +"port free"), but it means the dict grows monotonically per display context and the +FIRST entry becomes steadily less likely to be the relevant one over a machine's +lifetime. **Deliberately NOT changed here — see §6.** + +## 4. The fix + +**The liveness ladder now has one home.** The socket → `initialize` → `down` / `wedged` / +`responsive` ladder was four lines duplicated in `cli._probe_recorded_backend`, justified +there by a note saying `_probe_backend_status` "reads the FIRST recorded backend" and so +could not answer per-entry. That justification dies with this fix, and a duplicated +liveness ladder was a second way to answer one question regardless. It now has three +callers: the candidate walk, `restart_backend`, and the CLI form, which keeps only the one +word the ladder cannot reach ("no port recorded"). It lives in +`embedded/backend_liveness.py` as `probe_port`, beside the adoption walk +(`probe_recorded`), reached through `singleton._probe_port` / `_probe_backend_status` — +see §6 for why those wrappers exist and why the leaf takes its probes as arguments. + +**One home, no second way.** `_probe_backend_status` now walks +`backend_registry.adoption_candidates(SERVER_STATE_FILE, display_context.display_context())` +— the same list, from the same function, in the same order that `_find_running_server` +uses — and reports the first candidate that ANSWERS. With none responsive it reports the +most informative verdict it saw, `wedged` over `down`, because a wedged backend holds a +port and will be evicted while a `down` record names nothing running at all. No selection +policy is introduced: this function still only says which of the offered candidates is +up, and `adoption_candidates` remains the sole home for WHICH candidates those are. +Fixing it here fixes `status`, `doctor`, `stop` and `kill-orphans` at once, because all +four already consumed this one function. + +**The CLI status block now selects once and passes the answer down**, so its lines cannot +disagree with each other: + +- `_format_backend_status(status, port)` is pure formatting and performs no I/O. +- `_recorded_backend_pid(port)` reads `backend_on_port` — the entry on the port just + reported — instead of `first_backend`. This deletes a second selection rather than + adding a parallel one, and adopts the rule `stop_backend`/`restart_backend` already use. +- `_doctor_port_occupant_line(port)` takes that same port (falling back to + `DEFAULT_PORT` when nothing is reported, exactly as before), deleting the third. + +**`restart`'s report is pinned to the port it spawned on.** `restart_backend` took its +`status` from `_probe_backend_status()` while its `pid` came from `backend_on_port(…, +port)`, so on a multi-context record a responsive SIBLING could report "responsive" beside +the pid of a backend that had just come up wedged — the two halves of one return +describing two processes, and a direct contradiction of the docstring's promise that "a +restart that comes back wedged or down must be visible". Both halves now read the one +selected port via `_probe_port(port)`, so they agree by construction. The adoption walk +answers "is there a backend for me", which is the right question for `status` and the +wrong one here. + +**`status` now says what it is not speaking about.** `_other_records_note(port)` adds one +line when — and only when — the record holds entries besides the reported one: + +``` +backend : running (responsive) on port 52554 +pid : 53836 +log : C:\Users\amind\.stealth-mcp\logs\backend-53836.log +others : 2 backends recorded (win-session-2, headless) — run `doctor` for each one's state +version : 2.1.5 +``` + +It never re-decides which backend to report; it names what the reported one is not and +points at the verb that probes them all. A single-entry record prints no such line. +"Other" is decided on DISPLAY CONTEXT, not on port: `backends_in` stamps a context on +every entry, so the comparison is always against a real value, whereas a hand-edited entry +whose `port` is a string reads as `None` and would have matched a `None` reported port — +hiding itself in precisely the "nothing is running" case that needs it most. + +No new `STEALTH_MCP_*` knob, no new env read, no `typing.Any`, no LOC-budget change: +`cli.py` 645 → 691, `singleton.py` 979 → 999 → **985** once the `backend_liveness` +extraction landed (§6), and the new leaf is 91. + +## 5. Verification + +- `tests/test_cli_status_wedged.py::TestCliStatusReportsTheBackendThisClientWouldUse` + (4 tests, hermetic — `SERVER_STATE_FILE` redirected to `tmp_path`, both liveness + primitives patched so no socket is opened and no real process is touched): the observed + three-entry record reports 52554/53836 and its log path for a `win-session-1` client; + the same record reports the live backend for a `headless` client (whose adoption list + tries the dead capable entry FIRST); the other-records line appears with its contexts + and the `doctor` pointer; a single-entry record prints no such line. +- `tests/test_probe_backend_status.py::TestProbeWalksAdoptionOrder` (3 tests, hermetic, + real listeners on OS-assigned ports via the file's existing `responsive_stub` / + `wedged_stub` fixtures): a dead first entry does not hide the live own-context one; a + dead window-capable entry does not end an unproven client's search; a wedged candidate + outranks a dead one. +- `tests/test_singleton_stop_restart.py::TestRestartReportsTheSpawnedPort` (1 test): a + responsive SIBLING must not answer for a restart whose own fresh backend came up + wedged. RED against the pre-fix reporter with `('responsive', 4242) == ('wedged', 4242)` + — the sibling speaking for the wrong process — and it asserts BOTH halves, so it cannot + pass by the sibling merely being invisible: `_probe_backend_status()` is separately + shown to say `("responsive", sibling_port)` at the same moment. +- `tests/test_singleton_stop_restart.py::TestStopIsPerDisplayContext` (1 test): a record + holding only a FOREIGN proven context, own context a different proven desktop → + `stop_backend()` is `("not running", None)`, `_terminate_backend` is never called, and + the foreign entry is still recorded afterward. **Not RED-first, and deliberately so** — + it is green on this branch by construction, because the adoption walk IS the fix. It is + pinned because it is a BEHAVIOUR CHANGE riding on a bug fix (§6), and an unpinned + behaviour change is the kind a later "simplification" reverts without noticing. +- RED first: **6 of the 7 status tests failed before the product change** (plus the + restart pin above, 7 of 8 overall), the CLI ones with output identical to the + observation (§1). The seventh, + `test_a_single_recorded_backend_gets_no_other_records_line`, is green on BOTH sides by + construction and is not claimed as a RED: with a single-entry record `first_backend` and + the adoption walk select the same entry, and the assertion is a negative one about a + line that did not exist before. It earns its place as the boundary of the new note, not + as evidence of the defect. GREEN after. +- Regression scope run: `test_cli.py`, `test_cli_status_wedged.py`, + `test_probe_backend_status.py`, `test_backend_registry.py`, + `test_singleton_stop_restart.py`, `test_singleton_display_routing.py`, + `test_singleton_version_aware.py`, `test_singleton_port_fallback.py`, + `test_singleton_fast_handshake.py`, `test_singleton_backend_logging.py`, + `test_singleton_cold_start_logging.py`, `test_singleton_starvation_patience.py`, + `test_find_running_server_app_probe.py`, `test_proxy_selfheal.py`, + `test_no_silent_excepts.py`, `test_silent_excepts_log.py`, `test_doc_claims.py`, + `test_doc_examples.py`, `test_release_contract.py` — all green. + `ruff check` / `ruff format --check` / `tools/check_file_budgets.py` clean. + +## 6. Not claimed / follow-ups + +- **Pruning stale records is NOT done here, deliberately.** It is a separate decision + with a real safety argument on both sides, and it needs an owner: + - *Who would prune?* The only process that can prune honestly is one that has just + probed. A CLI verb (`status`/`doctor`) must stay read-only by contract (module + docstring, `STEALTH_MCP_NO_AUTO_RECOVERY=1`), so it may not. `stop` + already forgets what it stopped. The natural candidate is the cold-start path under + `_exclusive_lock` — it is the only writer that holds the lock — but a backend whose + pid is gone is not necessarily a backend whose PORT is free, and dropping the entry + loses the record that would let a later spawn step around a squatter. + - *What is the cost of not pruning?* Bounded: one small JSON object per display context + per machine lifetime, and (after this fix) no reader is misled by it. The cost of + pruning wrong is a live sibling made undiscoverable, which F-808 already paid for + once. +- **`stop` on a record with no adoptable entry** now reports `not running` where it + previously reported (and forgot) a foreign proven context's entry. That is the intended + consequence of the same rule — an operator's `stop` should not reach across into + another desktop's backend — but it is a behaviour change, not merely a bug fix, and it + is stated here rather than left to be discovered. +- **`restart`'s post-restart report: CLOSED here** (it was scoped out of the first draft + and put back on review). `_probe_port` made it a one-line change rather than a contract + change — see §4 and the `TestRestartReportsTheSpawnedPort` pin in §5. +- **`stop`'s narrowing is a behaviour change, and is now pinned** rather than only + described: `TestStopIsPerDisplayContext` (§5). On a record whose only entries are + foreign PROVEN contexts, `stop` reports `not running` and leaves them alone, where it + previously reported one, terminated nothing, and then forgot the record — quietly making + a possibly-live sibling undiscoverable. That pin is green on both sides of the fix by + construction; it exists to stop the change being reverted by accident, not as evidence. +- **The probe now costs up to one connect attempt per adoptable candidate** instead of + exactly one. Refused loopback connects are immediate; the only slow case is a candidate + whose socket is open but silent (one `LIVENESS_PROBE_TIMEOUT`, 2 s), and the walk stops + at the first responsive one. Not measured on a pathological record — `status` is an + interactive verb with no deadline. +- **`singleton.py`'s LOC: DONE, in its own commit.** The fix left that file at 999 of its + 1000-LOC default — one line of headroom, which is a gate that passes and a file nobody + can edit. On review this was escalated and the extraction was done as a SEPARATE commit, + so "the fix" and "the move" read independently: `embedded/backend_liveness.py` is now THE + one home for the ladder (`probe_port`) and the adoption walk (`probe_recorded`), on + `backend_watchdog`'s proven leaf pattern — the two primitives arrive as ARGUMENTS and the + record as a PATH, so it never imports `singleton`. `singleton` keeps thin + `_probe_port` / `_probe_backend_status` wrappers that bind OUR probes, OUR record path + and OUR display context, which is what keeps every existing + `monkeypatch.setattr(singleton, …)` in the suite reaching the code (a direct import would + bind at import time and silently stop seeing the patch — the standing lesson from the + "moving module globals breaks monkeypatch" incident). `singleton.py` 999 → **985**; + the new leaf is 91 lines. Two lines of the original growth were also 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 instead. diff --git a/src/stealth_chrome_devtools_mcp/cli.py b/src/stealth_chrome_devtools_mcp/cli.py index a4d0afc..cea0342 100644 --- a/src/stealth_chrome_devtools_mcp/cli.py +++ b/src/stealth_chrome_devtools_mcp/cli.py @@ -113,19 +113,19 @@ def _gb_to_bytes(gb: float | None, fallback: int) -> int: # ── commands ──────────────────────────────────────────────────────────────── -def _format_backend_status() -> str: - """Human-readable backend status for status/doctor, driven by - `_probe_backend_status` (plan_M1 SS2.1-D) instead of `_find_running_ - server`'s binary reuse-or-not answer. Closes F-301's "status prints - *running* through the whole outage" half: a wedged backend (socket open, - dispatch loop dead) now reports UNRESPONSIVE instead of a plain - "running" indistinguishable from a genuinely healthy one. Read-only - - this performs a single initialize+DELETE probe, self-cleaning, same as - every other consumer of `_backend_http_ready` (never evicts or spawns). +def _format_backend_status(status: str, port: int | None) -> str: + """Human-readable backend status for status/doctor, formatting what + `_probe_backend_status` (plan_M1 SS2.1-D) reported instead of + `_find_running_server`'s binary reuse-or-not answer. Closes F-301's "status + prints *running* through the whole outage" half: a wedged backend (socket + open, dispatch loop dead) now reports UNRESPONSIVE instead of a plain + "running" indistinguishable from a genuinely healthy one. + + The probe is the CALLER's (F-868): every line of the status block — + liveness, pid, log path, port occupant — must describe ONE backend, and the + only way to guarantee that is to select it once and pass it down. This + function is pure formatting and performs no I/O at all. """ - from stealth_chrome_devtools_mcp.embedded import singleton - - status, port = singleton._probe_backend_status() # "down" (a stale record but nothing actually listening) and "none" (no # record at all) both read as "not running" to an operator - there is no # live process to reconnect to either way; plan_M1 SS2.1-D's three @@ -141,20 +141,58 @@ def _format_backend_status() -> str: return f"running (responsive) on port {port}" -def _recorded_backend_pid() -> int | None: - """The pid singleton last recorded for the backend (server.json), or None - if there is no record. Independent of liveness — status/doctor combine +def _recorded_backend_pid(port: int | None) -> int | None: + """The pid singleton recorded for the backend on ``port``, or None when + that port names no entry. Independent of liveness — status/doctor combine this with `_format_backend_status()`'s liveness read separately (F-305). - The FIRST recorded backend: the record can now hold one per display context - (F-808), and naming them all belongs to `_doctor_backend_lines`, not to this - one-value line.""" + The backend on THAT port, never the first recorded one (F-868). The record + holds one entry per display context (F-808), so "first" is routinely a + different backend from the one the status line just reported — that split + is what printed a dead sibling's pid under a live backend's status. Same + agree-on-one-port rule `singleton.stop_backend` and `restart_backend` + already apply; naming every entry belongs to `_doctor_backend_lines`.""" from stealth_chrome_devtools_mcp.embedded import backend_registry, singleton - entry = backend_registry.first_backend(singleton._read_server_state()) + entry = backend_registry.backend_on_port(singleton._read_server_state(), port) return backend_registry.recorded_int(entry, "pid") +def _other_records_note(port: int | None) -> str: + """The display contexts recorded BESIDE the one reported, or "" when the + reported backend is the only entry there is (F-868). + + The status block is one summary line over a record that can hold an entry + per display context, and a summary that silently drops the rest reads as + "this is all there is". It never re-decides which backend to report — it + only names what the reported one is not, and points at the verb that probes + them all. + + "Other" is decided on DISPLAY CONTEXT, not on the port: `backends_in` + stamps a context on every entry (the v2 key, or `UNVERIFIED` for a v1 + record), so the comparison is always against a real value, whereas a + hand-edited entry whose `port` is a string reads as `None` and would have + matched a `None` reported port — silently hiding itself in exactly the + "nothing is running" case where the operator most needs to see it. With + nothing reported at all, every recorded entry is correctly an "other".""" + from stealth_chrome_devtools_mcp.embedded import backend_registry, singleton + + state = singleton._read_server_state() + reported = backend_registry.backend_on_port(state, port) + mine = reported.get("display_context") if reported else None + others = [ + str(entry.get("display_context")) + for entry in backend_registry.backends_in(state) + if entry.get("display_context") != mine + ] + if not others: + return "" + return ( + f"{len(others)} backends recorded ({', '.join(others)}) — " + "run `doctor` for each one's state" + ) + + def _backend_log_location(pid: int | None) -> str: """Where to look for backend logs (F-503's log-path half: M3 delivered "there is now a log"; this delivers "here is where"). Names the exact @@ -167,27 +205,22 @@ def _backend_log_location(pid: int | None) -> str: def _probe_recorded_backend(port: int | None) -> str: """One recorded backend's liveness on a port the caller already holds, in - `_probe_backend_status`'s exact vocabulary: down / wedged / responsive — - plus "no port recorded" for an entry naming nothing usable as a port, a - state that function cannot reach (it reports such a record as no backend - at all, and so has no word for it). - - Same two primitives in the same order producing the same three words as - `singleton._probe_backend_status` (ONE liveness vocabulary, plan_M8 - SS2.1-B) — all that differs is where the port comes from. That function - reads the FIRST recorded backend, which cannot answer for a record holding - one per display context (F-808), and singleton.py sits at its LOC budget, - so the per-port form lives here rather than beside it. It reaches the - primitives THROUGH the module (`singleton._server_is_healthy`), never by - importing their names, so a test that patches singleton still reaches them. + the ONE liveness vocabulary (plan_M8 SS2.1-B): down / wedged / responsive, + plus "no port recorded" for an entry naming nothing usable as a port. + + That fourth word is the whole of what this adds. The ladder itself is + `singleton._probe_port` and is CALLED, not copied (F-868) — it used to be + the same four lines in both files, justified by a note that + `_probe_backend_status` "reads the FIRST recorded backend" and so could not + answer per-entry. It no longer does, and duplicating a liveness ladder was + a second way to answer one question regardless. Reached THROUGH the module, + never by importing the name, so a test that patches singleton still wins. """ from stealth_chrome_devtools_mcp.embedded import singleton if port is None: return "no port recorded" - if not singleton._server_is_healthy(port): - return "down" - return "responsive" if singleton._backend_http_ready(port) else "wedged" + return singleton._probe_port(port) def _doctor_backend_lines() -> list[str]: @@ -200,7 +233,9 @@ def _doctor_backend_lines() -> list[str]: is refused when the backend serving it can neither display a window nor hand the launch to a logged-on desktop (F-810), and that refusal points the operator at this command, so it must be able to name every context — the - summary line, which reports the first recorded backend only, cannot. + summary line above, which reports the ONE backend this shell would be served + by (`_probe_backend_status`'s adoption walk, F-868), cannot: every entry a + proven-capable client will not adopt is missing from it by design. Ordering is `backend_registry.window_capable_first`'s, so doctor presents the same preference discovery applies rather than re-deriving one. @@ -271,15 +306,17 @@ def _doctor_backend_lines() -> list[str]: return lines -def _doctor_port_occupant_line() -> str: +def _doctor_port_occupant_line(port: int | None) -> str: """F-509 visibility: is the target port free, ours, or a NON-stealth process squatting it (which would otherwise silently block a backend - from binding)? Uses only existing helpers — no new port logic.""" - from stealth_chrome_devtools_mcp.embedded import backend_registry, singleton + from binding)? Uses only existing helpers — no new port logic. - entry = backend_registry.first_backend(singleton._read_server_state()) - recorded = entry.get("port") if entry else None - port = recorded if isinstance(recorded, int) else singleton.DEFAULT_PORT + ``port`` is the one the status block is already about (F-868), so this line + cannot describe a different backend's port than the two lines above it; with + nothing reported it falls back to the default the next spawn would prefer.""" + from stealth_chrome_devtools_mcp.embedded import singleton + + port = port if port is not None else singleton.DEFAULT_PORT our_pid = singleton._backend_pid_on_port(port) if our_pid is not None: return f"port {port} held by our backend (pid {our_pid})" @@ -293,10 +330,16 @@ def _cmd_status(_args) -> int: from stealth_chrome_devtools_mcp.embedded import singleton root = cs.default_session_root() - pid = _recorded_backend_pid() - print(f"backend : {_format_backend_status()}") + # ONE selection for the whole block (F-868): probe first, then report that + # backend's pid, its log and the records this line is not about. + status, port = singleton._probe_backend_status() + pid = _recorded_backend_pid(port) + others = _other_records_note(port) + print(f"backend : {_format_backend_status(status, port)}") print(f"pid : {pid if pid is not None else '-'}") print(f"log : {_backend_log_location(pid)}") + if others: + print(f"others : {others}") print(f"version : {singleton._server_version()}") print(f"browser-session root: {root} (exists: {root.exists()})") print( @@ -381,6 +424,8 @@ def _cmd_cleanup(args) -> int: def _cmd_doctor(_args) -> int: import platform + from stealth_chrome_devtools_mcp.embedded import singleton + cs = _clone_storage() ok = True @@ -388,11 +433,12 @@ def _cmd_doctor(_args) -> int: print(f"platform : {platform.platform()}") root = cs.default_session_root() print(f"browser-session root: {root} (exists: {root.exists()})") - pid = _recorded_backend_pid() - print(f"backend : {_format_backend_status()}") + status, port = singleton._probe_backend_status() + pid = _recorded_backend_pid(port) + print(f"backend : {_format_backend_status(status, port)}") print(f"pid : {pid if pid is not None else '-'}") print(f"log : {_backend_log_location(pid)}") - print(f"port : {_doctor_port_occupant_line()}") + print(f"port : {_doctor_port_occupant_line(port)}") print("contexts :") for line in _doctor_backend_lines(): print(f" {line}") @@ -471,9 +517,14 @@ def _cmd_restart(_args) -> int: "respawn it, or try `restart` again." ) return 1 - # "down" (spawned but the socket never came up) or "none" (no state at - # all afterward) - both mean the restart did not produce a running - # backend. Report honestly rather than implying success. + # "down": spawned, but the socket on the port we spawned on never came up - + # the restart did not produce a running backend, so report honestly rather + # than implying success. It used to read "or 'none' (no state at all + # afterward)"; since F-868 restart reports `singleton._probe_port` for that + # one port, whose vocabulary is down/wedged/responsive and has no "none" - + # that word belonged to the record-wide walk, which restart no longer uses. + # The branch stays as written: it is the "down" arm, and a status this + # function does not recognise still has to land somewhere truthful. print(f"backend restart did not bring the backend up (state: {status}, pid {pid}).") return 1 @@ -499,9 +550,9 @@ def _cmd_kill_orphans(args) -> int: _server() from stealth_chrome_devtools_mcp.embedded import process_cleanup, singleton - status, _ = singleton._probe_backend_status() + status, port = singleton._probe_backend_status() if status in ("responsive", "wedged") and not args.force: - pid = _recorded_backend_pid() + pid = _recorded_backend_pid(port) print( f"a backend is running (pid {pid if pid is not None else '-'}); " "use restart to recover it, or pass --force." diff --git a/src/stealth_chrome_devtools_mcp/embedded/backend_liveness.py b/src/stealth_chrome_devtools_mcp/embedded/backend_liveness.py new file mode 100644 index 0000000..92dfa56 --- /dev/null +++ b/src/stealth_chrome_devtools_mcp/embedded/backend_liveness.py @@ -0,0 +1,91 @@ +"""THE one home for "is the backend on this port alive, and which recorded +backend would THIS client be served by". + +Extracted from ``singleton`` by F-868, which left that file at 999 of its 1000 +LOC — the same gate that forced ``backend_watchdog`` out, and the same answer: +this is a self-contained pair that takes every collaborator through an +argument, so it belongs beside the module that wires it, not inside it. + +A leaf. The two liveness primitives arrive as PARAMETERS (``is_healthy`` / +``http_ready``), exactly as ``backend_watchdog`` takes its probes, so nothing +here imports ``singleton``; the record arrives as a path, so nothing here +decides WHICH record either. ``backend_registry`` — itself a leaf — is the only +import, because the adoption ORDER is its policy and is merely consumed here. + +``singleton`` keeps thin wrappers (``_probe_port`` / ``_probe_backend_status``) +that bind OUR probes, OUR record path and OUR display context to these two. +That is deliberate and not ceremony: 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. A caller that imported these names directly would bind them +at import time and silently stop seeing such a patch. + +The vocabulary is one closed set, shared with ``cli._probe_recorded_backend`` +(which adds the single word this cannot reach, "no port recorded", for an entry +naming nothing usable as a port): ``down`` | ``wedged`` | ``responsive``, plus +``none`` for "no adoptable entry names a port at all". +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from stealth_chrome_devtools_mcp.embedded import backend_registry + +if TYPE_CHECKING: + from collections.abc import Callable + from pathlib import Path + + +def probe_port( + port: int, + *, + is_healthy: Callable[[int], bool], + http_ready: Callable[[int], bool], +) -> str: + """THE liveness ladder for ONE port — socket, then a real MCP `initialize`: + "down" | "wedged" | "responsive" (F-301's third state, which a bare socket + check cannot see). Read-only. THE one home for those four lines (F-868), + with three readers: the candidate walk below, `restart_backend`'s report of + the port it spawned on, and doctor's `cli._probe_recorded_backend`, which + adds only the one word this cannot reach ("no port recorded") and was a + verbatim copy of this ladder until now. + """ + if not is_healthy(port): + return "down" + return "responsive" if http_ready(port) else "wedged" + + +def probe_recorded( + path: Path, own_context: str, *, probe: Callable[[int], str] +) -> tuple[str, int | None]: + """Report the state of the backend THIS process would be served by, for + display (CLI status/doctor) and for `stop`: `probe`'s verdict and the + port it was reached on, or ("none", None) when no adoptable entry names a + port. What this adds over that ladder is WHICH port to ask about. + + Candidates come in ADOPTION order (F-868) — `adoption_candidates`, the one + home `_find_running_server` already walks — never "whichever entry the + record lists first", which under one-entry-per-display-context is routinely + a dead sibling's: that is how `status` came to report "not running" beside + a backend serving 56 proxies, and `stop` to aim at the dead record. The + first candidate that ANSWERS wins, else the most informative verdict — + wedged over down: a wedged backend holds a port and will be evicted, a down + record names nothing running. The ORDER itself is not decided here. + + ``probe`` is a parameter rather than :func:`probe_port` called directly so + that ``singleton._probe_port`` — the binding the suite patches — stays the + thing this walk actually asks. + """ + best: tuple[str, int | None] = ("none", None) + for entry in backend_registry.adoption_candidates(path, own_context): + port = backend_registry.recorded_int(entry, "port") + if port is None: + continue + verdict = probe(port) + if verdict == "responsive": + return verdict, port + if best[0] == "none" or (best[0] == "down" and verdict == "wedged"): + best = (verdict, port) + return best diff --git a/src/stealth_chrome_devtools_mcp/embedded/singleton.py b/src/stealth_chrome_devtools_mcp/embedded/singleton.py index 3083543..c48db11 100644 --- a/src/stealth_chrome_devtools_mcp/embedded/singleton.py +++ b/src/stealth_chrome_devtools_mcp/embedded/singleton.py @@ -25,6 +25,7 @@ import psutil from stealth_chrome_devtools_mcp.embedded import ( + backend_liveness, backend_registry, backend_watchdog, display_context, @@ -157,28 +158,32 @@ def _clear_server_state() -> None: backend_registry.clear_record(SERVER_STATE_FILE, PORT_FILE) +def _probe_port(port: int) -> str: + """OUR binding of `backend_liveness.probe_port` — the ladder, with THIS + module's two probes handed in. The reasoning lives with the leaf. + + A wrapper on purpose, not a hop to delete: both probe names are resolved + HERE at call time, so `monkeypatch.setattr(singleton, "_server_is_healthy", + …)` still reaches the ladder. A caller importing the leaf's names directly + would bind them at import time and stop seeing such a patch. + """ + return backend_liveness.probe_port( + port, is_healthy=_server_is_healthy, http_ready=_backend_http_ready + ) + + def _probe_backend_status() -> tuple[str, int | None]: - """Report the recorded backend's actual state for display (CLI status/ - doctor), distinguishing what `_find_running_server`'s binary answer - collapses: not running, socket-dead, and wedged (the F-301 state a bare - socket check cannot see). Read-only: never evicts, never spawns. Doctor - runs this same ladder per-entry in `cli._probe_recorded_backend`. - - Returns ("none", None) no recorded backend | ("down", port) socket closed | - ("wedged", port) socket open, no real MCP initialize answer | - ("responsive", port) socket open AND initialize answers 200. + """OUR binding of `backend_liveness.probe_recorded`: this module's record + path, this process's display context, and `_probe_port` above as the + per-port probe (so a test that patches THAT still drives the walk). + + `stop_backend` and the CLI's status/doctor/kill-orphans verbs all call it + through this name; the adoption-order policy is the leaf's, and the order + itself is `backend_registry`'s. """ - entry = backend_registry.first_backend(_read_server_state()) - if entry is None: - return "none", None - port = entry.get("port") - if not isinstance(port, int): - return "none", None - if not _server_is_healthy(port): - return "down", port - if not _backend_http_ready(port): - return "wedged", port - return "responsive", port + return backend_liveness.probe_recorded( + SERVER_STATE_FILE, display_context.display_context(), probe=_probe_port + ) def _same_identity_backend_ready(port: int, patience: float | None = None) -> bool: @@ -198,14 +203,11 @@ def _same_identity_backend_ready(port: int, patience: float | None = None) -> bo ``patience=0.0`` (discovery) probes once and never sleeps; ``None`` means ``REUSE_PATIENCE_SECONDS``, read at call time so tests can shrink it. - F-856: the window is spent in FAIRLY SCHEDULED seconds, not wall seconds. - A probe timeout is evidence about the backend only while this process is - itself being scheduled, and on a machine at 100% CPU it is not — the - 2026-09-02 incident condemned a backend that had answered its six previous - confirmations. ``scheduling_lag.FairWindow`` measures that lateness from - the loop's own naps and discounts the budget by it, bounded at - ``MAX_STRETCH``. On an idle machine the measurement is 1.0 and this is - exactly the wall-clock deadline it replaced. + F-856: the window is spent in FAIRLY SCHEDULED seconds, not wall seconds — + a probe timeout is evidence about the backend only while this process is + itself being scheduled. The measurement, its bound and the 2026-09-02 + incident behind it are ``scheduling_lag.FairWindow``'s, THE one home for + that question; on an idle machine it is the wall-clock deadline it replaced. """ # The entry recorded ON THIS PORT, not merely the first: under F-808's # per-context record another desktop's backend says nothing about `port`. @@ -636,12 +638,17 @@ def restart_backend() -> tuple[str, int | None]: instead of a repeat 120s outage — the fallback port stays recorded (SSA1.5); `stop` clears `server.json`, the reset path to `DEFAULT_PORT`. Lock contention reports "busy" so the operator retries instead of racing. The - post-restart state is reported via `_probe_backend_status()` (binding - ruling: ONE liveness vocabulary) — a restart that comes back wedged or down - must be visible, not assumed "responsive". + post-restart state is `_probe_port`'s verdict for THE PORT WE SPAWNED ON + (binding ruling: ONE liveness vocabulary) — a restart that comes back + wedged or down must be visible, not assumed "responsive". + + That port, never `_probe_backend_status()`'s (F-868): the adoption walk + answers "is there a backend for ME", so on a multi-context record a + RESPONSIVE sibling reported "responsive" for a restart whose own backend + came up wedged. Both halves now read the one selected port. - Returns ``(status, pid)``: `_probe_backend_status`'s status or "busy"; - ``pid`` is the freshly recorded pid once the lock is acquired, else None. + Returns ``(status, pid)``: `_probe_port`'s verdict or "busy"; ``pid`` is the + freshly recorded pid once the lock is acquired, else None. """ # A PREFERENCE only: selection re-derives our own context's port itself. own = display_context.display_context() @@ -655,10 +662,9 @@ def restart_backend() -> tuple[str, int | None]: _start_server_process(port) _wait_for_server(port) - status, _ = _probe_backend_status() - # The port WE spawned on, not first_backend's - same agree-on-one-port rule. + # Both halves read the port WE spawned on - the agree-on-one-port rule. fresh = backend_registry.backend_on_port(_read_server_state(), port) - return (status, backend_registry.recorded_int(fresh, "pid")) + return (_probe_port(port), backend_registry.recorded_int(fresh, "pid")) def _port_is_foreign_held(port: int) -> bool: diff --git a/tests/fakes.py b/tests/fakes.py index 237c33e..d7811a7 100644 --- a/tests/fakes.py +++ b/tests/fakes.py @@ -145,6 +145,26 @@ def pretend_display_context(monkeypatch: Any, token: str) -> None: monkeypatch.setattr(display_context, "display_context", lambda: token) +def v2_record(**backends: dict) -> dict: + """A schema-v2 ``server.json`` record from ``context=entry`` kwargs, where + ``_`` in a kwarg name reads as ``-`` (``win_session_1`` → ``win-session-1``, + the token `display_context()` actually produces). + + Here rather than in a test module because both + `test_cli_status_wedged.py` and `test_probe_backend_status.py` build the + same record shape (F-868), and a record builder that disagreed with itself + between two files is exactly the drift this module exists to prevent. It + writes the schema literally, deliberately: `backend_registry.record_backend` + is the code under test in several of those cases, so a fixture that went + through it could not express a record that function would never write — a + hand-edited one, or a pre-supersede pair. + """ + return { + "schema": 2, + "backends": {ctx.replace("_", "-"): entry for ctx, entry in backends.items()}, + } + + # --------------------------------------------------------------------------- # Fake DOM tab — covers BOTH cloner seams (JS-eval + CDP) # --------------------------------------------------------------------------- diff --git a/tests/test_cli_status_wedged.py b/tests/test_cli_status_wedged.py index 1e1ad43..3293d5d 100644 --- a/tests/test_cli_status_wedged.py +++ b/tests/test_cli_status_wedged.py @@ -20,6 +20,7 @@ import pytest +from fakes import pretend_display_context, v2_record from stealth_chrome_devtools_mcp import cli from stealth_chrome_devtools_mcp.embedded import desktop_launch, singleton @@ -314,14 +315,6 @@ def _record(state: dict | None) -> None: return _record -def _v2(**backends) -> dict: - """A schema-v2 record from `context=entry` kwargs (`_` reads as `-`).""" - return { - "schema": 2, - "backends": {ctx.replace("_", "-"): entry for ctx, entry in backends.items()}, - } - - @contextmanager def _doctor_probes(fake_server, *, healthy=True, ready=True): """Run doctor with every live probe stubbed, so the ONLY input that varies @@ -387,7 +380,7 @@ def test_every_recorded_backend_is_listed_with_its_context( self, fake_server, recorded_backends, capsys ): recorded_backends( - _v2( + v2_record( win_session_1={"port": 55296, "pid": 4242, "version": "2.0.4"}, headless={"port": 19222, "pid": 909, "version": "2.0.4"}, ) @@ -407,7 +400,7 @@ def test_window_capable_backends_are_listed_first( """Recorded headless-first, printed capable-first: the order is `backend_registry.window_capable_first`'s, not the file's.""" recorded_backends( - _v2( + v2_record( headless={"port": 19222, "pid": 909, "version": "2.0.4"}, win_session_1={"port": 55296, "pid": 4242, "version": "2.0.4"}, ) @@ -424,7 +417,7 @@ def test_each_backend_is_probed_on_its_own_port( """Three recorded backends, three different liveness answers — a once-per-record probe would report one verdict for all of them.""" recorded_backends( - _v2( + v2_record( win_session_1={"port": 55296, "pid": 4242, "version": "2.0.4"}, win_session_2={"port": 55297, "pid": 4243, "version": "2.0.4"}, headless={"port": 19222, "pid": 909, "version": "2.0.4"}, @@ -448,7 +441,9 @@ def test_remedy_line_when_no_backend_can_show_a_window( """The actionable half: a headless-only machine with nobody logged on must be told what to do, in the same words `spawn_browser`'s refusal uses — that refusal is what sends the operator here.""" - recorded_backends(_v2(headless={"port": 19222, "pid": 909, "version": "2.0.4"})) + recorded_backends( + v2_record(headless={"port": 19222, "pid": 909, "version": "2.0.4"}) + ) with _doctor_probes(fake_server): cli._cmd_doctor(None) out = capsys.readouterr().out @@ -467,7 +462,9 @@ def test_the_remedy_does_not_claim_failure_when_delegation_will_deliver( would be doctor lying about the operator's own machine — the advice degrades from a fix to an optimisation, and must read that way.""" monkeypatch.setattr(desktop_launch, "available", lambda: True) - recorded_backends(_v2(headless={"port": 19222, "pid": 909, "version": "2.0.4"})) + recorded_backends( + v2_record(headless={"port": 19222, "pid": 909, "version": "2.0.4"}) + ) with _doctor_probes(fake_server): cli._cmd_doctor(None) out = capsys.readouterr().out @@ -484,7 +481,7 @@ def test_no_remedy_line_when_a_window_capable_backend_is_responsive( self, fake_server, recorded_backends, capsys ): recorded_backends( - _v2( + v2_record( headless={"port": 19222, "pid": 909, "version": "2.0.4"}, win_session_1={"port": 55296, "pid": 4242, "version": "2.0.4"}, ) @@ -510,7 +507,7 @@ def test_a_dead_window_capable_backend_does_not_suppress_the_remedy( operator's headed spawn is still refused, because discovery finds no LIVE capable backend to adopt.""" recorded_backends( - _v2(win_session_1={"port": 55296, "pid": 4242, "version": "2.0.4"}) + v2_record(win_session_1={"port": 55296, "pid": 4242, "version": "2.0.4"}) ) with _doctor_probes(fake_server, healthy=False): cli._cmd_doctor(None) @@ -526,7 +523,7 @@ def test_a_wedged_window_capable_backend_does_not_suppress_the_remedy( """Same ruling's other half: a wedged backend holds its socket open but cannot serve a spawn either.""" recorded_backends( - _v2(win_session_1={"port": 55296, "pid": 4242, "version": "2.0.4"}) + v2_record(win_session_1={"port": 55296, "pid": 4242, "version": "2.0.4"}) ) with _doctor_probes(fake_server, healthy=True, ready=False): cli._cmd_doctor(None) @@ -542,7 +539,7 @@ def test_an_entry_with_no_usable_port_says_so( is nothing to probe, and claiming "down" would assert a liveness we never tested.""" recorded_backends( - _v2(win_session_1={"port": "55296", "pid": 4242, "version": "2.0.4"}) + v2_record(win_session_1={"port": "55296", "pid": 4242, "version": "2.0.4"}) ) with _doctor_probes(fake_server): cli._cmd_doctor(None) @@ -578,3 +575,139 @@ def test_no_record_says_none_recorded_without_a_remedy( assert "backend (none recorded)" in out assert "no live backend can display a window" not in out + + +@contextmanager +def _live_only_on(port: int): + """Both liveness primitives answering for ONE port and no other. + + `_probe_backend_status` is left REAL on purpose — which recorded backend it + speaks about is exactly what F-868 is about, so stubbing it would stub the + defect out. Nothing here opens a socket: both probes are replaced, so the + port numbers are record data, never a connection. + """ + with ( + patch.object(singleton, "_server_is_healthy", lambda p: p == port), + patch.object(singleton, "_backend_http_ready", lambda p, **_kw: p == port), + ): + yield + + +class TestCliStatusReportsTheBackendThisClientWouldUse: + """F-868: `status` must answer for the backend THIS shell would be served + by — the one `_find_running_server` adopts — not for whichever entry the + record happens to list first. + + The record observed on 2026-09-14 (2.1.5, Windows 11) held three entries; + the first two named dead pids on ports nothing listened on, and the third + was the healthy backend serving 56 proxies in the operator's own session. + `status` reported "not running" and the first entry's dead pid. + """ + + def test_status_reports_the_own_context_live_backend_not_a_dead_sibling( + self, fake_server, recorded_backends, capsys, monkeypatch, tmp_path + ): + recorded_backends( + v2_record( + win_session_2={"port": 7169, "pid": 89892, "version": "2.1.1"}, + headless={"port": 19222, "pid": 67720, "version": "2.1.3"}, + win_session_1={"port": 52554, "pid": 53836, "version": "2.1.5"}, + ) + ) + pretend_display_context(monkeypatch, "win-session-1") + with ( + patch.object(cli, "_clone_storage", return_value=fake_server), + patch( + "stealth_chrome_devtools_mcp.embedded.logging_setup.resolve_log_dir", + return_value=tmp_path, + ), + _live_only_on(52554), + ): + cli._cmd_status(None) + out = capsys.readouterr().out + + assert "running (responsive) on port 52554" in out + assert "not running" not in out + # The pid and the log path must describe the SAME backend the status + # line is about — two independent record reads is how they diverged. + assert "53836" in out + assert "backend-53836.log" in out + assert "89892" not in out + + def test_status_reports_the_live_backend_for_a_client_that_adopts_anything( + self, fake_server, recorded_backends, capsys, monkeypatch, tmp_path + ): + """The other half of adoption's asymmetry: a HEADLESS/UNVERIFIED client + adopts any recorded backend, capable-first — so the dead window-capable + entry it tries first must not end the search.""" + recorded_backends( + v2_record( + win_session_2={"port": 7169, "pid": 89892, "version": "2.1.1"}, + win_session_1={"port": 52554, "pid": 53836, "version": "2.1.5"}, + ) + ) + pretend_display_context(monkeypatch, "headless") + with ( + patch.object(cli, "_clone_storage", return_value=fake_server), + patch( + "stealth_chrome_devtools_mcp.embedded.logging_setup.resolve_log_dir", + return_value=tmp_path, + ), + _live_only_on(52554), + ): + cli._cmd_status(None) + out = capsys.readouterr().out + + assert "running (responsive) on port 52554" in out + assert "53836" in out + + def test_status_names_the_records_it_is_not_speaking_about( + self, fake_server, recorded_backends, capsys, monkeypatch, tmp_path + ): + """One summary line over a three-entry record silently drops two of + them. It must say they exist and where to read them.""" + recorded_backends( + v2_record( + win_session_2={"port": 7169, "pid": 89892, "version": "2.1.1"}, + headless={"port": 19222, "pid": 67720, "version": "2.1.3"}, + win_session_1={"port": 52554, "pid": 53836, "version": "2.1.5"}, + ) + ) + pretend_display_context(monkeypatch, "win-session-1") + with ( + patch.object(cli, "_clone_storage", return_value=fake_server), + patch( + "stealth_chrome_devtools_mcp.embedded.logging_setup.resolve_log_dir", + return_value=tmp_path, + ), + _live_only_on(52554), + ): + cli._cmd_status(None) + out = capsys.readouterr().out + + assert "others : 2 backends recorded" in out + assert "win-session-2" in out + assert "headless" in out + assert "doctor" in out + + def test_a_single_recorded_backend_gets_no_other_records_line( + self, fake_server, recorded_backends, capsys, monkeypatch, tmp_path + ): + """The note is evidence, not decoration: with nothing else recorded + there is nothing it could honestly say.""" + recorded_backends( + v2_record(win_session_1={"port": 52554, "pid": 53836, "version": "2.1.5"}) + ) + pretend_display_context(monkeypatch, "win-session-1") + with ( + patch.object(cli, "_clone_storage", return_value=fake_server), + patch( + "stealth_chrome_devtools_mcp.embedded.logging_setup.resolve_log_dir", + return_value=tmp_path, + ), + _live_only_on(52554), + ): + cli._cmd_status(None) + out = capsys.readouterr().out + + assert "others :" not in out diff --git a/tests/test_probe_backend_status.py b/tests/test_probe_backend_status.py index 1d083a7..07b8a30 100644 --- a/tests/test_probe_backend_status.py +++ b/tests/test_probe_backend_status.py @@ -14,6 +14,7 @@ import pytest +from fakes import pretend_display_context, v2_record from stealth_chrome_devtools_mcp.embedded import singleton @@ -132,3 +133,76 @@ def test_responsive_backend_reports_responsive( status, reported_port = singleton._probe_backend_status() assert status == "responsive" assert reported_port == responsive_stub + + +class TestProbeWalksAdoptionOrder: + """F-868: the reporter must speak about the backend THIS process would be + served by — `backend_registry.adoption_candidates`' order, the same one + `_find_running_server` walks — not about whichever entry the record lists + first. Reading the first entry made `status` answer "not running" beside a + healthy own-context backend, and made `stop` aim at a dead sibling's record. + + There is no second selection policy here: the order arrives from the one + home, and this function only says which of those candidates is up. + """ + + def test_a_dead_first_entry_does_not_hide_the_live_own_context_one( + self, isolated_state, monkeypatch, responsive_stub + ): + dead = _free_closed_port() + (isolated_state / "server.json").write_text( + json.dumps( + v2_record( + win_session_2={"port": dead, "version": "2.1.1", "pid": 89892}, + win_session_1={ + "port": responsive_stub, + "version": "2.1.5", + "pid": 53836, + }, + ) + ) + ) + pretend_display_context(monkeypatch, "win-session-1") + + assert singleton._probe_backend_status() == ("responsive", responsive_stub) + + def test_a_dead_capable_entry_does_not_end_an_unproven_clients_search( + self, isolated_state, monkeypatch, responsive_stub + ): + """A HEADLESS/UNVERIFIED client adopts anything, window-capable first — + so the dead capable entry it tries first must not be its answer.""" + dead = _free_closed_port() + (isolated_state / "server.json").write_text( + json.dumps( + v2_record( + win_session_2={"port": dead, "version": "2.1.1", "pid": 89892}, + headless={ + "port": responsive_stub, + "version": "2.1.5", + "pid": 67720, + }, + ) + ) + ) + pretend_display_context(monkeypatch, "headless") + + assert singleton._probe_backend_status() == ("responsive", responsive_stub) + + def test_a_wedged_candidate_outranks_a_dead_one( + self, isolated_state, monkeypatch, wedged_stub + ): + """With nothing responsive the report still has to name the most + informative state: a wedged backend holds a port and will be evicted; + a down record names nothing running at all.""" + dead = _free_closed_port() + (isolated_state / "server.json").write_text( + json.dumps( + v2_record( + win_session_2={"port": dead, "version": "2.1.1", "pid": 89892}, + headless={"port": wedged_stub, "version": "2.1.5", "pid": 67720}, + ) + ) + ) + pretend_display_context(monkeypatch, "unverified") + + assert singleton._probe_backend_status() == ("wedged", wedged_stub) diff --git a/tests/test_singleton_stop_restart.py b/tests/test_singleton_stop_restart.py index 369ad2d..c57ddbc 100644 --- a/tests/test_singleton_stop_restart.py +++ b/tests/test_singleton_stop_restart.py @@ -31,6 +31,7 @@ import psutil import pytest +from fakes import pretend_display_context from stealth_chrome_devtools_mcp.embedded import backend_registry, singleton @@ -384,6 +385,9 @@ def test_none_reports_not_running(self, isolated_state, monkeypatch): assert pid is None def test_lock_contended_reports_busy(self, isolated_state, monkeypatch): + # A STOP test: stop_backend still reads _probe_backend_status (which + # record to act on), unlike restart, which since F-868 reports + # _probe_port for the port it spawned on. monkeypatch.setattr( singleton, "_probe_backend_status", lambda: ("responsive", 19222) ) @@ -436,10 +440,14 @@ class TestRestartBackend: the outage is survived, same as cold start - see TestRestartPortSelection below for that behavior's own pinning tests. - The final state reported is _probe_backend_status()'s, read AFTER the - lock releases (M1's one liveness vocabulary - binding ruling: no new - health check anywhere) - a restart that comes back wedged/down must be - visible, not assumed "responsive". Pinned by the third test below. + The final state reported is _probe_port(port)'s for THE PORT WE SPAWNED ON, + read AFTER the lock releases (M1's one liveness vocabulary - binding + ruling: no new health check anywhere) - a restart that comes back + wedged/down must be visible, not assumed "responsive". Pinned by the third + test below. It was _probe_backend_status() until F-868: that walks the + ADOPTION order, so on a multi-context record a responsive sibling answered + for a restart whose own backend came up wedged - pinned by the last test in + TestRestartReportsTheSpawnedPort. """ def test_terminate_then_spawn_ordering_under_the_lock( @@ -475,9 +483,7 @@ def _fake_spawn(port): monkeypatch.setattr( singleton, "_wait_for_server", lambda port: calls.append("wait") ) - monkeypatch.setattr( - singleton, "_probe_backend_status", lambda: ("responsive", 19222) - ) + monkeypatch.setattr(singleton, "_probe_port", lambda port: "responsive") result = singleton.restart_backend() @@ -531,9 +537,7 @@ def _fake_spawn(port): monkeypatch.setattr(singleton, "_wait_for_server", lambda port: None) # The backend comes back wedged - restart_backend must report that, # not the "responsive" the ordering test above pinned. - monkeypatch.setattr( - singleton, "_probe_backend_status", lambda: ("wedged", 19222) - ) + monkeypatch.setattr(singleton, "_probe_port", lambda port: "wedged") result = singleton.restart_backend() @@ -573,9 +577,7 @@ def test_recorded_port_is_rebound_when_still_free( ) monkeypatch.setattr(singleton, "_terminate_backend", lambda port: None) monkeypatch.setattr(singleton, "_wait_for_server", lambda port: None) - monkeypatch.setattr( - singleton, "_probe_backend_status", lambda: ("responsive", recorded_port) - ) + monkeypatch.setattr(singleton, "_probe_port", lambda port: "responsive") spawned_on = {} @@ -616,9 +618,7 @@ def test_recorded_port_now_foreign_held_falls_back( # touched by this test. monkeypatch.setattr(singleton, "_terminate_backend", lambda port: None) monkeypatch.setattr(singleton, "_wait_for_server", lambda port: None) - monkeypatch.setattr( - singleton, "_probe_backend_status", lambda: ("responsive", 0) - ) + monkeypatch.setattr(singleton, "_probe_port", lambda port: "responsive") spawned_on = {} @@ -660,9 +660,7 @@ def test_normal_case_return_shape_unchanged_vs_m8_5( ) monkeypatch.setattr(singleton, "_terminate_backend", lambda port: None) monkeypatch.setattr(singleton, "_wait_for_server", lambda port: None) - monkeypatch.setattr( - singleton, "_probe_backend_status", lambda: ("responsive", recorded_port) - ) + monkeypatch.setattr(singleton, "_probe_port", lambda port: "responsive") def _fake_spawn(port): singleton._write_server_state( @@ -730,9 +728,7 @@ def test_restart_terminates_our_own_context_and_leaves_the_sibling_alone( singleton, "_terminate_backend", lambda port: terminated.append(port) ) monkeypatch.setattr(singleton, "_wait_for_server", lambda port: None) - monkeypatch.setattr( - singleton, "_probe_backend_status", lambda: ("responsive", our_port) - ) + monkeypatch.setattr(singleton, "_probe_port", lambda port: "responsive") spawned: list[int] = [] @@ -801,9 +797,7 @@ def test_restart_with_no_entry_of_our_own_diverts_instead_of_killing_a_sibling( singleton, "_terminate_backend", lambda port: terminated.append(port) ) monkeypatch.setattr(singleton, "_wait_for_server", lambda port: None) - monkeypatch.setattr( - singleton, "_probe_backend_status", lambda: ("responsive", diverted_port) - ) + monkeypatch.setattr(singleton, "_probe_port", lambda port: "responsive") spawned: list[int] = [] @@ -824,3 +818,106 @@ def _fake_spawn(port): for e in backend_registry.read_backends(record) } assert surviving["headless"] == sibling_port + + +class TestStopIsPerDisplayContext: + """F-868's consequence for `stop`, stated as a contract rather than left to + be discovered. + + `stop` acts on whatever `_probe_backend_status` reports, and that now walks + `adoption_candidates`, which for a PROVEN-capable client excludes every + FOREIGN proven context. So an operator's `stop` no longer reaches across + into another desktop's backend — it says "not running", because for this + shell there is nothing it may act on, and it leaves the sibling's record + alone. Previously `first_backend` handed it that entry: it terminated + nothing (the port is not ours to hold) and then FORGOT the record, quietly + making a possibly-live sibling undiscoverable. + + This is green on the branch by construction — the adoption walk is the fix, + not something added for this test. It is pinned because it is a BEHAVIOUR + CHANGE riding on a bug fix, and an unpinned one is the kind a later + "simplification" reverts without noticing. + """ + + def test_stop_will_not_act_on_a_foreign_proven_context( + self, isolated_state, monkeypatch + ): + backend_registry.record_backend( + singleton.SERVER_STATE_FILE, + port=6000, + version="1.2.1", + pid=2222, + source_fingerprint="fp", + display_context="win-session-OTHER", + ) + pretend_display_context(monkeypatch, "win-session-OURS") + terminated: list[int] = [] + monkeypatch.setattr( + singleton, "_terminate_backend", lambda port: terminated.append(port) + ) + + # "none" short-circuits before the lock, so nothing is probed either: + # a foreign desktop's backend is not even asked whether it is alive. + assert singleton.stop_backend() == ("not running", None) + assert terminated == [] + assert [ + (e["display_context"], e["port"]) + for e in backend_registry.read_backends(singleton.SERVER_STATE_FILE) + ] == [("win-session-OTHER", 6000)] + + +class TestRestartReportsTheSpawnedPort: + """F-868: restart's status must be about the backend restart just brought + up, never about a sibling that happens to answer. + + `_probe_backend_status` asks "is there a backend for me" — the adoption + walk — which is the right question for `status` and the wrong one here: a + RESPONSIVE sibling would report "responsive" for a restart whose own + backend came up wedged, i.e. a promise about a different process than the + `pid` beside it. `restart_backend` now reports `_probe_port(port)` for the + port it selected, so both halves of its return describe one backend. + """ + + def test_a_responsive_sibling_does_not_answer_for_a_wedged_fresh_backend( + self, isolated_state, monkeypatch + ): + sibling_port = _free_closed_port() + our_port = _free_closed_port() + backend_registry.record_backend( + singleton.SERVER_STATE_FILE, + port=sibling_port, + version="1.2.1", + pid=1111, + source_fingerprint="fp", + display_context="win-session-1", + ) + # HEADLESS adopts anything, window-capable first — so the sibling is + # the adoption walk's answer, which is the whole point of the case. + pretend_display_context(monkeypatch, "headless") + monkeypatch.setattr( + singleton, "_exclusive_lock", lambda: _tracking_lock([], True) + ) + monkeypatch.setattr(singleton, "_select_backend_port", lambda port: our_port) + monkeypatch.setattr(singleton, "_terminate_backend", lambda port: None) + monkeypatch.setattr(singleton, "_wait_for_server", lambda port: None) + + def _fake_spawn(port): + singleton._write_server_state( + port=port, version="1.2.1", pid=4242, source_fingerprint="" + ) + + monkeypatch.setattr(singleton, "_start_server_process", _fake_spawn) + # The real `_probe_port` runs; only its two primitives are stated. The + # sibling answers, ours holds its socket open and says nothing. + monkeypatch.setattr(singleton, "_server_is_healthy", lambda port: True) + monkeypatch.setattr( + singleton, "_backend_http_ready", lambda port, **_kw: port == sibling_port + ) + + result = singleton.restart_backend() + + assert result == ("wedged", 4242) + # Both halves of the contrast, so this cannot pass by the sibling + # simply being invisible: the adoption walk DOES say responsive here, + # and restart deliberately does not use it. + assert singleton._probe_backend_status() == ("responsive", sibling_port)