Speed cold session loads

docs/design/boot-timing-instrumentation.md

@@ -1,17 +1,18 @@
 # Boot/reload performance instrumentation
 
-Opt-in instrumentation that measures the cost of a full page reload with hard
-numbers, so we can reason about reload disruption (e.g. while an agent is
-editing UI code under Vite) with data instead of estimates.
+Opt-in instrumentation that measures full-page reload and session transcript
+hydration costs with hard numbers, so reload and cold-navigation changes can be
+evaluated with data instead of estimates.
 
 ## Why
 
 A full reload under Vite dev re-does two expensive things: re-evaluating the
 unbundled module graph (the dev module waterfall) and rehydrating session state
-(WebSocket reconnect + `get_state` snapshot replay + full `MessageList`
-re-render). Both resist a small time budget, and the snapshot replay scales with
-transcript length. Rather than guess, this feature records named milestones
-across a reload and writes them somewhere agents can inspect.
+(WebSocket reconnect + `get_messages` transcript snapshot replay + full
+`MessageList` re-render). Cold session navigation uses the same transcript path.
+Both resist a small time budget, and snapshot rendering scales with transcript
+length. Rather than guess, this feature records named milestones and writes them
+somewhere agents can inspect.
 
 ## Surface
 
@@ -35,10 +36,10 @@ across a reload and writes them somewhere agents can inspect.
   `initApp-start`, `first-render-call` (main.ts), `first-paint`
   (`pwa-lifecycle.ts::finalizeBoot`, when `#app` actually paints), `ws-open`,
   `auth-ok` (WS auth handshake done — splits the ws-open→snapshot window into
-  handshake vs. server-side snapshot wait), `snapshot-received(N msgs)`,
-  `snapshot-applied`, `post-snapshot-paint` (`remote-agent.ts`). The raw
-  snapshot frame size is captured as `snapshotChars` to distinguish payload
-  transfer cost from server-side assembly.
+  handshake vs. server-side snapshot wait), `get-messages-sent`,
+  `snapshot-received(N msgs)`, `snapshot-applied`, `post-snapshot-paint`
+  (`remote-agent.ts`). The raw snapshot frame size is captured as `snapshotChars`
+  to distinguish payload transfer cost from server-side assembly.
 - **Server-side snapshot breakdown** (dev harness only): the `get_messages`
   handler attaches a `SnapshotServerTiming` (`rpcMs` agent assembly /
   `pipelineMs` server transform / `stampMs` / `stringifyMs` / `bytes` /
@@ -69,13 +70,159 @@ Each JSONL line carries `reason`, `isReload`, `total_ms`, `route`, `sessionId`,
 `Δ prev (ms)` deltas — the delta column shows where the time actually goes
 (module waterfall vs. first paint vs. snapshot replay).
 
+## Cold session navigation invariant
+
+A cold switch to an existing session must start transcript transfer before
+unrelated REST work. `connectToSession` in the session manager therefore owns
+the initial sequence:
+
+1. After initial WebSocket authentication resolves, call
+   `RemoteAgent.requestMessages()` as the first post-auth action. This emits
+   `get-messages-sent` and sends `get_messages`.
+2. Only then may side-panel workspace, proposal, draft, git, project/goal, or
+   session-list REST hydration start.
+3. Bind the transcript-bearing agent to the chat panel, then perform the single
+   initial side-panel workspace hydration.
+
+This ordering matters because the transcript is the primary session content;
+side-panel latency must not hold it behind an independent REST request. The
+initial `RemoteAgent` auth handler does not hydrate the workspace. `RemoteAgent`
+retains ownership of workspace hydration after non-initial authentication, so
+reconnects still refresh server state without duplicating the normal cold-load
+fetch.
+
+Workspace responses remain session-keyed. They can refresh the abandoned
+session's keyed cache, but compatibility/foreground mirrors update only when
+that session is still active. The session manager also checks the captured
+switch generation and selected session after asynchronous boundaries. Together,
+these guards prevent a late A response from overwriting B during an A→B switch,
+while preserving the exact cached agent and panel on A→B→A switch-back. The
+scheduling change does not remove hydration: side-panel tabs, active tab, size
+mode, proposals, and review documents still restore through their existing
+guarded paths. Workspace revision and conflict rules are unchanged.
+
+`tests2/dom/cold-session-workspace-ordering-repro.test.ts` pins this contract. It
+holds workspace hydration open while verifying that all 321 transcript rows
+render, `get_messages` precedes relevant REST requests, and only one initial
+workspace GET occurs. It also covers rapid navigation, pre-bind stale responses,
+workspace/review restoration, reconnect hydration, and cached switch-back. The
+committed test pins the 321-row render-before-workspace and single-fetch behavior;
+it does not run the seven timing samples, impose the benchmark's 250 ms delay, or
+assert the measurements below.
+
+## Controlled one-off cold-navigation evidence
+
+The following numbers are controlled one-off benchmark evidence, not a
+repository-reproducible benchmark or a timing threshold for arbitrary hosts. The
+fixed tree was `a4a811d75d05c76aabdab2108863a42c2b058cb5`; the measured
+pre-fix tree was `3ee000bb06b12e1f3bc5b80573cca9a67bafa427`. In the pre-fix
+tree, `connectToSession` awaited workspace hydration before requesting messages,
+while the initial `RemoteAgent` auth handler independently started a second
+workspace hydration.
+
+The exact source SHAs, environment, raw values, and method keep the result
+interpretable. However, the measurement harness and Vitest config lived under
+ignored `.bobbit/tmp` paths and were not retained in the repository. Re-running
+this comparison requires recreating that temporary fixture from the committed
+DOM regression test and running it against the actual fixed and pre-fix source
+trees; the commands below are therefore historical invocation records, not
+turnkey repeat commands.
+
+### Environment and method
+
+- Windows 11 `10.0.26200` x64; AMD Ryzen AI 9 HX 370; 24 logical CPUs; 63.1 GiB
+  RAM.
+- Node 24.13.1 / V8 13.6; Vitest 4.1.10 with happy-dom; one worker and
+  `retry: 0`.
+- The ignored fixture was derived from
+  `tests2/dom/cold-session-workspace-ordering-repro.test.ts`. It used a
+  321-record transcript and a real 250 ms `setTimeout` in the workspace endpoint
+  for seven sequential cold samples per variant.
+- Controlled WebSocket snapshots were delivered in a microtask. This excludes
+  real network latency, server assembly, and transport time so the comparison
+  isolates client request ordering and local rendering.
+- The clock started immediately before `connectToSession`, so this was a DOM cold
+  session switch—not a full app reload or deep-link. In contrast, the normal
+  full reload/deep-link instrumentation described above begins at browser
+  navigation, includes module and app boot, and writes its completed samples to
+  the JSONL sink.
+- Production code emitted `auth-ok`, `get-messages-sent`,
+  `snapshot-received(321 msgs)`, and `snapshot-applied`. Ready meant transcript
+  row 321 was present in the DOM and two `requestAnimationFrame` turns had
+  completed, matching the post-snapshot-paint boundary.
+- Baseline assertions required auth→request to be at least 235 ms and observed
+  two workspace GETs in every baseline sample. This proved the fixture exercised
+  the blocking and duplicate-hydration regression rather than merely comparing
+  noisy runs.
+
+### Recorded commands
+
+Fixed variant:
+
+```bash
+MEASURE_VARIANT=fixed MEASURE_SOURCE_SHA=a4a811d75d05c76aabdab2108863a42c2b058cb5 MEASURE_TRANSCRIPT_SIZE=321 MEASURE_SAMPLE_COUNT=7 MEASURE_WORKSPACE_DELAY_MS=250 node node_modules/vitest/vitest.mjs run --config .bobbit/tmp/cold-load-measure/vitest.measure.config.ts
+```
+
+Baseline variant, run from a temporary detached worktree at the pre-fix SHA:
+
+```bash
+MEASURE_ROOT=.bobbit/tmp/cold-baseline MEASURE_VARIANT=baseline MEASURE_SOURCE_SHA=3ee000bb06b12e1f3bc5b80573cca9a67bafa427 MEASURE_TRANSCRIPT_SIZE=321 MEASURE_SAMPLE_COUNT=7 MEASURE_WORKSPACE_DELAY_MS=250 node node_modules/vitest/vitest.mjs run --config .bobbit/tmp/cold-baseline/.bobbit/tmp/cold-load-measure/vitest.measure.config.ts
+```
+
+The referenced configs, generated measurement test, and temporary baseline
+worktree are absent from the repository. Recreate them before using these
+invocations.
+
+### Raw samples
+
+All timing values are milliseconds; values within each cell are in run order.
+
+| Metric | Pre-fix | Fixed |
+|---|---|---|
+| auth→request | `[255.375, 251.248, 253.282, 255.059, 259.189, 254.873, 261.206]` | `[0.471, 0.017, 0.011, 0.015, 0.015, 0.022, 0.014]` |
+| auth→snapshot | `[255.935, 251.566, 254.032, 255.401, 259.689, 255.894, 261.578]` | `[1.063, 0.290, 0.256, 0.254, 0.260, 0.543, 0.329]` |
+| navigation→ready | `[271.247, 258.330, 260.970, 259.569, 267.590, 269.112, 267.751]` | `[16.519, 5.747, 7.079, 3.856, 6.813, 5.835, 4.090]` |
+| Workspace GETs | `2` in every sample | `1` in every sample |
+
+### Median result
+
+| Metric | Pre-fix | Fixed | Change |
+|---|---:|---:|---:|
+| auth→request | 255.059 ms | 0.015 ms | −255.044 ms |
+| auth→snapshot | 255.894 ms | 0.290 ms | −255.605 ms (−99.9%) |
+| navigation→ready | 267.590 ms | 5.835 ms | −261.756 ms (−97.8%) |
+| Workspace GETs | 2 | 1 | one initial owner |
+
+The fixed transcript became ready about 244 ms before the delayed workspace
+response. For the fixed 321-record case, the median request→receipt time was
+0.273 ms, snapshot receipt→apply was 0.252 ms, and receipt→ready was 4.862 ms.
+The result isolates the intended change: transcript startup no longer inherits
+workspace latency, and duplicate initial hydration is gone.
+
+### Transcript-size scaling
+
+Fixed-tree medians from the same controlled setup show the remaining local cost:
+
+| Records | request→receipt | receipt→apply | receipt→ready | navigation→ready |
+|---:|---:|---:|---:|---:|
+| 1 | 0.084 ms | 0.025 ms | 0.565 ms | 1.057 ms |
+| 101 | 0.174 ms | 0.104 ms | 1.939 ms | 2.558 ms |
+| 321 | 0.273 ms | 0.252 ms | 4.862 ms | 5.835 ms |
+| 641 | 0.430 ms | 0.440 ms | 12.840 ms | 13.681 ms |
+
+This controlled DOM benchmark excludes real server assembly and network transfer
+latency, both of which remain after the request is sent immediately. Within the
+controlled path, rendering is now the dominant transcript-size-dependent local
+cost. This fix intentionally makes no snapshot protocol change. If real large
+histories show material latency, evaluate pagination or streaming for transfer
+and virtualized rendering for local paint as follow-up work.
+
 ## Tests
 
-- `tests/dev-boot-timing.test.ts` — sink: append/parse, dir creation, limit,
-  rejection of non-object/oversized samples, byte-cap trimming, malformed-line
-  skipping.
-- `tests/e2e/dev-boot-timing-api.spec.ts` — endpoint gating (403 off-harness),
-  write+read-back under the harness, 422 on a non-object body.
-- `tests/e2e/ui/perf-instrumentation-toggle.spec.ts` — toggle hidden without the
-  harness; visible/toggles/persists-across-reload and arms reload
-  instrumentation under the harness.
+- `tests2/dom/cold-session-workspace-ordering-repro.test.ts` — cold transcript
+  ordering, one initial workspace owner, stale navigation, reconnect, and cache
+  preservation.
+- `tests2/core/dev-boot-timing.test.ts` — sink append/parse, directory creation,
+  limits, malformed entries, and trimming.
+- `tests2/integration/dev-boot-timing-api.test.ts` — endpoint gating, write/read
+  under the harness, and invalid-body rejection.

docs/websocket-protocol.md

@@ -47,6 +47,80 @@ Bobbit keeps normal streaming responsive by bounding payloads before they enter
 
 Overflow diagnostics include `outerType`, `innerType` for `{ type: "event" }` frames, serialized `bytes`, recipient kind, and context such as `goalId`. These fields are the first place to look when a reconnect storm follows verification or reviewer activity.
 
+## Authenticated session work policy
+
+Live session sockets enforce `readOnly` and `nonInteractive` at the authenticated
+transport boundary, not only in the browser UI. The server checks both the live
+session and its persisted row; `true` in either source activates the restriction.
+This closes the window where one representation has updated before the other.
+If both restrictions apply, `readOnly` takes precedence.
+
+The guarded work classifier contains these frames:
+
+- Agent and queue work: `prompt`, `steer`, `steer_queued`, `remove_queued`,
+  `reorder_queue`, `retry`, `restart_agent`, `compact`.
+- Metadata and model writes: `set_title`, `generate_title`,
+  `summarize_goal_title`, `set_model`, `set_image_model`,
+  `set_thinking_level`.
+- Durable task and permission writes: `task_create`, `task_update`,
+  `task_delete`, `grant_tool_permission`.
+- Extension session writes: `ext_session_write_permit`, `ext_session_post`.
+
+### Read-only sessions
+
+Every guarded frame is rejected with `SESSION_READ_ONLY`. Ordinary guarded
+frames receive the generic error envelope:
+
+```json
+{ "type": "error", "code": "SESSION_READ_ONLY", "message": "..." }
+```
+
+Extension session-write callers require a correlated response so their Host API
+call does not wait for a generic error until timeout. They receive:
+
+```json
+{ "type": "ext_session_write_permit_result", "requestId": "...", "ok": false, "error": "SESSION_READ_ONLY" }
+```
+
+or:
+
+```json
+{ "type": "ext_session_post_result", "requestId": "...", "ok": false, "error": "SESSION_READ_ONLY" }
+```
+
+A read-only session has no streaming-steer exception.
+
+### Non-interactive sessions
+
+The response code identifies the rejected class:
+
+| Frame | Policy code |
+|---|---|
+| `prompt` | `NON_INTERACTIVE_PROMPT` |
+| `steer` while the current status is not `streaming` | `NON_INTERACTIVE_STEER` |
+| `steer_queued`, `remove_queued`, `reorder_queue` | `NON_INTERACTIVE_QUEUE_CONTROL` |
+| Every other guarded frame | `NON_INTERACTIVE_WORK_CONTROL` |
+
+A direct `steer` is permitted only while the session's current status is
+`streaming`; this allows an active automated review to be redirected without
+starting or queueing new reviewer work. The exception does not extend to
+`retry`, queue controls, extension posts, or any other guarded frame.
+
+Ordinary frames receive `{ "type": "error", "code": "<policy-code>",
+"message": "..." }`. As with read-only policy, `ext_session_write_permit` and
+`ext_session_post` instead return their respective request-correlated result
+envelope with the policy code in `error`.
+
+### Classifier scope
+
+`get_state`, `get_messages`, and `ping` remain available under these policies.
+`abort` and `deny_tool_permission` also remain outside the guarded work
+classifier because they decrease or stop active work. `resume` and the
+separately authorized extension channel and surface operations are likewise not
+classified here. This section documents only the session-work policy; those
+frames can still be rejected by their own authentication, authorization,
+lifecycle, validation, size, or replay rules.
+
 ## Client → Server
 
 | Type | Fields | Description |

src/app/remote-agent.ts

@@ -49,7 +49,7 @@ import { showFaviconBadge } from "./favicon-badge.js";
 import { isEffectivePlayFinishSoundEnabled, type FinishSoundSource } from "./play-finish-sound.js";
 import { needsHumanAttentionOnIdleTransition, needsImmediateHumanAttention } from "./notification-policy.js";
 import { scheduleGateStatusRefreshForGoal, refreshSessions, scheduleSessionListRefreshFromPush, scheduleStaffListRefreshFromPush } from "./remote-agent-refresh.js";
-import { applySidePanelWorkspaceFromServer, getSidePanelWorkspace, hydrateSidePanelWorkspace } from "./side-panel-workspace.js";
+import { applySidePanelWorkspaceFromServer, closeSidePanelTab, getSidePanelWorkspace, hydrateSidePanelWorkspace } from "./side-panel-workspace.js";
 import { shouldRefreshGateStatusForEvent } from "./gate-status-events.js";
 import { publishClientMessage, publishClientStatus } from "./session-event-bus.js";
 import { registerSessionPoster, unregisterSessionPoster, type SessionPostRequest } from "./session-write-bridge.js";
@@ -713,6 +713,37 @@ export class RemoteAgent {
 	get gatewaySessionId() {
 		return this._sessionId;
 	}
+	/**
+	 * Remove review tabs restored from the server after this session's review
+	 * was already submitted. Initial/reconnect workspace hydration can finish
+	 * after transcript replay performed the same cleanup against an empty local
+	 * workspace, so callers replay it at the workspace-apply boundary.
+	 *
+	 * This intentionally has no active-session guard: cached/background sessions
+	 * still own their keyed workspace and must not retain a submitted review tab.
+	 */
+	async reconcileSubmittedReviewWorkspace(): Promise<void> {
+		const sessionId = this._sessionId;
+		if (!sessionId || !isReviewSubmitted(sessionId)) return;
+		const isReviewTab = (tab: { id?: string; kind?: string }): boolean =>
+			tab.kind === "review" || tab.id?.startsWith("review:") === true;
+		const reviewTabIds = getSidePanelWorkspace(sessionId).tabs
+			.filter(isReviewTab)
+			.map((tab) => tab.id);
+		if (reviewTabIds.length === 0) return;
+
+		// Keep every deletion on the normal mutation path. A confirmed compatible
+		// 204 settles the optimistic close, while a 409 workspace remains
+		// authoritative and may be retried once at its newer revision. Refetches,
+		// network failures, and rollbacks are never locally filtered afterward.
+		for (const tabId of reviewTabIds) {
+			try {
+				await closeSidePanelTab(tabId, { sessionId, retryConflictOnce: true });
+			} catch (err) {
+				console.warn("[RemoteAgent] submitted-review workspace cleanup failed:", err);
+			}
+		}
+	}
 	private _isActiveSession(): boolean {
 		return this._sessionId !== "" && state.selectedSessionId === this._sessionId;
 	}
@@ -843,7 +874,13 @@ export class RemoteAgent {
 						this._reconnectAttempt = 0;
 						this._setConnectionStatus("connected");
 						resolve();
-						void hydrateSidePanelWorkspace(this._sessionId);
+						// Initial hydration is owned by connectToSession after ChatPanel
+						// binding. Reconnects still refresh the server workspace here and
+						// then replay submitted-review cleanup against the hydrated tabs.
+						if (!initial) {
+							void hydrateSidePanelWorkspace(this._sessionId)
+								.then(() => this.reconcileSubmittedReviewWorkspace());
+						}
 						// S2: deliver any prompts/steers/retries the user issued while
 						// the socket was reconnecting, before resume/snapshot traffic.
 						this._flushOutbox();
@@ -1778,7 +1815,7 @@ export class RemoteAgent {
 								if (!this._isActiveSession()) break;
 							}
 						} else {
-							closeReviewWorkspaceTabs(undefined, { sessionId: reviewSessionId, select: false });
+							await this.reconcileSubmittedReviewWorkspace();
 						}
 						if (this._isActiveSession()) {
 							const reviewSources = await loadReviewSources();