Skip to content

fix(server): type no-tab rejections so they stop reading as version mismatches - #205

Merged
chrischall merged 1 commit into
mainfrom
fix/no-tab-error-hint
Aug 5, 2026
Merged

fix(server): type no-tab rejections so they stop reading as version mismatches#205
chrischall merged 1 commit into
mainfrom
fix/no-tab-error-hint

Conversation

@chrischall

Copy link
Copy Markdown
Owner

Closes #204.

Correction to the issue

The issue blamed error-kind.ts:88 and a missing key in the CLI's hint table. That was the wrong classifier. classifyFetchError (error-kind.ts) is what MCP consumers use on fetch results; the CLI uses classifyBridgeError, which dispatches on error type:

if (err instanceof FetchproxyProtocolError) return 'protocol';

So a no-tab rejection was never going to be anything but protocol, and adding a no_tab key to the hint table would have done nothing. The symptom and the remedy were right; the mechanism was not.

The fix

FetchproxyNoTabError, built by protocolErrorFrom like every other typed rejection, carrying the remedy on .hint. Same shape as FetchproxyScopeError, which exists for exactly this reason.

Before

bridge error (protocol): no tab matching https://api.creditkarma.com/
  — extension/server version mismatch — update both.

After (verified live against the original reproduction)

bridge error (protocol): no tab matching https://api.creditkarma.com/
  — open a tab on that host and sign in, then re-run. This is not a
    version problem and does not need an update.

Two decisions worth reviewing

FetchproxyHintedError as a shared base. The CLI now branches on that rather than FetchproxyScopeError. Keying on the concrete subclass is how this bug happened — a hinted error that wasn't a scope error fell through to the blanket advice. With the base, the next hinted error renders correctly with no new branch. FetchproxyScopeError keeps its own identity and ancestry, so existing instanceof catches are unaffected either way.

The content-script variant is deliberately excluded. no tab matching <url> has the fetchproxy content script loaded (N URL matches, none responded) means a tab did match — the remedy is to refresh the page, not open one, and the extension's own message already says so. The regex uses a negative lookahead rather than a bare prefix match, and there's a test pinning that.

Tests

1142 pass across the monorepo (up from 1140). New packages/server/tests/no-tab-error.test.ts covers the typing, the hint content, the raw-error preservation, back-compat with FetchproxyProtocolError/classifyBridgeError, the excluded content-script wording, and non-collision with scope errors — plus a shared-contract block asserting both hinted subclasses render identically. Two CLI tests cover the rendered output.

Note

Anyone on the published CLI (1.4.0) still sees the old hint until the CLI ships too — the branch on FetchproxyHintedError lives in packages/cli. That's how I first "verified" this and got the old string back: the linked local server was producing the new error, but the npm CLI didn't recognise it.

🤖 Generated with Claude Code

https://claude.ai/code/session_012o2nXwu7tov6j7ciBEpigo

…ismatches

`classifyBridgeError` dispatches on error type, not message, so "no tab
matching <url>" arrived as a plain FetchproxyProtocolError and inherited
whatever blanket advice a consumer gives `protocol` errors. In the CLI
that advice is "extension/server version mismatch — update both", so a
user on a current CLI and a current extension was sent to update both:

  bridge error (protocol): no tab matching https://api.creditkarma.com/
    — extension/server version mismatch — update both.

Nothing was mismatched. Nothing was open on that host.

This is the same misdirection FetchproxyScopeError was introduced to
stop, so it gets the same treatment: a FetchproxyNoTabError carrying the
remedy on `.hint`, built by `protocolErrorFrom` like every other typed
rejection.

Two things beyond the obvious fix:

`FetchproxyHintedError` now holds the shared originalError/hint shape,
and the CLI branches on that base rather than on FetchproxyScopeError.
Keying on the concrete subclass is precisely how this bug happened — the
next hinted error would have inherited the wrong blanket advice too.
Existing `instanceof FetchproxyScopeError` and
`instanceof FetchproxyProtocolError` catches are unaffected.

The "matched a tab but its content script never answered" wording is
deliberately excluded. Its remedy is to refresh the page, not to open
one, and the extension's message already says so — retyping it would
staple contradictory advice onto it.

Verified live against the original reproduction:
  no tab matching https://api.creditkarma.com/ — open a tab on that host
  and sign in, then re-run. This is not a version problem…

Closes #204

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012o2nXwu7tov6j7ciBEpigo
@github-actions github-actions Bot added the auto-review Trigger Claude + Copilot review on this PR label Aug 5, 2026
@claude

claude Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @chrischall's task in 2m 8s —— View job


Reviewing PR #205

  • Read CLAUDE.md for conventions
  • Read PR diff (git diff origin/main...HEAD)
  • Review packages/server/src/index.ts and ws-server.ts changes
  • Review packages/cli/src/bridge-errors.ts changes
  • Review new/updated tests
  • Post findings and verdict

Review

