feat(replace-dashboard-retry-command-with-protocol-message): typed retry_session protocol message

openspec/changes/archive/2026-08-24-replace-dashboard-retry-command-with-protocol-message/proposal.md

@@ -0,0 +1,143 @@
+# Replace the /__dashboard_retry command with a first-class protocol message
+
+## Why
+
+The dashboard's settled-error **Retry** button re-drives a failed turn by
+smuggling a magic string through the user-prompt channel: the client sends
+`{ type: "send_prompt", sessionId, text: "/__dashboard_retry" }`, and the bridge
+recovers the intent only via an exact string match in `command-handler.ts`
+(`text === "/__dashboard_retry"` → `{ type: "retry" }`). This is fragile and
+dishonest at the wire level:
+
+- **Channel abuse.** A control signal (re-drive this turn) rides the same field
+  as a real user prompt. The transport cannot distinguish intent from content.
+- **Stringly-typed coupling.** Retry depends on the slash-command parser. Any
+  refactor of `parseCommand` / `parseSendPrompt`, or a user literally typing
+  `/__dashboard_retry`, reaches the same branch.
+- **No typed contract.** `send_prompt` carries no signal that this is a
+  non-user, no-replay re-drive; reviewers must trace the string to understand it.
+
+The **pi call the bridge ultimately makes is already correct** and is what
+pi-core recommends: `pi.sendMessage({ customType: "pi-dashboard:retry",
+content: …, display: false }, { triggerTurn: true })` — the sole public
+primitive for "append a non-user entry and start a new turn without replaying
+the user's message" (`AgentSession.sendCustomMessage`, verified against the
+installed `@earendil-works/pi-coding-agent`). Only the **client→bridge
+transport** is wrong. This change fixes the transport, not the pi call.
+
+> **Revised after cross-model doubt-review (luna + terra, 2 clean probes).**
+> Both reviewers independently caught a false lifecycle claim, a missing
+> server hop, and an unsafe deletion. Corrections folded below; superseded
+> claims struck.
+
+- Add a dedicated `retry_session` message `{ type: "retry_session";
+  sessionId: string }` across **all three hops** it must traverse:
+  1. `packages/shared/src/browser-protocol.ts` — browser→server union.
+  2. `packages/shared/src/protocol.ts` — `ServerToExtensionMessage`
+     (server→bridge) union.
+  3. **Server routing** — a `retry_session` case in the browser gateway
+     switch (`packages/server/src/pairing/browser-gateway.ts`) that forwards
+     to the owning bridge. The default forwarder drops unknown types
+     (`directory-handler.ts`), so adding the unions alone is NOT enough — the
+     server would silently swallow the message.
+- Update the client's `handleRetrySession` (`useSessionActions.ts`) to send
+  `{ type: "retry_session", sessionId }`. The existing stale-click guard
+  (re-read `lastError` / `retryState` / `retryCancelled` / `isStreaming` from
+  `sessionStatesRef`) is preserved.
+- Handle `retry_session` in the bridge directly, calling the same
+  `pi.sendMessage({ customType: "pi-dashboard:retry", display: false },
+  { triggerTurn: true })` it calls today.
+- **Do NOT delete the `/__dashboard_retry` branch this release.** An older
+  browser client (version skew) still sends `send_prompt("/__dashboard_retry")`;
+  deleting the bridge parse branch would route it as an ordinary slash/user
+  prompt and replay it into history — violating the no-replay contract. Keep
+  the branch as a **deprecated alias** that maps to the same `retry_session`
+  handler, marked for removal one release after clients are known upgraded.
+- On button press the bridge emits **no synthetic retry-start**, and the
+  bridge SHALL guard against a still-armed `RetryTracker` chain converting the
+  manual retry's `agent_start` into a synthetic `auto_retry_start`
+  (`bridge.ts` routes every `agent_start` through
+  `RetryTracker.observeAgentStart`). ~~the resulting native `agent_start`
+  clears `retryState`/`lastError`~~ — **FALSE** (reducer preserves both across
+  `agent_start`; the *first non-error assistant completion* clears them). ~~the
+  optimistic `prompt_received { fresh:true }` ack drives sending state~~ —
+  **FALSE** (`prompt_received` is a no-op without a `pendingPrompt`, and a
+  retry creates none). The banner therefore clears on the recovered turn's
+  first clean assistant completion, exactly as the auto-retry path already
+  does — no new UI signal is introduced.
+
+## Resolved decisions (doubt-review + design spike)
+
+1. **Dispatch-failure channel — KEEP `auto_retry_end{attempt:0}` (decision: a).**
+   The failure folds into `lastError` and clears `retryState`, so it surfaces as
+   a plain error and NEVER renders the attempt counter — the auto-retry counter
+   surface is untouched. **Spike addendum:** `sendCustomMessage` is async and the
+   bridge's current synchronous `try/catch` only traps a sync throw. Wrap the
+   `pi.sendMessage(...)` call in `.catch()` as well so an async rejection ALSO
+   emits `auto_retry_end{success:false, finalError}` — otherwise an async
+   dispatch failure escapes as an unhandled rejection and strands the surface.
+2. **Delivery ack — structured negative-ack, per repo convention (decision:
+   follow `plugin_action_error`).** The codebase rule is "unknown → structured
+   error to the sender, never a silent drop" (`browser-gateway.ts:996`). Add a
+   `retry_session_error` (mirroring `plugin_action_error` / `spawn_error`) that
+   the server or bridge emits when it cannot deliver (unknown/disconnected
+   session, or bridge lacks the handler); the client re-enables the one-shot
+   Retry + toasts on it. In the DELIVERED case the retry turn's own
+   `agent_start` / `lastError` change already self-heals the button, so the
+   negative-ack is only the not-delivered path. The pure old-server skew window
+   (an old server that doesn't know the type → `default` → `handlePiGatewayForward`)
+   is identical for every new message type and is closed by the co-versioned
+   deploy flow (`/api/restart` + `npm run reload`) — **accepted + documented**,
+   no client timeout added.
+3. **Eligibility — trust the client ref check (decision: b).** Consistent with
+   `send_prompt`, which is also only client-guarded today. **Spike validates
+   this is safe:** if `retry_session` lands while the session is streaming,
+   `sendCustomMessage` branch 1081 QUEUES it as steer/followUp — it does not
+   collide or corrupt state. A mis-timed retry degrades to a queued no-op, so no
+   server/bridge idle-guard is required.
+
+## Capabilities
+
+### New Capabilities
+
+None.
+
+### Modified Capabilities
+
+- `session-status-banner`: the settled-error Retry action is dispatched via a
+  typed `retry_session` message; no behavioural change to what the banner shows.
+
+## Discipline Skills
+
+- `review-code`: review the extension/client/shared transport diff after the
+  focused protocol tests pass.
+- `systematic-debugging`: only if a regression surfaces — the failure mode is a
+  transport-routing change, not a lifecycle change; verify with synthetic
+  message sequences, not transcript inspection.
+
+(`security-hardening`, `performance-optimization`,
+`observability-instrumentation` do not apply: no new external input surface, no
+latency-budgeted path, no new runtime state — the change swaps one already-
+authenticated WS message shape for another.)
+
+## Impact
+
+- Protocol unions: `packages/shared/src/browser-protocol.ts`,
+  `packages/shared/src/protocol.ts` (+ the shared protocol tests).
+- Server routing: `packages/server/src/pairing/browser-gateway.ts` (new
+  `retry_session` switch case + forward-to-bridge).
+- Client dispatch: `packages/client/src/hooks/useSessionActions.ts`.
+- Bridge routing: `packages/extension/src/bridge.ts` /
+  `packages/extension/src/command-handler.ts` (add `retry_session` handler;
+  KEEP `/__dashboard_retry` as a deprecated alias, do NOT delete; add the
+  armed-chain disarm guard around the manual `agent_start`).
+- Negative-ack: `retry_session_error` type in `browser-protocol.ts`
+  (server→browser), emitted by the gateway/bridge on undeliverable retry;
+  client handler re-enables the one-shot Retry + toast.
+- Test updates (existing tests encode the deprecated wire contract and must be
+  re-pointed, not just added to):
+  `packages/client/src/hooks/__tests__/useSessionActions.optimistic-prompt.test.tsx`,
+  `packages/server/src/browser-handlers/__tests__/session-action-handler.test.ts`,
+  plus the extension command-handler / protocol test files.
+- No new dependency, no persistence change, no live-dashboard test, no provider
+  regex, no transcript parsing.

openspec/changes/archive/2026-08-24-replace-dashboard-retry-command-with-protocol-message/specs/session-status-banner/spec.md

@@ -0,0 +1,93 @@
+# session-status-banner (delta)
+
+## ADDED Requirements
+
+### Requirement: Settled-error Retry is dispatched as a typed protocol message
+
+The settled-error **Retry** action SHALL be dispatched as a first-class
+`retry_session` protocol message `{ type: "retry_session"; sessionId: string }`,
+NOT by sending a `send_prompt` whose `text` is the sentinel `/__dashboard_retry`.
+The message SHALL traverse three hops, each of which SHALL carry the type: the
+browser→server union (`browser-protocol.ts`), the server→bridge union
+(`protocol.ts` `ServerToExtensionMessage`), and the server gateway routing that
+forwards a browser `retry_session` to the owning session bridge. The underlying
+pi call the bridge makes SHALL remain
+`pi.sendMessage({ customType: "pi-dashboard:retry", display: false },
+{ triggerTurn: true })` — this change alters the transport, not the pi call.
+
+The client SHALL preserve the pre-dispatch stale-click guard: it SHALL NOT
+dispatch when `lastError` is absent, or `retryState` is set, or `retryCancelled`
+is set, or `isStreaming` is true.
+
+#### Scenario: Client dispatches retry_session, not the sentinel prompt
+- **GIVEN** a session with `lastError` set, `retryState` undefined,
+  `retryCancelled` false, `isStreaming` false
+- **WHEN** the user activates the settled-error Retry control
+- **THEN** the client SHALL send `{ type: "retry_session", sessionId }`
+- **AND** it SHALL NOT send a `send_prompt` carrying `/__dashboard_retry`
+
+#### Scenario: Stale-click guard blocks dispatch in every ineligible state
+- **WHEN** Retry is activated while ANY of: `lastError` absent, `retryState`
+  set, `retryCancelled` set, or `isStreaming` true
+- **THEN** the client SHALL send no `retry_session` message
+
+#### Scenario: Server forwards retry_session to the owning bridge
+- **GIVEN** a browser `retry_session { sessionId }` for a live, bridged session
+- **WHEN** the server gateway receives it
+- **THEN** it SHALL forward a `retry_session` to that session's bridge
+- **AND** it SHALL NOT drop it through the unknown-type default path
+
+#### Scenario: Bridge re-drives the turn via the custom-message primitive
+- **GIVEN** the bridge receives `retry_session` for an idle settled session
+- **WHEN** it handles the message
+- **THEN** it SHALL call `pi.sendMessage({ customType: "pi-dashboard:retry",
+  display: false }, { triggerTurn: true })`
+- **AND** a native `agent_start` for the re-driven turn SHALL follow
+- **AND** no user message SHALL be appended or replayed
+
+### Requirement: A manual retry is not mapped onto the auto-retry surface
+
+A `retry_session`-initiated turn SHALL NOT render pi's auto-retry attempt
+counter. The bridge SHALL guard against a still-armed `RetryTracker` chain
+converting the manual turn's `agent_start` into a synthetic `auto_retry_start`.
+
+#### Scenario: Armed tracker chain does not synthesize a counter for a manual retry
+- **GIVEN** a `RetryTracker` chain is still armed for the session
+- **WHEN** the manual `retry_session` turn emits `agent_start`
+- **THEN** the bridge SHALL NOT forward a synthetic `auto_retry_start` for it
+- **AND** no attempt counter SHALL render on the banner
+
+#### Scenario: Dispatch failure surfaces as an error, not a counter
+- **GIVEN** `pi.sendMessage` throws synchronously OR rejects asynchronously
+- **WHEN** the bridge handles the failure
+- **THEN** it SHALL forward `auto_retry_end { success: false, attempt: 0,
+  finalError }`
+- **AND** the banner SHALL show the error with no attempt counter
+
+### Requirement: Undeliverable retry is negatively acked, never silently dropped
+
+When the server or bridge cannot deliver a `retry_session` (unknown or
+disconnected session, or a bridge lacking the handler), it SHALL emit a
+structured `retry_session_error` to the sender (mirroring `plugin_action_error`),
+never a silent drop. The client SHALL re-enable the one-shot Retry control and
+surface a toast on receipt.
+
+#### Scenario: Unknown/disconnected session yields a structured error
+- **GIVEN** a `retry_session` for a session with no reachable bridge
+- **WHEN** the server processes it
+- **THEN** it SHALL send `retry_session_error { sessionId, error }` to the sender
+- **AND** the client SHALL re-enable Retry and toast the error
+
+### Requirement: The /__dashboard_retry sentinel remains a deprecated alias
+
+For backward compatibility with un-upgraded clients during a version-skew
+window, the bridge SHALL continue to accept a `send_prompt` whose `text` equals
+`/__dashboard_retry` and route it to the same retry handler as `retry_session`.
+The alias SHALL NOT be removed in this change; removal is a separate change after
+clients are known upgraded.
+
+#### Scenario: Legacy sentinel still triggers a retry
+- **GIVEN** an older client sends `send_prompt { text: "/__dashboard_retry" }`
+- **WHEN** the bridge parses it
+- **THEN** it SHALL invoke the same retry dispatch as `retry_session`
+- **AND** it SHALL NOT append or replay the sentinel as a user message

openspec/changes/archive/2026-08-24-replace-dashboard-retry-command-with-protocol-message/tasks.md

@@ -0,0 +1,41 @@
+# Tasks — replace-dashboard-retry-command-with-protocol-message
+
+## 1. Protocol + shared types
+- [x] 1.1 Add `RetrySessionBrowserMessage { type: "retry_session"; sessionId }` to `BrowserToServerMessage` in `packages/shared/src/browser-protocol.ts`.
+- [x] 1.2 Add `RetrySessionExtensionMessage { type: "retry_session"; sessionId }` to `ServerToExtensionMessage` in `packages/shared/src/protocol.ts`.
+- [x] 1.3 Add `RetrySessionErrorMessage { type: "retry_session_error"; sessionId; error }` to `ServerToBrowserMessage` in `packages/shared/src/browser-protocol.ts` (mirror `plugin_action_error`).
+
+## 2. Server routing
+- [x] 2.1 Add a `retry_session` case in the browser gateway switch (`packages/server/src/pairing/browser-gateway.ts`) that forwards to the owning session bridge; do NOT let it fall through to the unknown-type `handlePiGatewayForward` default.
+- [x] 2.2 On unknown/disconnected session, emit `retry_session_error` to the sender (follow the `plugin_action_error` "never a silent drop" convention).
+
+## 3. Client dispatch + UI
+- [x] 3.1 Change `handleRetrySession` (`packages/client/src/hooks/useSessionActions.ts`) to send `{ type: "retry_session", sessionId }`; keep the stale-click guard unchanged.
+- [x] 3.2 Handle `retry_session_error` in the client: re-enable the one-shot Retry in `SessionBanner` and surface a toast.
+
+## 4. Bridge handler
+- [x] 4.1 Handle `retry_session` in the bridge/command-handler, calling `pi.sendMessage({ customType: "pi-dashboard:retry", display: false }, { triggerTurn: true })`.
+- [x] 4.2 Wrap the `pi.sendMessage(...)` call in BOTH a synchronous `try/catch` AND `.catch()` (it is async) so a sync throw OR an async rejection emits `auto_retry_end { success:false, attempt:0, finalError }`. (Spike caveat 1.)
+- [x] 4.3 Add a disarm guard so a still-armed `RetryTracker` chain does not convert the manual retry's `agent_start` into a synthetic `auto_retry_start`.
+- [x] 4.4 KEEP the `text === "/__dashboard_retry"` branch as a deprecated alias routing to the same handler; do NOT delete it this change. Mark it for removal in a follow-up.
+
+## 5. Tests (folded from test-plan.md — one per automated scenario)
+- [x] 5.1 Client dispatches `retry_session`, never the sentinel. input: settled+idle state · trigger: `handleRetrySession` · observable: one `retry_session` send, no `/__dashboard_retry`. see `packages/client/src/hooks/__tests__/useSessionActions.optimistic-prompt.test.tsx` (test-plan #1).
+- [x] 5.2 Stale-click guard blocks all 4 ineligible states. input: `lastError`-absent / `retryState`-set / `retryCancelled`-true / `isStreaming`-true · trigger: `handleRetrySession` each · observable: zero sends in all four. see useSessionActions test (test-plan #2).
+- [x] 5.3 Server forwards `retry_session` to the owning bridge. input: browser `retry_session` for a live bridged session · trigger: gateway handler · observable: forwarded to bridge, not the unknown-type default. see `packages/server/src/browser-handlers/__tests__/session-action-handler.test.ts` (test-plan #3).
+- [x] 5.4 Bridge sync dispatch failure emits `auto_retry_end`. input: `pi.sendMessage` throws sync · trigger: bridge handles · observable: `auto_retry_end{success:false,attempt:0,finalError}` once, no `agent_start`. see `packages/extension/src/__tests__/command-handler.test.ts` (test-plan #4).
+- [x] 5.5 Bridge async rejection ALSO emits `auto_retry_end`. input: `pi.sendMessage` returns rejected promise · trigger: bridge handles + microtask drain · observable: `auto_retry_end{success:false}` forwarded (the `.catch()` path). see command-handler test (test-plan #5).
+- [x] 5.6 Armed tracker chain yields no counter for a manual retry. input: armed `RetryTracker` chain · trigger: manual retry `agent_start` · observable: no synthetic `auto_retry_start`, no `retry-banner-attempt`. see `packages/extension/src/__tests__/retry-tracker.test.ts` (test-plan #6).
+- [x] 5.7 Legacy `/__dashboard_retry` still triggers retry. input: `send_prompt{text:"/__dashboard_retry"}` · trigger: parse · observable: same retry dispatch, no user-message replay. see command-handler test (test-plan #7).
+- [~] 5.8 DEFERRED to ship-it (L3 e2e, docker harness). Needs a stateful fail-then-succeed faux scenario not in `qa/fixtures/faux-scenarios.ts`; the transport swap does not touch the reducer/banner convergence path (fully covered by L1 5.1–5.10). Banner clears on the recovered turn's first clean completion. input: settled-error banner · trigger: Retry → re-drive → first non-error `message_end` · observable: error → no-counter → hidden; no `retry-banner-attempt`. see nearest `tests/e2e/` banner/retry spec, docker harness derived port (test-plan #8).
+- [x] 5.9 Negative-ack re-enables the one-shot Retry. input: `SessionBanner` post-press (disabled) · trigger: `retry_session_error` arrives · observable: Retry enabled + toast. see `packages/client/src/components/session/__tests__/SessionBanner.test.tsx` (test-plan #9).
+- [x] 5.10 `retry_session` while streaming degrades to a queued no-op. input: `isStreaming` true (guard bypassed) · trigger: bridge receives `retry_session` · observable: pi queues (steer/followUp), no corruption, no duplicate `agent_start`. see command-handler test with mock `isStreaming` (test-plan #10).
+- [~] 5.11 DEFERRED to ship-it (L3 e2e, docker harness). Same reason as 5.8 — no stateful faux retry scenario; nearest `error-lifecycle.spec.ts` is stale (written for the removed single-card design). Happy-path: click Retry re-drives and completes. input: settled overloaded_error banner · trigger: click Retry · observable: new turn streams, banner hides on success, no injected user message. see nearest `tests/e2e/` retry/banner spec (test-plan #11).
+
+## 6. Manual verification (deferred post-merge)
+- [x] 6.1 (manual-only #12, validated post-merge) Old-server version skew: new client + pre-`retry_session` server → Retry drops, button stays disabled; confirm the co-versioned deploy (`/api/restart` + `npm run reload`) closes it. (test-plan: manual-only #12).
+
+## 7. Validate
+- [x] 7.1 `npm test` green for the touched packages (shared, server, client, extension) — 194 focused tests + all touched-package suites pass. The 8 repo-wide failures are pre-existing worktree-env issues (missing `@earendil-works/pi-coding-agent`, `pi-dashboard-cost-estimator`, `node_modules/.bin/tsc`, fs.watch attach behavior), none in this diff.
+- [~] 7.2 DEFERRED to ship-it (needs a running instance). Rebuild matrix: `npm run reload` (extension), `curl -X POST .../api/restart` (server/shared), `npm run build && restart` (client).
+- [x] 7.3 `openspec status --change replace-dashboard-retry-command-with-protocol-message --json` task counts match the plain checkboxes.