fix(slack): defer parked-tab keeper to pinned workspace instead of forcing reopen (t098)

Author: duongdev

CLAUDE.md

@@ -61,7 +61,7 @@ The server-side, authoritative Slack capture modality (ADR-0011). The web server
 _Avoid_: scraper, poller, backfill.
 
 **Workspace Registry**:
-The server-side persistence (`slack-workspaces.json`) mapping each Slack `teamId` → `{ url, name, enterpriseId, lastSeen }`, populated the first time a workspace tab is seen live (ADR-0011). Persisting `enterpriseId` lets a cold start resolve each workspace's **Grid Group** without live creds. It drives two things: re-extraction targets for stale creds, and the **Parked Tab** keep-alive loop, which ensures exactly one tab per registered workspace exists on the remote browser (recreated via `/json/new` if closed or after a browser restart) so creds self-refresh and the hijack stays armed. Distinct from ADR-0010 **Workspaces** (multi-CDP-host UI), though both stamp entries with a workspace key.
+The server-side persistence (`slack-workspaces.json`) mapping each Slack `teamId` → `{ url, name, enterpriseId, lastSeen }`, populated the first time a workspace tab is seen live (ADR-0011). Persisting `enterpriseId` lets a cold start resolve each workspace's **Grid Group** without live creds. It drives two things: re-extraction targets for stale creds, and the **Parked Tab** keep-alive loop. The keeper ensures at least one Slack tab is live so creds self-refresh and the hijack stays armed — but it **defers to a Pin** (t098): a registered workspace whose URL is pinned is owned by its Pin and the keeper never spawns an anonymous duplicate for it (closing its tab no longer resurrects a stray). When no Slack tab is live at all, a single cred lifeline opens one tab, preferring a pinned URL, so shared creds remain available to the sweep. Per-workspace anonymous tabs are kept only for unpinned workspaces. Distinct from ADR-0010 **Workspaces** (multi-CDP-host UI), though both stamp entries with a workspace key.
 _Avoid_: account list, team store, tenant table.
 
 **Grid Group**:
@@ -108,7 +108,7 @@ _Avoid_: native tab, page view, webview tab.
 - **Viewport Transform** maps canvas coordinates to **Remote Page** coordinates for both drawing **Screencast Frames** and hit-testing **Input Forwarding**.
 - **Adaptive Viewport** (when enabled) resizes the **Remote Page** to the canvas so **Screencast Frames** fill it without letterbox bars.
 - A **Notification Side-Channel** attaches to a background **Tab**'s target and uses a **Notification Adapter** to run **Notification Capture** — independent of the **Active Tab**'s screencast socket. Clicking the result activates the owning Tab and, if the entry carries an `activate` intent, the activation registry maps it to a **Remote Page** deep-open intention.
-- For Slack, the **Slack Content Sweep** is the authoritative **Notification Capture** writer; the in-page hijack provides only an instant foreground toast. The sweep reads creds from a live **Tab**, persists workspaces in the **Workspace Registry**, keeps a **Parked Tab** alive per registered workspace, and respects the **Channel Exclude** list.
+- For Slack, the **Slack Content Sweep** is the authoritative **Notification Capture** writer; the in-page hijack provides only an instant foreground toast. The sweep reads creds from a live **Tab**, persists workspaces in the **Workspace Registry**, uses the **Parked Tab** keeper (pin-deferred since t098 — a pinned workspace is owned by its Pin), and respects the **Channel Exclude** list.
 - A **Local Tab** renders a real local web page (in-DOM `<webview>`) alongside CDP Tabs — it does not use **Screencast Frames** or **Input Forwarding**; it gets direct device access instead.
 
 ## Example dialogue

CONTEXT.md

@@ -44,15 +44,37 @@ function liveTeamIds(targets) {
 }
 
 // Which registered workspaces need a parked tab created: those with no live tab and not
