Resources: subscriptions/listen model on modern servers (era fork) (#1630)

[cliffhall]

Closes #1630

Stack: SDK V2 + New Spec — 7 of 10.

Forks resource subscriptions by protocol era. On modern (2026-07-28) servers resources/subscribe/unsubscribe are gone — subscriptions are a filter over one long-lived subscriptions/listen stream (SEP §7.4). Legacy stays byte-identical.

Core (InspectorClient)

• subscribeToResource / unsubscribeFromResource fork on the negotiated era:
  • Legacy: unchanged — one resources/subscribe per URI.
  • Modern: owns a single McpSubscription whose filter's resourceSubscriptions mirrors the subscribed-URI set, folding in the listChanged opt-ins (config ∩ capability) so one stream carries every opted-in type. Mutating the set re-lists (close old, open new); a no-op mutation (re-subscribe / unsubscribe of an unknown URI) skips the re-list; the last unsubscribe closes the stream.
• Reconnect-by-re-listen on an unexpected "remote" drop (the stream isn't resumable), with capped exponential backoff and a bound on consecutive rapid reconnects (past the cap it marks the stream ended rather than looping). A modernListenGeneration guard drops stale re-lists/reconnects; the stream + reconnect timer are torn down and reset on disconnect.
• getResourceSubscriptionStreamState() + a resourceSubscriptionStreamChange event expose the acknowledged / reconnecting / ended lifecycle.
• notifications/resources/updated is era-transparent (delivered through the same handler on both eras), so the notification-receive path and the resources.subscribe support gate are unchanged.

State + hook

ResourceSubscriptionsState tracks the stream state (seed + streamStateChange); useResourceSubscriptions returns { subscriptions, streamState }.

Web UI

The Subscriptions section gains modern-only chrome (legacy unchanged): a single labelled stream-status badge (Listening / Reconnecting… / Stream ended, tooltip-explained) in the accordion header, next to the count, so it stays visible while the section is collapsed. New SubscriptionStreamBadge element. subscriptionStreamState / protocolEra threaded App → InspectorView → ResourcesScreen → ResourceControls. Also: the per-frame "modern" badge is dropped from ProtocolEntry (the era is shown once in the panel header), and the sub … listen-id tag rides the compact entry's top line so long method badges don't truncate against the pin.

Tests

• Unit: store/hook stream state, the badge element, ResourceControls era gating (+ stories).
• Integration (real modern + legacy servers): subscriptions/listen (not resources/subscribe) on modern, legacy unchanged, re-list on partial unsubscribe, no-op skips, filter contents (URIs + listChanged), and the reconnect / backoff give-up / error-rollback branches.
• npm run ci green (validate → coverage ≥90 per-file → smoke → storybook).

Showcase

Manual test-server configs (subscriptions-{legacy,modern}-http.json) + README docs + pr-screenshots/ are included. Caveat: a tool-triggered resources/updated doesn't reach the modern subscriptions/listen stream on the SDK's stateless modern leg (the tool call and the listen stream are separate connections), so the live-update round-trip is demonstrated on the legacy server; the modern listen path (send + acknowledged + reconnect) is proven both in the integration suite and the screenshots.

Screenshots

Modern era — the stream-status badge in the Subscriptions header (Connecting… → Listening):

Modern era — Protocol view: subscriptions/listen (PENDING) + notifications/subscriptions/acknowledged, no per-entry era badge, sub … listen:0 on the top line:

Legacy era — resources/subscribe, no stream chrome:

Legacy era — live update: update_resource → notifications/resources/updated stamps the subscribed tile:

🤖 Generated with Claude Code

https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 3m 49s —— View job

---

Review — Resources: subscriptions/listen model on modern servers (#1630)

• Read the diff against origin/v2/main
• Review core InspectorClient era-fork logic
• Review state/hook/types changes
• Review web UI changes
• Review tests & config

Overall this is a clean, well-documented era fork. Legacy stays byte-identical, the modern path is isolated behind isModernEra(), the modernListenGeneration race guard is genuinely careful, the optimistic add/rollback keeps subscribedResources consistent with the wire filter, and the test suite (unit + hook + store + real-server integration + private-state internals for the unreachable branches) is thorough. Nice work.

A few things worth a look — mostly questions and robustness notes, nothing I'd call a hard blocker.

---

1. Reconnect-by-re-listen has no backoff (moderate)

core/mcp/inspectorClient.ts — onModernSubscriptionClosed → refreshModernSubscription

On a "remote" drop we immediately re-list, and the fresh stream registers a new closed.then(onModernSubscriptionClosed). If a server flaps (opens, then drops "remote" again right away), this becomes a tight reconnect loop with zero delay — each successful re-listen that immediately drops spawns another re-listen synchronously off the resolved closed promise. Consider a small delay / capped exponential backoff, or a bound on consecutive reconnects before falling back to ended. Fix this →

2. Does requestTimeout get applied to the long-lived listen stream? (question, potentially real)

refreshModernSubscription calls this.client.listen(filter, this.getRequestOptions()), and getRequestOptions() sets opts.timeout = this.requestTimeout whenever it's defined. For a normal request that's correct, but subscriptions/listen is a long-lived stream. Please confirm the SDK applies that timeout only to the acknowledgement (the listen() promise), not to the stream's lifetime. If the timeout ticks against the open stream, it would tear the stream down after requestTimeout and drive the reconnect path (see #1) on a perfectly healthy server. If that's the case, listen() should omit/override timeout.

3. Modern list-changed is coupled to having ≥1 resource subscription (question)

buildSubscriptionFilter() folds resourcesListChanged / toolsListChanged / promptsListChanged into the listen filter, but the stream is only opened while subscribedResources.size > 0 and is only rebuilt on subscribe/unsubscribe. Two consequences worth confirming are intended:

• On modern, list-changed notifications only flow while at least one resource is subscribed (no resource subscription → no listen stream → no list-changed delivery).
• Toggling a list-changed opt-in mid-session does not re-list, so the change won't reach the server until the next subscribe/unsubscribe.

If modern list-changed is handled by a separate mechanism/PR in the stack, ignore this; otherwise it looks like a gap.

4. Redundant re-list on no-op mutations (minor)

Re-subscribing an already-subscribed URI (covered by the "idempotent" test) and unsubscribing a URI that isn't in the set both still call refreshModernSubscription(), which tears down and reopens the server-side stream even though the filter is unchanged. An early return when the set didn't actually change would avoid needless stream churn.

5. active: true + status: "ended" after a failed reconnect (minor UX)

When the reconnect re-listen also fails, state becomes { active: true, status: "ended" }, so the badge shows a gray "Stream ended" while subscriptions remain — and the only way to recover is to unsubscribe/resubscribe. That may be the intended UX, but consider surfacing that recovery path (tooltip hint or a manual retry affordance).

6. PR description is stale vs. the branch (doc nit)

• The body says the showcase config + screenshots are a deferred follow-up, but test-servers/configs/subscriptions-{legacy,modern}-http.json, pr-screenshots/*, and the README section are all in this PR now.
• The body describes "a stream-status badge atop the panel and a bare status dot in the accordion header (tooltip-only)", but the implementation renders a single labelled SubscriptionStreamBadge in the accordion header. Worth aligning the description with what shipped.

Nits

• subscriptionStreamPresentation (subscriptionStreamUtils.ts) has an exhaustive switch with no default/assertNever. Fine today; an exhaustiveness guard would future-proof against a new ResourceSubscriptionStreamStatus silently returning undefined.

---

Tests, coverage, and the Mantine/theme conventions all look in good shape. Points 1–3 are the ones I'd most like a response on before merge; 4–6 are polish.
· v2/resource-subscriptions-listen-1630

[cliffhall]

Thanks for the thorough review! Responses below — fixes are in 349e5a1.

1. Reconnect backoff — fixed. onModernSubscriptionClosed now schedules the re-listen through a capped exponential backoff (500 ms base, ×2, capped at 15 s) with a 30 s reset window, and bounds consecutive rapid reconnects at 8 — past the cap it marks the stream ended rather than looping. The timer is cleared/reset on disconnect and on user-initiated subscribe/unsubscribe. Added fake-timer tests for the give-up, the reset window, and the timer-bail guard.

2. requestTimeout on the listen stream — confirmed not a bug (no change). The SDK's listen() applies options.timeout only to the acknowledgement phase: it arms ackTimer = setTimeout(…, ackTimeout) and settle() clears that timer the moment notifications/subscriptions/acknowledged arrives (client dist/index.mjs, the settle/ackTimer block). After ack there's no timeout on the stream's lifetime, so passing our requestTimeout correctly bounds only the ack wait. (The ~60 s churn I saw while capturing screenshots was the browser-proxied SSE dropping, not the SDK timeout — direct Node/CLI connections hold the stream open.)

3. Modern list-changed coupled to ≥1 resource subscription — intended for this card's scope. #1630 is specifically resource subscriptions, and the issue models the filter with the listChanged flags, so folding them in means the one open stream also carries list-changed (the spec's unified-stream model). You're right that on modern, list-changed only flows while a resource is subscribed, and a mid-session listChanged-opt-in toggle doesn't re-list. Full modern list-changed delivery independent of resource subscriptions is a broader modern-notifications concern that belongs in a separate card in the stack, not here — flagging it there.

4. Redundant re-list on no-op mutations — fixed. Re-subscribing an already-subscribed URI and unsubscribing an unknown URI now early-return (filter unchanged → no re-listen). Tests assert no subscriptions/listen is sent in those cases.

5. active: true + status: "ended" after a failed reconnect — kept, with recovery via resubscribe. The subscriptions genuinely remain while the stream is gone, and re-subscribing (a user action) resets the backoff and re-establishes the stream — that's the recovery path, and the Stream ended tooltip says it won't reconnect on its own. A manual "retry" affordance would be a nice follow-up but felt out of scope here.

6. Stale PR description — fixed. Updated the body: the showcase configs + screenshots are in this PR (not deferred), and the UI is described as the single labelled badge in the accordion header.

Nit (exhaustiveness) — fixed. subscriptionStreamPresentation is now a Record<ResourceSubscriptionStreamStatus, …> — a new status is a compile error, with no unreachable branch to annotate.

npm run ci green across all clients.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 1s —— View job

---

Re-review — subscriptions/listen model on modern servers (#1630)

Re-reviewed the current branch (through 349e5a14) with a focus on the fixes from my last pass plus a fresh look at the whole diff.

• Read the diff against origin/v2/main
• Re-verify the backoff/reconnect fix (point 1)
• Re-verify no-op skip (point 4) + exhaustiveness nit
• Fresh pass over core / state / hook / UI
• Post review

---

Prior points — all resolved ✅

• 1 — Reconnect backoff. onModernSubscriptionClosed now runs capped exponential backoff (base 500 ms ×2, cap 15 s), a 30 s reset window (now - modernLastReconnectMs > MODERN_RECONNECT_RESET_MS resets the run), and a hard cap at 8 rapid attempts before it marks the stream ended instead of looping. Timer is cleared on disconnect and on any user-initiated refresh (fromReconnect = false). The modernListenGeneration guard + the in-timer isTerminalStatus / size === 0 bail cover the races. Clean.
• 2 — requestTimeout on the stream. Confirmed reasoning accepted — the SDK's listen() timeout arms only the ack timer and settle() clears it on notifications/subscriptions/acknowledged, so no timeout ticks against the open stream. No change needed.
• 3 — list-changed coupled to ≥1 resource sub. Acknowledged as intentional for this card's scope. Fine.
• 4 — no-op re-lists. Both subscribeToResource (already-subscribed) and unsubscribeFromResource (unknown URI) now early-return before refreshModernSubscription, and integration tests assert no subscriptions/listen is sent. Good.
• 5 — active: true + ended. Kept as intended; recovery via re-subscribe resets backoff. Fine.
• 6 / nit — stale body + exhaustiveness. Body updated; subscriptionStreamPresentation is now a Record<ResourceSubscriptionStreamStatus, …> — a new status is a compile error. 👍

The optimistic add/rollback, the generation guard, the era-transparent resources/updated path, and the store→hook seeding all still read correctly, and the test suite (unit + hook + store + real-server integration + fake-timer backoff branches) is thorough.

---

One small thing I noticed this pass (minor, non-blocking)

Inconsistent badge visibility between "graceful close" and "gave up after reconnects." In onModernSubscriptionClosed:

• The !shouldReconnect branch (a "graceful" close) sets active: false, status: "ended".
• The give-up branch (too many rapid reconnects) sets active: this.subscribedResources.size > 0, status: "ended".

Because ResourceControls gates the badge on subscriptionStreamState?.active === true, a give-up leaves the gray "Stream ended" badge visible while subscriptions remain, but a graceful server-side close of the listen stream (while the connection itself stays up and URIs are still subscribed) sets active: false and hides the badge entirely — so the user sees subscriptions listed with no stream indicator and updates silently stopped. In practice "graceful" usually coincides with the connection tearing down (which clears subscribedResources anyway), so this is an edge case, but if you want the two "stream is gone but subs remain" outcomes to look the same, the graceful branch could use active: this.subscribedResources.size > 0 too. Purely cosmetic. Fix this →

---

Verdict

The fixes fully address the earlier feedback and I found nothing new that would block. The one note above is cosmetic. Nice, careful work on the race guards and the backoff bound.

CI (Run install, format, lint, build, and test) is still in progress at review time — worth a glance before merge, but you already reported npm run ci green locally.
· v2/resource-subscriptions-listen-1630

[cliffhall]

Thanks — addressed the badge-consistency note in e7ba4fc.

Graceful-close badge parity — fixed. onModernSubscriptionClosed's !shouldReconnect (graceful/local) branch now sets active: this.subscribedResources.size > 0, matching the reconnect give-up branch. So both "stream is gone but subscriptions remain" outcomes render the same gray Stream ended badge; on a terminal-status drop the disconnect reset still forces the inactive state. Added coverage for both graceful cases (subs remain → active, subs cleared → inactive).

While there, a bit of test hygiene: the internals tests that drive onModernSubscriptionClosed by hand now swap the real listen stream for a controllable fake (closing the real one first), so they no longer leave a live stream that rejects Connection closed on teardown.

npm run ci is green.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 13s —— View job

---

Re-review — subscriptions/listen model on modern servers (#1630)

Fresh pass over the current branch (through e7ba4fce), focused on the round-3 badge-parity fix plus a whole-diff sweep.

• Read the diff against origin/v2/main
• Re-verify prior fixes (backoff, no-op skip, graceful-close parity)
• Fresh pass over core / state / hook / UI
• Post review

---

Prior points — all resolved ✅

• Graceful-close parity (round 2 note). onModernSubscriptionClosed's !shouldReconnect branch now sets active: this.subscribedResources.size > 0 (inspectorClient.ts:3742), matching the give-up branch (:3764) and the reconnect-failure branch (:3792). Both "stream gone but subs remain" outcomes now render the same gray Stream ended badge; on a terminal-status drop, disconnect()'s reset (:1572–1579) still forces INACTIVE_SUBSCRIPTION_STREAM_STATE. Covered by the two new graceful-close cases in the integration suite (:476, :495). Clean.
• Backoff / give-up / reset window. Capped exponential (MODERN_RECONNECT_BASE_MS ×2, cap 15 s), a 30 s drop-to-drop reset, and a hard cap at 8 before it marks ended. Timer cleared on disconnect and on any user-initiated refresh (fromReconnect = false), with the modernListenGeneration guard + the in-timer isTerminalStatus / size === 0 bail covering the races.
• No-op re-lists. Both subscribeToResource (already-subscribed, :3821) and unsubscribeFromResource (unknown URI, :3860) early-return before refreshModernSubscription.
• Exhaustiveness. subscriptionStreamPresentation is a Record<ResourceSubscriptionStreamStatus, …> — a new status is a compile error.
• Test hygiene. The internals tests now swap the real listen stream for a controllable fake (closing the real one first, fakeSubscription helper at :254–:314), so they no longer leak a live stream that rejects on teardown. Good catch.

The optimistic add/rollback, the era-transparent resources/updated path (:1437), and the store→hook seeding all still read correctly.

---

One minor observation this pass (non-blocking)

The reconnect counter isn't reset on a successful acknowledged reconnect — only on the 30 s drop-to-drop window. refreshModernSubscription(true) (the reconnect path) never resets modernReconnectAttempts, and onModernSubscriptionClosed only resets it when now - modernLastReconnectMs > MODERN_RECONNECT_RESET_MS (inspectorClient.ts:3754). Because modernLastReconnectMs is stamped at each drop, the window measures drop→drop spacing, which includes the time the reconnected stream was healthy.

Concretely: a server that drops, reconnects, holds a healthy stream for ~20 s, then drops again keeps each gap under 30 s, so the run never resets and the counter marches to the cap — giving up after ~8 drops (~2–3 min) even though every reconnect succeeded and held. That's arguably not the "tight flapping loop" the cap is meant to stop.

If you want the cap to count only consecutive rapid failures, resetting modernReconnectAttempts = 0 right after a successful ack (in refreshModernSubscription, in the generation === modernListenGeneration success block at :3700) would let a stream that recovers and holds start fresh. Purely a tuning choice — the current behavior is safe and bounded, just slightly more eager to give up on an intermittently-healthy server. Fix this →

---

Verdict

All prior feedback is fully addressed and I found nothing blocking. The one note above is a tuning nuance, not a defect. The race guards, backoff bound, era-fork isolation (legacy stays byte-identical), and the unit + hook + store + real-server + fake-timer test coverage are all in good shape. Nice, careful work.
· v2/resource-subscriptions-listen-1630

[cliffhall]

Good call — implemented the reset-on-success semantics in 48c21de.

The backoff/give-up now counts only consecutive failed re-lists, not drops of streams that reconnect and hold:

• refreshModernSubscription resets modernReconnectAttempts = 0 on a successful acknowledgement.
• A "remote" drop schedules a re-listen at the current backoff (base delay after a healthy stream). A re-listen that fails increments the count and retries with a longer backoff (scheduleModernReconnect → onModernReconnectFailed); past the cap it gives up (ended). So a server that reconnects-and-holds but flaps every ~20s now reconnects indefinitely (paced by the base delay, no tight loop), while a persistently-failing re-list stays bounded.
• Dropped the now-unused drop-to-drop reset window (modernLastReconnectMs / MODERN_RECONNECT_RESET_MS).

Tests rewritten for the new semantics: retry-then-give-up on repeated re-list failures, and reset-of-the-count after a successful ack. npm run ci green.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 2m 6s —— View job

---

Re-review — subscriptions/listen model on modern servers (#1630)

Fresh pass over the current branch (through 48c21de2), focused on the round-3 fix (reset backoff on a successful ack) plus a whole-diff sweep.

• Read the diff against origin/v2/main
• Re-verify round-3 fix (reset backoff on successful ack)
• Fresh pass over core / state / hook / UI
• Post review

---

Round-3 point — resolved ✅

Reset reconnect counter on a successful ack. refreshModernSubscription now sets modernReconnectAttempts = 0 in the generation-match success block (inspectorClient.ts:3702), so the backoff run counts only consecutive failed re-lists. The old drop-to-drop 30 s window (modernLastReconnectMs) is gone entirely — simpler, and it no longer marches a repeatedly-recovering stream to the give-up cap. onModernReconnectFailed (:3793) advances the counter and gives up past MODERN_RECONNECT_MAX_ATTEMPTS, and the give-up + graceful branches both keep the badge via active: subscribedResources.size > 0. Clean.

All earlier points (backoff/give-up, no-op skips, graceful-close parity, exhaustive presentation Record, era-transparent resources/updated, disconnect teardown at :1571, store→hook seeding) still read correctly.

---

One tradeoff the round-3 change introduces (minor, non-blocking)

An "ack-then-immediate-drop" flap now loops indefinitely and never hits the give-up cap. The counter resetting on every successful ack means the cap bounds only runs where the listen() call itself rejects. Consider a server that accepts listen(), sends notifications/subscriptions/acknowledged, then drops the stream "remote" right away, repeatedly:

1. refreshModernSubscription(true) → ack arrives → modernReconnectAttempts = 0 (:3702).
2. Stream drops "remote" → onModernSubscriptionClosed → shouldReconnect → scheduleModernReconnect() at delay = 500 × 2^0 = 500 ms (:3770).
3. Timer fires → back to step 1.

onModernReconnectFailed is never reached (the re-listen succeeded each time), so modernReconnectAttempts never climbs and the MODERN_RECONNECT_MAX_ATTEMPTS bound never trips — the client reconnects forever at the 500 ms floor.

This is the flip side of the round-1/round-3 fix: it's now safe (a 500 ms-throttled loop, not the original zero-delay tight loop or a CPU burn), and it's arguably the right call for a genuinely-recovering stream — but the "bound on consecutive rapid reconnects" only actually bounds failed re-lists, not a successful-ack-then-drop flap. If you want the cap to also catch that pattern, you'd need to count time-in-healthy-stream (e.g. only reset the counter if the stream held for ≥ some threshold after ack, rather than resetting immediately on ack). Purely a tuning nuance — the current behavior is bounded-rate and correct for the common case; flagging so it's a conscious choice rather than an oversight. Consider →

Nit (cosmetic)

The ended badge tooltip (subscriptionStreamUtils.ts:35) reads "The server ended the stream deliberately (for example, on shutdown)…". ended is also the give-up state after MODERN_RECONNECT_MAX_ATTEMPTS failed reconnects, where the client stopped trying rather than the server closing gracefully — so the copy is slightly off for that path. The "will not reconnect on its own" half is accurate for both; only the attribution differs. Could soften to "The stream is closed and won't reconnect on its own (the server ended it, or reconnection was abandoned) — re-subscribe to try again."

---

Verdict

The round-3 fix is correctly implemented and all prior feedback is fully addressed. Nothing blocking — the one note above is a deliberate tuning tradeoff (bounded-rate, not a defect), and the nit is copy. Era-fork isolation (legacy byte-identical), the race guards, the backoff bound, and the unit + hook + store + real-server + fake-timer coverage remain in good shape. Nice work.

CI (Run install, format, lint, build, and test) is still in progress at review time — worth a glance before merge, but you reported npm run ci green locally.
· v2/resource-subscriptions-listen-1630

[cliffhall]

Thanks!

Reconnect tradeoff — deliberate, keeping as-is. Confirming this is a conscious choice, not an oversight. After going back and forth between drop-based and failure-based give-up, I'm settling on: reset the backoff on a successful ack and bound only consecutive failed re-lists. The flip side you noted — an ack-then-immediate-drop server reconnecting forever — is throttled at the 500 ms floor (no tight loop, no CPU burn), and I think perpetually retrying a server that is acknowledging is the right behavior: it's genuinely reachable, just unstable, and the alternative (giving up on a server that keeps saying "yes") is worse. Counting time-in-healthy-stream to also cap that pattern is a reasonable enhancement, but it adds state for a pathological edge, so I'm leaving it out for now.

Tooltip nit — fixed (a57cbeb). The ended badge tooltip no longer attributes the close solely to the server, since ended is also the client's give-up state: "…the server ended it (for example, on shutdown) or reconnection was abandoned after repeated failures. Re-subscribe to try again."

npm run ci green.

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 1m 13s —— View job

---

Re-review — subscriptions/listen model on modern servers (#1630)

Fresh pass over the current branch (through a57cbebe), focused on the round-4 fix (tooltip copy) plus a whole-diff sweep.

• Read the diff against origin/v2/main
• Re-verify round-4 fix (tooltip copy)
• Fresh pass over core / state / hook / UI
• Post review

---

Round-4 point — resolved ✅

Tooltip attribution. The ended badge copy (subscriptionStreamUtils.ts:35) no longer pins the close solely on the server: "…won't reconnect on its own — either the server ended it (for example, on shutdown) or reconnection was abandoned after repeated failures. Re-subscribe to try again." Accurate for both the graceful-close and give-up paths that share the ended status. Clean.

All earlier points remain resolved and still read correctly:

• Backoff / give-up semantics. refreshModernSubscription resets modernReconnectAttempts = 0 on a successful ack (inspectorClient.ts:3702); scheduleModernReconnect uses capped exponential backoff (MODERN_RECONNECT_BASE_MS ×2, cap MODERN_RECONNECT_MAX_MS, :3770); onModernReconnectFailed advances the counter and gives up past MODERN_RECONNECT_MAX_ATTEMPTS (:3793). The modernListenGeneration guard (:3693, :3727) drops superseded streams, and the in-timer isTerminalStatus / size === 0 bail (:3779) covers the disconnect/unsubscribe race. The ack-then-immediate-drop "reconnect forever at the 500 ms floor" tradeoff was flagged in round 3 and confirmed as a deliberate choice — no change expected.
• Graceful-close badge parity. Both the graceful (:3744) and give-up (:3800) branches use active: this.subscribedResources.size > 0, so "stream gone but subs remain" renders identically; disconnect()'s reset still forces the inactive state on a terminal-status drop.
• No-op re-lists skipped. subscribeToResource early-returns when already subscribed (:3831); unsubscribeFromResource early-returns on !subscribedResources.delete(uri) (:3870).
• Optimistic add/rollback + intentional unsubscribe non-rollback. Subscribe rolls the URI back out of the set if the re-listen throws (:3838); unsubscribe keeps the removal even if the re-listen fails (documented as the user's intent, :3871). Consistent and well-commented.
• Era-transparent resources/updated, exhaustive presentation Record, store→hook seeding, disconnect teardown all intact.

The buildSubscriptionFilter (:3618) folding of list-changed opt-ins (config ∩ capability) into the single stream matches the SEP §7.4 unified-stream model, and the state store's belt-and-braces subscribedUris.includes re-check (resourceSubscriptionsState.ts:102) and terminal-status reset (:107) are correctly conservative.

---

One cosmetic edge I noticed this pass (non-blocking)

Stream badge can render inside a disabled, empty-looking Subscriptions control when a search filter hides all subs. In ResourceControls.tsx, the subscriptions Accordion.Control is disabled={filteredSubscriptions.length === 0} (:300), but streamStatus is derived from subscriptionStreamState?.active (:131), which reflects the actual subscription set, not the filtered one. So if you have live modern subscriptions and type a search query that matches none of them, the section header shows Subscriptions (0) (disabled) with a Listening badge still beside it. Harmless and arguably even informative (the stream is still live), but it's a slight mismatch between the count and the badge. If you'd rather the badge track what's visible, gating it on filteredSubscriptions.length > 0 && streamStatus would hide it alongside the emptied section. Purely cosmetic. Consider →

---

Verdict

The round-4 tooltip fix is correct and all prior feedback across the four rounds is fully addressed. I found nothing blocking — the one note above is a cosmetic search-interaction edge, not a defect. The era-fork isolation (legacy stays byte-identical), the generation/backoff race guards, and the unit + hook + store + real-server + fake-timer coverage are all in good shape. This looks ready to merge. Nice, careful iteration.
· v2/resource-subscriptions-listen-1630

[cliffhall]

Thanks — nice catch, fixed in 630a9a4.

Search vs. badge mismatch — fixed. streamStatus is now also gated on filteredSubscriptions.length > 0, so a search query that matches none of the live subscriptions hides the Listening badge along with the emptied (disabled) Subscriptions (0) section, instead of leaving the badge orphaned next to a zero count. Added a test for it.

npm run ci green. Appreciate the careful multi-round pass!

[cliffhall]

@claude review

[claude]

Claude finished @cliffhall's task in 59s —— View job

---

Re-review — subscriptions/listen model on modern servers (#1630)

Fresh pass over the current branch (through 630a9a4c), focused on the round-5 fix (badge hidden when a search empties the section) plus a whole-diff sweep.

• Read the diff against origin/v2/main
• Re-verify round-5 fix (badge hidden when search filters out all subs)
• Fresh pass over core / state / hook / UI
• Post review

---

Round-5 point — resolved ✅

Search vs. badge mismatch. streamStatus in ResourceControls.tsx (:133) is now gated on filteredSubscriptions.length > 0 in addition to isModernEra(protocolEra) and subscriptionStreamState?.active === true, so a query that matches none of the live subscriptions hides the Listening badge alongside the emptied, disabled Subscriptions (0) header instead of orphaning it. The comment (:126–132) documents the reasoning, and there's a dedicated test asserting the badge disappears when the search empties the section (ResourceControls.test.tsx, "hides the stream badge when a search filters out all subscriptions"). Clean.

All earlier points across rounds 1–4 remain resolved and still read correctly:

• Backoff / give-up semantics. refreshModernSubscription resets modernReconnectAttempts = 0 on a successful ack (inspectorClient.ts:3702); scheduleModernReconnect uses capped exponential backoff (MODERN_RECONNECT_BASE_MS ×2, cap MODERN_RECONNECT_MAX_MS, :3770); onModernReconnectFailed advances the counter and gives up past MODERN_RECONNECT_MAX_ATTEMPTS (:3793). The modernListenGeneration guard (:3693, :3727) drops superseded streams, and the in-timer isTerminalStatus / size === 0 bail (:3779) covers the disconnect/unsubscribe race. The ack-then-immediate-drop "reconnect forever at the 500 ms floor" tradeoff was confirmed as a deliberate choice.
• Graceful-close badge parity. Both the graceful (:3744) and give-up (:3801) branches use active: this.subscribedResources.size > 0; disconnect()'s reset still forces the inactive state on a terminal-status drop, and the state store's onStatusChange (resourceSubscriptionsState.ts:107) clears the set + seeds INACTIVE_SUBSCRIPTION_STREAM_STATE on a terminal status.
• No-op re-lists skipped. subscribeToResource early-returns when already subscribed (:3831); unsubscribeFromResource early-returns on !subscribedResources.delete(uri) (:3870).
• Optimistic add/rollback + intentional unsubscribe non-rollback. Subscribe rolls the URI back out if the re-listen throws (:3838); unsubscribe keeps the removal even if the re-listen fails (documented as user intent, :3871).
• Era-transparent resources/updated, exhaustive presentation Record, tooltip copy covering both ended paths, store→hook seeding, buildSubscriptionFilter folding list-changed opt-ins (config ∩ capability), belt-and-braces subscribedUris.includes re-check all intact.

---

Verdict

The round-5 fix is correctly implemented and tested, and all feedback across the five rounds is fully addressed. I found nothing blocking and nothing new worth flagging this pass. The era-fork isolation (legacy stays byte-identical), the generation/backoff race guards, and the unit + hook + store + real-server + fake-timer coverage are all in good shape. This is ready to merge — nice, careful iteration across the review rounds.
· v2/resource-subscriptions-listen-1630

[cliffhall]

Thanks for the thorough multi-round review — no further changes on my end. Ready for merge whenever.