Fix #1472: bound the mailbox escalation-toast dedupe set

apps/vscode/src/__tests__/mailbox-escalation-toast.test.ts

@@ -18,7 +18,7 @@ vi.mock('vscode', () => ({
  },
 }));
 
-const { activateMailboxEscalationToasts } = await import('../notifications/mailbox-escalation-toast.js');
+const { activateMailboxEscalationToasts, MAX_SEEN } = await import('../notifications/mailbox-escalation-toast.js');
 
 type SSEHandler = (e: { type: string; data: string }) => void;
 
@@ -51,10 +51,35 @@ function escalationEvent(overrides: Record<string, unknown> = {}): string {
  return JSON.stringify({ type: 'mailbox-escalation', body: JSON.stringify(payload) });
 }
 
-function activate(cm: ReturnType<typeof makeConnectionManager>) {
+/**
+ * Minimal OverviewCache double: the workspace-level `mailboxEscalated` flag plus a
+ * manual change hook, so a test can drive the de-escalation prune (#1472) the same
+ * way Tower's overview refresh would.
+ */
+function makeOverviewCache(initial: { mailboxEscalated: boolean } | null = null) {
+ let data = initial;
+ const listeners: (() => void)[] = [];
+ return {
+ getData: () => data,
+ onDidChange: (fn: () => void) => {
+ listeners.push(fn);
+ return { dispose() {} };
+ },
+ /** Simulate an overview refresh committing a new escalated state. */
+ set: (next: { mailboxEscalated: boolean } | null) => {
+ data = next;
+ for (const fn of listeners) { fn(); }
+ },
+ };
+}
+
+function activate(
+ cm: ReturnType<typeof makeConnectionManager>,
+ cache: ReturnType<typeof makeOverviewCache> = makeOverviewCache(),
+) {
  const ctx = makeCtx();
- // Structural fakes stand in for vscode.ExtensionContext / ConnectionManager.
- activateMailboxEscalationToasts(ctx as any, cm as any);
+ // Structural fakes stand in for vscode.ExtensionContext / ConnectionManager / OverviewCache.
+ activateMailboxEscalationToasts(ctx as any, cm as any, cache as any);
  return ctx;
 }
 
@@ -129,3 +154,71 @@ describe('activateMailboxEscalationToasts', () => {
  expect(h.showWarningMessage).not.toHaveBeenCalled();
  });
 });
+
+/**
+ * Issue #1472: the dedupe set must be bounded. Its eviction key is the mailbox row
+ * leaving the escalated set — observed here as the workspace-level
+ * `mailboxEscalated` flag going false — with a {@link MAX_SEEN} cap as backstop.
+ * Each test observes eviction the only way it is observable from outside: an id
+ * that was deduped before is toasted again after it has been evicted.
+ */
+describe('activateMailboxEscalationToasts — bounded dedupe set (#1472)', () => {
+ it('drops seen ids once no held row in the workspace is escalated', () => {
+ const cm = makeConnectionManager('/ws');
+ const cache = makeOverviewCache({ mailboxEscalated: true });
+ activate(cm, cache);
+
+ cm.fire(escalationEvent({ mailboxId: 'a' }));
+ expect(h.showWarningMessage).toHaveBeenCalledTimes(1);
+
+ // The row resolved: nothing is escalated any more, so 'a' has left the set.
+ cache.set({ mailboxEscalated: false });
+
+ cm.fire(escalationEvent({ mailboxId: 'a' }));
+ expect(h.showWarningMessage).toHaveBeenCalledTimes(2);
+ });
+
+ it('keeps seen ids while the workspace is still escalated', () => {
+ const cm = makeConnectionManager('/ws');
+ const cache = makeOverviewCache({ mailboxEscalated: true });
+ activate(cm, cache);
+
+ cm.fire(escalationEvent({ mailboxId: 'a' }));
+ cache.set({ mailboxEscalated: true });
+ cm.fire(escalationEvent({ mailboxId: 'a' }));
+
+ expect(h.showWarningMessage).toHaveBeenCalledTimes(1);
+ });
+
+ it('does not prune on an empty cache — no data says nothing about the escalated set', () => {
+ const cm = makeConnectionManager('/ws');
+ const cache = makeOverviewCache(null);
+ activate(cm, cache);
+
+ cm.fire(escalationEvent({ mailboxId: 'a' }));
+ cache.set(null);
+ cm.fire(escalationEvent({ mailboxId: 'a' }));
+
+ expect(h.showWarningMessage).toHaveBeenCalledTimes(1);
+ });
+
+ it('caps the set when the window never de-escalates, evicting oldest-first', () => {
+ const cm = makeConnectionManager('/ws');
+ // Permanently escalated: the prune above never runs, so only the cap bounds it.
+ const cache = makeOverviewCache({ mailboxEscalated: true });
+ activate(cm, cache);
+
+ for (let i = 0; i < MAX_SEEN; i++) {
+ cm.fire(escalationEvent({ mailboxId: `id-${i}` }));
+ }
+ expect(h.showWarningMessage).toHaveBeenCalledTimes(MAX_SEEN);
+
+ // One past the cap evicts the oldest id, which can therefore toast again;
+ // the newest id is still deduped.
+ cm.fire(escalationEvent({ mailboxId: 'overflow' }));
+ cm.fire(escalationEvent({ mailboxId: 'id-0' }));
+ cm.fire(escalationEvent({ mailboxId: 'overflow' }));
+
+ expect(h.showWarningMessage).toHaveBeenCalledTimes(MAX_SEEN + 2);
+ });
+});