-// created within the cooldown. `createdAt` maps teamId → last create timestamp. Pure.
-function planParkedTabs(registry, live, createdAt, now) {
+// created within the cooldown. `createdAt` maps teamId → last create timestamp.
+//
+// `pinUrlByTeam` (t098) maps a pinned workspace's teamId → its pin URL. A pinned workspace
+// is considered OWNED BY ITS PIN: the keeper never spawns an anonymous duplicate for it
+// (closing its tab no longer resurrects a stray). Capture is unaffected because one live
+// Slack tab refreshes creds for ALL workspaces and the sweep polls each over the web API
+// regardless of which tab is live. So per-workspace tabs aren't needed — only one live tab
+// is. When NO Slack tab is live and nothing else would open one, a single cred lifeline
+// plan keeps one alive, preferring a pinned URL (so it adopts into the pin on next reload).
+// Omitting `pinUrlByTeam` preserves the prior per-workspace behavior. Pure.
+function planParkedTabs(registry, live, createdAt, now, pinUrlByTeam = {}) {
+  const offCooldown = (teamId) => {
+    const last = createdAt[teamId]
+    return !(last && now - last < CREATE_COOLDOWN_MS)
+  }
   const plans = []
   for (const teamId of Object.keys(registry)) {
     if (live.has(teamId)) continue
-    const last = createdAt[teamId]
-    if (last && now - last < CREATE_COOLDOWN_MS) continue
+    if (pinUrlByTeam[teamId]) continue // pin owns it — don't reopen
+    if (!offCooldown(teamId)) continue
     plans.push({ teamId, url: registry[teamId].url })
   }
+  // Cred lifeline: nothing live and nothing else planned → keep exactly one Slack tab alive
+  // via a pinned workspace, so shared creds keep refreshing. Cooldown-gated; one is enough.
+  if (live.size === 0 && plans.length === 0) {
+    for (const teamId of Object.keys(pinUrlByTeam)) {
+      if (!offCooldown(teamId)) continue
+      plans.push({ teamId, url: pinUrlByTeam[teamId] })
+      break
+    }
+  }
   return plans
 }
 

core/slack-workspaces.js

@@ -83,4 +83,61 @@ describe("planParkedTabs — recreate registered workspaces with no live tab", (
     const plans = planParkedTabs(reg, new Set(["T1"]), { T2: 4000 }, 40000)
     expect(plans).toEqual([{ teamId: "T2", url: CLIENT("T2") }])
   })
+
+  it("omitting pinUrlByTeam is byte-identical to the prior behavior", () => {
+    expect(planParkedTabs(reg, new Set(["T1"]), {}, 5000)).toEqual([
+      { teamId: "T2", url: CLIENT("T2") },
+    ])
+  })
+})
+
+describe("planParkedTabs — defers to a pinned workspace (t098)", () => {
+  const reg = {
+    T1: { teamId: "T1", url: CLIENT("T1"), name: "A", lastSeen: 1 },
+    T2: { teamId: "T2", url: CLIENT("T2"), name: "B", lastSeen: 1 },
+  }
+  const PIN = (team: string) => `https://app.slack.com/client/${team}/CPIN`
+
+  it("skips a registered workspace that has a pin — the pin owns it", () => {
+    // T1 live, T2 not live but pinned → no reopen for T2.
+    const plans = planParkedTabs(reg, new Set(["T1"]), {}, 5000, { T2: PIN("T2") })
+    expect(plans).toEqual([])
+  })
+
+  it("still plans an unpinned workspace alongside a pinned one", () => {
+    // T1 pinned (skip), T2 unpinned + not live → only T2 reopens.
+    const plans = planParkedTabs(reg, new Set(), {}, 5000, { T1: PIN("T1") })
+    expect(plans).toEqual([{ teamId: "T2", url: CLIENT("T2") }])
+  })
+
+  it("cred lifeline: opens one pinned workspace (at the pin URL) when nothing is live and nothing else is planned", () => {
+    // Both pinned, neither live → no normal plan, but the lifeline opens exactly one at its pin URL.
+    const plans = planParkedTabs(reg, new Set(), {}, 5000, { T1: PIN("T1"), T2: PIN("T2") })
+    expect(plans).toHaveLength(1)
+    expect(plans[0]).toEqual({ teamId: "T1", url: PIN("T1") })
+  })
+
+  it("no lifeline when a Slack tab is already live", () => {
+    const plans = planParkedTabs(reg, new Set(["T1"]), {}, 5000, { T1: PIN("T1"), T2: PIN("T2") })
+    expect(plans).toEqual([])
+  })
+
+  it("no lifeline when an unpinned plan already keeps a tab alive", () => {
+    // T1 pinned, T2 unpinned + not live → T2's normal plan covers cred-refresh; no extra lifeline.
+    const plans = planParkedTabs(reg, new Set(), {}, 5000, { T1: PIN("T1") })
+    expect(plans).toEqual([{ teamId: "T2", url: CLIENT("T2") }])
+  })
+
+  it("cred lifeline respects the create cooldown", () => {
+    // Only T1 registered + pinned, not live, but just created at t=4000 (cooldown) → no lifeline.
+    const oneReg = { T1: reg.T1 }
+    const plans = planParkedTabs(oneReg, new Set(), { T1: 4000 }, 5000, { T1: PIN("T1") })
+    expect(plans).toEqual([])
+  })
+
+  it("cred lifeline bootstraps from a pin even when the workspace is not yet registered", () => {
+    // Fresh start: empty registry, no live tab, a pin exists → open it to seed creds.
+    const plans = planParkedTabs({}, new Set(), {}, 5000, { T9: PIN("T9") })
+    expect(plans).toEqual([{ teamId: "T9", url: PIN("T9") }])
+  })
 })

