feat(browser-bridge): support --annotated with --frame

[ZacxDev]

What

Support --annotated with --frame for the text op. Previously this returned annotated_with_frame_unsupported, but annotatedTextFn is self-contained and works identically when injected into a frame's document.

Changes

• service_worker.js: remove the annotated_with_frame_unsupported throw, add annotated branch inside the --frame path
• protocol.js: extract byteCapElements() helper to deduplicate the byte-cap loop across 3 paths (wake, standalone, frame)
• manifest.json: 0.7.0 → 0.7.1 (see Discriminator below) + the two literal assertions in test_server.py that pin it
• Docs: remove incompatibility notes from 6 files (errors.md, read-envelopes.md, SKILL.md, README.md, handoff docs); document the 0.7.1 discriminator in extension/README.md
• Tests: 6 new tests for byteCapElements, plus 2 regression tests in service_worker.test.mjs for the annotated-in-frame branch
• Merged origin/main (a0a5d73, fix(browser-bridge): nav/open --wake, fast ping, split routing errors, orientation telemetry, screenshot Read hint #278) — the SKILL.md ops-table conflict is resolved by taking this branch's text row and fix(browser-bridge): nav/open --wake, fast ping, split routing errors, orientation telemetry, screenshot Read hint #278's nav/wake rows. SKILL.md is 11,999 B vs the enforced 12,038 budget (39 B slack).

Discriminator (why 0.7.1)

There is no new wire op, so ping's ops list is byte-identical between builds and cannot tell you which one Brave loaded. The version bump restores the extension_stale tell, and the capability itself is the exact discriminator:

browser --instance <label> frames                       # pick a real frameId
browser --instance <label> --frame <frameId> text --annotated
  # 0.7.0 → annotated_with_frame_unsupported: --annotated is not supported with --frame
  # 0.7.1 → the per-element elements[] payload (frame-relative CSS paths)

Run it against a page that HAS an iframe — on a frameless page both builds fail identically with frame_not_found.

Verification

Live (laptop host 192.168.50.155, profiles personal+work, deployed extension 0.7.0):

• Nested-OOPIF rig served on 127.0.0.1:8901, top.html opened + waked (never activate), tab closed after. frames → 0 (top) → 4410 (127.0.0.1.sslip.io/mid.html) → 4411 (127.0.0.1.nip.io/leaf.html), cross-origin, nested.

• Control --frame 4411 text → "LEAF (site: 127.0.0.1.nip.io) — a GRANDCHILD OOPIF (nested inside mid)\n\nleaf-marker", url: http://127.0.0.1.nip.io:8901/leaf.html. The frame plumbing is sound.

• Discriminator, "before" reading --frame 4411 text --annotated → annotated_with_frame_unsupported. This is the running 0.7.0 build, i.e. the error this PR removes is live and observable today.

• Staleness detection restored — fed the live /health versions into the server's own annotate_staleness() (read-only, nothing deployed):

  LIVE loaded versions : personal=0.7.0, work=0.7.0
  vs DEPLOYED manifest (0.7.0)   → extension_stale: false, false   ← the defeated tell
  vs THIS BRANCH  (0.7.1)        → extension_stale: true,  true    ← restored
  

🔴 NOT verified live: the feature working. The deployed extension is 0.7.0 and loading 0.7.1 needs a home-manager switch + an extension reload, neither of which was done here. Operator steps to complete it: home-manager switch → brave://extensions ↻ → browser --instance <label> ping must report extensionVersion 0.7.1 (if it still says 0.7.0, the ↻ no-op'd — fully quit and reopen Brave) → re-run the discriminator above and expect elements[].

Tests (measured on the merged tree, this branch @ 325e73a):

suite	command	result
node	node --test --test-reporter=tap scripts/browser-bridge/tests/*.test.mjs	462 tests / 462 pass / 0 fail / 0 skipped
pytest	python3 -m pytest scripts/browser-bridge/tests -q (pytest 9.0.2)	421 passed / 0 failed

test_surface_parity.py (#277) is not present on this branch, so its deliberate AGENT SURFACE PARTITION failure does not appear here.

Red-before-change (base origin/main = a0a5d73):

• service_worker.test.mjs annotated-in-frame tests → 2 fail, both with annotated_with_frame_unsupported: --annotated is not supported with --frame; the other 15 tests in the file pass, so the harness is sound and the failures are attributable.
• annotated.test.mjs → the whole file fails to load at base (byteCapElements is not exported).
• test_manifest_version_reads_current / ..._falls_back_to_repo_when_not_deployed → mutation-checked by reverting manifest.json to 0.7.0: both fail with assert '0.7.0' == '0.7.1'.

Notes

• CSS paths from annotated-in-frame are frame-relative (correct — relative to the frame's DOM, not the top page)
• --wake + --frame remains refused (wake_with_frame_unsupported, fix(browser-bridge): nav/open --wake, fast ping, split routing errors, orientation telemetry, screenshot Read hint #278's parse-time CLI guard). That is a different flag from --annotated and is not a contradiction with the row above it in SKILL.md.
• ⚠️ The original body claimed "561 tests pass". That figure is wrong and unreproducible — see the correction comment below.

[ZacxDev]

Reviewed while integration-testing this against five other open browser-bridge PRs. It merges clean with all of them and the merged tree is green (pytest 406/0; node 466 with only an unrelated intended failure from #277). Two things worth fixing before merge — the first I'd call blocking.

🔴 manifest.json is not bumped, and isn't in the diff

extension/protocol.js (+22) and extension/service_worker.js (+21/-37) both change, but manifest.json stays byte-identical at 0.7.0.

Per the repo's own rule, an extension change that must be provably loaded has to bump manifest.json and add a discriminator. Without the bump:

• browser --instance <label> ping reports extensionVersion: 0.7.0 whether or not the new code loaded;
• whoami/health compute extension_stale: false — not stale — even if Brave is still running the old service worker.

So the deterministic tell that exists specifically to catch a no-op ↻ reload is defeated for exactly this change. Given how often a ↻ silently doesn't take (the long-poll keeps the old worker alive), there's currently no way to confirm this shipped rather than sitting inert.

Note the extension id is path-derived, not version-derived, so it won't disambiguate this either.

🟡 The "561 tests pass" figure doesn't reconcile

Measured on this branch: 460 node tests, 460 pass, 0 fail (node --test --test-reporter=tap, counted from # pass). pytest on main is 369 — now 370 since #275. No combination reproduces 561. The "0 failures" half matches.

Not claiming it's wrong — I don't know the command used — but worth stating it, since an unreproducible count is the kind of claim that's bitten this repo before.

Verification

I couldn't find live-verification evidence in the body. This changes extension code, so it needs a full Brave restart (not ↻), and per the above there's no version tell to confirm it took until the manifest is bumped.

The good part

The removal of annotated_with_frame_unsupported is thorough and consistent — gone from all six places it appeared (service_worker.js, SKILL.md, both READMEs, reference/errors.md, reference/read-envelopes.md), no dangling references to a now-unreachable error. ALLOWED_OPS is unchanged, so no new wire op and no impact on the surface-parity tests in #277.

—Reviewed while landing #266/#272/#274/#275; not merging this one since it isn't mine.

[ZacxDev]

Heads-up: #278 just merged (a0a5d73), and it touches the adjacent row in the SKILL.md ops table — so this PR now conflicts there. Posting the resolution I already verified so you don't have to re-derive it.

The conflict

Both PRs rewrite neighbouring rows in the ops table:

• feat(browser-bridge): support --annotated with --frame #271 (yours) — the text [selector] … row: refused with \--frame`→works with `--frame` (frame-relative CSS paths)`
• fix(browser-bridge): nav/open --wake, fast ping, split routing errors, orientation telemetry, screenshot Read hint #278 — the nav <url> row, which gained [--wake[=MS]], and the wake row, which gained nav|open to its flag list

The resolution — take BOTH, one row each

Keep your text row verbatim and #278's nav row verbatim. I merged and gated this exact combination before #278 landed:

gate	result
SKILL.md	11,999 B vs the enforced budget 12,038 → 39 B slack
pytest	436 passed / 0 failed
node	466 tests / 465 pass / 1 fail — the single failure is #277's deliberate AGENT SURFACE PARTITION, unrelated

🔴 The byte gate is the trap here. SKILL.md is capped at MAX_BYTES 12_288 with MIN_HEADROOM_BYTES 250 (tests/test_skill_size.py), so the enforced budget is 12,038 and there are 39 bytes of slack. A resolution that keeps both phrasings of the --frame clause — the natural thing to do when a merge tool shows you two plausible lines — blows the gate. Take one row from each side; do not merge the sentences.

Not a contradiction, in case it looks like one

After resolution the file says --annotated works with --frame (your row) and --wake is refused with --frame (the wake row, wake_with_frame_unsupported). Those are different claims about different flags and both are correct — #278 added a parse-time CLI guard for the --wake+--frame case. Don't "fix" the second one while resolving.

Still outstanding from my earlier review

The 🔴 manifest.json bump is unaddressed — extension/protocol.js and extension/service_worker.js both change while manifest.json stays byte-identical at 0.7.0, so ping will report 0.7.0 and extension_stale will compute false whether or not the new code actually loaded. That's the tell that exists to catch a no-op ↻ reload, and it's defeated for exactly this change.

[ZacxDev]

Correction: the "561 tests pass" figure was wrong

Flagging this explicitly because the body has been edited and a reviewer may have read the original claim.

The original body said "561 tests pass, 0 failures". That number is not reproducible and has been removed. Measured counts:

tree	node (node --test --test-reporter=tap scripts/browser-bridge/tests/*.test.mjs)	pytest (python3 -m pytest scripts/browser-bridge/tests -q)
original PR head f5fbe82 (pre-merge)	460 tests / 460 pass / 0 fail	—
this branch 325e73a (post-merge of a0a5d73 + the 0.7.1 work)	462 / 462 / 0	421 passed / 0 failed

The 460 figure at f5fbe82 matches @ZacxDev's independent measurement exactly. No combination of node + pytest on any tree here reaches 561, so the number appears to have been asserted rather than counted. The "0 failures" half was correct.

Two counting gotchas worth recording, since they produce false numbers in this repo:

• node --test tests/ (directory form) reports # tests 1 / # fail 1 — a false red. The glob form is the correct invocation.
• a bare python3 -m pytest here exits 0 printing No module named pytest. Run it under nix-shell -p python312Packages.pytest (9.0.2 used above) and read the N passed line, not the exit code.

The 🔴 manifest finding is now addressed

manifest.json is bumped 0.7.0 → 0.7.1 in 325e73a, along with the two literal assertions in test_server.py that pin it (mutation-checked — reverting the manifest fails both with assert '0.7.0' == '0.7.1').

Since there is no new wire op, ping's ops list cannot discriminate this build, so the capability itself is documented as the discriminator in extension/README.md. Both halves are live-confirmed against the currently-deployed 0.7.0 build on the laptop host (nested-OOPIF rig, wake only, tab closed after):

--frame 4411 text              → "…GRANDCHILD OOPIF…\n\nleaf-marker"  (control: frame plumbing works)
--frame 4411 text --annotated  → annotated_with_frame_unsupported     (the "old build" reading)

and feeding the live /health versions through the server's own annotate_staleness() (read-only):

LIVE loaded: personal=0.7.0, work=0.7.0
expected 0.7.0 (deployed)    → extension_stale: false, false   ← the defeated tell
expected 0.7.1 (this branch) → extension_stale: true,  true    ← restored

🔴 What is still unverified: the feature actually working in Brave. That needs a home-manager switch + an extension reload, which I could not do (a full Brave restart would lose the operator's tabs — restore_on_startup is unset). Operator steps are in the body.

Also resolved here: the SKILL.md conflict with #278, using exactly the resolution posted above — this branch's text row + #278's nav/wake rows, one from each side. SKILL.md lands at 11,999 B against the 12,038 budget (39 B slack), and test_skill_size.py passes. Merged (not rebased) so the push is a fast-forward and the branch checked out in the shared base clone is not disturbed.