The fix introduces FetchproxyHintedError as a shared base for FetchproxyScopeError and the new FetchproxyNoTabError, and switches the CLI's branch (packages/cli/src/bridge-errors.ts:46) from the concrete FetchproxyScopeError to the shared base — exactly the class of bug the PR describes (a hinted error falling through to blanket "protocol" advice because a consumer keyed on one concrete subclass).

Verified in detail:

  • Regex correctness (packages/server/src/ws-server.ts:598): NO_TAB_REJECTION = /no tab matching (?!.*content script loaded)/. Checked against the actual extension wording in packages/extension-core/src/background.ts:1569-1575 — the plain rejection (no tab matching <url>) has no content script loaded substring so the negative lookahead succeeds and it matches; the content-script-unreachable variant (no tab matching <url> has the fetchproxy content script loaded (...)) does contain that substring later in the string, so the lookahead fails and the message correctly falls through to plain FetchproxyProtocolError. This matches the PR's stated intent and the new test at packages/server/tests/no-tab-error.test.ts:51-62.
  • Backward compatibility: FetchproxyScopeError extends FetchproxyHintedError extends FetchproxyProtocolError, so classifyBridgeError (packages/server/src/classify-bridge-error.ts:42-47, unchanged) still buckets both as 'protocol', and any existing instanceof FetchproxyProtocolError/FetchproxyScopeError catch sites remain correct — confirmed by the shared-contract test block (no-tab-error.test.ts:83-99) and scope-error.test.ts still passing conceptually (class identity unchanged, just re-parented).
  • No other in-repo consumers key on FetchproxyScopeError directlygrep'd the whole repo; only packages/cli/src/bridge-errors.ts and the server's own tests reference it, so there's no second call site silently left un-migrated.
  • packages/cli/src/verbs/health.ts:69's pre-existing /^no tab matching /.test(err.message) regex still matches, since FetchproxyNoTabError.message is ${originalError} — ${hint} and still starts with no tab matching — no regression there.
  • Version-annotation comment style (1.12.0+) matches the existing convention used throughout ws-server.ts for prior additions (0.8.0+, 1.10.0+, 1.4.0+, etc.).
  • Note: I was unable to execute the test suite in this sandbox (npm test / npx vitest required approval that wasn't available), so I did not independently verify the 1142-pass count claimed in the PR description — this is an open question, not a finding, per review scope rules. The test file itself (no-tab-error.test.ts) reads as a solid, well-targeted set of assertions covering the typing, hint content, raw-error preservation, backward-compat classification, and the excluded content-script wording.

No correctness, convention, or test-coverage issues found.

No issues found.

Verdict: pass

Branch: fix/no-tab-error-hint

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

✅ Auto-review verdict: pass — The fix correctly types no-tab rejections via a new FetchproxyNoTabError sharing a FetchproxyHintedError base with FetchproxyScopeError; regex, class hierarchy, and CLI branch were all verified against source and no regressions or convention violations were found.

@chrischall chrischall added the ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green label Aug 5, 2026
@chrischall
chrischall enabled auto-merge (squash) August 5, 2026 22:03
@chrischall
chrischall merged commit dc30bd9 into main Aug 5, 2026
11 checks passed
@chrischall
chrischall deleted the fix/no-tab-error-hint branch August 5, 2026 22:03
chrischall added a commit that referenced this pull request Aug 6, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.0.0](v1.11.0...v2.0.0)
(2026-08-06)


### ⚠ BREAKING CHANGES

* **protocol:** bind the ephemeral key into the ready signature
([#222](#222))

### Features

* **protocol:** add write_cookies, the one verb that can repair a
rotated session
([#211](#211))
([b2557c2](b2557c2))
* **protocol:** bind the ephemeral key into the ready signature
([#222](#222))
([c13aeed](c13aeed))
* **server:** let a request name the tab that relays it
([#207](#207))
([c5d3f4d](c5d3f4d))
* **server:** pin the extension's identity, and verify it on the peer
path ([#213](#213))
([0eeced7](0eeced7))


### Bug Fixes

* **cli:** let a real filesystem error be itself, not "no extension pin"
([#221](#221))
([c87a864](c87a864)),
closes [#220](#220)
* **cli:** validate --via-tab before connecting, like the request URL
([#210](#210))
([959fcc5](959fcc5))
* **extension:** reattach the write_cookies doc block, and name the
writable cookies as writable
([#215](#215))
([2730c4a](2730c4a))
* **extension:** use the guarded caps local for the cookie heading
([#217](#217))
([f95c832](f95c832))
* **server:** release only our own extension claim, and stop guessing
scoped names
([#219](#219))
([3d90a64](3d90a64)),
closes [#218](#218)
* **server:** type no-tab rejections so they stop reading as version
mismatches ([#205](#205))
([dc30bd9](dc30bd9))


### Refactor

* **server:** drop the concatBytes imports the signature change orphaned
([#224](#224))
([4985ba7](4985ba7)),
closes [#223](#223)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review Trigger Claude + Copilot review on this PR ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"no tab matching" is reported to CLI users as an extension/server version mismatch

1 participant