core/slack-workspaces.test.ts

@@ -1,6 +1,6 @@
 # ADR-0012: Phone triage surface: inbox-rooted shell + conversation reader
 
-- **Status:** Proposed
+- **Status:** Accepted (all phases shipped t076–t081)
 - **Date:** 2026-06-11
 
 ## Context

docs/adr/0012-phone-triage-surface-inbox-rooted-shell-conversati.md

@@ -87,7 +87,7 @@ Any non-trivial design decision gets an ADR. Examples of "non-trivial":
 
 ADRs are **append-only**. When a decision changes, write a new ADR that supersedes the old one. Update the old ADR's *Status* line to reference the superseder, but never edit its body. The history is the documentation.
 
-Current ADRs: `docs/adr/0001-single-remote-page.md`, `0002-adaptive-viewport.md`, `0003-notifications-side-channel.md`, `0004-pin-live-tab-model.md`, `0005-local-tabs-base-window.md`, `0006-web-proxy-sse-transport.md`, `0007-web-websocket-transport.md`, `0008-defer-monorepo-shared-cjs-core.md`, `0009-touch-first-co-primary-input-surface.md`, `0010-multiple-workspaces-deferred-design.md`, `0011-slack-content-sweep-guaranteed-delivery.md`, `0012-phone-triage-surface-inbox-rooted-shell-conversati.md`, `0013-per-device-notification-delivery.md`, `0014-endpoint-reconciled-per-device-push-identity.md`.
+Current ADRs: `docs/adr/0001-single-remote-page.md`, `0002-adaptive-viewport.md`, `0003-notifications-side-channel.md`, `0004-pin-live-tab-model.md`, `0005-local-tabs-base-window.md`, `0006-web-proxy-sse-transport.md`, `0007-web-websocket-transport.md`, `0008-defer-monorepo-shared-cjs-core.md`, `0009-touch-first-co-primary-input-surface.md`, `0010-multiple-workspaces-deferred-design.md`, `0011-slack-content-sweep-guaranteed-delivery.md`, `0012-phone-triage-surface-inbox-rooted-shell-conversati.md`, `0013-per-device-notification-delivery.md`, `0014-endpoint-reconciled-per-device-push-identity.md`, `0015-prefer-thin-handlers-over-reducer-indirection.md`.
 
 ---
 

docs/conventions/docs-discipline.md