apps/vscode/src/extension.ts

@@ -1519,7 +1519,7 @@ export async function activate(context: vscode.ExtensionContext) {
 	// Spec 1313 Phase 8: toast when a held message crosses the escalation age
 	// (the `mailbox-escalation` SSE event). Visibility only — read/dismiss via
 	// `afx inbox`. Respects `codev.mailboxEscalationToasts.enabled`.
-	activateMailboxEscalationToasts(context, connectionManager);
+	activateMailboxEscalationToasts(context, connectionManager, overviewCache);
 
 	// Auto-open builder terminals on Tower spawn events
 	const builderSpawnHandler = new BuilderSpawnHandler(connectionManager, terminalManager, outputChannel);

apps/vscode/src/notifications/mailbox-escalation-toast.ts

@@ -3,6 +3,16 @@ import type { MailboxEscalationPayload } from '@cluesmith/codev-types';
 import { parseSseEnvelope, parseSseBody } from '../sse-envelope.js';
 import { escalationToastText, escalationMatchesWorkspace } from '../mailbox-indicators.js';
 import type { ConnectionManager } from '../connection-manager.js';
+import type { OverviewCache } from '../views/overview-data.js';
+
+/**
+ * Backstop bound on the dedupe set for a window that stays escalated all day, so
+ * the de-escalation prune below never gets to run. Oldest-first eviction (a `Set`
+ * iterates in insertion order); an evicted id can only re-toast if Tower re-emits
+ * an escalation for that same row, which the one-way server-side `escalated` flag
+ * makes impossible. Sized far above any plausible live escalated set.
+ */
+export const MAX_SEEN = 500;
 
 /**
  * Spec 1313 Phase 8: toast on `mailbox-escalation`.
@@ -22,13 +32,44 @@ import type { ConnectionManager } from '../connection-manager.js';
  * - gated by `codev.mailboxEscalationToasts.enabled` (default true) — the same
  * mute affordance `codev.gateToasts.enabled` gives the gate toasts. The
  * persistent status-bar count/attention state is unaffected by the mute.
+ *
+ * Issue #1472: the dedupe set is BOUNDED. Its eviction key is the mailbox row
+ * leaving the escalated set — the same signal that would let a legitimate
+ * re-escalation re-notify — mirroring how `activateGateToasts` prunes a builder
+ * that leaves the blocked set. The overview carries no per-row mailbox ids, only
+ * the workspace-level `mailboxEscalated` flag, so `false` (no held row in this
+ * workspace is escalated) is the finest-grained signal available here: at that
+ * point every id in the set has left the escalated set, and the set is dropped
+ * whole. A {@link MAX_SEEN} cap backstops a window that never sees that `false`.
+ * (Tower also reports `false` when it cannot read the mailbox at all, which prunes
+ * a little early — harmless: a row escalates exactly once server-side and there is
+ * no SSE replay, so an evicted id has no second event to be deduped against.)
+ *
+ * The prune cannot fire on a stale snapshot: `OverviewCache.refresh()` is
+ * last-write-wins by sequence, and the escalation event itself triggers a refresh
+ * whose request starts after Tower flagged the row, so an older in-flight
+ * `mailboxEscalated: false` response can never commit after it.
  */
 export function activateMailboxEscalationToasts(
  context: vscode.ExtensionContext,
  connectionManager: ConnectionManager,
+ cache: OverviewCache,
 ): void {
  const seen = new Set<string>();
 
+ context.subscriptions.push(
+ cache.onDidChange(() => {
+ const data = cache.getData();
+ // No data yet (or a transient read) says nothing about the escalated set.
+ if (!data) {
+ return;
+ }
+ if (!data.mailboxEscalated) {
+ seen.clear();
+ }
+ }),
+ );
+
  context.subscriptions.push(
  connectionManager.onSSEEvent(({ data }) => {
  const enabled = vscode.workspace
@@ -56,6 +97,10 @@ export function activateMailboxEscalationToasts(
  return;
  }
  seen.add(payload.mailboxId);
+ while (seen.size > MAX_SEEN) {
+ const oldest = seen.values().next().value as string;
+ seen.delete(oldest);
+ }
 
  void vscode.window.showWarningMessage(escalationToastText(payload));
  }),

codev/projects/bugfix-1472-vscode-bound-the-mailbox-escal/status.yaml

@@ -0,0 +1,22 @@
+id: bugfix-1472
+title: vscode-bound-the-mailbox-escal
+protocol: bugfix
+phase: verified
+plan_phases: []
+current_plan_phase: null
+gates:
+ pr:
+ status: approved
+ requested_at: '2026-08-17T23:14:58.384Z'
+ approved_at: '2026-08-17T23:24:05.907Z'
+iteration: 1
+build_complete: false
+history: []
+started_at: '2026-08-17T23:03:26.300Z'
+updated_at: '2026-08-17T23:24:09.666Z'
+pr_history:
+ - phase: pr
+ pr_number: 1484
+ branch: builder/bugfix-1472
+ created_at: '2026-08-17T23:14:54.493Z'
+pr_ready_for_human: false

codev/state/bugfix-1472_thread.md

@@ -0,0 +1,116 @@
+# bugfix-1472 — VSCode mailbox escalation-toast `seen` Set is unbounded
+
+Issue #1472 · protocol BUGFIX (strict) · branch `builder/bugfix-1472`
+
+## Architect constraints (2026-08-17)
+
+- We are **not** cluesmith/codev maintainers: open the PR, address review feedback,
+ then **park it for the maintainer** — do not merge it myself. Report protocol-complete.
+- Design note from tracking issue #1483: prefer eviction keyed on **the mailbox row leaving
+ the escalated set** (the same signal that would let a legitimate re-escalation re-notify)
+ over a bare LRU cap, *if that signal is available in the toast module*.
+
+## Investigate
+
+**Root cause** — `apps/vscode/src/notifications/mailbox-escalation-toast.ts:30`:
+`const seen = new Set<string>()` lives for the whole extension-host activation. Line 58
+`seen.add(payload.mailboxId)` is the only mutation in the file — grep confirms **zero**
+`seen.delete` / `seen.clear` anywhere. So every escalated mailbox row the window ever
+observes leaves a permanent entry. Add-only container, activation-scoped lifetime =
+unbounded growth. Reproduction is static and conclusive: there is no code path that can
+ever remove an entry.
+
+**Is the de-escalation signal available?** Yes, at workspace granularity:
+
+- `OverviewData.mailboxEscalated: boolean` (packages/types/src/api.ts) — "at least one held
+ row in this workspace has crossed the escalation age". `OverviewData` carries **no
+ per-row mailbox ids** (only `heldCount` + this flag), so `false` is the finest-grained
+ "left the escalated set" signal a client can see. When it is false, *every* id in `seen`
+ has left the escalated set → the whole set can be dropped.
+- The signal reaches a notification module the same way `activateGateToasts` gets it:
+ `OverviewCache.onDidChange` + `getData()` (`apps/vscode/src/views/overview-data.ts`).
+ `overviewCache` is already in scope at the `activateMailboxEscalationToasts(...)` call
+ site (extension.ts:1522), so this is a one-argument signature change, one call site.
+- Precedent to mirror: `gate-toast.ts` keeps a `(builderId, gateName)` seen-set and prunes
+ entries once they leave the blocked set — same shape of fix.
+
+**Server-side semantics checked** (so the doc comment is accurate, not assumed):
+`db/mailbox.ts` sets `escalated = 1` once, guarded by `escalated = 0`, on a still-held row;
+the flag is never reset and terminal rows are pruned. So the same `mailboxId` cannot
+legitimately re-escalate — eviction here is purely about memory, never about re-notifying.
+
+**Stale-snapshot race** — `OverviewCache.refresh()` is last-write-wins by `latestSeq`, and
+the escalation SSE event itself triggers a refresh whose request starts *after* the row was
+flagged in the DB. So an older in-flight `mailboxEscalated: false` response cannot commit
+after the escalation's own `true` response and wrongly clear `seen`.
+
+**Fix shape** (~40 LOC + tests, well inside the BUGFIX ceiling):
+1. Inject `OverviewCache`; on `onDidChange`, clear `seen` when `data.mailboxEscalated` is
+ false (the preferred eviction key — the row leaving the escalated set).
+2. Keep a hard cap with oldest-first eviction as a backstop, for the pathological window
+ where the workspace is *continuously* escalated all day so the flag never falls false.
+ Insertion-ordered `Set` makes this trivial.
+3. Regression tests that fail without the fix: re-toast after de-escalation, and the cap
+ bounding a continuously-escalated window.
+
+Signal: PHASE_COMPLETE.
+
+## Fix
+
+Implemented as planned, commit `850242bf` (~50 LOC of source + 4 tests):
+
+- `mailbox-escalation-toast.ts` takes `OverviewCache` and prunes `seen` whole on
+ `mailboxEscalated === false`; `MAX_SEEN = 500` oldest-first cap as backstop.
+- `extension.ts` — the single call site passes `overviewCache` (already in scope).
+
+**Verified failing without the fix**, not assumed: with the eviction neutered the suite went
+2 failed / 10 passed; restored, 12/12. Full vscode unit suite 72 files / 850 tests green;
+`pnpm check-types` clean; `pnpm lint` has one pre-existing warning in the unrelated
+`src/commands/tunnel.ts`.
+
+Environment note for the next builder in this worktree: it starts with **no `node_modules`**.
+`pnpm install` at the worktree root, then `pnpm --filter @cluesmith/codev-types build &&
+pnpm --filter @cluesmith/codev-sdk build` — without the package builds, 18 vscode unit files
+fail on `Cannot find package '@cluesmith/codev-sdk/...'`, which looks like a code failure and
+is not one.
+
+## PR
+
+PR #1484 — https://github.com/cluesmith/codev/pull/1484. Per the architect's constraint we are
+not maintainers here: the PR is parked for a maintainer to merge; I do not merge it.
+
+`consult` did not auto-detect the project from this worktree (it listed every project and
+exited); `--issue 1472 --project-id bugfix-1472` is required.
+
+### CMAP (PR #1484)
+
+gemini = APPROVE (HIGH) · codex = APPROVE (HIGH) · claude = APPROVE (HIGH). No blocking issues
+from any lane. The claude lane died once on a transient API 500 and was re-run.
+
+Non-blocking notes and what I did with them:
+
+- *Doc slightly stronger than the server guarantees* — Tower also reports
+ `mailboxEscalated: false` when it cannot read the mailbox at all. **Fixed**: the doc comment
+ now says so, and why it is harmless (one escalation per row, no SSE replay).
+- *Uncommitted thread file* — **fixed**, committed with the PR.
+- *A null-workspace window toasts every workspace but reads Tower's fallback-workspace flag, so
+ it can prune on a foreign `false`* — pre-existing quirk of `escalationMatchesWorkspace`'s
+ deliberate null-matches-everything rule; this fix neither introduces nor worsens it, and it
+ is unobservable given single-emission. **Left alone** — widening it into a workspace-scoping
+ change is outside a BUGFIX.
+- *The `while (seen.size > MAX_SEEN)` loop can only iterate once* — deliberate; **left as is**
+ (the reviewer agreed).
+
+## Protocol complete — PR parked, NOT merged
+
+Human approved the `pr` gate (relayed by the architect 2026-08-17T23:24Z); I ran
+`porch approve bugfix-1472 pr --a-human-explicitly-approved-this` and `porch done`. Porch
+reports **PROTOCOL COMPLETE**, phase `verified`.
+
+`porch next` then hands out a final "Merge the pull request" task. **Deliberately not done.**
+We are not cluesmith/codev maintainers on this project; the architect's standing constraint is
+that a maintainer merges. GitHub agrees independently — PR #1484 is `reviewDecision:
+REVIEW_REQUIRED`, `mergeStateStatus: BLOCKED`. I also did NOT run `porch done --merged 1484`,
+since nothing was merged; whoever merges should record it.
+
+Anyone picking this up: the PR is complete and green, waiting only on maintainer review+merge.