fix(browser-bridge): nav/open --wake, fast ping, split routing errors, orientation telemetry, screenshot Read hint - #278
Conversation
|
Adversarial audit run. Verdict: merge after fixing one 🔴. Two corrections to this PR body first, posted here rather than edited in, since a reviewer may already have read the original. Correction 1 — the base-red count is 18, not 17Re-measured as three disjoint runs at base: The substantive half of the claim — all new tests, every pre-existing test green — is confirmed. Correction 2 — one base-red row is red for the wrong reason
Commit isolation is otherwise clean: 🔴 The blocking finding: the 2 s ping deadline false-negatives a busy extension
Measured at two points on one rig (a
Failure scenario: two sessions drive one Brave profile — the documented, rate-limit-backstopped case. Agent B runs The 68 ms healthy-path measurement that motivated 2 s was taken against an idle extension. Both S2 tests use a wedged or an idle extension; none uses a busy one — so the negative control that would have caught this was never constructed. Confirmed clean
Follow-ups being addressed in the same pass: |
…ge audit S1 --wake[=MS] on `nav` and `open` (F2). The flag existed on exactly html/text/js; 13 nav->wake / open->wake adjacent pairs in transcripts and one opencode session that is literally `nav wake eval` x5 were paying for the gap. The three copied parse blocks are consolidated into one `_wake_flag` helper (validation still at PARSE time). nav/open send NO `wake` field on the wire — the extension honours cmd.wake only on getHtml/text/eval — so `--wake` is a client-side compose (`_wake_after`): the nav/open op, then the existing `wake` op, annotated at result.data.wake exactly where a `--wake` read reports it. S2 `ping` gets its own 2s deadline (F7): 38.2% failure rate, every failure burning the full 20s CMD_TIMEOUT while 21 healthy pings averaged 3-4ms. New BROWSER_BRIDGE_PING_TIMEOUT / PING_TIMEOUT_DEFAULT; no other op's timeout moves. S3 unknown_instance / no_extension now separate "wrong label" from "right label, profile disconnected" (F6): 48 of 52 failures used the CORRECT label, with `eval` retried 37 times. The disconnected branch names when the profile went away, says FULLY RESTART Brave, and says DO NOT RETRY. S4 /whoami and /health emit metadata-only telemetry (F8) — 53 invocations were invisible to the only structured source. No domain (they are global ops), no key; /instances and /poll still emit nothing. S5 an explicit-path `screenshot` prints one `#`-prefixed hint that the .png must be Read (F4: 7 of 63 captures never read). Line 1 stays the bare path. S6 the CLI's "SKILL.md -> Concurrency" pointers named a heading that has never existed; a test now proves every SKILL.md heading the CLI names resolves. server.py changes need a `home-manager switch` + `systemctl --user restart browser-bridge` to go live; the CLI is a working-tree symlink and is live on pull. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…omment `--help` was `grep -E '^#( |$)' "$0"`, which matched EVERY column-0 comment in the file — so it shipped the whole implementation commentary as user-facing help: 25,440 bytes against a ~15 KB header, and it grew with every internal comment anyone added (the five audit fixes in the parent commit would alone have put 4,129 bytes of maintainer notes onto the help screen). Now: the contiguous `#` block after the shebang, stopping at the first line of code. Measured on this branch: 25,440 -> 15,532 bytes per `--help`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y extension Adversarial audit of #278, confirmed by reproduction. The extension's poll loop is strictly serial, so `ping` skips the per-tab FIFO but still cannot be DEQUEUED until the running command finishes. Measured on a FakeExtension running a legitimate 6s getHtml, then a ping: ping_timeout=2.0 -> 504 timeout at 2.00s (the getHtml completed fine) ping_timeout=10.0 -> 200 pong at ~6.0s Real blast radius: two sessions on one profile; agent B runs `browser ping` — the skill's documented FIRST action — while agent A is mid-nav. B was told "is Brave focused / responsive?", whose documented remedy is a FULL Brave restart of the operator's live session. Root cause of the miss: the healthy 68ms measurement was against an IDLE extension, and both S2 tests used a wedged or idle one. NO test used a busy one. Fixes: * PING_TIMEOUT_DEFAULT 2.0 -> 10.0, justified as an INVARIANT rather than a number: it must exceed the longest a HEALTHY extension can be busy with a command the CLI can ASK for. ACTIVATE_WAIT_MAX_MS (8s) is that ceiling; 10s is the smallest value above it with slack for the result POST + poll turnaround. The residual (10s < EXEC_OP_BUDGET_MS 18s) is stated, not hidden, and the rejected "escalate when the instance is busy" alternative is recorded with the reason it does not work (pending>0 cannot tell BUSY from WEDGED). * The 504 message is now op-aware: a ping timeout says the extension may be BUSY, says to re-run once the other op finishes, names the env override, and explicitly refuses to recommend a Brave restart. * New regression test with a BUSY (not wedged, not idle) extension. Red at the previous tip at exactly 2.00s. Also hardened test_the_short_deadline_applies_to_ping_ONLY, which silently stopped discriminating when the default rose above its cmd_timeout: a lower bound alone now survives the `op_timeout = ping_timeout` mutant (10 >= 3.5). It is bounded on both sides and asserts its own precondition. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e, docs
A. A failing `--wake` no longer swallows the successful primary op. `_wake_after`
used `|| exit 1`, which exited rc 1 with EMPTY stdout on all three measured
failure modes (429, 504, op-level unknown_op) — so `T=$(browser nav --wake
"$url")` yielded an empty T for a tab that really had navigated, and only the
op-level branch even mentioned `wake`. Now: the primary JSON is always emitted,
the failure is attached at result.data.wake.ok=false, stderr says the primary
SUCCEEDED and which half failed, and the exit code is a distinct 3.
B. `--frame X nav --wake <url>` is refused at PARSE time. `--frame` is spliced into
every op by cmd_op; `nav`/`open` ignore it but the standalone `wake` is refused
by the extension's assertWakeNotFramed — so the composed form navigated and THEN
failed the wake. This half-state did not exist before `--wake` was added to
nav/open (it was a hard parse error), i.e. the feature opened it. One shared
predicate, both callers. VERIFIED LIVE, not inferred.
C. Restored the drop evidence the S3 rewrite dropped: `last_unanswered_op` on every
rendered instance, and the last-seen/last-op lines on the never-seen 404 branch
(render_missing used to print them). server.py calls that field "the single fact
that turns the next silent drop from inference into evidence".
D. reference/errors.md: `404 unknown_instance` said "the --instance key matches no
connected instance" — exactly the conflation S3 exists to break. Now documents
both branches with their opposite actions, plus the same split for 503 and the
`ping` exception to `504 timeout`.
E. SKILL.md documents the flag where agents actually read it: `nav`/`open` rows and
the `wake` row. 11,911 -> 12,010 B; 278 B under the 12,288 ceiling, 28 B above
the 250 B floor. Agents read SKILL.md, not `browser --help`, so without this S1
changed agent behaviour approximately zero.
F. The --help test now pins the LAST line and the full header line-for-line. `awk`
exits at the first non-`#` line, so a blank line inserted near the end of the
header truncated --help with landmarks + an upper length bound still green.
G. `_wait_connected`/`_wait_count` use `body["count"]`, not `.get("count", 0)`: a
missing key must raise, not read as "nothing connected" — which for want=False
was an instantly vacuous pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
8a26983 to
8daee7f
Compare
Audit round 1 addressed — 🔴 + A–GRebased onto Two corrections to my original PR body — acknowledged, not silently edited
🔴 BLOCKING — fixed, and reproduced firstReproduced exactly as reported before changing anything: a 1.
Justified against the extension's own constants: I did not take the suggested ~6 s: 6 < Residual, stated in the code rather than hidden: 10 s < Structural alternative considered and REJECTED, recorded in the code so nobody re-derives it: "use the short deadline only when the instance is idle, escalate when busy." 2. The 504 is now op-aware. A ping timeout says the extension executes one command at a time, that a queued ping times out while the profile is healthy, to wait and re-run, that only a repeat failure on an idle profile implicates the build, and names the env override. It ends: "do NOT restart Brave on the strength of this alone." The generic message is unchanged for every other op. 3. The busy-extension test. Establishes the precondition (polls
A second bug the raise exposed, which I would have shipped otherwise: 🟡 A–GA — a failing wake no longer swallows the primary result. Always emits the primary JSON, attaches the failure at B — C — restored the lost evidence. D — E — SKILL.md. F — the G — Red → green (this round). Base =
|
| test | base | HEAD | kind |
|---|---|---|---|
test_ping_survives_a_BUSY_but_perfectly_healthy_extension |
RED @ 8a26983 (504 @ 2.00 s) |
green | regression (🔴) |
test_a_failing_wake_never_swallows_the_primary_result × 6 (nav/open × op_error/429/504) |
RED | green | regression (A) |
test_a_SUCCESSFUL_wake_still_exits_zero[nav]/[open] |
green | green | NEGATIVE CONTROL for rc 3 |
test_frame_plus_wake_is_refused_before_anything_reaches_the_wire[nav]/[open] |
RED | green | regression (B) |
test_frame_WITHOUT_wake_and_wake_WITHOUT_frame_both_still_work[nav]/[open] |
green | green | NEGATIVE CONTROL — the guard must fire on the conjunction only |
test_unknown_instance_KNOWN_but_disconnected... |
RED | green | regression (C) |
test_unknown_instance_NEVER_SEEN_key_says_wrong_label |
RED | green | regression (C) + branch control |
test_no_extension_distinguishes_dropped_from_never_wired_up |
RED | green | regression (C) |
test_help_prints_the_HEADER_block_only... (F's new pins) |
green | green | INVARIANT GUARD — test-only hardening; its evidence is MF1, below |
Mutations (each red with THAT guard's own error)
| # | mutation | result |
|---|---|---|
| M1/M2 | re-run against the 10 s default | both still RED with their own messages |
| MA1 | if true — exit 3 unconditional |
RED, and the negative control test_a_SUCCESSFUL_wake_still_exits_zero is what fired |
| MA2 | drop the data["wake"] failure annotation |
RED — KeyError: 'wake' |
| MB1 | neuter _reject_frame_with_wake |
RED on both frame_plus_wake cases only |
| MB2 | fire the guard on --frame alone (over-broad) |
RED on the negative control only — positive test stayed green |
| MC1 | drop last_unanswered_op |
RED ×3 — assert 'last unanswered op: eval' in … |
| MF1 | blank line 6 lines from the end of the header | RED — "--help was truncated before the end of the header block" |
| MG1 | /instances loses count |
KeyError: 'count' — raises instead of reading as "not connected" |
| M5/M6 | re-run the S3 branch mutants after C's edit | both still RED, each on its own branch |
MF1 is worth calling out. I checked whether F's pin was actually necessary by running the pre-F assertions against the mutant — every one of them passes while 7 lines of help are silently lost:
landmark 'FLAG ORDER / END OF FLAGS' -> PASS
landmark 'browser nav <url> [--wake[=MS]]' -> PASS
landmark 'Global flags (before the subcommand)' -> PASS
no-leak (all four) -> PASS
length bound 15049 < 15856 -> PASS
LOST LINES: 7
So the audit's F was right and the pin is load-bearing, not decorative.
Counts (counted, not exit codes)
- pytest 407 passed, 0 failed (was 393 → +14). Clean tree at the time of the run.
- node tests 454 / pass 454 / fail 0 / skipped 0;
grep -ci 'test timed out'→ 0.
Verified live vs. tests only
Laptop .155, profile personal, ext 0.7.0, deployed server f5fbe82, worktree clean and committed at the time.
B — verified LIVE, not inferred. Both halves:
- The premise:
browser --frame wake-rig --tab <T> wake→wake_with_frame_unsupported: un-throttling is tab-level, not per-frame. The extension really does refuse a framed wake. - The guard, with a before/after URL proving nothing was half-applied:
url BEFORE: "/deep0.html"
$ browser --tab <T> --frame child nav --wake http://127.0.0.1:8901/wake-rig.html
browser: --frame and --wake cannot be combined on 'nav'.
browser: refused before anything was sent (nothing has been navigated or opened)
rc=1
url AFTER: "/deep0.html" <- unchanged
S1 re-verified live after the _wake_after rewrite (it is on the hot path of A): nav --wake → woke:true, WAKE-RIG-RENDERED | raf=30 | vis=hidden, rc 0. No focus moved; tab closed afterwards.
S3 re-verified live for the never-seen branch, now including the drop evidence.
Still NOT verified live, unchanged from before: S2's timeout path and S4's emit (both in server.py, which is not deployed — needs home-manager switch + systemctl --user restart browser-bridge); S3's known-but-disconnected branch (would require killing a Brave profile's long-poll on the operator's live session). A is covered by canned-server tests only — all three failure modes (429/504/op-level) are exercised there, but I did not manufacture a live wake failure.
Byte deltas, updated
| surface | delta |
|---|---|
browser --help |
−9,908 B (25,440 → 15,532) |
SKILL.md |
+99 B (11,911 → 12,010) — every browser task pays this |
screenshot <path> stdout |
+20 B + len(path) |
unknown_instance stderr |
+326 B → now ~+390 B with the restored evidence; error path only |
ping 504 stderr |
~+700 B, and only on a ping timeout (13 in the 2-day window) |
nav/open without --wake, all read ops, whoami/health |
0 |
…t a constant
PRIMARY (audit 🟡-2, and it dissolves 🟡-1 and 🟢-3). The audit was right that a
distinguishing signal exists on the Instance, and right that my ✗REJECTED note was
about the wrong field. `inst.pending` cannot tell BUSY from WEDGED — but the AGE of
the outstanding work can, because the extension self-bounds one command at
EXEC_OP_BUDGET_MS = 18s.
`inst.last_dispatch` cannot carry it, for two concrete reasons:
* it is OVERWRITTEN by every new enqueue, so it names the NEWEST outstanding
command, not the oldest — a fresh enqueue behind a wedged one makes the wedge
look young;
* its `at` is wall-clock time.time(), while all deadline math uses the injectable
monotonic self._clock(); an NTP step would move it.
Both are properties of that field, not of the idea. So `last_dispatch` stays
DIAGNOSTIC and a new `Instance.inflight` (cid -> monotonic enqueue reading, every
outstanding command) does the timing.
Registry._effective_timeout_locked is the gate:
* nothing in flight -> a ping that does not answer within the fast deadline IS
the wedge. PING_TIMEOUT_DEFAULT is back to 2s and is now ONLY the idle floor.
* work in flight -> allow it its own budget: N * EXEC_OP_BUDGET_S +
WEDGE_GRACE_S - age(oldest). N scales it because the loop drains serially.
* clamped to cmd_timeout above (so this can never be slower than the behaviour
predating the fast ping) and to fast_timeout below (an overdue instance still
gets its full fast deadline, not an instant fail).
This dominates the tuned constant on both axes: wedge detection returns to ~2s
(it was ~10s), and a busy profile is never reported dead — including the case a
constant cannot express at all, busy AND wedged.
🟡-1 is therefore moot rather than fixed: the invariant no longer has to out-bid
any ceiling. The old comment claimed 8s was "the caller-requestable ceiling" and
was wrong twice over — `screenshot --fullpage` (CDP_OP_BUDGET_MS 15s) and
`wake --wait 6000` composed with an 8s CDP attach both exceed 10s. Both are now
named in the code as the reason the tuned approach was abandoned.
EXEC_OP_BUDGET_S mirrors a constant across a language boundary, so
test_exec_budget_matches_the_extension reads protocol.js and fails on drift.
🟡-3 The ping-timeout guidance — the entire mitigation for the disclosed residual —
had zero test coverage; deleting the block left both suites green. Now pinned, with
a negative control asserting a NON-ping timeout keeps the generic wording. The text
is also rewritten for the new semantics (it claimed a "default 10s SHORT deadline").
🟡-4 result.data.wake.error now carries the REAL cause (`rate_limited`, `timeout`,
`wake_with_frame_unsupported`) instead of the constant "wake_failed_after_nav", so
a script can tell transient from permanent. cmd_op stderr is captured, re-emitted
VERBATIM, then classified. The test pins the error VALUE per failure mode.
🟢-1 result.data.wake is now shape-symmetric: `ok` is always present. Success used
to write the raw payload, which has no `ok` key, so `if (data.wake.ok)` was falsy
on every successful wake. Three shapes became two.
🟢-2 Exit code 3 is documented in the CLI header block (not SKILL.md, which has
28 B of slack).
Also fixed: two background threads in the new tests raised into pytest as
unhandled daemon-thread exceptions and were attributed to an unrelated later test
(test_release_drops_ownership_without_dispatch). The suite now runs clean under
-W error::pytest.PytestUnhandledThreadExceptionWarning.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 2 addressed — structural gate implemented (
|
| test | base | HEAD | kind |
|---|---|---|---|
test_ping_fast_fails_even_while_BUSY_once_the_work_has_blown_its_budget |
RED | green | regression (🟡-2) |
test_effective_timeout_gate_unit |
RED | green | regression (🟡-2) |
test_exec_budget_matches_the_extension |
RED | green | regression (🟡-2) |
test_inflight_is_released_on_every_exit_path |
RED | green | regression (🟡-2) |
test_ping_does_not_wait_out_cmd_timeout |
RED (10 s) | green (~2 s) | regression — retargeted to the idle path |
test_the_short_deadline_applies_to_ping_ONLY |
RED | green | control, re-bounded for the new semantics |
test_a_failing_wake_never_swallows_the_primary_result × 6 |
RED (wake_failed_after_nav) |
green | regression (🟡-4) |
test_a_SUCCESSFUL_wake_still_exits_zero[nav]/[open] |
RED (KeyError: 'ok') |
green | regression (🟢-1) |
test_a_ping_timeout_does_NOT_steer_the_operator_into_restarting_brave |
RED | green | ⚠ entangled — red on the reworded text, since the block existed at 8daee7f. Its clean evidence is MS7 below. |
test_a_NON_ping_timeout_keeps_the_generic_wording |
green | green | NEGATIVE CONTROL |
15 red at 8daee7f, all new/retargeted; no pre-existing test red.
Mutations — each red with THAT guard's own error
| # | mutation | result |
|---|---|---|
| MS1 | gate ignores fast_timeout (always cmd_timeout) |
RED ×4 incl. the unit gate |
| MS2 | gate is busy-blind (always fast_timeout) |
RED — "a BUSY but healthy extension was reported dead after 2.00s" |
| MS3 | drop inflight.pop in the finally |
RED — "leaked after success" |
| MS4 | EXEC_OP_BUDGET_S 18 → 17 |
RED — "protocol.js says 18000ms, server.py says 17.0s" |
| MS5 | drop the LOWER clamp | RED — assert -10.0 == 2.0 (overdue would fail instantly) |
| MS6 | drop the UPPER clamp | RED — "must never exceed cmd_timeout" |
| MS7 | delete the whole ping-504 block | RED — only the 🟡-3 test; the negative control stayed green |
| MS8 | revert the wake error to a constant | RED ×6, each naming the cause it lost |
| MS9 | success shape loses ok |
RED ×2 — KeyError: 'ok' |
A defect I introduced and caught
The two new threaded tests left unhandled daemon-thread exceptions, which pytest attributed to an unrelated later test (test_release_drops_ownership_without_dispatch) — the "a crashed sweep poisons the next run" class. Both now capture their exception and join. The suite is run with -W error::pytest.PytestUnhandledThreadExceptionWarning and is clean.
Counts (counted, not exit codes)
- pytest 413 passed, 0 failed (was 407), under
-W error::…ThreadExceptionWarning. Run undernix-shell -p python312Packages.pytest, and I checked the count is plausible rather than reading the exit code — a barepython3 -m pyteston this host would exit 0 printingNo module named pytest. - node tests 454 / pass 454 / fail 0 / skipped 0;
test timed outgrep → 0. - Clean tree at the time of the run;
git stash listunchanged at 9.
Live
Laptop .155 / personal / ext 0.7.0 / deployed server f5fbe82, tree clean. Healthy ping → pong, ext 0.7.0. The gate itself is NOT live-verified — it is server.py, which needs home-manager switch + systemctl --user restart browser-bridge. Everything about it is covered by the threaded + unit tests above, including the wedged-while-busy case (via a monkeypatched EXEC_OP_BUDGET_S, so it runs in test time rather than 18 real seconds).
Noted on the #271 SKILL.md conflict — I have not touched #271 and will leave the resolution to you.
Hardens the CLI dispatch parser in test_surface_parity.py. It matched `\bcmd_op\s+([A-Za-z]+)` on any line whose first non-space character was not `#`, so any prose containing the phrase `cmd_op <word>` was harvested as a wire op. MEASURED on the merged tree of this PR + #278 (tip a0a0021): #278 added a Python docstring inside a `python3 -c` block reading "The machine-readable cause, from `cmd_op stderr` it already emits" — and the parser harvested a phantom op named `stderr`. Merged-tree pytest went 1 failed / 424 passed, with a diagnostic pointing at a wire op that does not exist. Invisible on either branch alone. Fixed here as a CLASS, not by special-casing the word. The rule now requires TWO independent conditions: 1. the occurrence survives `mask_shell_noncode()` — a small shell lexer that blanks comments, single-/double-quoted strings (including MULTI-LINE ones), backquoted spans and heredoc bodies, preserving byte offsets. `$( … )` is deliberately NOT masked even inside double quotes: that is genuine command position. 2. it sits in command position — start of statement, or after `;` `&&` `||` `|` `(` `$(` `then` `do` `else`. New fixture tests/fixtures/cmd_op_parse_rig.sh pins 8 dispatch shapes that MUST be harvested and 11 mention shapes that MUST NOT be. RED-FIRST (the whole point): against the rig, the previous parser harvested 7 phantoms — phantombacktick, phantomdocstring, phantomdq, phantomheredoc, phantommultilinedq, phantomquotedheredoc, phantomsq — while keeping all 8 real ones. Reverting the parser (mutation P1) reproduces exactly that. Over-tightening controls, because a permanently-green no-op is the obvious wrong fix: - the rig set must match EXACTLY (misses are named, not just leaks); - the real CLI must still yield all 19 ops == server.py's inventory; - `screenshot` is pinned by name — it is dispatched from `resp="$(cmd_op screenshot "$full")"`, a command substitution nested in double quotes. An earlier version of the masker blanked the `$(` and lost exactly that one op, 19 -> 18, while every other test stayed green. Caught by this control, not by review. - a new test pins that the CLI contains no LIVE backtick substitution, since backquoted spans are masked conservatively (113 backticks, all inside comments — MEASURED). Mutation sweep, 10 mutations, all red, each naming its specific phantom or missing op. Two SURVIVED the first pass and are the reason the rig grew two more cases: - P5 (drop command-position anchoring) was green — every phantom was already being caught by masking. Now caught by an UNQUOTED bareword mention (`echo usage: cmd_op phantombareword`). - P6 (drop single-quote masking) was green for the WRONG reason: the `"""` in the docstring case toggled double-quote state, so a different defence was catching it. Now caught by a single-quoted `python3 -c` block containing no double quotes. Also re-verified P7, the M14-style "new wire op with no CLI subcommand" case, still goes red naming `sniff`. Verified against the real #278 shape: injecting that exact docstring into a throwaway COPY of `browser` (the real one is owned by #278 and untouched) leaves the op set at 19 -> 19 and does not harvest `stderr`. Counts (from reporter lines, not exit codes; pytest 9.0.2 confirmed importable — a bare `python3 -m pytest` on this host exits 0 printing "No module named pytest"): pytest 381 -> 385 passed, 0 failed node 460 tests, 459 pass, 1 fail — unchanged, still the deliberate AGENT SURFACE PARTITION finding awaiting the operator's ruling on context/emulate/ping. Not pre-empted here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…, wake causes 🔴-A A docstring inside a `python3 -c` block read "<dispatch-helper> stderr", and #277's surface-parity gate harvests wire ops with a regex that skips only lines whose first non-space char is `#`. A docstring line is not one, so it harvested a phantom wire op named `stderr` and reddened the MERGED tree (1 failed / 424 passed) while both branches were green alone. Reworded, and a local test now runs the same harvest so the trap is closed on this side too rather than depending on the other PR — either fix alone closes it, either alone can regress. 🟡-B The submitter abandoning a command does NOT free the extension: its serial loop keeps executing for up to EXEC(18) + RESULT(10) = 28s, LONGER than the 20s cmd_timeout. Popping the `inflight` entry at submitter-exit therefore opened a window where the instance looked IDLE while provably still busy, and the next ping fast-failed at 2s against a healthy extension — in that window a0a0021 was worse than both main (20s flat) and the interim constant (10s flat). Now the entry is dropped only when the command never left the outbox (it will never run); when the extension already took it the entry SURVIVES the submitter, is released when the late result arrives, and otherwise expires at INFLIGHT_STALE_S (EXEC + RESULT) so one wedged command cannot make ping slow forever. 🟡-C `unknown_op` was the one wake failure still losing its cause: its branch uses neither the "failed in the browser:" prefix nor an echoed body, so both classifier arms had nothing to read. It is also the most likely `--wake` failure here (a stale loaded extension) and the PERMANENT one the exit-3 contract promises callers can distinguish. Added the arm + a parametrize case. 🟢-E The headline "busy AND wedged" test was INSENSITIVE to the `- age` term it advertises: EXEC was monkeypatched to 0.2 so the no-age budget (2.2s) sat under its own 5.0s bound, and deleting `- age` left it green. Recalibrated (EXEC 3.0 / GRACE 0.5 / age ~4s) so correct=0.3s vs no-age=3.5s with the bound between them, and cmd_timeout lowered below _req's 10s urlopen timeout so the `return timeout` mutant now dies on THIS test's assertion instead of a transport error. 🟢-D `fast_timeout` is clamped into [.., cmd_timeout] before use — both are operator-settable and nothing validates the relation, so fast=60/cmd=20 escaped the "never above cmd_timeout" claim through the lower clamp (measured: 60.0). 🟢-F/G/H Comment above the ping-504 branch still described a "default 10s SHORT deadline"; the inflight-release test claimed three exits but had two blocks describing the same case (it now covers success / abandoned-while-queued / abandoned-while-running / late-result-release); and the "a busy profile is never reported as dead" claims are now conditional — the derived budget is still capped at cmd_timeout, so >cmd_timeout of legitimate serial work can still time a ping out. RESULT_BUDGET_S joins EXEC_OP_BUDGET_S with its own protocol.js drift guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 3 addressed (
|
| test | base | HEAD | kind |
|---|---|---|---|
test_ping_still_waits_after_the_SUBMITTER_gave_up_on_a_running_command |
RED (504 @ 2.00 s) | green | regression (🟡-B) |
test_inflight_release_distinguishes_queued_from_running_on_abandon |
RED | green | regression (🟡-B, 🟢-G) |
test_a_kept_inflight_entry_expires_if_the_result_never_arrives |
RED | green | regression (🟡-B bound) |
test_result_budget_matches_the_extension |
RED | green | drift guard |
test_effective_timeout_gate_unit (cases 7) |
RED | green | regression (🟢-D) |
test_a_failing_wake_never_swallows_the_primary_result[op_error-unknown_op-*] |
RED (wake_failed) |
green | regression (🟡-C) |
test_no_prose_line_looks_like_a_wire_op_dispatch |
RED (['stderr']) |
green | regression (🔴-A) |
test_ping_fast_fails_even_while_BUSY_... |
green | green | ⚠ test-hardening, not a code fix — its evidence is MU1/MU2, not a base-red |
8 red at a0a0021; no pre-existing test red.
Mutations — each red with THAT guard's own error
| # | mutation | result |
|---|---|---|
| MU1 | drop the - age term |
RED at 3.50 s with the test's own message — the mutant that used to survive |
| MU2 | gate always returns cmd_timeout |
RED at 8.00 s on this test's assertion, not a transport error |
| MU3 | always pop inflight on abandon |
RED ×2 — "told the extension is dead after 2.00s", "the entry must survive" |
| MU4 | late result no longer releases | RED — "must release the kept entry, not leave it to expire" |
| MU5 | disable the staleness prune | RED — "a stale entry must be pruned, not just ignored" |
| MU6 | remove the 🟢-D clamp | RED — "idle: fast_timeout must be capped at cmd_timeout" |
| MU7 | remove the unknown_op arm |
RED ×2 — 'wake_failed' == 'unknown_op' |
| MU8 | restore the phantom prose | RED — "prose in the CLI reads as a wire-op dispatch: ['stderr']" |
Counts
- pytest 419 passed, 0 failed (was 413), under
-W error::pytest.PytestUnhandledThreadExceptionWarning. Run undernix-shell -p python312Packages.pytest; count is plausible (+6 = 4 new tests + 2 new parametrize cases). - node tests 454 / pass 454 / fail 0 / skipped 0;
test timed outgrep → 0. - Clean tree at run time. SKILL.md untouched: 12,010 B — no bytes added, per your 39 B merged-tree note.
Live
Laptop .155 / personal / ext 0.7.0 / deployed server f5fbe82. Healthy ping → pong; --help intact at 16,152 B; the phantom-op harvest verified clean against the real file. The gate and the abandon window are server.py, so not live-verified — they need a switch + systemctl --user restart browser-bridge.
Untouched: #271, and the SKILL.md merge resolution.
…ll awaiting
🟡-1 (round-4 audit). INFLIGHT_STALE_S is measured from ENQUEUE, but a queued
command's EXEC_OP_BUDGET_S does not start until the SERIAL extension dequeues it.
So age alone cannot mean "a healthy extension has certainly answered" — and it
fails precisely at N>=2, the case the `len(inst.inflight) * EXEC_OP_BUDGET_S` term
exists to model.
Measured with the injected clock, _effective_timeout_locked(inst, cmd_timeout, 2.0):
N=3, cmd_timeout=60: age 27s -> 29.0s ; age 29s -> 2.0s (should be 27.0s)
N=2, cmd_timeout=20: age 27s -> 11.0s ; age 29s -> 2.0s (should be 9.0s)
Scenario: an operator follows the advice THIS BRANCH added at
BROWSER_BRIDGE_CMD_TIMEOUT ("raise CMD_TIMEOUT if that is your workload"), sets
60s and submits 3 ops. At t=29s all three entries are pruned while their submitters
are still blocked, the instance reads IDLE, and `ping` fast-fails at 2s with ~25s
of legitimate work left. A regression versus a0a0021 in that window, and it
falsified the remediation the same commit documented.
Fix: prune requires BOTH age > INFLIGHT_STALE_S AND no live submitter
(`cid not in inst.waiters`). The memory bound is not weakened, it is split by
owner: a live-submitter entry is bounded by that submitter's own deadline (it
unwinds through the finally, which pops it); an abandoned entry has no one to bound
it, which is what INFLIGHT_STALE_S is for.
`waiters` was verified to be exactly the live-submitter set by reading every
mutation, not by assumption — and submit()'s finally now discards it too, so an
unexpected raise inside the wait loop cannot strand an entry into permanent
prune-exemption. A new test pins the property on both reachable exits.
🟢-2 The N=1 expiry test claimed "the staleness window bounds memory, not the
verdict". True at N=1 ONLY — it holds there because a lone entry's budget is
already negative at 20s, so pruning at 28s is verdict-neutral by construction. The
docstring now names that scope and points at the N>=2 test.
🟢-3 The abandon-late-result branch popped `inflight` but left `last_dispatch`
naming that cid, whose contract is "the command it never answered" and which is
surfaced by health/whoami. After that path it DID answer. Cleared, on the same
condition the normal path uses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 4 addressed (
|
| test | base | HEAD | kind |
|---|---|---|---|
test_stale_prune_never_evicts_an_entry_whose_submitter_is_STILL_WAITING |
RED (2.0 == 27.0) |
green | regression (🟡-1) |
test_inflight_release_distinguishes_queued_from_running_on_abandon |
RED (last_dispatch still names…) |
green | regression (🟢-3) |
test_waiters_is_exactly_the_live_submitter_set |
green | green | INVARIANT GUARD — pins the premise the prune now reads; not regression coverage |
test_a_kept_inflight_entry_expires_if_the_result_never_arrives |
green | green | unchanged behaviour; docstring scoped (🟢-2) |
Mutations
| # | mutation | result |
|---|---|---|
| MV1 | drop the c not in inst.waiters condition (restore the bug) |
RED — "three live submitters were pruned as stale, so the instance read idle while the extension was legitimately working" |
| MV2 | drop the age condition, keep only waiters |
RED ×2 — the abandoned entry is pruned instantly, breaking both 🟡-B's test and the expiry bound. Proves both conditions necessary, not just the new one. |
| MV3 | remove the last_dispatch clear on the abandon path |
RED — "last_dispatch still names a command that has now been answered" |
Disclosed, not papered over: the finally's extra waiters.discard is a defensive net for a raise inside the wait loop that no test can reach from outside. It is an invariant guard with no mutation coverage, labelled as such in both the source and the test docstring. I'm not claiming it's covered.
Counts
- pytest 421 passed, 0 failed (was 419; +2 new tests), under
-W error::pytest.PytestUnhandledThreadExceptionWarning, vianix-shell -p python312Packages.pytest. Count plausible: +2. - node tests 454 / pass 454 / fail 0 / skipped 0 using the glob form — thanks for the harness note; I hit that exact false red (
tests 1 / fail 1, MODULE_NOT_FOUND) early in this task and have been using the glob since. - Clean tree at run time. SKILL.md untouched: 12,010 B.
Noted, no action
🟢-4 — agreed on all three points, including that my copy is strictly stricter on same-line cases (a visible false alarm, not a silent hole) and that the newline-spanning \s case is covered by #277's rig and not mine. Recorded rather than chased; my guard's job is CLI prose, not policing #277's masker.
Live: healthy ping → pong, ext 0.7.0. The gate and prune are server.py, so not live-verified — they need a switch + systemctl --user restart browser-bridge. #271 and the SKILL.md resolution untouched.
Hardens the CLI dispatch parser in test_surface_parity.py. It matched `\bcmd_op\s+([A-Za-z]+)` on any line whose first non-space character was not `#`, so any prose containing the phrase `cmd_op <word>` was harvested as a wire op. MEASURED on the merged tree of this PR + #278 (tip a0a0021): #278 added a Python docstring inside a `python3 -c` block reading "The machine-readable cause, from `cmd_op stderr` it already emits" — and the parser harvested a phantom op named `stderr`. Merged-tree pytest went 1 failed / 424 passed, with a diagnostic pointing at a wire op that does not exist. Invisible on either branch alone. Fixed here as a CLASS, not by special-casing the word. The rule now requires TWO independent conditions: 1. the occurrence survives `mask_shell_noncode()` — a small shell lexer that blanks comments, single-/double-quoted strings (including MULTI-LINE ones), backquoted spans and heredoc bodies, preserving byte offsets. `$( … )` is deliberately NOT masked even inside double quotes: that is genuine command position. 2. it sits in command position — start of statement, or after `;` `&&` `||` `|` `(` `$(` `then` `do` `else`. New fixture tests/fixtures/cmd_op_parse_rig.sh pins 8 dispatch shapes that MUST be harvested and 11 mention shapes that MUST NOT be. RED-FIRST (the whole point): against the rig, the previous parser harvested 7 phantoms — phantombacktick, phantomdocstring, phantomdq, phantomheredoc, phantommultilinedq, phantomquotedheredoc, phantomsq — while keeping all 8 real ones. Reverting the parser (mutation P1) reproduces exactly that. Over-tightening controls, because a permanently-green no-op is the obvious wrong fix: - the rig set must match EXACTLY (misses are named, not just leaks); - the real CLI must still yield all 19 ops == server.py's inventory; - `screenshot` is pinned by name — it is dispatched from `resp="$(cmd_op screenshot "$full")"`, a command substitution nested in double quotes. An earlier version of the masker blanked the `$(` and lost exactly that one op, 19 -> 18, while every other test stayed green. Caught by this control, not by review. - a new test pins that the CLI contains no LIVE backtick substitution, since backquoted spans are masked conservatively (113 backticks, all inside comments — MEASURED). Mutation sweep, 10 mutations, all red, each naming its specific phantom or missing op. Two SURVIVED the first pass and are the reason the rig grew two more cases: - P5 (drop command-position anchoring) was green — every phantom was already being caught by masking. Now caught by an UNQUOTED bareword mention (`echo usage: cmd_op phantombareword`). - P6 (drop single-quote masking) was green for the WRONG reason: the `"""` in the docstring case toggled double-quote state, so a different defence was catching it. Now caught by a single-quoted `python3 -c` block containing no double quotes. Also re-verified P7, the M14-style "new wire op with no CLI subcommand" case, still goes red naming `sniff`. Verified against the real #278 shape: injecting that exact docstring into a throwaway COPY of `browser` (the real one is owned by #278 and untouched) leaves the op set at 19 -> 19 and does not harvest `stderr`. Counts (from reporter lines, not exit codes; pytest 9.0.2 confirmed importable — a bare `python3 -m pytest` on this host exits 0 printing "No module named pytest"): pytest 381 -> 385 passed, 0 failed node 460 tests, 459 pass, 1 fail — unchanged, still the deliberate AGENT SURFACE PARTITION finding awaiting the operator's ruling on context/emulate/ping. Not pre-empted here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ry (contains 1 DELIBERATE failure) (#277) * test(browser-bridge): anchor the agent surface to the real op inventory Closes the structural gap the 2026-08-02 surface audit called its most important non-token finding (F9/V3, G1, G2). The four-source agent parity test pinned browser.js's typed enum, the agent-md capability table, the README contract and ALLOWED_OPS_DEFAULT to EACH OTHER and to nothing upstream. Because all four omit `context`, `ping` and `emulate` identically, the set was self-consistent, CI was green, and the omission was invisible. Structurally the same defect as `context` being dead on main (in protocol.js, absent from server.py's ALLOWED_OPS), one layer further out - at the surface the agent actually touches. tests/test_server.py::test_ping_op_set_mirrors_the_extension_protocol_js already does this correctly for the wire layer: it PARSES protocol.js and asserts set-equality. Same approach here, at two more layers. browser_tool.test.mjs (+6 tests) - the existing four-source parity test now names WHICH source is missing WHICH op before the bare deepEqual (assertions added, none removed) - UPSTREAM ANCHOR: every agent-facing op name must resolve through OP_TO_SERVER to an op server.py actually allows - AGENT SURFACE PARTITION: every wire op must be REACHABLE by the agent or a DECLARED exclusion in REVIEWED_AGENT_EXCLUSIONS, each carrying a written reason AND a `source` snippet asserted to exist verbatim in browser_tool_impl.mjs - so an exclusion cannot be padded with an invented rationale, and cannot outlive the comment it points at - two HARNESS SELF-CHECK tests: plausible non-empty cardinality, and the parser failing loudly on an unreadable/wrong-shaped source tests/test_surface_parity.py (new, +12 tests, IS in the flake gate) - CLI SUBCOMMANDS <-> op inventory. Not set-equality: 24 CLI names vs 18 wire ops + 1 server op. Every name is classified wire-op / server-op / alias / client-only, the classification is asserted EXHAUSTIVE and a partition, and it is anchored to the CLI's real `cmd_op <op>` call sites so the table cannot claim a mapping the script does not make. - SKILL.md ops table <-> SUBCOMMANDS, both directions, plus server.py -> SKILL.md to close the triangle. - parser negative controls: missing file and wrong-shaped file must RAISE, never return an empty set (an empty set makes every parity assertion pass vacuously). ONE TEST FAILS ON PURPOSE - that failure IS the finding: AGENT SURFACE PARTITION: UNDECLARED EXCLUSION(S): context, emulate, ping Those three wire ops are unreachable by the autonomous agent (absent from OP_TO_SERVER, so not even BROWSER_AGENT_ALLOWED_OPS re-enables them) and carry no written rationale, unlike `open`/`close`/`tabs`/`activate`, which all do. Whether each is a deliberate exclusion is an operator decision, not one this test may make. Do NOT resolve it by tuning the test. Counts (from the reporter lines, not exit codes): node 454 pass / 0 fail -> 460 tests, 459 pass, 1 fail (the finding) pytest 369 passed -> 381 passed, 0 failed Mutation sweep: 25 mutations across delete / rename / add / alias-retarget / unclassified-name / fabricated-rationale / harness-sabotage. All 25 went red, each naming the specific op and source; verified red for its OWN assertion, not an earlier one. Notably a fabricated exclusion rationale does NOT silence the failure - it fails with "cited rationale is NOT present in browser_tool_impl.mjs". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(browser-bridge): a `cmd_op` MENTION is not a dispatch Hardens the CLI dispatch parser in test_surface_parity.py. It matched `\bcmd_op\s+([A-Za-z]+)` on any line whose first non-space character was not `#`, so any prose containing the phrase `cmd_op <word>` was harvested as a wire op. MEASURED on the merged tree of this PR + #278 (tip a0a0021): #278 added a Python docstring inside a `python3 -c` block reading "The machine-readable cause, from `cmd_op stderr` it already emits" — and the parser harvested a phantom op named `stderr`. Merged-tree pytest went 1 failed / 424 passed, with a diagnostic pointing at a wire op that does not exist. Invisible on either branch alone. Fixed here as a CLASS, not by special-casing the word. The rule now requires TWO independent conditions: 1. the occurrence survives `mask_shell_noncode()` — a small shell lexer that blanks comments, single-/double-quoted strings (including MULTI-LINE ones), backquoted spans and heredoc bodies, preserving byte offsets. `$( … )` is deliberately NOT masked even inside double quotes: that is genuine command position. 2. it sits in command position — start of statement, or after `;` `&&` `||` `|` `(` `$(` `then` `do` `else`. New fixture tests/fixtures/cmd_op_parse_rig.sh pins 8 dispatch shapes that MUST be harvested and 11 mention shapes that MUST NOT be. RED-FIRST (the whole point): against the rig, the previous parser harvested 7 phantoms — phantombacktick, phantomdocstring, phantomdq, phantomheredoc, phantommultilinedq, phantomquotedheredoc, phantomsq — while keeping all 8 real ones. Reverting the parser (mutation P1) reproduces exactly that. Over-tightening controls, because a permanently-green no-op is the obvious wrong fix: - the rig set must match EXACTLY (misses are named, not just leaks); - the real CLI must still yield all 19 ops == server.py's inventory; - `screenshot` is pinned by name — it is dispatched from `resp="$(cmd_op screenshot "$full")"`, a command substitution nested in double quotes. An earlier version of the masker blanked the `$(` and lost exactly that one op, 19 -> 18, while every other test stayed green. Caught by this control, not by review. - a new test pins that the CLI contains no LIVE backtick substitution, since backquoted spans are masked conservatively (113 backticks, all inside comments — MEASURED). Mutation sweep, 10 mutations, all red, each naming its specific phantom or missing op. Two SURVIVED the first pass and are the reason the rig grew two more cases: - P5 (drop command-position anchoring) was green — every phantom was already being caught by masking. Now caught by an UNQUOTED bareword mention (`echo usage: cmd_op phantombareword`). - P6 (drop single-quote masking) was green for the WRONG reason: the `"""` in the docstring case toggled double-quote state, so a different defence was catching it. Now caught by a single-quoted `python3 -c` block containing no double quotes. Also re-verified P7, the M14-style "new wire op with no CLI subcommand" case, still goes red naming `sniff`. Verified against the real #278 shape: injecting that exact docstring into a throwaway COPY of `browser` (the real one is owned by #278 and untouched) leaves the op set at 19 -> 19 and does not harvest `stderr`. Counts (from reporter lines, not exit codes; pytest 9.0.2 confirmed importable — a bare `python3 -m pytest` on this host exits 0 printing "No module named pytest"): pytest 381 -> 385 passed, 0 failed node 460 tests, 459 pass, 1 fail — unchanged, still the deliberate AGENT SURFACE PARTITION finding awaiting the operator's ruling on context/emulate/ping. Not pre-empted here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(browser-agent): make `context` reachable; DECLARE `ping` + `emulate` excluded Resolves the deliberate failing test from #277 (AGENT SURFACE PARTITION), which reported three wire ops that were neither agent-REACHABLE nor DECLARED exclusions. - `context` -> REACHABLE. Its absence was provably an oversight, not a decision: browser_tool_impl.mjs was last edited 2026-07-31 (#243, 1e2ad9e) and `context` only reached server.py's ALLOWED_OPS on 2026-08-01 (#263, 55ad035), so the agent-surface mapping has never been touched since the op existed. It is a cheap read of page state (url/domain/path/searchParams/title/tabId), no DOM content, strictly less powerful than the `text`/`html` the agent already has. Added to OP_TO_SERVER, ALLOWED_OPS_DEFAULT and the other three agent-facing sources the parity test holds in lockstep (browser.js enum, browser-agent.md capability table, README op contract), plus a field-pinned summarizeResult branch so a later server-side payload addition cannot silently widen what reaches the model. - `ping` -> DECLARED EXCLUDED. Operator diagnostic for extension staleness; the model cannot act on the answer (cannot reload an extension or restart Brave) and it reads no page state. - `emulate` -> DECLARED EXCLUDED. Mutates the tab and leaves STICKY per-tab state (device metrics, UA-CH, media overrides) that outlives the op until an explicit reset, so an agent that emulates and never resets hands back a silently altered tab. Both exclusions carry the rationale comment in browser_tool_impl.mjs that REVIEWED_AGENT_EXCLUSIONS asserts verbatim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s (2026-08-02) (#286) Covers the 2026-08-01 handoff's three open items taken through to merged-deployed-verified: 11 devrc PRs, homelab-infra#274 + clawgate 0.7.82, and issue #273 opened/probed/closed on measurement. The four durable lessons, all measured: - "Merged != deployed" fired twice in OPPOSITE directions. The kubeclaw chart merged to trunk and sat inert (go:embed behind a literal image pin, no Flux image automation). The workbench ran OLD code behind a green ship.sh, because an orphaned non-systemd process from the previous day held port 8788 while the unit crash-looped. Check the consumer is running your artifact. - Twelve false-signal harnesses, and a rule already in RULES.md did not inoculate against its own class: `diff` unified output made `^>`/`^<` greps report "0 lines differ" for files differing by 1,445 bytes. cmp settled it. - A count of DECLARATIONS is not a count of INSTANCES: two skipif decorators were reported as "2 skips" and actually gated 123 tests. 60x, and it was the difference between a nit and the session's most valuable fix. - The merged-tree gate earned itself: a docstring in #278 containing the phrase `cmd_op stderr` made #277's parser harvest a phantom wire op. Red only on the merged tree, invisible on either branch alone. Both hosts are at 81e2d76; main has since moved to 9254361 (other sessions), so the next session should re-run ship.sh. A skills/docs update PR was dispatched at end of session and should be checked for landing. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Five CLI/behaviour fixes from the 2026-08-02 usage audit (
claudedocs/browser-bridge-usage-audit-2026-08-02.md, PR #274), plus one regression found while measuring them.Files:
scripts/browser-bridge/browser,scripts/browser-bridge/server.py,scripts/browser-bridge/tests/test_browser_cli_args.py,scripts/browser-bridge/tests/test_server.py. NoSKILL.md, noreference/**, nobrowser_tool.test.mjs, no extension.What changed
S1 —
--wake[=MS]onnavandopen(audit F2). The flag existed on exactlyhtml/text/js. The three copy-pasted parse blocks are consolidated into ONE_wake_flaghelper that all five branches call; validation still happens at PARSE time (thedieis in the flag loop, never insidewake_fields' command substitution).nav/opensend nowakefield on the wire — the extension honourscmd.wakeonly ongetHtml/text/eval, and the extension is out of this PR's scope.--wakeis therefore a client-side compose (_wake_after): the nav/open op, then the existingwakeop, annotated atresult.data.wake— the same place a--wakeread reports it. The round trip it removes is the agent's, not the bridge's. Two wire ops, one invocation.S2 —
pinggets its own 2 s deadline (F7). NewPING_TIMEOUT_DEFAULT = 2.0+BROWSER_BRIDGE_PING_TIMEOUT, resolved through a new_env_floatthat falls back on an unparseable value rather than killing the service at startup. Applied at exactly one place,op_timeout = ping_timeout if op == "ping" else cmd_timeout— no other op's timeout moves.S3 —
unknown_instance/no_extensionsplit into two messages (F6). 48 of 52unknown_instancefailures used the correct label, withevalretried 37×. The server already returnsknown_instancesin both bodies; the CLI now reads it. Key known-but-disconnected → names when it went away, says FULLY RESTART Brave, says DO NOT RETRY. Key never seen → says WRONG LABEL and lists the keys that exist, with no restart advice.no_extensiongets the same split.S4 —
/whoamiand/healthemit telemetry (F8). 53 invocations were invisible to the only structured source. Metadata-only via the existingemit_cmd_event, after the response, best-effort. No domain and no key — these endpoints are global (they describe every connected profile at once), so emitting per-profile domains would widen the privacy contract atserver.py's PRIVACY CONTRACT./instancesand/pollstill emit nothing.health's output shape is untouched.S5 — explicit-path
screenshotprints a Read hint (F4; 7 of 63 captures never Read). Line 1 stays the bare path; line 2 is# Read <path> to view it.The#prefix keeps it from ever being mistaken for a second path. Not printed for--data-urlor the temp-file form.S6 — the broken doc pointer.
browser:582/:588pointed at "SKILL.md → Concurrency", a heading that has never existed. Repointed atreference/tabs-instances.md+ the Concurrent-drivers paragraph ofSKILL.md → This is the user's LIVE session, and a test now proves everySKILL.md → <heading>the CLI names resolves.Bonus (second commit) —
--helpstopped dumping the whole file. Found while measuring S1–S6's byte cost:--helpwasgrep -E '^#( |$)' "$0", i.e. every column-0 comment in the file. It shipped 25,440 bytes of implementation commentary as user-facing help and grew with every internal comment — S1–S6's comments alone would have added +4,129 bytes to every--help. Now the contiguous#block after the shebang, stopping at the first line of code.Red → green matrix
Base =
origin/main(ff0fe41). Method: commit, thengit checkout origin/main -- browser server.pykeeping the new tests, run, restore withgit checkout HEAD -- …. Nogit stashanywhere (git stash listverified unchanged: 58 entries,stash@{0}intact).Harness validation first. The initial base run reported 33 failures — contaminated: the new
_serve(..., ping_timeout=)raisesTypeErroragainst the pre-S2make_handler, so every_serve()call errored and pre-existing tests failed for a reason that had nothing to do with the change. That green/red was worthless. Re-run with a measurement-only signature shim (reverted, not shipped) and the base run came back exactly 17 failures, all of them new tests, every pre-existing test green — which is also the control proving the test-file helper edits are base-compatible.test_wake_on_nav_and_open_issues_the_wake_in_one_invocation[nav],[open]test_wake_on_nav_and_open_is_order_free_and_carries_ms[nav],[open]test_nav_and_open_validate_wake_ms_at_parse_time[nav],[open]test_nav_and_open_still_reject_an_unknown_flag[nav],[open]test_no_wake_on_nav_and_open_sends_exactly_one_unchanged_command[nav],[open]test_open_with_no_url_still_means_about_blanktest_ping_does_not_wait_out_cmd_timeouttest_ping_timeout_is_env_overridable_and_survives_a_malformed_valuetest_the_short_deadline_applies_to_ping_ONLYtest_a_healthy_ping_is_unaffected_and_answers_in_millisecondstest_unknown_instance_KNOWN_but_disconnected_says_stop_retryingtest_unknown_instance_NEVER_SEEN_key_says_wrong_labeltest_no_extension_distinguishes_dropped_from_never_wired_uptest_routing_failure_explainer_degrades_against_an_OLD_servertest_orientation_ops_emit_exactly_one_metadata_only_event[/whoami],[/health]test_instances_and_poll_still_do_not_emittest_orientation_emit_never_breaks_the_responsetest_browser_cli_screenshot_explicit_path_prints_path_then_a_read_hinttest_browser_cli_screenshot_read_hint_not_printed_where_it_would_be_wrongtest_every_skill_md_heading_the_cli_points_at_actually_exists['Concurrency'])test_help_prints_the_HEADER_block_only_not_the_whole_files_commentsMutation results (each confirmed red with THAT guard's own error)
op_timeout = cmd_timeout(ping loses its deadline)ping waited 8.0s — cmd_timeout, not its own. The negative control stayed green, proving it discriminates.op_timeout = ping_timeout(leaks to every op)getHtml returned in 2.00s — the short ping deadline leaked onto every op. Only the negative control fired./healthemitexpected exactly one event, got [],[/health]only;[/whoami]stayed green.domain(privacy weakening)[/whoami]and[/health]— payload equality catches it.assert 'is UNKNOWN' in "…is KNOWN but NOT CONNECTED…".if False:)navparses--wakebut never composes the wakeassert ['nav'] == ['nav', 'wake'].--wake=MSvalidation from_wake_flag[nav]and[open]from a single deletion — the consolidation is real, one rule in one place.SKILL.md -> Concurrencyassert ['Concurrency'] == [].grep-everything--help--help leaked an internal comment: wake_fields WAKE WAITMS.The S6 pointer test carries its own inline harness negative control: the extractor is run against a known-bad pointer (must report
["Concurrency"]) and a known-good one (must report[]) before its verdict on the real file is read.Test counts (counted, not exit codes)
git status -sempty).grep -ci 'test timed out'→ 0. No.mjsfile was touched, so 454 is also the base count.Verified live vs. tests only
Live host: laptop
192.168.50.155, profilepersonal, extension 0.7.0, deployed server gitf5fbe82. My worktree was CLEAN at the time (committed). ThebrowserCLI is a working-tree symlink so the CLI under test was my branch's code;server.pywas the deployed artifact, i.e. pre-S2/S4.S1 — live-verified end to end against
tests/fixtures/oopif-rig/wake-rig.htmlserved on127.0.0.1:8901:rafpinned at 1 whiletimerclimbed — live and throttled, not dead — so the control is not vacuous.vis=hiddenthroughout: no focus was moved,activatewas never called. Tab closed afterwards.S3 — live-verified for the never-seen branch (
browser --instance nosuchlabel tabsprinted the WRONG-LABEL message and listedpersonal, work). The known-but-disconnected branch was NOT live-verified — producing it means killing a Brave profile's long-poll, which I would not do to the operator's session. It is covered by canned-server tests only.S2 — NOT live-verified. The change is in
server.py, which is not deployed. I confirmed only that the healthy path is unaffected:browser --instance personal ping→pong, extension 0.7.0, 68 ms end to end against the deployed server. The 2 s failure path is covered by wall-clock tests only.S4 — NOT live-verified (same reason:
server.pyis not deployed). Covered by tests.S5, S6,
--help— tests + direct CLI invocation only (no bridge involved).🔴
server.pychanges needhome-manager switch+systemctl --user restart browser-bridgeto go live. The CLI half (S1, S3, S5, S6,--help) is amkOutOfStoreSymlinkonto the working tree and goes live on pull. Until the switch, S1'snav --wakeworks (it is pure CLI) but S2/S4 do not.Per-call byte deltas of added output
browser --helpscreenshot <path>stdout\nunknown_instancestderrnav/openwithout--wakenav/openwith--wakeresult.data.wakeobjectbrowser wakeinvocation, which printed a larger standalone envelope — net negativewhoami/health/ every read opCould not verify
home-manager switchthis PR does not perform.healthprints both profiles' full active-tab URLs to its caller is a problem worth fixing — explicitly out of scope;health's output shape is unchanged here.Note on a test-harness change
test_server.py's_wait_connectednow polls/instancesinstead of/health, and a new_wait_countdoes the same for two telemetry tests. Reason: S4 makes/healthemit, and these helpers call it in a tight loop, so they injected spurious events into the spool of every test that waits for a connection (14 went red). The fix makes the harness silent rather than loosening the exact-count assertions._wait_instancesstays on/healthbecause its callers read health-only fields. Both are commented in place so nobody points them back.🤖 Generated with Claude Code