@@ -0,0 +1,109 @@
+# 098 — slack keeper defers to pinned workspace tab instead of forcing reopen
+
+- **Status:** in-progress
+- **Mode:** HITL
+- **Estimate:** 0.5d
+- **Depends on:** none
+- **Blocks:** none
+
+## Goal
+
+The Slack parked-tab keeper (t070, ADR-0011) reopens an anonymous background tab
+for every registered workspace whose tab is not currently live — **even when the
+user has that workspace pinned**. Closing the workspace's tab immediately spawns a
+stray duplicate in the Tabs list, which is annoying. After this task, the keeper
+**defers to a pin**: a workspace that has a pin is considered covered by its pin and
+the keeper never spawns a duplicate for it. Capture is unaffected because **one live
+Slack tab refreshes creds for all workspaces** (shared `d` cookie + `localConfig_v2`
+holds every team's token) and the sweep polls every workspace over the web API
+regardless of which tab is live. A cred lifeline keeps exactly one Slack tab alive
+(preferring a pinned URL) only when no Slack tab is live and nothing else would open
+one.
+
+## Why now
+
+Daily-driver annoyance on the priority web/PWA surface: the user pins their Slack
+workspaces and the keeper keeps resurrecting closed tabs against their intent. The
+fix is small and server-only, and it tightens guaranteed delivery (ADR-0011) rather
+than weakening it — capture no longer depends on a per-workspace tab.
+
+## Acceptance criteria
+
+- [ ] A registered workspace that has a pin gets **no** anonymous parked tab from
+      the keeper (it is not re-created on close).
+- [ ] A registered workspace with **no** pin keeps today's anonymous parked-tab
+      behavior (unchanged).
+- [ ] Cred lifeline: when **no** Slack tab is live and no unpinned workspace would
+      open one, the keeper opens exactly **one** tab, preferring a pinned URL, so
+      creds keep refreshing.
+- [ ] The cred lifeline respects the existing create-cooldown (no spam).
+- [ ] Omitting the pin map preserves byte-identical prior behavior (back-compat).
+
+## Test plan
+
+### Layer 1 — Pure logic (TDD)
+
+- [ ] `core/slack-workspaces.js` `planParkedTabs(registry, live, createdAt, now, pinUrlByTeam)`
+      — skips a pinned registered workspace; still plans an unpinned one alongside it.
+- [ ] cred lifeline opens one pinned workspace when `live` is empty and no unpinned
+      plan exists; uses the pin URL; respects cooldown; does nothing when a tab is
+      live or an unpinned plan already keeps one alive.
+- [ ] back-compat: omitting `pinUrlByTeam` equals the prior result.
+
+### Layer 2 — Manual smoke (CDP/IPC)
+
+Web build (`pnpm web`) against a live Remote Browser with ≥1 Slack workspace pinned:
+
+- [ ] Pin a Slack workspace, open it, then close its tab → keeper does **not** reopen
+      a stray; the workspace still receives notifications (sweep continues).
+- [ ] Close **every** Slack tab → keeper opens exactly one (a pinned workspace's URL);
+      creds recover and sweeps resume.
+- [ ] An unpinned workspace closed → still auto-reopens (unchanged).
+
+### Layer 3 — Visual review
+
+n/a — no renderer UI is touched (server-side keeper only).
+
+## Design notes
+
+- **Contracts changed:** `planParkedTabs` gains an optional 5th arg
+  `pinUrlByTeam: { [teamId]: url }`. A workspace whose `teamId` is a key is skipped
+  (pin owns it). When `live` is empty and the normal plan list is empty, the planner
+  appends **one** lifeline plan from `pinUrlByTeam` (cooldown-gated) so a single
+  Slack tab stays alive for shared-cred refresh. Absent/empty map → prior behavior.
+- The server (`web/server.mjs` `keepSlackTabsAlive`) derives `pinUrlByTeam` from
+  `settings.getPins()` via `teamIdOf(pin.url)` and passes it in. No renderer change.
+- **New modules:** none.
+- **New ADR needed?** No — this is a tuning of the t070 keeper within ADR-0011.
+  Document in CLAUDE.md (keeper bullet + `slack-workspaces.js` note) and the code.
+
+## Out of scope
+
+- Renderer pin-adoption of a keeper-opened tab mid-session (ADR-0004 keeps
+  URL-adoption startup-only; the lifeline tab adopts on next reload). Not needed
+  for the chosen "don't reopen — pin owns it" behavior.
+- Reducing the unpinned per-workspace keeper to one-tab-total (kept as-is per the
+  decision).
+
+## Definition of Done
+
+- [ ] Layer 1 tests written and green
+- [ ] Layer 2 smoke checklist completed with a live Remote Browser (web build)
+- [ ] `pnpm check:changed` clean
+- [ ] `pnpm typecheck` clean
+- [ ] `pnpm test` green
+- [ ] CLAUDE.md updated (keeper bullet + `slack-workspaces.js` note)
+- [ ] No commented-out code, no `console.log` debris, no AI attribution
+- [ ] Task closed: status → done, file moved to `docs/tasks/done/`, t098 in commit
+
+## Notes
+
+Decided with the user via two grills: (1) "reuse the pin" over "hands-off"; then,
+after surfacing the code fact that one live Slack tab refreshes all workspaces'
+creds, (2) "don't reopen — pin owns it" with a single-tab cred lifeline. The keeper
+was over-aggressive: per-workspace tabs are not needed for the sweep — only one live
+Slack tab is.
+
+---
+
+_When task status flips to `done`, move this file to `done/`._

docs/tasks/098-slack-keeper-defers-to-pinned-workspace-tab-instea.md

@@ -634,9 +634,17 @@ async function keepSlackTabsAlive(targets) {
     )
     if (!before || before.enterpriseId !== enterpriseId) changed = true
   }
-  // Recreate a tab for any registered workspace that has no live tab.
+  // Recreate a tab for any registered workspace that has no live tab — UNLESS the user has
+  // it pinned (t098): a pinned workspace is owned by its pin, so the keeper never spawns a
+  // stray duplicate for it. One live Slack tab refreshes every workspace's creds, so capture
+  // is unaffected; a cred lifeline (inside planParkedTabs) keeps one tab alive when none is.
   const live = slackLiveTeamIds(targets)
-  const plans = slackPlanParkedTabs(slackRegistry, live, slackCreatedAt, Date.now())
+  const pinUrlByTeam = {}
+  for (const pin of settings.getPins()) {
+    const tid = slackTeamIdOf(pin.url || "")
+    if (tid && !pinUrlByTeam[tid]) pinUrlByTeam[tid] = pin.url
+  }
+  const plans = slackPlanParkedTabs(slackRegistry, live, slackCreatedAt, Date.now(), pinUrlByTeam)
   for (const plan of plans) {
     slackCreatedAt[plan.teamId] = Date.now()
     try {