From 4e636511ae748d606d8871f5068a2bd18b386bd0 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Sat, 30 May 2026 23:15:18 +0900 Subject: chore(notes): collect loose root docs into notes/; add reconcile edge-cases note Move all loose root-level .md files (plans, reports, gemini reviews, incident notes) into a single notes/ directory, and update the doc-reference breadcrumbs in code comments/test labels to the notes/ path. Add notes/queue-interrupt-reconcile-edge-cases.md: documents why the queue/interrupt/turn-sealed reconcile path keeps surfacing edge cases (a catalog of the four review-pass bugs, the no-loss/no-duplicate invariants, the recommended membership-based reconcile refactor, and interleaving-test guidance). --- notes/cache-miss-report.md | 180 ++++ notes/changes-report.md | 54 ++ notes/changes.md | 133 +++ notes/claude-auth-report.md | 282 ++++++ notes/claude-report.md | 114 +++ notes/context.md | 169 ++++ notes/eviction-limitation.md | 105 ++ notes/gemini-chunk-eviction-review-2.md | 95 ++ notes/gemini-chunk-eviction-review-3.md | 118 +++ notes/gemini-chunk-eviction-review.md | 69 ++ notes/gemini-chunk-log-review.md | 90 ++ notes/harness-comparison.md | 373 +++++++ notes/plan-bg-restore.md | 1294 +++++++++++++++++++++++++ notes/plan-chunk-eviction.md | 251 +++++ notes/plan-chunk-log.md | 271 ++++++ notes/plan-chunk-refactor.md | 245 +++++ notes/plan-v6-upgrade.md | 450 +++++++++ notes/plan.md | 451 +++++++++ notes/problem.md | 79 ++ notes/queue-interrupt-reconcile-edge-cases.md | 183 ++++ notes/report.md | 38 + notes/requirements.md | 353 +++++++ notes/tool-runner-duplication-incident.md | 147 +++ notes/wishlist.md | 26 + 24 files changed, 5570 insertions(+) create mode 100644 notes/cache-miss-report.md create mode 100644 notes/changes-report.md create mode 100644 notes/changes.md create mode 100644 notes/claude-auth-report.md create mode 100644 notes/claude-report.md create mode 100644 notes/context.md create mode 100644 notes/eviction-limitation.md create mode 100644 notes/gemini-chunk-eviction-review-2.md create mode 100644 notes/gemini-chunk-eviction-review-3.md create mode 100644 notes/gemini-chunk-eviction-review.md create mode 100644 notes/gemini-chunk-log-review.md create mode 100644 notes/harness-comparison.md create mode 100644 notes/plan-bg-restore.md create mode 100644 notes/plan-chunk-eviction.md create mode 100644 notes/plan-chunk-log.md create mode 100644 notes/plan-chunk-refactor.md create mode 100644 notes/plan-v6-upgrade.md create mode 100644 notes/plan.md create mode 100644 notes/problem.md create mode 100644 notes/queue-interrupt-reconcile-edge-cases.md create mode 100644 notes/report.md create mode 100644 notes/requirements.md create mode 100644 notes/tool-runner-duplication-incident.md create mode 100644 notes/wishlist.md (limited to 'notes') diff --git a/notes/cache-miss-report.md b/notes/cache-miss-report.md new file mode 100644 index 0000000..03342af --- /dev/null +++ b/notes/cache-miss-report.md @@ -0,0 +1,180 @@ +# Cache Miss Investigation — Dispatch / Claude prompt caching + +> Read-only investigation. No code was modified. File:line references are to the +> state of the tree at the time of writing. + +## TL;DR + +The beta header and cache-breakpoint placement are **correct**. The cache misses +come from a **message-serialization instability inside multi-step turns**: +dispatch stores an entire multi-step assistant turn as a *single growing +assistant message*, and `toModelMessages` + the Anthropic Pass-3 normalization +**re-bucket every tool-call and every tool-result on every step**. This moves +earlier steps' `tool_use` / `tool_result` blocks to new positions each step, so +Anthropic can only match the prefix up to the *first* step's text/thinking. +Everything after is re-written as a new cache entry. + +Result: `cache_write` grows every step while `cache_read` stays flat — exactly +the panel readout (write 100,470 ≫ read 40,448; last request 19% < session 29%). + +## Evidence (the Cache Rate panel) + +``` +readCache hits 40,448 +writeCache writes 100,470 +freshUncached input 18 +Total input 140,936 (= read + write + fresh, confirms inputTokens is the TOTAL prompt) +Output 5,130 +9 req +Session (this tab) 29% +Last request 19% +``` + +Two tells: + +1. **writes ≫ reads.** In a healthy rolling cache over a growing turn, reads + accumulate and dominate writes. Here writes are ~2.5× reads → the cacheable + prefix is being invalidated and re-written almost every request. +2. **Last request (19%) < cumulative (29%).** The *largest* request has the + *lowest* hit rate. In a working rolling cache the last turn should be the + *highest* (biggest cached prefix, smallest delta). The inversion means the + newest, biggest request re-wrote the most. + +## What is fine (ruled out) + +- **Beta header present** — `prompt-caching-scope-2026-01-05` is sent + (`packages/core/src/credentials/anthropic-betas.ts:13-20`, wired in + `packages/core/src/llm/provider.ts:123`). The earlier `claude-report.md` + "Root Cause 1" (missing beta) is fixed. +- **Breakpoint placement matches OpenCode exactly** — dispatch's + `applyAnthropicCaching` (`packages/core/src/agent/agent.ts:448-466`) marks + first-2 system + last-2 non-system messages at the *message* level, identical + to OpenCode's `applyCaching` + (`references/opencode/packages/opencode/src/provider/transform.ts:345-394`) + for `providerID === "anthropic"`. +- **System prompt is deterministic** — `buildSystemPrompt` + (`packages/api/src/agent-manager.ts:127-147`) has no date/cwd/env/timestamp. + The billing-header `cch` derives from the first user message + (`packages/core/src/credentials/claude.ts:354-372`), stable within a + conversation. So the `tools + system` prefix does **not** churn. +- **OAuth body transform is a faithful port** of the reference plugin + (`packages/core/src/llm/anthropic-oauth-transform.ts` ≈ + `references/opencode-claude-auth/src/transforms.ts`). + +## Root cause (primary) — multi-step turns reshuffle their own prefix every step + +### The architecture + +A whole turn (all tool steps) accumulates into ONE assistant message whose +`chunks` array is shared across steps (`agent.ts:786`, pushed once at +`agent.ts:923-926`). Each step's first `tool-call` opens a *new* `tool-batch` +chunk because the preceding chunk is text/thinking +(`packages/core/src/chunks/append.ts:111-124`). So after 2 steps the single +message is: + +``` +assistant.chunks = [ text0, think0, batch0{A:+resultA}, text1, think1, batch1{B:+resultB} ] +``` + +### The serialization + +`toModelMessages` (`agent.ts:162-256`) walks all chunks of that one message, +pushing **all** tool-calls into `parts` and **all** results into a single +trailing `tool` message. Then `applyAnthropicStructuralNormalisations` Pass 3 +(`agent.ts:399-413`) splits the assistant message because tool-calls are +followed by later-step text. + +Concrete trace of the request prefix: + +``` +# Step-1 request (after step 0) # Step-2 request (after step 1) +assistant:[text0, think0, callA] assistant:[text0, think0, text1, think1] <- Pass-3 non-tool +tool:[resultA] assistant:[callA, callB] <- Pass-3 tool bucket + tool:[resultA, resultB] +``` + +After the `@ai-sdk/anthropic` converter merges the two consecutive assistant +messages, the single Anthropic assistant turn is: + +- step 1 cached: `[text0, think0, tool_use_A]` +- step 2 sends: `[text0, think0, text1, think1, tool_use_A, tool_use_B]` + +The two diverge **right after `think0`** (cached had `tool_use_A` next; step 2 +has `text1`). Anthropic's longest-prefix match ends at roughly +`static + user1 + text0 + think0`. Everything from there on — including +`resultA`, which was the cached tail one step earlier but is now re-grouped with +`resultB` — becomes a **cache write**. + +Every additional step pushes all prior `tool_use` blocks further back and +re-groups all results, so: + +- the matchable read prefix stays ≈ constant (static prefix + first step), +- the re-written suffix grows with the entire turn, +- cumulative `cache_write` balloons, `cache_read` stays low, and the **largest + (last) request has the lowest hit rate**. + +This reproduces every symptom in the panel. + +### Why it slipped through + +The caching tests (`packages/core/tests/agent/agent.test.ts:1034`, `:1078`) only +exercise a *single* step with parallel calls (3 reads → one tool message), which +is correct. The **multi-step accumulation path has no test**, and that's the +path that churns. + +### Contrast with OpenCode + +OpenCode's native loop appends each step as its own stable `assistant` + `tool` +message pair (`[system, user, assistant0, tool0, assistant1, tool1, …]`). Those +never move, so its rolling cache accumulates. Dispatch's "one growing assistant +message, re-derive the split each step" is the divergence. + +## Secondary findings (smaller, worth noting) + +1. **Per-turn session-id rotation.** `createProvider` runs inside `run()` and + mints a new `X-Claude-Code-Session-Id` per turn (`provider.ts:92`, called at + `agent.ts:745`). Within a turn it's stable, but if the `prompt-caching-scope` + beta keys cache by session, cross-turn reads are lost on top of the + within-turn churn. *Confidence: medium — verify against Anthropic's scope + semantics.* +2. **Reasoning empty-text filter diverges from OpenCode.** Dispatch drops any + `reasoning` part with `text === ""` (`agent.ts:356-360`); OpenCode keeps it + when a signature / `redactedData` is present (`transform.ts:144-150`). + Dropping a signed-but-empty thinking block changes assistant bytes and can + break thinking-signature round-trips. Minor vs. the primary issue. +3. **5-minute ephemeral TTL** — generic: idle gaps > 5 min between turns force a + cold re-write regardless of the above. + +## Recommended fix direction (not yet implemented) + +Make the wire history **stable across steps** by emitting one `assistant` + one +`tool` message **per step (per `tool-batch` chunk)** instead of collapsing the +whole turn into one assistant message and one trailing tool message. Concretely, +in `toModelMessages`, segment the assistant chunks at each `tool-batch` boundary +and emit `[assistant(text, think, tool-calls), tool(results)]` per segment. This: + +- keeps the intended within-step grouping (parallel calls in one step → one tool + message), satisfying the existing "Root Cause 2" tests, +- preserves earlier steps' block positions so the rolling cache accumulates, +- removes the Pass-3 reshuffle trigger (no later-step text after a tool-call + within a single message), +- and matches OpenCode's stable message layout. + +Add a regression test for a **3-step sequential** turn asserting the step-0 and +step-1 message blocks are byte-identical between the step-2 and step-3 requests. + +## Key files + +| Area | Location | +| --- | --- | +| Turn accumulator (single assistant msg) | `packages/core/src/agent/agent.ts:786`, `:923-926` | +| New tool-batch chunk per step | `packages/core/src/chunks/append.ts:111-124` | +| Rebuild + group all results | `packages/core/src/agent/agent.ts:162-256` | +| Pass-3 reshuffle (split) | `packages/core/src/agent/agent.ts:399-413` | +| Breakpoints (faithful port) | `packages/core/src/agent/agent.ts:448-466` | +| OpenCode reference `applyCaching` | `references/opencode/packages/opencode/src/provider/transform.ts:345-394` | +| Beta headers | `packages/core/src/credentials/anthropic-betas.ts:13-20` | +| Per-run session id | `packages/core/src/llm/provider.ts:92` | +| OAuth body transform | `packages/core/src/llm/anthropic-oauth-transform.ts` | +| Cache Rate panel + aggregation | `packages/frontend/src/lib/components/CacheRatePanel.svelte`, `packages/frontend/src/lib/tabs.svelte.ts:856-877` | +| Caching tests (single-step only) | `packages/core/tests/agent/agent.test.ts:1034`, `:1078` | diff --git a/notes/changes-report.md b/notes/changes-report.md new file mode 100644 index 0000000..f30b611 --- /dev/null +++ b/notes/changes-report.md @@ -0,0 +1,54 @@ +# Changes Report + +## Overview +This report reviews the uncommitted changes in the `dispatch` repository. The changes can be broadly categorized into two parts: +1. **Codebase Updates**: The removal of the `todo` (task list) tool and a fix to a `summon` tool test. +2. **Documentation Additions**: Several new untracked markdown documents regarding planning and incident reports. + +## Per-File Analysis + +### Untracked Files +- **`claude-auth-report.md`** & **`tool-runner-duplication-incident.md`**: New post-mortem and incident investigation reports. +- **`cyberdeck/credentials.md`**: Documentation regarding credentials. +- **`skill-plan/` directory**: Multiple markdown files outlining a multi-step plan for building a new "skill" architecture (tool definitions, registration, routes, etc.). + +**Assessment**: Adding detailed markdown documents for incidents and project planning is a very good practice. They are cleanly separated from the application source code. + +### Modified Files (Staged & Unstaged) + +#### 1. `packages/core/src/tools/task-list.ts` +- **Change**: Removed the `createTaskListTool` function and the Zod/ToolDefinition imports. The `TaskList` class remains. +- **Correctness**: The removal of the factory is clean. + +#### 2. `packages/api/src/agent-manager.ts` +- **Change**: Removed `createTaskListTool` imports and invocations. Removed `todo` from the `TOOL_DESCRIPTIONS` and the lengthy `TODO_GUIDANCE` string from the agent's system prompt. +- **Correctness**: Consistently applies the removal of the `todo` tool. However, it still retains `tabAgent.taskList = new TaskList()`, which may now be dead state (see Issues section). + +#### 3. `packages/core/src/agents/loader.ts` & `packages/core/src/tools/summon.ts` +- **Change**: Removed `"todo"` from the default tool arrays and documentation comments for child agents. +- **Correctness**: Follows through with the removal of the `todo` tool across tool lists, ensuring child agents don't request a missing tool. + +#### 4. `packages/core/src/index.ts` +- **Change**: Updated exports to remove `createTaskListTool`, while keeping `TaskList`. +- **Correctness**: Correctly reflects the module changes. + +#### 5. Tests (`packages/api/tests/*.test.ts` & `packages/core/tests/agents/loader.test.ts`) +- **Change**: Removed mocked `createTaskListTool` injections and updated assertions that previously verified the automatic injection of the `todo` tool. +- **Correctness**: Properly aligns tests with the new implementation. Build/tests should pass cleanly. + +#### 6. `packages/core/tests/tools/summon.test.ts` +- **Change**: Updated the "surfaces child errors when blocking" test to correctly anticipate the `agent_id: ` prefix before the child output. Added an excellent explanatory comment about why this prefix exists (for frontend `ToolCallDisplay` regex parsing). +- **Correctness**: The change is highly robust. Explicitly asserting `.toContain()` before the final `.toBe()` provides great debugging signals if the test fails in the future. The comment is an excellent practice. + +## Correctness Assessment +The changes successfully and cleanly remove the `todo` tool logic from the backend tools and system prompt. The test fix for `summon.test.ts` is technically sound and well-documented. The changes are internally consistent from a backend tooling perspective. + +## Issues & Concerns Found +**Incomplete Refactoring (Dead Code)**: +While the `todo` tool was completely removed, the `TaskList` class itself and its instantiation in `AgentManager` (`tabAgent.taskList = new TaskList()`) were kept. Because the agent no longer has a tool to interact with this list, any tasks will remain empty. +Consequently, the UI component that relies on this (`TaskListPanel.svelte` in the `frontend` package) will now always render an empty state. This represents an incomplete feature removal/refactor. + +## Recommendations +1. **Clean Up `TaskList`**: If the todo feature is permanently removed, you should also delete the `TaskList` class from `packages/core`, remove the `taskList` property from `tabAgent` in `packages/api`, and delete `TaskListPanel.svelte` (along with its imports in `SidebarPanel.svelte`) from the frontend. +2. **Staging**: If the untracked markdown documents are ready, ensure they are intentionally added (`git add`) and committed. +3. **Commit the Changes**: The code removal of the `todo` tool is safe to commit. I recommend summarizing it as "refactor: remove todo tool from agent capabilities". \ No newline at end of file diff --git a/notes/changes.md b/notes/changes.md new file mode 100644 index 0000000..66389d9 --- /dev/null +++ b/notes/changes.md @@ -0,0 +1,133 @@ +# Changes + +## May 27, 2026 + +### Chunk-Based Message Refactor (`ca6ee91`) + +Replaced the flat `content: string` + `thinking: string` message model with an ordered +`chunks: Chunk[]` union that preserves actual temporal ordering of events from the model. + +**New chunk types:** + +| Type | Body | Emitted on | +|------|------|-----------| +| `text` | `text: string` | `text-delta` events, coalesced | +| `thinking` | `text: string` | `reasoning-delta` events, coalesced | +| `tool-batch` | `calls: Array<{id, name, arguments, result?, isError?, shellOutput?}>` | `tool-call` events, batched | +| `error` | `message: string, statusCode?: number` | Error events | +| `system` | `text: string, kind` | System notices (model-changed, config-reload, cancelled, rate-limit) | + +**Key design decisions:** +- System events during active turn append inline to the assistant message's chunks +- System events outside turns create/append `role: "system"` messages +- `toCoreMessages` strips `error`/`system` chunks and `role: "system"` messages +- `MessageRole` changed from `user | assistant | tool` → `user | assistant | system` +- Tool calls/results embedded in `tool-batch` chunks, no separate `role: "tool"` messages + +**Files changed:** +- `packages/core/src/types/index.ts` — `Chunk` union, `MessageRole` update +- `packages/frontend/src/lib/types.ts` — mirrored types +- `packages/core/src/chunks/append.ts` — `appendEventToChunks()` state machine + `applySystemEvent()` router +- `packages/core/tests/chunks/append.test.ts` — 35 unit tests +- `packages/core/src/db/index.ts` — removed `thinking` column from messages schema +- `packages/core/src/db/messages.ts` — updated `appendMessage`/`getMessagesForTab` +- `packages/core/src/agent/agent.ts` — single `chunks[]` accumulator, updated `toCoreMessages` +- `packages/api/src/agent-manager.ts` — progressive persistence, system event routing +- `packages/frontend/src/lib/tabs.svelte.ts` — unified `applyChunkEvent`, `openAgentTab` reads chunks +- `packages/frontend/src/lib/components/ChatMessage.svelte` — per-type chunk renderers +- `packages/frontend/src/lib/components/ToolCallDisplay.svelte` — prop updates + +**Database:** Messages and tabs tables dropped; settings, keys, credentials preserved. +Backup at `~/.local/share/dispatch/dispatch.db.bak-20260527-181334`. + +--- + +### Frontend Fixes + +#### Wire-format drift (`5261879`) + +`openAgentTab` expected `contentJson: string` on the wire but the API now returns +`chunks: Chunk[]`. Fixed to read `m.chunks` directly with `Array.isArray` fallback. + +Also added diagnostic debug info to `copyConversation`: store state block +(connection status, agentStatus, message counts) and per-message chunk summaries. + +#### structuredClone → $state.snapshot (`faeb8fe`) + +Svelte 5 `$state` proxies throw `DataCloneError` on native `structuredClone()`. +Fixed by switching to `$state.snapshot()` in `applyChunkEvent` and `routeSystemEvent`. +This was the root cause of `chunks=0` in production — every content event after +placeholder creation silently failed. + +#### WS error swallowing (`faeb8fe`) + +`ws.svelte.ts` wrapped all callbacks in a single `try{} catch{}` that swallowed +errors. Split into per-callback try/catch with `console.error`. Future bugs of +this class are now diagnosable from the browser console. + +#### statuses reconnect handler (`faeb8fe`) + +Added `statuses` variant to `AgentEvent` union. On WS reconnect, handler syncs +`agentStatus` for all tabs, detects desync (frontend thinks running, backend says +idle/error), calls `reloadTabMessagesFromApi` to pull persisted chunks, and clears +`currentAssistantId` and streaming flags. + +--- + +### Model Routing Fix (`9ac04b9`) + +When a user selected a model (e.g., Gemini via configured key) but the corresponding +API key environment variable was not set, `getOrCreateAgentForTab` in `packages/api/src/agent-manager.ts:610` +set `useOverride = true` without updating `model` or `baseURL` from their defaults. +The request silently went to `https://opencode.ai/zen/go/v1` with `model: "deepseek-v4-flash"` +and no API key — OpenCode Go routed this to Claude, causing every model selection +to respond with "I'm Claude, made by Anthropic." + +Fixed by setting `baseURL = key.base_url` and `model = effectiveModelId` in the +missing-key branch so requests target the correct endpoint and produce a diagnosable +auth error instead of a silent model-swap. + +--- + +### Test Infrastructure Rewrite (`1e3f67e`) + +Replaced the POJO (plain-old-JavaScript-object) test harness in `packages/frontend/tests/chat-store.test.ts` +with real `$state`-backed store instances via an exported `createTabStore()` factory +and `handleEvent()` method. + +**What this catches that the old harness couldn't:** +- Logic bugs in the actual `handleEvent` / `applyChunkEvent` / `routeSystemEvent` code +- Drift between harness and production (now the same code) +- Reactivity contract issues with real `$state` proxies + +**Known limitation:** The `structuredClone(svelteProxy)` bug cannot be reproduced in +these tests because Bun's `structuredClone` (used by vitest) is more permissive than +browser `structuredClone`. Catching that class of bug requires a browser-runtime test +layer (Playwright, vitest browser mode). + +**Mocks:** `wsClient`, `config`, and `fetch` are mocked so module-load side effects +(WebSocket connection, localStorage access, HTTP calls) don't interfere. + +**Files:** +- `packages/frontend/src/lib/tabs.svelte.ts` — exported `createTabStore`, added `handleEvent` +- `packages/frontend/tests/chat-store.test.ts` — 32 tests through the real reactive store + +--- + +### Earlier: Read-System Fixes + +#### Path resolution (`da57842`) + +`read-file.ts`, `read-file-slice.ts`, `write-file.ts`, and `list-files.ts` used +`resolve(join(workingDirectory, path))` which mangled absolute paths (e.g., +spill paths like `/tmp/dispatch/tool-results/...`). `join()` concatenates rather +than short-circuiting on absolute segments. + +Fixed to use a shared `canonicalize()` helper in `packages/core/src/tools/path-utils.ts` +that resolves via `realpath` and walks up to the nearest existing ancestor when the +leaf doesn't exist (handles `write_file` creating new files through symlinked parent dirs). + +#### DEFAULT_LIMIT alignment + +Changed `DEFAULT_LIMIT` from 2000 → `MAX_LINES` (500) in `read-file.ts` so default +reads don't always trigger truncator spills. diff --git a/notes/claude-auth-report.md b/notes/claude-auth-report.md new file mode 100644 index 0000000..db2c6e6 --- /dev/null +++ b/notes/claude-auth-report.md @@ -0,0 +1,282 @@ +# Claude Auth Report — "old tab 401s, new tab works" + +## Symptom + +A tab that has been open for a while starts failing **every** Claude request with: + +``` +Invalid authentication credentials +status=401 authentication_error +url=https://api.anthropic.com/v1/messages +model=claude-opus-4-8 baseURL=https://api.anthropic.com/v1 +``` + +Opening a **new tab** and selecting the same Claude key works fine. The old tab +keeps 401-ing no matter what you send. Both tabs are configured against the same +`anthropic` key (`claude-pro` / `claude-max`), so "they should be using the same +auth" — but they are not. + +This is an **authentication-layer** problem, not an agent/tool-wiring/permission +bug. The 401 comes back from Anthropic on the chat request itself. + +--- + +## Decisive clue: new tab works, old tab doesn't + +This single fact rules out most hypotheses and points at exactly one mechanism: +**the per-tab `Agent` instance is cached, and it freezes the access token it was +constructed with.** A new tab builds a fresh `Agent`, re-resolves credentials +from the DB, and gets a currently-valid token. The old tab never re-resolves. + +--- + +## Root cause #1 (proximate, confirmed): the cached per-tab Agent freezes a stale access token + +### Where the token is captured + +Credentials are resolved **only when a new `Agent` is constructed**, inside +`getOrCreateAgentForTab` in `packages/api/src/agent-manager.ts`: + +- The `anthropic` branch (lines ~579–636) resolves `claudeCredentials = + { accessToken: }` and passes it into the `Agent` constructor + (lines ~686–700, `...(claudeCredentials ? { claudeCredentials } : {})`). +- That token is stored on the Agent's `this.config.claudeCredentials` and never + updated for the life of the Agent. + +In `packages/core/src/agent/agent.ts`, every `run()` call rebuilds the provider +from that frozen config: + +```ts +const providerFactory = createProvider({ + apiKey: this.config.apiKey, + baseURL: this.config.baseURL, + provider: this.config.provider, + claudeCredentials: this.config.claudeCredentials, // <-- frozen at construction +}); +``` + +And in `packages/core/src/llm/provider.ts`, `createClaudeOAuthProvider` sends it +as the bearer token: + +```ts +authToken: config.claudeCredentials?.accessToken ?? config.apiKey, +``` + +So the access token used on the wire is whatever was captured when the tab's +Agent was first built. Calling `run()` again does **not** refresh it. + +### Why the cache is never invalidated on expiry + +`getOrCreateAgentForTab` (agent-manager.ts lines ~361–368) only discards the +cached Agent when the key, model, or permissions change: + +```ts +if ( + tabAgent.agent && + (effectiveKeyId !== tabAgent.keyId || + effectiveModelId !== tabAgent.modelId || + permKey !== tabAgent._lastPermKey) +) { + tabAgent.agent = null; +} +``` + +**Token expiry is not one of the conditions.** A tab that keeps the same key + +model + permissions will reuse the same `Agent` — and therefore the same frozen +token — indefinitely. The credential-refresh code below this gate only runs when +`tabAgent.agent` is null, which for a stable tab never happens again. + +### Why this produces the exact symptom + +- **Old tab:** built its `Agent` earlier with token **A**. Time passes. Token A + is no longer accepted by Anthropic (it either reached its ~8h TTL, or a refresh + elsewhere — a new tab, the Model Status panel, model listing — rotated the + credential in the DB and **revoked A** server-side). The cached Agent still + holds A. Cache gate sees no key/model/perm change → Agent reused → every + request sends the dead token A → `401 authentication_error`. +- **New tab:** no cached Agent → runs the resolution path → reads the **current** + token from the DB (and/or refreshes) → token **B** → works. + +Both tabs "use the same key," but the old tab is pinned to a now-invalid +*instance* of that key's token. That is the whole discrepancy. + +> Note on OAuth token rotation: Anthropic's refresh flow rotates the refresh +> token and can invalidate the previously issued access token. So the moment any +> code path successfully refreshes the credential (updating the DB), every +> already-constructed Agent still holding the old access token is now holding a +> *revoked* token — not merely an expired one. This is why the old tab can fail +> even when its captured token's local `expiresAt` hasn't elapsed yet. + +--- + +## Root cause #2 (contributing, confirmed by reference): the OAuth refresh call is malformed + +Even when the resolution path *does* run, the refresh half of it is broken, so +the system can't reliably self-heal an expired token. + +`packages/core/src/credentials/claude.ts`: + +```ts +const OAUTH_TOKEN_URL = "https://claude.ai/v1/oauth/token"; // line 27 — wrong host + +async function refreshViaOAuth(refreshToken: string) { + const body = new URLSearchParams({ grant_type: "refresh_token", client_id: OAUTH_CLIENT_ID, refresh_token: refreshToken }); + const response = await fetch(OAUTH_TOKEN_URL, { + method: "POST", + headers: { "Content-Type": "application/x-www-form-urlencoded" }, // wrong content-type + body: body.toString(), // form-encoded + }); + if (!response.ok) return null; // failure is swallowed silently + ... +} +``` + +Two concrete defects, both contradicted by the working reference implementation +in `references/oh-my-pi/packages/ai/src/utils/oauth/anthropic.ts`: + +1. **Wrong endpoint host.** The token exchange/refresh endpoint lives on the API + host, not the web-app host: + ```ts + const TOKEN_URL = "https://api.anthropic.com/v1/oauth/token"; // reference, line 11 + ``` + The reference's test suite asserts exactly this URL + (`references/oh-my-pi/packages/ai/test/anthropic-oauth.test.ts`). `claude.ai` + is correct only for the *authorize* step (`https://claude.ai/oauth/authorize`). + +2. **Wrong body format.** Anthropic's `/v1/oauth/token` expects **JSON**, not + form-encoding. The reference posts: + ```ts + headers: { "Content-Type": "application/json", Accept: "application/json" }, + body: JSON.stringify(body), + ``` + +Because both are wrong, `refreshViaOAuth` effectively always returns `null`. When +a token has genuinely expired, the manager's `anthropic` branch hits its final +`else` and does the worst possible thing — proceeds with the **stale** token: + +```ts +console.warn(`dispatch: unable to refresh Claude credentials for "${account.label}" — using stale token`); +claudeCredentials = { accessToken: account.credentials.accessToken }; // expired +... +useOverride = true; +``` + +So the contributing failure mode is: +- New tabs work **only as long as the DB token is still valid** (e.g. because + Claude Code refreshed it on disk and it was re-imported, or it simply hasn't + expired yet). Dispatch's *own* refresh cannot renew it. +- Once the DB token expires with no external renewal, even new tabs will 401, + and the warning above appears in the server log. + +**Diagnostic check:** look in the server logs for +`dispatch: unable to refresh Claude credentials for "..." — using stale token`. +Its presence confirms the refresh path is failing and the stale-token fallback +fired. + +> History: both the wrong URL and the form-encoded body were introduced in the +> original commit `8151447` ("feat: claude max oauth support…"). They have never +> been correct in this repo. + +--- + +## Secondary suspect (unverified): the chat provider omits `anthropic-beta` + +`createClaudeOAuthProvider` (provider.ts lines ~76–84) sets only: + +```ts +headers: { + "anthropic-dangerous-direct-browser-access": "true", + "x-app": "cli", + "user-agent": "claude-cli/2.1.112 (external, sdk-cli)", +} +``` + +It does **not** send `anthropic-beta: …,oauth-2025-04-20,…` or +`anthropic-version`. Every other path in this codebase and in the reference does: + +- `getAnthropicHeaders()` (claude.ts:387) builds the full set, including + `oauth-2025-04-20`, and is used by the models/profile/usage calls. +- The reference's chat path always sends `anthropic-beta: ANTHROPIC_OAUTH_BETA` + with the OAuth bearer (`references/oh-my-pi/.../provider-models/openai-compat.ts`, + `.../providers/anthropic.ts:1542`). Anthropic generally requires the + `oauth-2025-04-20` beta for Bearer/OAuth requests. + +This is flagged **secondary** because chat works at all on a fresh token, which +suggests `@ai-sdk/anthropic` may inject the oauth beta itself for `authToken` +requests. **I could not verify this** — the workspace deps are not installed in +this environment (`node_modules/@ai-sdk/anthropic` is absent) and outbound +network is blocked, so the SDK could not be inspected and the request could not +be reproduced live. If fixing #1 and #2 doesn't fully resolve things, make this +provider reuse `getAnthropicHeaders()` so the chat path carries the same +`anthropic-beta` / `anthropic-version` as every other Anthropic call. + +--- + +## Minor, unrelated observation + +`agent.ts:836` gates adaptive thinking on a hardcoded model string: + +```ts +const isOpus47 = this.config.model === "claude-opus-4-7"; +``` + +The failing model is `claude-opus-4-8`, so it silently takes the non-adaptive +`thinking: { type: "enabled", budgetTokens }` branch. Not related to the 401, but +that literal will need updating for opus-4-8 to get adaptive thinking. + +--- + +## Recommended fixes + +In priority order: + +### 1. Stop freezing the token in the cached Agent (fixes the new-vs-old-tab bug) + +Pick one of: + +- **Re-resolve credentials per `run()` for `anthropic` keys**, instead of caching + them in the Agent's config. e.g. have the Agent pull the access token through a + callback/getter at request time (`() => refreshAccountCredentials(account)`), + so each `run()` uses the live DB token. This is the cleanest fix. +- **Or** invalidate the cached Agent when its captured token is near/after + expiry. Add an expiry check to the cache-invalidation gate in + `getOrCreateAgentForTab` (store the captured `expiresAt` on `tabAgent` and null + the agent when `Date.now() > expiresAt - 60_000`). Coarser, but localized. +- **Or** when a refresh rotates the DB token, proactively null every cached + `tabAgent.agent` whose `keyId` matches, forcing re-resolution on next send. + +The getter approach is preferred because it also survives token rotation +(root-cause note above) without any cache bookkeeping. + +### 2. Fix the OAuth refresh call in `credentials/claude.ts` + +- `OAUTH_TOKEN_URL` → `https://api.anthropic.com/v1/oauth/token`. +- In `refreshViaOAuth`, send JSON: `Content-Type: application/json`, + `Accept: application/json`, `body: JSON.stringify({ grant_type, client_id, refresh_token })`. +- Don't swallow failures silently — log `response.status` and the body so the + next stale-token event is diagnosable. (Mirror the reference's `postJson`, + which throws with `status` + `body` included.) + +### 3. (If still needed) add `anthropic-beta` to the chat provider + +Have `createClaudeOAuthProvider` reuse `getAnthropicHeaders(token)` so the chat +path carries `anthropic-beta` (incl. `oauth-2025-04-20`) and `anthropic-version` +like every reference path and every other Anthropic call in this repo. + +--- + +## What was verified vs. assumed + +- **Verified by reading the code:** the per-tab Agent cache, the cache-invalidation + gate that ignores token expiry, the frozen `claudeCredentials` flowing into the + provider, the wrong refresh URL + form-encoded body, the silent failure + + stale-token fallback, the missing `anthropic-beta` on the chat provider, the + `opus-4-7` hardcode. +- **Verified against the reference** (`references/oh-my-pi`): correct token URL + (`api.anthropic.com/v1/oauth/token`), JSON body, and that the OAuth chat path + sends `anthropic-beta`. +- **Could not run/reproduce:** deps aren't installed here and network is + sandboxed, so the live request, the SDK's default-header behavior, and the + exact DB token `expiresAt` values were not inspected. Root cause #1 fully + explains the reported new-vs-old-tab behavior on its own; #2 explains why the + system can't self-heal once the DB token lapses. diff --git a/notes/claude-report.md b/notes/claude-report.md new file mode 100644 index 0000000..6635fa4 --- /dev/null +++ b/notes/claude-report.md @@ -0,0 +1,114 @@ +# Cache Miss Analysis for Dispatch's Claude Code Integration + +## Executive Summary + +The massive token burn and 0% cache hit rate observed after upgrading to AI SDK v6 are the result of two interacting flaws in how the Dispatch harness constructs requests for Anthropic: + +1. **Missing Beta Header:** The Claude OAuth provider completely omits the required `anthropic-beta: prompt-caching-scope-2026-01-05` header. Without this specific beta header, the Anthropic API silently ignores all `cache_control` markers. +2. **Suboptimal Breakpoint Placement:** The logic that assigns `cache_control` breakpoints misinterprets the structure of AI SDK v6 messages. Because each tool result is serialized as an independent `role: "tool"` message, the caching logic places markers on the last two *individual tool results* rather than the actual conversational turns, wasting breakpoints and failing to cache the preceding `assistant` message. + +The tool duplication incident (where tools were echoed 150+ times) is highly likely a symptom of the context window blowing up due to these cache misses, causing the model's generation loop to degenerate into repetitive tool hallucinations. + +--- + +## Root Cause 1: The Missing Beta Header + +### Analysis +Anthropic's prompt caching relies on explicit `cache_control` markers embedded in the request payload. However, for the Claude CLI ecosystem and OAuth flow, caching features are gated behind specific beta headers. + +In `packages/core/src/llm/provider.ts`, the `createClaudeOAuthProvider` factory initializes the AI SDK Anthropic provider. While it correctly mimics the `x-app` and `user-agent` headers of the Claude CLI, it **fails to include the `anthropic-beta` header**: + +```typescript +function createClaudeOAuthProvider(config: ProviderConfig): ModelFactory { + const anthropic = createAnthropic({ + // ... + headers: { + "anthropic-dangerous-direct-browser-access": "true", + "x-app": "cli", + "user-agent": "claude-cli/2.1.112 (external, sdk-cli)", + // MISSING: "anthropic-beta": getAnthropicBetas().join(",") + }, + }); +} +``` + +The AI SDK's `createAnthropic` constructor (in `@ai-sdk/anthropic`) adds `anthropic-version: 2023-06-01` but does *not* automatically inject `prompt-caching-scope-2026-01-05`. + +Because this header is missing from the underlying fetch request, the Anthropic API treats the request as a standard (non-cached) `/messages` call and ignores the ephemeral cache breakpoints completely. + +--- + +## Root Cause 2: Inefficient Caching Breakpoints + +### Analysis +Even if the beta header were present, the current breakpoint strategy in `agent.ts` defeats the purpose of caching due to how the AI SDK v6 structures tool results. + +In `toModelMessages()`, the agent unpacks an internal `tool-batch` chunk by creating a separate AI SDK message for every single tool result: +```typescript +for (const tr of trailingToolResults) { + result.push({ role: "tool", content: [ { type: "tool-result", ... } ] }); +} +``` + +Immediately after, `applyAnthropicCaching()` attempts to apply rolling cache markers: +```typescript +const nonSystem = msgs.filter((m) => m.role !== "system").slice(-2); +for (const m of nonSystem) targets.add(m); +// applies cacheControl: { type: "ephemeral" } +``` + +If the assistant emitted a batch of 10 tool calls, `msgs` ends with 10 individual `role: "tool"` messages. The `.slice(-2)` logic grabs only the 9th and 10th tool result messages and applies `cacheControl` to them. + +When the AI SDK translates this to the Anthropic wire format (in `convert-to-anthropic-messages-prompt.ts`), it groups consecutive `tool` messages into a single `role: "user"` Anthropic block. The cache markers are applied strictly to the 9th and 10th tool result parts at the very end of this user block. + +**Why this fails:** +1. **Wasted Breakpoints:** Anthropic requires at least 1,024 tokens between breakpoints to effectively cache the delta. Placing breakpoints on two consecutive tool results at the end of the chain wastes a breakpoint because the token delta between them is microscopic. +2. **Missing Assistant Context:** The `assistant` message (which contains the expensive reasoning tokens and the tool calls themselves) receives NO cache marker. + +### Comparison with `oh-my-pi` +The `oh-my-pi` reference implementation avoids this by applying cache controls logically across the wire format: +- `applyCacheControlToLastBlock(params.system)` +- `applyCacheControlToLastBlock(params.tools)` +- `penultimate user message` +- `last user message` + +By targeting the end of distinct conversational phases (the system block, the tools definition, and the entire user response block), `oh-my-pi` maximizes the cached prefix length. + +--- + +## Tool Duplication Incident Correlation + +The documented incident describes 150+ duplicated `read_file` results for `package.json` with wildly uneven counts. + +This behavior is not indicative of the execution harness blindly retrying tools. In `agent.ts`, tool calls are harvested directly from the AI SDK stream (`event.type === "tool-call"`) and executed synchronously. If 150 identical `read_file` results were yielded, it is because **the LLM actively generated 150 identical `` blocks** in a single turn. + +This generation loop is a known failure mode for Claude when the context window becomes excessively bloated or loses coherence (often compounded by a lack of cached prefixes breaking the model's structural attention). Fixing the cache miss will likely resolve the generation instability. + +--- + +## Recommendations + +Since this is a read-only analysis, no code has been edited. The following changes should be applied to fix the system: + +1. **Inject the Beta Headers:** + Modify `packages/core/src/llm/provider.ts` to import `getAnthropicBetas` from `../credentials/claude.js` and inject it into the Claude OAuth headers: + ```typescript + headers: { + "anthropic-beta": getAnthropicBetas().join(","), + "anthropic-dangerous-direct-browser-access": "true", + // ... + } + ``` + +2. **Group Tool Results in `toModelMessages`:** + Instead of pushing an individual `role: "tool"` message for every single tool result, group all `trailingToolResults` into a single `content` array inside one `role: "tool"` message. This accurately represents the tool-batch as a single turn. + +3. **Revise the Breakpoint Strategy in `applyAnthropicCaching`:** + Instead of naively grabbing the last two elements of the array, specifically target: + - The first `system` message. + - The **last** `assistant` message in the array. + - The **last** `user` (or `tool`) message in the array. + This ensures that the entire prefix leading up to the most recent reasoning step is successfully cached. + +4. **Add a Tool Deduplication Safeguard (Optional but recommended):** + In `agent.ts`'s execution loop, hash the tool name and arguments. If an identical tool call appears in the exact same `stepToolCalls` batch, automatically copy the result from the first execution rather than re-running it or blowing up the LLM's next prompt with redundant shell outputs. \ No newline at end of file diff --git a/notes/context.md b/notes/context.md new file mode 100644 index 0000000..896eed4 --- /dev/null +++ b/notes/context.md @@ -0,0 +1,169 @@ +# Dispatch — Phase 1 Implementation Context + +This file captures all decisions, open questions, and constraints established during planning. It serves as the source of truth for any agent working on Phase 1 implementation. + +--- + +## Stack Decisions + +| Layer | Choice | Notes | +|---|---|---| +| Runtime | **Bun** | Runtime + package manager. Native SQLite, fast installs/execution | +| Backend framework | **Hono.js** | Lightweight, WebSocket support | +| Frontend framework | **Vite + Svelte + DaisyUI** | Strict TypeScript throughout | +| Language | **TypeScript (strict mode)** | Both frontend and backend | +| LLM SDK | **Vercel AI SDK (`ai`)** | Provider-agnostic, streaming, tool calling | +| Default LLM | **DeepSeek V4 Flash Free** | Via OpenCode Go (Zen). Hardcoded for Phase 1 | +| Database | **SQLite (no ORM)** | `bun:sqlite` (native), no Drizzle or other ORM | +| Testing | **Vitest** | Set up across all packages from day one | +| Linting/Formatting | **Biome** | Single tool for both linting and formatting. Confirmed compatible with OpenCode | +| Package Manager | **Bun workspaces** | Monorepo with `@dispatch/*` packages | +| Dev Server | **Separate ports + CORS** | Frontend `:5173`, backend `:3000`, explicit CORS | + +--- + +## Resolved Decisions + +### Streaming Architecture: WebSocket Only +All real-time communication flows over a single WebSocket connection: +- Chat messages (user -> server) +- Streaming LLM tokens (server -> client) +- Tool call notifications and results (server -> client) +- Agent status updates (server -> client) + +`POST /chat` is not a streaming endpoint — it just queues/sends a message. The WebSocket handles all real-time data. `GET /status` remains as a simple REST endpoint for polling agent state. + +### Database: SQLite Without ORM +No Drizzle ORM. Use `better-sqlite3` (or equivalent) directly with raw SQL. Keep it simple. The question of whether to set up persistence in Phase 1 or defer to Phase 5 is still open — the user indicated willingness to use SQLite but the timing wasn't finalized. **Default assumption: defer heavy persistence to Phase 5, but the library choice is locked in.** + +### Tool Scoping: Working Directory +File tools (`read_file`, `write_file`, `list_files`) will be scoped to a configurable working directory from Phase 1. This prevents accidental filesystem damage and provides a clean foundation for the permission system in Phase 2. + +### Testing: Vitest From Day One +Vitest set up across the monorepo. Unit tests for core logic (agent loop, tool registry, individual tools). + +### DaisyUI Theme: Configurable With Persistence +Support multiple DaisyUI themes with a user-selectable theme switcher in settings. The selected theme is remembered across sessions (localStorage or equivalent). + +### Default LLM Provider: DeepSeek V4 Flash via OpenRouter +The Phase 1 hardcoded model is DeepSeek V4 Flash, accessed through OpenRouter. This means: +- The Vercel AI SDK OpenRouter provider will be used +- A single `OPENROUTER_API_KEY` env var (or equivalent) is needed +- Model ID will be the OpenRouter model string for DeepSeek V4 Flash + +--- + +## Resolved Decisions (Previously Open Questions) + +### 1. Package Manager and Monorepo Tooling: Bun Workspaces +**Decision:** Bun as both runtime and package manager, using Bun workspaces. + +Bun workspaces work similarly to pnpm workspaces — `packages/core`, `packages/api`, and `packages/frontend` reference each other as `@dispatch/*` dependencies, and Bun symlinks them locally. + +### 2. Runtime: Bun +**Decision:** Bun is the runtime. + +Implications: +- Native SQLite via `bun:sqlite` — no need for `better-sqlite3` +- Bun is faster for installs, script execution, and testing +- Vitest is the test runner (works with Bun) +- No Node.js version to manage + +### 3. Dev Server Setup: Separate Ports + CORS +**Decision:** Frontend on `:5173` (Vite dev server), backend on `:3000` (Hono). Explicit CORS configuration on the backend. + +This means: +- Hono backend needs CORS middleware allowing the frontend origin +- WebSocket connections go directly to `:3000` from the frontend +- No Vite proxy configuration needed +- Production will also be cross-origin (matches the split deployment model) + +### 4. Biome Compatibility: Confirmed +**Decision:** Biome is fully compatible with OpenCode. + +OpenCode has Biome as a **built-in formatter**. It auto-detects `biome.json(c)` config files and handles `.js`, `.jsx`, `.ts`, `.tsx`, `.html`, `.css`, `.md`, `.json`, `.yaml`, and more. Just needs a `biome.json` in the project root and formatters enabled in OpenCode config (`"formatter": true` or `"formatter": {}`). + +--- + +## Phase 1 Scope (from plan.md) + +**Goal:** Chat with one agent in a browser, watch it read and write files. + +### Backend Tasks +- Project scaffolding (monorepo with `packages/core`, `packages/api`, `packages/frontend`) +- Agent runtime: message -> LLM -> tool call -> result -> repeat loop +- Vercel AI SDK integration with streaming responses +- Single provider config (DeepSeek V4 Flash via OpenRouter, env var for API key) +- Basic tools: + - `read_file` — read file contents (scoped to working directory) + - `write_file` — write/overwrite a file (scoped to working directory) + - `list_files` — glob/list directory contents (scoped to working directory) +- HTTP API: + - `POST /chat` — send a message (non-streaming, queues to agent) + - `GET /status` — agent status (idle, running, etc.) +- WebSocket: stream agent output tokens and tool calls in real-time + +### Frontend Tasks +- Single chat panel — text input field, send button +- Streamed response rendering (tokens appear as they arrive via WebSocket) +- Tool call display (collapsible: show tool name, arguments, result) +- Model/provider indicator in header +- Basic layout: chat takes full screen, clean and minimal +- Theme switcher in settings (DaisyUI themes, persisted to localStorage) + +### Done When +Open a browser, type "read the contents of package.json and summarize it," see the agent call `read_file`, stream back a summary. Ask it to create a new file — it calls `write_file` and confirms. + +--- + +## Project Structure (Planned) + +``` +dispatch/ + packages/ + core/ # Agent runtime, LLM integration, tools + src/ + agent/ # Agent loop, lifecycle + llm/ # Vercel AI SDK wrapper, provider config + tools/ # Tool registry, built-in tools (read_file, write_file, list_files) + types/ # Shared TypeScript types + tests/ + api/ # Hono HTTP + WebSocket server + src/ + routes/ # HTTP route handlers + ws/ # WebSocket handlers + tests/ + frontend/ # Vite + Svelte + DaisyUI client + src/ + lib/ # Svelte components, stores, utilities + routes/ # Page routes (if using SvelteKit) or views + tests/ + biome.json # Biome config (pending compatibility check) + tsconfig.base.json # Shared TypeScript config + package.json # Root workspace config + .env.example # Environment variables template (OPENROUTER_API_KEY, etc.) +``` + +--- + +## Concurrency Map (What Can Be Built in Parallel) + +Once scaffolding is complete, the following sections can be developed concurrently: + +``` +[A] Scaffolding (sequential — must be first) + | + +---> [B] Core Agent Runtime (agent loop, LLM, tool system) + | + +---> [C] API Server Shell (Hono setup, route stubs, WebSocket setup) + | + +---> [D] Frontend Shell (Svelte app, chat UI, WebSocket client, theme system) + +Then integration (sequential — depends on B, C, D): + +[E] Wire core into API (connect agent runtime to routes/WebSocket) +[F] Wire frontend to API (connect UI to live WebSocket) +[G] End-to-end testing and polish +``` + +Sections B, C, and D are independent and can be coded by concurrent agents. Section E requires B and C. Section F requires C and D. Section G requires everything. diff --git a/notes/eviction-limitation.md b/notes/eviction-limitation.md new file mode 100644 index 0000000..91d1a88 --- /dev/null +++ b/notes/eviction-limitation.md @@ -0,0 +1,105 @@ +# Known Limitation: Frontend eviction is whole-message, not per-chunk + +Status: **RESOLVED.** Fixed by the chunk-native frontend store (see +`plan-chunk-eviction.md`). The frontend's source of truth for history is now a +flat `ChunkRow[]` (`tab.chunks`, real per-tab `seq`); the live turn is a +transient tail (`tab.live`) reconciled into the sealed log on a `turn-sealed` +event. Eviction (`evictChunks`) is a rolling per-chunk trim of the oldest rows — +so a single oversized turn is trimmed chunk-by-chunk instead of pinned whole. +Pagination loads raw chunks (`GET /tabs/:id/chunks`), deduped by `seq`. The +historical analysis below is retained for context. + +--- + +Documented from the append-only chunk-log work (see `plan-chunk-log.md`). The +backend was not the problem — this was purely a frontend in-memory concern. + +## TL;DR + +The append-only chunk log made **loading** per-chunk (you fetch the last N +*chunks*, not N whole turns), but the frontend's in-memory **eviction** still +drops whole *messages* (turns). A single pathological turn (e.g. the 150-tool-call +incident — one assistant message holding ~150 chunks) therefore stays resident in +browser memory in full until it scrolls out of the protected window. On a +memory-constrained device, one giant turn can still blow the budget. + +So: the chunk log helps long *histories* (many normal turns), but does **not** +yet help a single oversized *turn*. + +## What "eviction" is (for the record) + +`evictMessages` (`packages/frontend/src/lib/tabs.svelte.ts:345`) trims a tab's +in-memory `messages` array when its total chunk count exceeds +`tab.chunkLimit`, to bound browser RAM. It never deletes anything from the DB — +the `chunks` table is the durable source of truth and evicted content is +re-fetched on scroll-up via `loadMoreMessages` +(`tabs.svelte.ts:402`). The active/streaming turn and the last user+assistant +pair are pinned; eviction is suppressed while the user is scrolled up. + +## Root cause + +Eviction operates at message granularity and explicitly refuses to trim within a +message. From `tabs.svelte.ts` (the `evictMessages` comment, ~`:360-372`): + +> We never trim chunks from WITHIN a message: messages are the +> persistence/pagination unit ... Whole-message eviction from the front is the +> correct granularity. + +That comment is now stale in spirit: persistence/pagination is per-**chunk** +(the flat `chunks` table + `GET /messages` windowing by chunk `seq`), but the +in-memory store still holds **grouped `ChatMessage[]`** as its source of truth, +so the smallest evictable unit is a whole turn. + +``` +DB / wire: per-chunk ✅ (chunks table, chunk-seq pagination) +Frontend load: per-chunk ✅ (GET /messages?limit=N windows the chunk log) +Frontend evict: per-MESSAGE ❌ (tab.messages is grouped; a turn is atomic) +``` + +## Why it wasn't fixed in this pass + +True per-chunk eviction requires the frontend store's **source of truth to be the +flat chunk list**, with `messages` derived for rendering (this was P5 in +`plan-chunk-log.md`). That means: + +- store `tab.chunks: ChunkRow[]` (+ the in-flight live turn) instead of + `tab.messages: ChatMessage[]`; +- rewrite every streaming handler (`applyChunkEvent`, `routeSystemEvent`, the + `done` / `status` / `statuses` / error paths) to mutate the flat list; +- derive `messages` via `groupRowsToMessages` for the render layer. + +That touches ~10 handler sites in `tabs.svelte.ts` and the ~60 frontend tests in +`chat-store.test.ts`, all of which currently assert against the grouped +`tab.messages` model. It was deferred to keep the test suite green and the +diff bounded. The hard part (flat storage + DB-free `explode`/`group` transforms +in `packages/core/src/chunks/transform.ts`) is already done and reusable. + +## Fix options (later) + +1. **Flat-chunk store + derived messages (recommended, the "proper" fix).** + Make `tab.chunks: ChunkRow[]` the source of truth; derive `messages` with + `groupRowsToMessages` (already shared from + `@dispatch/core/src/chunks/transform.js`). Eviction then trims the flat array + at chunk granularity; pin the in-flight turn + the tail. Re-fetch on + scroll-up by chunk `seq` (already supported). Highest effort (handler + + test rewrite), but fully solves it and matches `plan-chunk-log.md` P5. + +2. **Partial trim of an oversized message (incremental, lower effort).** + Keep the grouped model, but when the *oldest* in-memory message alone exceeds + the limit, drop its leading chunks (whole `text`/`thinking`/`tool-batch` + units) and record a per-message `oldestChunkSeq` so `loadMoreMessages` can + re-hydrate it. Caveat: must keep the message renderable and merge correctly on + scroll-up (the `turnId` merge in `loadMoreMessages` already handles the + boundary case). A pragmatic stopgap. + +3. **Render virtualization (separate concern).** + Windowing the *DOM* (only mount visible bubbles) reduces render cost but not + the JS-heap cost of holding the chunks. Complementary to 1/2, not a + substitute. + +## Acceptance check for whichever fix + +Load a tab whose history contains one turn with ≫ `chunkLimit` chunks; confirm +in-memory chunk count stays ≤ `chunkLimit` (± the pinned tail) while scrolled to +the bottom, and that scrolling up re-hydrates the trimmed chunks of that same +turn without duplication. diff --git a/notes/gemini-chunk-eviction-review-2.md b/notes/gemini-chunk-eviction-review-2.md new file mode 100644 index 0000000..a2a1ac3 --- /dev/null +++ b/notes/gemini-chunk-eviction-review-2.md @@ -0,0 +1,95 @@ +# Code Review (Pass 2): Chunk-Native Frontend Eviction + +## Executive Summary + +The fixes applied since Pass 1 successfully address the two major Blockers related to reconciling active turns and optimistic UI state. The decoupling of reconcile logic from `agentStatus` in favor of `liveTurnId` elegantly solves the race conditions around deferred reconciliations and interrupt boundaries. + +However, a **NEW Blocker** was identified in how `turn_id`s are backfilled onto user messages when a turn starts. The loop indiscriminately tags pending `queued-` messages that belong to *future* turns, causing them to be wiped when the *current* turn finishes. + +**Verdict: DO NOT SHIP.** The new Blocker must be fixed. The fix is a trivial one-line condition. + +--- + +## Pass 1 Fixes & Invariants + +* **Fix #1: Deferred reconcile wipes concurrent active turns.** + **VERDICT: FIXED.** `reloadChunksFromApi`'s new `preserveTurnId` logic perfectly handles overlapping turns. It correctly preserves Turn B's in-flight chunks while dropping Turn A's, and cleanly nullifies state when appropriate. The Map `pendingReconcileTabs` ensures any intermediate turns are fetched comprehensively from the DB. +* **Fix #2: Optimistic queued user messages are dropped on reconcile.** + **VERDICT: FIXED (Partially).** The condition `(m.turnId === undefined && m.role === "user")` inside `keptLive` effectively retains optimistic user messages. However, see the New Blocker below regarding `turnId` assignment. +* **Cache Safety Invariant.** + **VERDICT: SAFE.** `toModelMessages` and Anthropic normalization logic were completely untouched. The backend's prompt caching remains strictly bound to DB-persisted rows. +* **`messages` -> `renderGroups` Rename Completeness.** + **VERDICT: COMPLETE.** No stray references to `tab.messages` were found. Reactivity safely bounds derived state within `updateTab`. + +--- + +## NEW Findings + +### 1. `turn-start` wrongly tags future queued messages (Block) +**Location:** `packages/frontend/src/lib/tabs.svelte.ts:873-882` (inside `handleEvent` case `turn-start`) + +**Description:** +When a turn starts, the frontend loops backwards through `tab.live` to tag untagged user messages with the new `turnId`. It breaks only when it hits a non-user or an already-tagged message. + +If multiple messages are queued (e.g. `[queued-B, queued-C]`), and the backend dequeues `B`, `message-consumed` removes the prefix and puts `B` at the end of the array: `[queued-C, B]`. Immediately after, `turn-start` for Turn B fires, looping backward. It tags `B` with `"Turn B"`, but continues and ALSO tags `queued-C` with `"Turn B"`. When Turn B subsequently seals, `queued-C` is wiped from the UI because its `turnId` matches the sealing turn, and it loses the protection of `turnId === undefined`. It vanishes until Turn C actually finishes processing. + +This same bug happens if the user queues a message in the tiny race-window between sending their first prompt and the WS `turn-start` event arriving. + +**Direction:** The backfill loop must explicitly ignore queued messages. Add a check to prevent tagging them: `if (m && m.role === "user" && m.turnId === undefined && !m.id.startsWith("queued-"))`. + +--- + +## Evaluation of New Regression Tests + +The two new regression tests are highly valuable, but miss critical permutations: + +1. **"preserves an optimistic queued user message..."** + * **Genuinely covers:** Fix #2. It proves `keptLive` preserves an untagged queued message during a reconcile. + * **Untested edge case:** It queues the message *after* `turn-start` fires. Therefore, it completely bypasses the `turn-start` backfill loop. A test should queue a message *before* `turn-start` fires (or queue *two* messages and consume one) to expose the new Blocker above. +2. **"preserves a concurrent newer turn..."** + * **Genuinely covers:** Fix #1. It correctly validates `preserveTurnId` logic, the `currentAssistantId` mapping, and `liveTurnId` integrity when a deferred reconcile flushes across a newer streaming turn. + * **Untested edge case (Map last-write-wins):** It only seals *one* turn while scrolled up. A test should seal *two* turns while scrolled up, then scroll down, ensuring `pendingReconcileTabs` handles the latest `turnId` and the entire window is correctly loaded. + +--- + +## Resolution (OpenCode) + +### Block #1 (`turn-start` wrongly tags future queued messages) — FIXED +`packages/frontend/src/lib/tabs.svelte.ts` `handleEvent` case `turn-start`. + +Backend reality check (not in Gemini's model): a queued message **never** gets its +own `turn-start`. `turn-start` is emitted exactly once per user-initiated +`processMessage` (`agent-manager.ts:1128`), which persists exactly one user row via +`explodeUserText`. Queued messages are drained *into* the running turn through +`dequeueMessages`/`message-consumed` (`agent.ts:1241,1308`). So the turn's initiator +is always the single most-recent **non-queued** untagged user row, and any `queued-` +row in the live tail belongs to a future turn. + +Gemini's literal one-liner (`&& !m.id.startsWith("queued-")` with the existing +`else break`) is **insufficient**: when the queued row is the trailing element the +loop would `break` on it and never tag the real initiator, leaving the initiator +untagged → it duplicates against its own sealed chunk row on reconcile. + +Implemented fix: the backfill now (a) stops at the first non-user row, (b) **skips +past** (`continue`) pending `queued-` rows, and (c) tags exactly the one most-recent +non-queued untagged user row, then breaks. `keptLive` is unchanged — untagged +`queued-` rows remain untagged and are preserved on reconcile. + +### Test gap — ADDRESSED +Added `"turn-start backfill skips a pending queued row trailing the turn initiator"` +in `chat-store.test.ts`: constructs live = `[, queued-q2]`, fires +`turn-start`, asserts the initiator is tagged and `queued-q2` is **not**, then seals +and asserts `queued-q2` survives with no duplicate initiator bubble +(`renderGroups` roles = `[user, assistant, user]`). This exercises the `continue` +(skip-queued) branch and covers the queue-present-before-`turn-start` race Gemini +flagged as untested. + +### Nits (deferred, non-blocking) +- `pendingReconcileTabs` last-write-wins doc/test (seal two turns while scrolled up): + acceptable as-is — the deferred flush refetches the full window from the DB, so the + latest sealed `turnId` correctly supersedes; left as a follow-up test. + +### Verification +Full suite: **326 tests pass** (core 223 incl. 33-test agent/cache-stability suite, +api 35, frontend 68). Biome clean; `tsc` core+api and `svelte-check` report 0 errors. +Cache invariant remains untouched (no `agent.ts` wire/folding/caching changes). \ No newline at end of file diff --git a/notes/gemini-chunk-eviction-review-3.md b/notes/gemini-chunk-eviction-review-3.md new file mode 100644 index 0000000..3637946 --- /dev/null +++ b/notes/gemini-chunk-eviction-review-3.md @@ -0,0 +1,118 @@ +# Code Review (Pass 3): Verify the `turn-start` backfill Block fix + +## Executive Summary + +The fix to the `turn-start` backfill loop successfully prevents pending `queued-` messages from being wiped by correctly skipping them. However, it exposes a critical flaw in how **consumed** messages are handled. + +When a queued message is consumed (`message-consumed`), its `queued-` prefix is stripped, leaving it as an untagged, plain user row in `live`. Because it lacks a `turnId`, it survives `reconcileSealedTurn` and lingers in the UI forever, floating to the bottom and duplicating the `[USER INTERRUPT]` text already present in the sealed chunks. Additionally, in multi-client scenarios with no local initiator, the backfill loop will incorrectly tag this lingering consumed message with a future turn's ID. + +**Verdict: DO NOT SHIP.** A new Blocker must be fixed. The fix is a one-line change in the `message-consumed` handler to bind consumed messages to the active turn. + +--- + +## Detailed Findings + +### Q1. Backend Claims Verification +**Confirmed.** The backend code in `packages/api/src/agent-manager.ts` and `packages/core/src/agent/agent.ts` confirms that: +1. `turn-start` is emitted exactly once per `processMessage`. +2. Exactly one user row draft is persisted via `explodeUserText`. +3. Queued messages are pulled into a running turn via `dequeueMessages` and NEVER trigger a `turn-start`. + +### Q2. Block Fix Correctness +The new backfill logic correctly skips `queued-` rows and tags exactly one initiator when a local initiator exists. +* **(a) `[initiator, queued-X]`**: Correctly skips `queued-X`, tags `initiator`, and breaks. +* **(b) `[queued-X, initiator]`**: Logically impossible; the initiator is appended before a queue can form. +* **(c) `[queued-X, queued-Y]` consumed**: When `queued-X` is consumed, its prefix is stripped. It becomes an untagged user message. When the NEXT turn starts, if there is a local initiator, it tags the new initiator and ignores the consumed message. +* **(d) Multi-client (no local initiator)**: If Client B starts a turn, Client A receives `turn-start` but has no local initiator in `live`. Client A's backfill loop will find the untagged consumed message from the *previous* turn and incorrectly tag it with the *new* turn's ID. + +### Q3. No Duplication / No Loss +**Failed.** Because the backfill loop `break`s after tagging the new initiator, the consumed message from the previous turn remains permanently untagged (`turnId === undefined`). It survives the `turn-sealed` reconcile process and floats to the bottom of the live tail. Since the backend injects the consumed message's text into the `[USER INTERRUPT]` chunk row, the user sees BOTH the tool result chunk text AND a lingering user bubble. + +### Q4. `keptLive` Unchanged — Still Correct? +**Failed.** `keptLive` preserves optimistic rows by checking `m.turnId === undefined && m.role === "user"`. This was intended for pending initiators and queues, but it inadvertently preserves consumed messages because they never received a `turnId`. Consumed messages MUST be bound to the turn that consumed them so they are cleanly dropped when that turn seals. + +### Q5. Test Adequacy +**The new test simulates an impossible backend sequence.** +The test creates `q1`, consumes it, and then fires `turn-start`, assuming `turn-start` applies to the consumed `q1`. The backend never emits a `turn-start` for a consumed message. By forcing the backfill loop to tag a consumed message, the test mistakenly verifies an action that in reality constitutes cross-turn ID theft. + +### Q6. Prompt-Caching Invariant +**SAFE.** The backend logic in `packages/core/src/agent/agent.ts` was untouched. Cache stability is preserved. + +--- + +## NEW Blockers + +### Block #1: Consumed messages linger and duplicate due to missing `turnId` +**Location:** `packages/frontend/src/lib/tabs.svelte.ts:1226` (inside `handleEvent` case `"message-consumed"`) + +**Description:** +When `message-consumed` extracts a queued message, it strips the prefix but leaves `turnId` undefined. The message survives `turn-sealed` and lingers in `live` forever, duplicating the chunk data. In multi-client scenarios, it also acts as a trap for the next `turn-start` backfill. + +**Direction:** +In `message-consumed`, bind the consumed message to the actively streaming turn so `reconcileSealedTurn` drops it cleanly: +```typescript +if (mcEvent.messageIds.includes(queuedId)) { + consumed.push({ + ...m, + id: queuedId, + turnId: mcTab.liveTurnId ?? undefined // Bind to active turn + }); + continue; +} +``` +With this fix, the `turn-start` loop's `break` behavior is perfectly safe, as consumed messages will no longer be "untagged". + +--- + +## Resolution (OpenCode) + +### Block #1 (consumed interrupt messages linger + duplicate) — FIXED +`packages/frontend/src/lib/tabs.svelte.ts`, `handleEvent` case `"message-consumed"`. + +Verified the finding against the code: `keptLive` (in `reloadChunksFromApi`) preserves +every `m.turnId === undefined && m.role === "user"` row, and `message-consumed` was +pushing `{ ...m, id: queuedId }` with **no** `turnId`. So a consumed interrupt bubble +was kept on every reconcile — lingering at the tail and duplicating the +`[USER INTERRUPT]` text the backend folds into the sealed tool-result chunk +(`agent.ts:1248-1255`). This contradicted the intended "collapse to persisted shape" +behavior. + +Applied Gemini's direction (with the codebase's conditional-spread style): bind the +consumed row to the in-flight turn so reconcile drops it on seal — +```ts +consumed.push({ + ...m, + id: queuedId, + ...(mcTab.liveTurnId !== null ? { turnId: mcTab.liveTurnId } : {}), +}); +``` +`liveTurnId` is always set while a turn runs (the only time a consume happens). The +ChatPanel keyer (`${turnId}:${role}:${n}`, per-(turn,role) counter) gives the consumed +row a distinct key from the initiator, so no key collision during the live interrupt +split. This also makes the Pass-2 `turn-start` backfill `break` fully safe (no +untagged consumed rows remain to be mis-tagged in the multi-client path Q2(d)). + +### Test fixes (addressing Q5) +The Pass-2 test was rewritten — Gemini correctly flagged that consuming a message and +then firing `turn-start` at it is not a real backend sequence. Replaced with two +realistic tests in `chat-store.test.ts`: +1. `"turn-start backfill skips a pending queued row (race), tags only the initiator"` — + drives the **real** race via `store.sendMessage`: an idle send (plain optimistic + row) followed by a running send (queued row), then a late `turn-start`. Asserts the + initiator is tagged, the queued row is NOT, and the queued row survives the seal + with no duplicate initiator. +2. `"a consumed interrupt message collapses into the sealed turn (no lingering bubble)"` + — realistic interrupt flow (turn-start → stream → queue → consume → stream → seal); + asserts the consumed row is bound to the turn and dropped on reconcile. Fails on the + pre-fix code (row lingers untagged), passes after. + +### Verdict response +- Q1 backend claims: confirmed by Gemini and re-confirmed here. +- Q2(d) multi-client mis-tag: eliminated — consumed rows are no longer untagged. +- Q3/Q4 lingering+duplication: FIXED. +- Q6 cache invariant: SAFE — change is frontend store/test only; `agent.ts` untouched. +- Q7: no new Blockers introduced. + +### Verification +Frontend **69 tests pass** (chat-store 54 incl. the two new tests, sidebar 15); +`svelte-check` 0 errors; Biome clean. Core (223) + api (35) unaffected → 327 total. \ No newline at end of file diff --git a/notes/gemini-chunk-eviction-review.md b/notes/gemini-chunk-eviction-review.md new file mode 100644 index 0000000..373917a --- /dev/null +++ b/notes/gemini-chunk-eviction-review.md @@ -0,0 +1,69 @@ +# Code Review: Chunk-Native Frontend Eviction (Dispatch) + +## Executive Summary + +The rewrite to a chunk-native frontend store successfully resolves the unbounded memory pinning caused by oversized turns. The migration from grouped messages to a flat chunk log (`tab.chunks`) as the source of truth, with a decoupled `renderGroups` cache, correctly achieves rolling chunk-level eviction and seamless pagination. + +However, while the backend invariants and cache safety are perfectly maintained, there are two **Blocker** regressions in the frontend's reconcile logic relating to the handling of concurrent/queued messages and deferred UI updates. `reloadChunksFromApi` assumes it is strictly processing a single sequential turn and acts destructively on in-flight UI state when edge cases overlap. + +**Verdict: DO NOT SHIP.** The cache invariant holds, but the Blocks must be fixed first. + +--- + +## Cache Safety Invariant + +**VERDICT: SAFE.** +The diff contains **zero** modifications to `packages/core/src/agent/agent.ts`. The prompt-cache cohesion logic, `toModelMessages`, `applyAnthropicCaching`, and normalisation remain 100% server-side and entirely isolated from the frontend's transient render representations. Model-bound bytes are unchanged. + +--- + +## Findings + +### 1. Deferred reconcile wipes concurrent active turns (Block) +**Location:** `packages/frontend/src/lib/tabs.svelte.ts:658` (`reconcileSealedTurn`) and `:636` (`reloadChunksFromApi`) + +**Description:** +If the user scrolls up, automatic eviction and reconciliation are suppressed. If Turn A finishes while scrolled up, its reconciliation is deferred (`pendingReconcileTabs.add`). If the agent then starts Turn B (e.g. via queue processor), it establishes a new live streaming bubble (`liveTurnId = 'Turn B'`). +When the user subsequently scrolls down, the deferred flush fires for Turn A, calling `reloadChunksFromApi`. This function blindly sets `live: []`, `liveTurnId: null`, and `currentAssistantId: null` — immediately deleting all in-flight chunks for the *currently active* Turn B. Turn B will then spawn a fresh disconnected live bubble on its next stream delta, permanently losing its prior chunks and its `turnId` tag (causing it to remount/flash on seal). + +**Direction:** `reloadChunksFromApi` must not unconditionally nuke `live` and `currentAssistantId` if a *new* turn is actively in-flight (`agentStatus === "running" && liveTurnId !== turnIdToReconcile`). The reconcile must be turn-aware. + +### 2. Optimistic queued user messages are dropped on reconcile (Block) +**Location:** `packages/frontend/src/lib/tabs.svelte.ts:636` (`reloadChunksFromApi`) + +**Description:** +When a user sends a message while the agent is busy, it is added to `tab.live` as an optimistic unsealed bubble. When the current active turn completes and emits `turn-sealed`, `reloadChunksFromApi` wipes the entire `live` array (`live: []`). The queued user message vanishes from the UI entirely. While it remains safely in `queuedMessages` and will eventually be processed by the backend, the UI will not reflect it again until its own eventual `turn-sealed` event completes the backend loop. + +**Direction:** `reloadChunksFromApi` must preserve unsealed optimistic user messages (e.g. those matching `queuedMessages` IDs) when clearing `live`. + +### 3. Edge-case fetch race on WS reconnect desync (Ship-with-followup) +**Location:** `packages/frontend/src/lib/tabs.svelte.ts:860` (`handleEvent` case `statuses`) + +**Description:** +The backend emits `status: idle` immediately before making the synchronous SQLite `flushAssistant` write, and emits `turn-sealed` immediately after. If a WS reconnect happens exactly in this microsecond window, the frontend's `hydrateFromBackend` sees `backendStatus !== "running"` and triggers `reloadChunksFromApi`. Because the DB write hasn't landed, it loads a chunk window missing the just-finished turn. + +**Direction:** This is non-fatal because the backend will still emit `turn-sealed` milliseconds later, which triggers a second `reloadChunksFromApi` that corrects the UI state. It will manifest as a sub-second flicker. Safe to ship, but ideally, `statuses` should infer completion from the presence of unpersisted chunks rather than the raw agent status. + +### 4. Tool-batches undercount the live eviction budget (Nit) +**Location:** `packages/frontend/src/lib/tabs.svelte.ts:76` (`countLiveChunks`) + +**Description:** +In `countLiveChunks`, a single `tool-batch` render bubble counts as `1` against the live budget (`m.chunks.length`). However, upon sealing, `explodeTurn` expands each batch into `N * 2` rows (a `tool_call` and `tool_result` for each parallel call). This means an in-flight turn with heavily parallelized tool execution will temporarily consume more memory than `chunkLimit` targets. + +**Direction:** Minor inconsistency. Once the turn seals, the correct DB row count will be respected. Acceptable trade-off for simplicity in the live rendering path. + +### 5. `trimLiveChunks` drops the prompt under extreme pressure (Nit) +**Location:** `packages/frontend/src/lib/tabs.svelte.ts:431` (`trimLiveChunks`) + +**Description:** +If a single streaming turn heavily exceeds `chunkLimit`, `trimLiveChunks` enforces a strict rolling window on `live`. Since it trims oldest-first indiscriminately, it will `shift()` away the user's prompt chunk from the UI entirely before it touches the assistant's streaming chunks. + +**Direction:** This technically aligns with the design ("never the chunk currently being streamed"), but means the user's initiating context disappears mid-stream under memory pressure. Expected behavior for a raw chunk limit. + +--- + +## Notable Correctness Risks Validated + +- **Seq Cursor Logic:** Perfect. `oldestLoadedSeq` rigidly derives from `ChunkRow.seq` via `minSeqOf(sealed)`. Live transient state is securely walled off from pagination logic. +- **Transaction Wrapper:** Sound. `db.transaction()` around the `appendChunks` batch is synchronously robust and correctly yields monotonic `seq` allocations per turn. +- **Scroll-up pagination:** Deduplication works flawlessly. `mergeChunksBySeq` efficiently handles overlap at the pagination boundary without duplicating message bubbles. diff --git a/notes/gemini-chunk-log-review.md b/notes/gemini-chunk-log-review.md new file mode 100644 index 0000000..e04f6c3 --- /dev/null +++ b/notes/gemini-chunk-log-review.md @@ -0,0 +1,90 @@ +# Review: Append-Only Chunk-Log Refactor + +## Executive Summary + +The refactor successfully transitions the codebase from a "message-as-container" model to a flat, append-only **chunk log**. This is a major structural improvement that enables granular pagination and addresses the primary root cause of Anthropic prompt-cache churn by segmenting multi-step turns into stable message pairs. + +**Status: Partially Correct.** +- **Goal 1 (Cache Fix):** **Achieved for happy-path turns**, but **broken for turns involving user interrupts**. The planned "New model" for interrupts (append-only user chunks) was not implemented; the legacy mutation-based stripping remains in `agent.ts`, which continues to bust the cache prefix across steps when an interrupt occurs. +- **Goal 2 (Flat Storage/Pagination):** **Fully Achieved.** The explode/group transforms are robust, lossless, and handle window boundaries correctly via `turnId` merging in the frontend. + +--- + +## Findings & Questions + +### 1. Cache Stability +**Severity: Should-Fix** +The implementation of `toModelMessages` (`agent.ts:162`) correctly segments assistant turns into stable `[assistant, tool]` pairs per step. However, the **interrupt logic** (`agent.ts:241`) reintroduces instability. +- In Step 1 of a turn, an interrupt is marked "freshest" and included in the Step 1 `tool` message. +- In Step 2, that same Step 1 `tool` message is now "stale" and has the interrupt stripped. +- **Result:** The serialized content of Step 1 changes between Request 1 and Request 2, shattering the Anthropic cache prefix for everything following the Step 1 text. +- *Reference:* `packages/core/src/agent/agent.ts:241-247` and `packages/core/src/agent/agent.ts:80-86`. + +### 2. Explode/Group Fidelity +**Severity: Verified Correct** +The round-trip between `Chunk[]` and `ChunkRow[]` is lossless. +- `explodeTurn` correctly splits `tool-batch` into paired `tool_call` and `tool_result` rows. +- `groupRowsToMessages` correctly reconstructs turns using `turnId` and `step`, and gracefully handles orphan `tool_result` rows by creating synthetic entries in the batch. This is vital for pagination. +- *Reference:* `packages/core/src/chunks/transform.ts`. + +### 3. Step Derivation +**Severity: Verified Correct** +The assumption that a `tool-batch` marks the end of an LLM step is consistent with the `Agent.run()` loop. The `step` increment in `explodeTurn` (`transform.ts:89`) and the segmentation in `toModelMessages` (`agent.ts:251`) are in sync. + +### 4. Migration Safety +**Severity: Verified Correct** +The migration in `db/index.ts` correctly detects the legacy `messages` table and performs a one-shot nuke of messages/tabs. +- It is safe for repeat runs (idempotent check on `sqlite_master`). +- Tests are safe as they mock `getDatabase()` and use in-memory fakes. +- *Reference:* `packages/core/src/db/index.ts:106-118`. + +### 5. Persistence Correctness +**Severity: Verified Correct** +The "Write-on-seal" strategy is correctly implemented. +- `processMessage` accumulates chunks in memory and flushes exactly once via `flushAssistant()` when the turn settles. +- The fallback-retry path correctly avoids calling `flushAssistant()` for failed attempts, preventing partial/duplicate turns in the log. +- *Reference:* `packages/api/src/agent-manager.ts:1081-1085` and `:1168`. + +### 6. Rebuild Correctness +**Severity: Nit** +`getMessagesForTab` fetches the *entire* chunk log for a tab to rebuild the Agent's in-memory history. For very long conversations (thousands of chunks), this will cause increasing latency and memory pressure whenever an Agent is reconstructed (e.g., on model switch). +- *Reference:* `packages/core/src/db/chunks.ts:121`. + +### 7. Pagination Correctness +**Severity: Verified Correct** +Frontend pagination correctly handles turns split across the 50-chunk window. +- `loadMoreMessages` in `tabs.svelte.ts` detects `turnId` + `role` matches at the boundary and merges the chunks. +- This ensures that scrolling up restores the "tail" of a turn and prepends the "head" seamlessly. +- *Reference:* `packages/frontend/src/lib/tabs.svelte.ts:445-467`. + +### 8. Interrupt Handling +**Severity: Blocker (for Caching Goal)** +The refactor failed to implement the "New model" described in `plan-chunk-log.md` (Section 4). +- The plan called for interrupts to be appended as their own `user/text` chunks, making history immutable. +- The implementation instead kept the legacy `[USER INTERRUPT]` string injection and the unstable `stripUserInterruptBlock` logic. +- This preserves the cache-churn bug for any session involving interrupts. +- *Reference:* `packages/core/src/agent/agent.ts:162-256`. + +### 9. Anthropic Wire Validity +**Severity: Verified Correct** +- `applyAnthropicStructuralNormalisations` correctly handles Anthropic's strict requirements, including splitting assistant messages if tool-calls are followed by text (Pass 3) and scrubbing tool IDs (Pass 2). +- Empty reasoning blocks are stripped to avoid API errors while maintaining the signature in the DB for future turns. + +--- + +## Verified Correct +The following components were reviewed and found to be implementation-perfect: +- **`packages/core/src/chunks/transform.ts`**: Pure logic for flattening and re-grouping. +- **`packages/core/src/db/chunks.ts`**: Monotonic `seq` allocation and pagination queries. +- **`packages/api/src/routes/tabs.ts`**: Cursor-based history endpoint. +- **`packages/core/src/agent/agent.ts`**: `applyAnthropicCaching` breakpoint placement. + +--- + +## Recommendations + +1. **Fix Interrupt Immutability (High Priority):** Follow the original plan: remove `USER_INTERRUPT_MARKER` injection into tool results. Instead, when an interrupt is dequeued in `Agent.run()`, append it as a new `user` role chunk to the log *after* the current step's tool results. Remove the stripping logic from `toModelMessages`. This makes the prefix 100% stable. +2. **Explicit Transactions in `appendChunks` (Nit):** Wrap the loop in `appendChunks` (`db/chunks.ts:40`) in an explicit `db.transaction()` to ensure atomicity and improve performance for large turns. +3. **Optimize History Rebuild (Nit):** Consider limiting the history rebuild in `getOrCreateAgentForTab` to the last N turns or the last M chunks, rather than the entire history, to bound startup time for long-lived tabs. +4. **Tab-level Lock in `processMessage` (Nit):** Add a primitive lock or a "running" check at the start of `processMessage` to prevent concurrent execution on the same tab, which could otherwise corrupt the `tabAgent` state or the chunk log if two turns are interleaved. +5. **Update `eviction-limitation.md`**: The document accurately reflects that eviction is still whole-message; this remains a valid technical debt item. diff --git a/notes/harness-comparison.md b/notes/harness-comparison.md new file mode 100644 index 0000000..1fd7aec --- /dev/null +++ b/notes/harness-comparison.md @@ -0,0 +1,373 @@ +# Open-Source AI Agent Harness Comparison + +This report evaluates 20+ open-source AI agent frameworks against the Dispatch requirements. Frameworks are rated on a 15-point checklist covering architecture, configuration, tooling, integration, and session management. Each requirement scores 1 (fully supported), 0.5 (partial), or 0 (not supported), for a maximum of 15. + +Frameworks that are archived, in maintenance mode, or clearly unsuitable (GPT-Engineer, Mentat, Sweep, Bolt.diy, SuperAGI, Semantic Kernel) are excluded from the main comparison but noted in the appendix. + +--- + +## The Critical Gap: No Framework Has a Three-Layer Hierarchy + +The single most distinctive Dispatch requirement -- a three-layer **dispatch -> orchestrator -> subagent** architecture -- does not exist natively in any open-source framework evaluated. Every framework is either: + +- **Single-agent** (Aider, Crush, Plandex, Pi) +- **Two-layer** (Claude Code, Goose, Cline, Agency Swarm, CrewAI) +- **Flat pool** (AutoGen, MetaGPT, CAMEL) +- **DAG-based** (LangGraph, ChatDev 2.0) -- the closest to true hierarchy via subgraph nesting + +This means **any choice involves building the hierarchical orchestration layer yourself**. The question becomes: which framework gives you the best foundation to build on? + +--- + +## Top Contenders + +### Tier 1: Highest Feature Alignment (87% match) + +#### Goose (Block / AAIF) -- 13/15 + +| Requirement | Rating | Notes | +|---|---|---| +| 1. Three-layer hierarchy | Partial | Main agent -> subagents (2 layers). Cannot nest further. | +| 2. Config-driven orchestrators | **Full** | YAML recipes define subagent behavior, extensions, parameters | +| 3. Parallel subagent execution | **Full** | Native parallel subagent support via trigger keywords | +| 4. Strict hierarchy communication | **Full** | Subagents cannot spawn further subagents or manage extensions | +| 5. User-to-agent messaging | **Full** | Continuous sessions, real-time subagent visibility | +| 6. Conflict prevention | Partial | Process isolation; no explicit file-scope assignment | +| 7. Role-scoped tooling | **Full** | Per-subagent extension sets via recipes | +| 8. Skills system | **Full** | `~/.agents/skills/` and `.agents/skills/` with SKILL.md | +| 9. LSP integration | **None** | No LSP support | +| 10. Shell + directory perms | **Full** | Permission modes (auto/approve/smart_approve), allowlists | +| 11. Session management | **Full** | Start, resume, search, model switching | +| 12. HITL checkpoints | **Full** | Permission modes, per-action approval | +| 13. State persistence | **Full** | Session persistence across restarts | +| 14. Provider-agnostic LLM | **Full** | 15+ providers | +| 15. Multiple interfaces | **Full** | Desktop app, CLI, API (ACP server) | + +**Language**: Rust + TypeScript. **Stars**: 45.5k. **Status**: Very active, moved to Linux Foundation AAIF (Apr 2026). Apache 2.0 license. + +**Strengths**: Closest to Dispatch's architecture out of the box. Config-driven recipes, parallel subagents, strict hierarchy enforcement, skills system, permission model, and multi-interface support all map directly to requirements. The MCP extension ecosystem (70+ extensions) provides broad tool coverage. + +**Weaknesses**: Only 2 layers of hierarchy (no recursive nesting). No LSP. Written in Rust, which makes deep architectural modification harder than Python/TypeScript. Subagents cannot spawn sub-subagents. + +**Extensibility verdict**: Adding a third layer would mean building an orchestrator abstraction that manages multiple Goose agent instances, each of which manages its own subagents. The recipe system could potentially be extended to define orchestrator types. + +--- + +#### Cline -- 13/15 + +| Requirement | Rating | Notes | +|---|---|---| +| 1. Three-layer hierarchy | Partial | Coordinator -> specialist agents (2 layers via SDK) | +| 2. Config-driven orchestrators | Partial | Code-driven SDK; CLI flags provide some config | +| 3. Parallel subagent execution | **Full** | Kanban enables parallel agents with separate worktrees | +| 4. Strict hierarchy communication | Partial | Coordinator pattern implies parent-mediated, not enforced | +| 5. User-to-agent messaging | **Full** | Interactive CLI, `ask_question` tool | +| 6. Conflict prevention | Partial | Kanban uses Git worktrees for isolation | +| 7. Role-scoped tooling | **Full** | Per-agent tool sets via plugin system | +| 8. Skills system | **Full** | `.agents/skills/` with SKILL.md files | +| 9. LSP integration | **Full** | VS Code extension integrates with editor LSP | +| 10. Shell + directory perms | **Full** | Command permission allow/deny lists | +| 11. Session management | **Full** | History, persistence, model override per run | +| 12. HITL checkpoints | **Full** | Plan/Act modes, per-action approval, auto-approve toggle | +| 13. State persistence | **Full** | Sessions persist across restarts, snapshot/restore | +| 14. Provider-agnostic LLM | **Full** | 200+ models via OpenRouter, plus direct providers | +| 15. Multiple interfaces | **Full** | CLI, VS Code, JetBrains, Kanban web, SDK | + +**Language**: TypeScript. **Stars**: 62k. **Status**: Very active (v3.0.7, May 2026). Apache 2.0 license. + +**Strengths**: The only framework with real LSP integration (through VS Code). Broadest interface coverage (IDE extensions, CLI, web Kanban, SDK). Git worktree isolation in Kanban is a creative approach to conflict prevention. The SDK (`@cline/core`, `@cline/agents`, `@cline/llms`) is well-layered and embeddable. + +**Weaknesses**: Orchestration is code-driven, not config-driven -- no YAML orchestrator definitions. LSP integration is tied to the VS Code extension host; unclear if it works outside IDE context. Hierarchy enforcement is not strict. TypeScript-only. + +**Extensibility verdict**: The layered SDK architecture is a strong foundation. You could build the dispatch and orchestrator layers on top of `@cline/core` and `@cline/agents`. The plugin system (`AgentPlugin`) provides lifecycle hooks. However, adding config-driven orchestrator definitions would require custom work. + +--- + +#### Claude Code (Anthropic) -- 13/15 (NOT fully open source) + +| Requirement | Rating | Notes | +|---|---|---| +| 1. Three-layer hierarchy | Partial | Main agent -> subagents (2 layers). Agent teams add peer coordination | +| 2. Config-driven orchestrators | Partial | Subagents defined via YAML frontmatter in .md files | +| 3. Parallel subagent execution | **Full** | Multiple concurrent subagents + agent teams | +| 4. Strict hierarchy communication | **Full** | Subagents report to parent only | +| 5. User-to-agent messaging | **Full** | Shift+Down for in-process subagent messaging | +| 6. Conflict prevention | Partial | Git worktrees, file locking in agent teams | +| 7. Role-scoped tooling | **Full** | `tools` and `disallowedTools` in subagent YAML frontmatter | +| 8. Skills system | **Full** | Full Agent Skills standard, YAML frontmatter, multi-scope | +| 9. LSP integration | **Full** | Through IDE integrations (VS Code, JetBrains) | +| 10. Shell + directory perms | **Full** | allow/ask/deny rules, wildcard matching, sandboxing | +| 11. Session management | **Full** | Resume, fork (`/fork`), model switch (`/model`), persistence | +| 12. HITL checkpoints | **Full** | Permission modes, plan mode, hooks for custom approval | +| 13. State persistence | **Full** | Full session persistence in `~/.claude/projects/` | +| 14. Provider-agnostic LLM | Partial | Primarily Claude; Bedrock/Vertex/Azure as backends | +| 15. Multiple interfaces | **Full** | CLI, VS Code, JetBrains, Desktop, Web, Slack, SDKs | + +**Language**: Proprietary core (distributed as npm binary); Shell/Python/TypeScript for installer, plugins, examples. **Stars**: 125k. **Status**: Very active. **License**: Partially open source -- core engine is proprietary. + +**Strengths**: Highest overall feature coverage. Best permission system. Best skills system. Chat forking built-in. Agent SDK available in Python and TypeScript. Hooks system (PreToolUse, PostToolUse, SubagentStart/Stop) provides excellent lifecycle control. + +**Weaknesses**: **Core engine is proprietary.** You cannot fork or modify the agent loop. Primarily Claude-only for models. Building on top means depending on Anthropic's closed binary. This is a dealbreaker if full ownership of the codebase is required. + +**Extensibility verdict**: If you're comfortable depending on a proprietary core, Claude Code's Agent SDK is probably the fastest path to a Dispatch-like system. But you'd be building on a dependency you cannot modify or audit internally. + +--- + +### Tier 2: Strong Architectural Foundation (53-60% match) + +#### LangGraph -- 9/15 + +| Requirement | Rating | Notes | +|---|---|---| +| 1. Three-layer hierarchy | **Full** | Arbitrary subgraph nesting with private state schemas | +| 2. Config-driven orchestrators | **None** | Purely code-defined (Python) | +| 3. Parallel subagent execution | **Full** | Parallel edges, `Send()` for map-reduce fan-out | +| 4. Strict hierarchy communication | **Full** | Subgraph state isolation via separate schemas | +| 5. User-to-agent messaging | **Full** | `interrupt()` / `Command(resume=...)` anywhere | +| 6. Conflict prevention | **None** | No file-scope mechanisms | +| 7. Role-scoped tooling | **Full** | Per-node tool sets | +| 8. Skills system | **None** | No skills/instruction injection system | +| 9. LSP integration | **None** | No LSP | +| 10. Shell + directory perms | **None** | No built-in shell or permissions | +| 11. Session management | Partial | Checkpointer history, time travel, `update_state()` | +| 12. HITL checkpoints | **Full** | `interrupt()`, static breakpoints, approval patterns | +| 13. State persistence | **Full** | Multiple checkpointers (SQLite, Postgres, CosmosDB) | +| 14. Provider-agnostic LLM | **Full** | All LangChain providers + standalone | +| 15. Multiple interfaces | Partial | Python API, LangSmith Studio, LangGraph API | + +**Language**: Python. **Stars**: 32.4k. **Status**: Very active (v1.2.0, May 2026). + +**Key insight**: LangGraph is the **only framework that natively supports arbitrary hierarchy depth** via subgraph nesting. This is the single most important Dispatch requirement. The trade-off: it has zero application-layer features (no skills, no shell, no LSP, no session management in the user-facing sense). It's a low-level orchestration runtime, not an end-user tool. + +**Extensibility verdict**: LangGraph provides the best orchestration primitives but you'd need to build everything else on top -- the CLI, the skills system, the shell with permissions, the LSP integration, session management UI. It's essentially a graph execution engine, not an agent harness. + +--- + +#### CrewAI -- 9/15 + +| Requirement | Rating | Notes | +|---|---|---| +| 1. Three-layer hierarchy | Partial | Manager -> agents (2 layers); Flows chain crews | +| 2. Config-driven orchestrators | **Full** | YAML `agents.yaml`, `tasks.yaml` | +| 3. Parallel subagent execution | **Full** | `async_execution=True` on tasks | +| 4. Strict hierarchy communication | Partial | Manager delegates but `allow_delegation` enables P2P | +| 5. User-to-agent messaging | Partial | `@human_feedback` at configured points only | +| 6. Conflict prevention | **None** | No file-scope mechanisms | +| 7. Role-scoped tooling | **Full** | Per-agent tools, task tool overrides | +| 8. Skills system | Partial | Agent templates, prompt customization | +| 9. LSP integration | **None** | No LSP | +| 10. Shell + directory perms | **None** | Code execution deprecated | +| 11. Session management | Partial | Flow persist/fork via `@persist` | +| 12. HITL checkpoints | **Full** | `@human_feedback`, `human_input=True` | +| 13. State persistence | **Full** | `@persist` with SQLite | +| 14. Provider-agnostic LLM | **Full** | Many providers via LiteLLM | +| 15. Multiple interfaces | Partial | CLI + Python API | + +**Language**: Python. **Stars**: 51.7k. **Status**: Very active, backed by CrewAI Inc. + +**Extensibility verdict**: Best config-driven agent/task definitions. The YAML approach maps well to Dispatch's config-driven orchestrators. However, no shell access, no LSP, no skills directory system. Better suited for general automation than coding-specific workflows. + +--- + +#### ChatDev 2.0 -- 9/15 + +| Requirement | Rating | Notes | +|---|---|---| +| 1. Three-layer hierarchy | Partial | DAG + subgraph nesting, not strict tree | +| 2. Config-driven orchestrators | **Full** | Full YAML workflow definitions | +| 3. Parallel subagent execution | **Full** | Map/Tree modes with `max_parallel` | +| 4. Strict hierarchy communication | Partial | Edge-routed but no parent-only enforcement | +| 5. User-to-agent messaging | Partial | Human nodes in DAG at predefined points | +| 6. Conflict prevention | **None** | No file-scope mechanisms | +| 7. Role-scoped tooling | **Full** | Per-node tooling in YAML | +| 8. Skills system | Partial | `.agents/skills` directory exists | +| 9. LSP integration | **None** | No LSP | +| 10. Shell + directory perms | **None** | No permission system | +| 11. Session management | Partial | Context snapshots, no forking/resume | +| 12. HITL checkpoints | **Full** | Human nodes + edge conditions | +| 13. State persistence | Partial | Artifacts persist, no execution recovery | +| 14. Provider-agnostic LLM | **Full** | Per-node provider config | +| 15. Multiple interfaces | **Full** | Web UI, CLI, HTTP API, Python SDK | + +**Language**: Python + Vue.js. **Stars**: 33.1k. **Status**: Active (v2.2.0, Mar 2026). Very new (released Jan 2026). + +**Extensibility verdict**: Best zero-code workflow definition system. YAML DAGs with subgraphs, parallel execution, and multiple interfaces. The main risk is maturity -- ChatDev 2.0 is only months old and documentation is still evolving. + +--- + +#### OpenHands -- 8.5/15 + +| Requirement | Rating | Notes | +|---|---|---| +| 1. Three-layer hierarchy | **None** | Single Conversation -> Agent -> Tools pipeline | +| 2. Config-driven orchestrators | Partial | SDK is code-driven, config template exists | +| 3. Parallel subagent execution | **None** | One agent step at a time per conversation | +| 4. Strict hierarchy communication | **None** | No hierarchy enforced | +| 5. User-to-agent messaging | **Full** | `send_message()` at any time via WebSocket | +| 6. Conflict prevention | **None** | No scope assignment | +| 7. Role-scoped tooling | **Full** | Per-agent tool sets via typed Action/Observation | +| 8. Skills system | **Full** | Three skill types, YAML frontmatter, MCP integration | +| 9. LSP integration | **None** | No LSP | +| 10. Shell + directory perms | Partial | Risk-based security (LOW/MEDIUM/HIGH), not directory-based | +| 11. Session management | Partial | Persistence + resume, no forking or model switching | +| 12. HITL checkpoints | **Full** | ConfirmationPolicy with configurable thresholds | +| 13. State persistence | **Full** | Auto-save, resume, incremental events | +| 14. Provider-agnostic LLM | **Full** | 100+ providers via LiteLLM | +| 15. Multiple interfaces | **Full** | CLI, React GUI, Cloud, Enterprise, SDK, REST API | + +**Language**: Python + TypeScript. **Stars**: 74.1k. **Status**: Very active (v1.7.0, May 2026). MIT license. + +**Extensibility verdict**: The four-package SDK architecture (`openhands.sdk`, `openhands.tools`, `openhands.workspace`, `openhands.agent_server`) is clean and composable. The event-driven design provides good extension points. However, no native multi-agent hierarchy -- you'd build the orchestration layer from scratch using the SDK primitives. Best choice if you want a battle-tested single-agent SDK with excellent security and skills, and are willing to build hierarchy on top. + +--- + +#### Crush (OpenCode successor) -- 8/15 + +| Requirement | Rating | Notes | +|---|---|---| +| 1. Three-layer hierarchy | **None** | Single-agent with `agent` tool for sub-tasks | +| 2. Config-driven orchestrators | **None** | No orchestrator concept | +| 3. Parallel subagent execution | **None** | Sub-agent tool is sequential | +| 4. Strict hierarchy communication | Partial | Agent tool returns results; no P2P | +| 5. User-to-agent messaging | **None** | User types at main session only | +| 6. Conflict prevention | **None** | No scope assignment | +| 7. Role-scoped tooling | **Full** | Different agents can have different tool sets via config | +| 8. Skills system | **Full** | Agent Skills standard, reads `.crush/`, `.claude/`, `.agents/` | +| 9. LSP integration | **Full** | Built-in LSP with configurable language servers | +| 10. Shell + directory perms | Partial | `allowed_tools` allowlist, no directory scoping | +| 11. Session management | **Full** | Save/load/switch, model switching, SQLite persistence | +| 12. HITL checkpoints | **None** | No checkpoint system | +| 13. State persistence | **Full** | SQLite-based session persistence | +| 14. Provider-agnostic LLM | **Full** | 15+ providers | +| 15. Multiple interfaces | **Full** | Interactive TUI, CLI, scripting | + +**Language**: Go. **Stars**: 24.4k. **Status**: Very active (v0.70.0, May 2026). MIT license. + +**Key insight**: The only CLI-native framework with built-in LSP integration for compiler diagnostics. If LSP is a hard requirement, Crush is one of only two options (the other being Cline's VS Code extension). However, it has no multi-agent architecture at all. + +--- + +#### Pi.dev -- 6.5/15 + +| Requirement | Rating | Notes | +|---|---|---| +| 1. Three-layer hierarchy | **None** | Single-agent. Subagent extension is a bash demo. | +| 2. Config-driven orchestrators | **None** | No orchestrator concept | +| 3. Parallel subagent execution | **None** | Subagent extension demo: 8 tasks, 4 concurrent, via bash | +| 4. Strict hierarchy communication | **None** | No agent communication framework | +| 5. User-to-agent messaging | **Full** | Built-in steer/follow-up message queuing | +| 6. Conflict prevention | **None** | "YOLO mode" by design | +| 7. Role-scoped tooling | Partial | `--tools` flag restricts globally, no role system | +| 8. Skills system | Partial | Agent Skills standard, but no `default/agents/project/` dirs | +| 9. LSP integration | **None** | No LSP | +| 10. Shell + directory perms | **None** | Full unrestricted access by design | +| 11. Session management | **Full** | Tree-structured, fork, clone, resume, model switch | +| 12. HITL checkpoints | Partial | `tool_call` event can block; no built-in checkpoint system | +| 13. State persistence | **Full** | JSONL auto-save, sessions survive restarts | +| 14. Provider-agnostic LLM | **Full** | 15+ providers, cross-provider context handoff | +| 15. Multiple interfaces | **Full** | TUI, CLI, JSON, RPC, SDK, web UI package | + +**Language**: TypeScript. **Stars**: 51.4k. **Status**: Very active (v0.75.3, May 2026). MIT license. + +**Key insight**: Pi has the best session management (tree-structured branching with fork/clone/resume) and the most powerful extension system (30+ lifecycle events, full tool registration, UI components). The `@earendil-works/pi-ai` and `@earendil-works/pi-agent-core` packages are clean, well-documented TypeScript libraries. However, the maintainer explicitly rejects multi-agent patterns ("sub-agents are an anti-pattern"). Building Dispatch on Pi means fighting its philosophy. + +**Best use**: Harvest `pi-ai` (LLM abstraction) and `pi-agent-core` (agent runtime) as libraries in a custom system, rather than extending the Pi CLI. + +--- + +### Eliminated Frameworks + +| Framework | Score | Reason for Elimination | +|---|---|---| +| AutoGen (AG2) | 8.5/15 | **Maintenance mode.** Microsoft recommends migrating to Agent Framework. No new features. | +| Agency Swarm | 8/15 | 2-layer only, code-defined, no LSP/shell/sessions. Active but smaller community (4.4k stars). | +| CAMEL | 6.5/15 | Research-oriented. Sequential workforce. No production features (sessions, perms). | +| Plandex | 6/15 | Single-agent. Strong for plan-then-execute but no hierarchy or multi-agent. | +| MetaGPT | 5/15 | Flat role pool, sequential, broadcast communication. Team pivoting to commercial MGX. | +| Aider | 5/15 | Single-agent pair programmer. No hierarchy, no subagents, no persistence. | +| Semantic Kernel | 4.5/15 | SDK-only, no interfaces, experimental orchestration. Being superseded by MS Agent Framework. | +| SuperAGI | 5.5/15 | Stale (v0.0.11). Single-agent. No hierarchy. | +| SWE-agent | 5/15 | Maintenance mode, superseded by mini-SWE-agent. Academic benchmarking tool. | +| GPT-Engineer | N/A | Archived April 2026. | +| Mentat | N/A | Archived January 2025. | +| Sweep | N/A | Pivoted to JetBrains plugin. | +| Bolt.diy | N/A | Web app builder, not an agent harness. | +| Continue | N/A | Pivoted to CI/CD checks product. | + +--- + +## Summary: Ratings at a Glance + +| Rank | Framework | Score | % | Language | Stars | Key Strength | Key Gap | +|---|---|---|---|---|---|---|---| +| 1 | **Goose** | 13/15 | 87% | Rust/TS | 45.5k | Closest architecture match (recipes, subagents, permissions) | No LSP, no 3rd layer | +| 1 | **Cline** | 13/15 | 87% | TypeScript | 62k | Only framework with LSP + parallel agents + SDK | Config-driven orchestration is weak | +| 1 | **Claude Code** | 13/15 | 87% | Proprietary | 125k | Most complete feature set, best permissions/skills | **Not fully open source** | +| 4 | **LangGraph** | 9/15 | 60% | Python | 32.4k | Only native arbitrary-depth hierarchy | Zero application features (no shell, skills, UI) | +| 4 | **CrewAI** | 9/15 | 60% | Python | 51.7k | Best config-driven agents (YAML) | No shell, no LSP, no skills dirs | +| 4 | **ChatDev 2.0** | 9/15 | 60% | Python/Vue | 33.1k | Best zero-code YAML workflows | Very new (Jan 2026), immature | +| 7 | **OpenHands** | 8.5/15 | 57% | Python/TS | 74.1k | Best SDK architecture, 100+ LLM providers | No hierarchy, no parallel agents | +| 8 | **Crush** | 8/15 | 53% | Go | 24.4k | Only CLI with built-in LSP | No multi-agent anything | +| 9 | **Pi.dev** | 6.5/15 | 43% | TypeScript | 51.4k | Best session management + extension system | Anti-multi-agent philosophy | + +--- + +## Recommendation: Build vs. Extend + +No existing framework is a drop-in match. The choice depends on which gaps you're most willing to fill: + +### Option A: Extend Goose +**Best if**: You want the most features out of the box and are comfortable with Rust/TypeScript. +- **Already have**: Subagents, parallel execution, config recipes, skills, permissions, session management, multi-interface +- **Must build**: Third orchestrator layer, LSP integration, custom directory permissions, skills directory restructuring +- **Risk**: Rust codebase makes deep architectural changes harder. Goose's subagent model may resist being generalized into a full orchestrator pattern. + +### Option B: Build on Cline SDK +**Best if**: You want LSP and a well-layered TypeScript SDK. +- **Already have**: LSP, parallel agents (Kanban), skills, permissions, sessions, plugins, IDE integration +- **Must build**: Config-driven orchestrator definitions, third dispatch layer, strict hierarchy enforcement +- **Risk**: Cline is IDE-first; extracting the SDK for standalone use may have rough edges. + +### Option C: Build on LangGraph +**Best if**: You prioritize getting the hierarchy right and are comfortable building everything else. +- **Already have**: Arbitrary hierarchy depth, parallel execution, interrupts, state persistence, provider support +- **Must build**: Skills system, shell + permissions, LSP, session management UI, CLI/TUI, config-driven orchestrator definitions +- **Risk**: Enormous amount of application-layer work. LangGraph is an execution engine, not an end-user tool. + +### Option D: Build from Scratch, Harvest Libraries +**Best if**: You want full architectural control and no framework fights. +- **Harvest from Pi.dev**: `@earendil-works/pi-ai` (LLM abstraction), `@earendil-works/pi-agent-core` (agent runtime) +- **Harvest from Cline**: `@cline/llms` (provider gateway), `@cline/agents` (stateless agent loop) +- **Harvest from Crush**: LSP integration patterns (Go) +- **Must build**: Everything else -- dispatch layer, orchestrator management, hierarchy enforcement, skills system, permissions, session management +- **Risk**: Highest initial effort, but cleanest architecture alignment. + +--- + +## Sources + +- [Goose GitHub](https://github.com/aaif-goose/goose) -- Architecture, subagents, skills, security docs +- [Goose Documentation](https://goose-docs.ai/) -- Subagents, recipes, config, sessions, security guides +- [Cline GitHub](https://github.com/cline/cline) -- SDK, multi-agent, Kanban, plugins +- [Cline Documentation](https://docs.cline.bot/) -- SDK architecture, tools, CLI, building agents +- [Claude Code GitHub](https://github.com/anthropics/claude-code) -- Installer, plugins, examples +- [Claude Code Documentation](https://code.claude.com/docs/en/overview) -- Subagents, skills, permissions, hooks, agent teams, SDK +- [LangGraph GitHub](https://github.com/langchain-ai/langgraph) -- Graph API, subgraphs, interrupts, persistence +- [LangGraph Documentation](https://docs.langchain.com/oss/python/langgraph/) -- Overview, subgraphs, interrupts, persistence +- [CrewAI GitHub](https://github.com/crewAIInc/crewAI) -- YAML config, agents, tasks, flows +- [CrewAI Documentation](https://docs.crewai.com/) -- Agents, tasks, flows, processes, memory +- [ChatDev GitHub](https://github.com/OpenBMB/ChatDev) -- Workflow authoring guide, YAML definitions +- [OpenHands GitHub](https://github.com/OpenHands/OpenHands) -- SDK overview +- [OpenHands SDK Docs](https://docs.openhands.dev/sdk) -- Architecture, agent, conversation, LLM, skills, security +- [Crush GitHub](https://github.com/charmbracelet/crush) -- LSP, sessions, skills, providers +- [Pi.dev GitHub](https://github.com/earendil-works/pi) -- Extensions, skills, SDK, sessions +- [Pi.dev Extensions Docs](https://github.com/earendil-works/pi/blob/main/packages/coding-agent/docs/extensions.md) -- Event lifecycle, tool registration +- [Pi.dev Blog](https://mariozechner.at/posts/2025-11-30-pi-coding-agent/) -- Design philosophy +- [Agency Swarm GitHub](https://github.com/VRSEN/agency-swarm) -- Communication flows, agents +- [Agency Swarm Docs](https://agency-swarm.ai/) -- Agencies, agents, running +- [AutoGen GitHub](https://github.com/microsoft/autogen) -- Maintenance mode notice, teams, HITL +- [CAMEL GitHub](https://github.com/camel-ai/camel) -- Workforce, toolkits, model factory +- [MetaGPT GitHub](https://github.com/geekan/MetaGPT) -- Roles, teams, serialization +- [Plandex GitHub](https://github.com/plandex-ai/plandex) -- Plan-execute workflow, diff sandbox +- [Aider GitHub](https://github.com/Aider-AI/aider) -- Conventions, modes, LLM support +- [SWE-agent GitHub](https://github.com/SWE-agent/SWE-agent) -- Architecture, tools, batch mode diff --git a/notes/plan-bg-restore.md b/notes/plan-bg-restore.md new file mode 100644 index 0000000..e622669 --- /dev/null +++ b/notes/plan-bg-restore.md @@ -0,0 +1,1294 @@ +# Plan: Background-running agents + layout restore on browser reopen + +> **Audience**: this plan is consumed by two flash subagents running in parallel, +> plus a Gemini review pass. Flash agents are weak and cheap — every code shape, +> import path, function signature, expected behavior, and test assertion is +> spelled out below. Do NOT improvise. Do NOT rename anything. Do NOT touch +> files outside your segment's "Files owned" list. + +--- + +## 1. Spec (the goal) + +Three user-visible behaviors: + +1. **Browser-close keeps agents alive.** If the user closes the browser + window / reloads the page / loses the network, any running agent + continues processing on the backend. No cancellation is triggered. + +2. **Layout restore on browser reopen.** When the page next loads, every + tab that existed at the time the window was closed is restored, in + the same order, with full message history. Tabs whose agents finished + while disconnected appear with the completed message. Tabs whose + agents are still running appear streaming live (the in-flight + assistant message is reconstructed from the backend's in-memory + `currentChunks` plus any new deltas). + +3. **Explicit tab-close cancels + forgets.** Clicking the X on a tab in + the sidebar still cancels the running agent (existing behavior) and + also prevents that tab from being restored next time (existing + behavior — `DELETE /tabs/:id` already archives the row by setting + `is_open = 0`). + +The only NEW work is in Behavior 2. Behaviors 1 and 3 already work on +the current `dev` branch; the implementation must preserve them. + +--- + +## 2. Current state (verified findings) + +The investigation report from explore agent (`task_id ses_19461dcf5ffe4r7wLyAji7Bn5b`) is the canonical +source. Key facts the implementation MUST respect: + +### 2.1 Backend +- The `tabs` table has columns `id, title, key_id, model_id, parent_tab_id, status, is_open, position, created_at, updated_at` (`packages/core/src/db/index.ts:77-88`). +- `archiveTab(id)` sets `is_open = 0` — never hard-deletes. Used by `DELETE /tabs/:id`. (`packages/core/src/db/tabs.ts:118-124`). +- `listOpenTabs()` returns rows where `is_open = 1`, ordered by `position` (`packages/core/src/db/tabs.ts:80-86`). +- `agentManager.tabAgents` is the in-memory `Map` where each TabAgent tracks `agent`, `status`, `currentChunks: Chunk[] | null`, `currentAssistantId: string | null`, `messageQueue: QueuedMessage[]` (`packages/api/src/agent-manager.ts:137-177`). +- `getAllStatuses()` currently returns `Record` — just the strings (`packages/api/src/agent-manager.ts:714-720`). +- `processMessage` is fire-and-forget: `app.ts:77-79` calls `.processMessage(...).catch(console.error)` and returns immediately. +- WS `onClose` does NOT stop agents — it only unsubscribes the per-client event listener (`packages/api/src/index.ts:58-66`). +- `flushAssistant` (the per-turn DB write) is only called at turn-end (`done`) or on system events — NOT on every delta. Mid-stream chunks live in memory only. + +### 2.2 Frontend +- `App.svelte:onMount` currently does: `wsClient.connect()` → `fetchModels()` → `if (tabStore.tabs.length === 0) tabStore.createNewTab()`. **It never reads existing backend tabs.** Every page load is a clean slate. +- Tab state lives only in Svelte `$state` — zero `localStorage` / `IndexedDB` persistence of tab ids. +- The existing `statuses` WS event handler (`tabs.svelte.ts:419-446`) reconciles status drift but only for tabs the frontend already has in `$state`. Tabs the frontend doesn't know about are silently ignored. + +### 2.3 The wire shapes (the flash agents MUST match these exactly) +- `GET /tabs` → `{ tabs: TabRow[] }` where each `TabRow = { id, title, keyId, modelId, parentTabId, status, isOpen, position, createdAt, updatedAt }`. **Note camelCase** — the DB function `listOpenTabs` translates snake_case to camelCase. +- `GET /tabs/:id/messages` → `{ messages: Array<{ id, role, chunks, ... }> }`. +- `GET /status` → `{ status, messageCount, statuses }`. The `statuses` field's shape is being changed in this plan. +- `DELETE /tabs/:id` → `{ success: true }`. Already cancels + archives. Unchanged. +- `POST /tabs` body `{ id?: string; title?: string }` → returns the new tab. Unchanged. + +--- + +## 3. Design + +### 3.1 Strategy + +The backend already runs agents independently of WS subscribers (Behavior 1 +works for free). The persistent record (DB) already survives across browser +sessions (Behavior 2's data is already on disk). The X-button already +cancels and archives (Behavior 3 works for free). + +So the entire feature reduces to: **on browser reopen, the frontend must +fetch the persisted tabs and rebuild the UI state, and for any tab that's +still streaming it must pick up the live event flow without losing the +chunks that were emitted before the WS handshake completed.** + +### 3.2 The mid-stream catch-up problem + +If a tab is `running` at the moment the new browser session connects, the +DB has chunks from the most recent `flushAssistant` call (turn-end or +system event), NOT the live in-memory `currentChunks` array. The frontend +needs that in-memory array to render the streaming assistant message +correctly. + +**Solution**: enrich the existing `statuses` snapshot (sent over both +`GET /status` HTTP and the WS `onOpen`) to include `currentChunks` and +`currentAssistantId` for every running tab. The frontend uses this +snapshot to seed the in-flight assistant message before live deltas +arrive. The race is safe because: + +1. JS is single-threaded — reading `currentChunks` and serializing it in + the WS `onOpen` handler is atomic with respect to other event-loop + ticks. +2. The frontend treats the snapshot as authoritative initial state for + the in-flight assistant message; subsequent live deltas append on top + via the existing `applyChunkEvent` path. + +### 3.3 What's NOT in scope + +- Per-delta DB flushing (not needed — snapshot covers in-flight state). +- Persisting queued messages across server restart (server restart kills + the in-memory agent anyway; queued messages were always best-effort). +- Subagent (`parentTabId != null`) ordering changes — they restore the + same way as user tabs; the existing `TabBar.svelte` already separates + parent vs child rows. +- LocalStorage cache of tabs (the backend DB is the source of truth). +- Migrating any DB schema (the existing schema already supports + everything we need via `is_open`). + +--- + +## 4. Phase plan + +``` +Phase 0 (sequential, I do it) + ├─ Add shared `TabStatusSnapshot` type in packages/core/src/types/index.ts + ├─ Re-export from packages/core/src/index.ts + └─ Verify core typecheck + biome + +Phase 1 (parallel, two flash agents) + ├─ Segment A: Backend — flash agent A + │ ├─ packages/api/src/agent-manager.ts + │ └─ packages/api/tests/agent-manager.test.ts + │ + └─ Segment B: Frontend — flash agent B + ├─ packages/frontend/src/lib/types.ts (mirror type) + ├─ packages/frontend/src/lib/tabs.svelte.ts (hydrateFromBackend + statuses handler update) + ├─ packages/frontend/src/App.svelte (onMount sequencing) + └─ packages/frontend/tests/chat-store.test.ts + +Phase 2 (sequential, I do it) + ├─ Run typecheck on all three packages + ├─ Run tests on all three packages + ├─ Run biome + └─ Sanity-check the integration manually + +Phase 3 (I do it) + ├─ Launch gemini subagent for read-only review (writes report.md only) + ├─ Do my own review in parallel + └─ WAIT for gemini to finish before applying any fixes + +Phase 4 (I do it, possibly spawning more flash agents) + └─ Apply fixes from gemini + self-review +``` + +--- + +## 5. Phase 0 — Shared type (main agent only) + +### 5.1 File: `packages/core/src/types/index.ts` + +Add the following ABOVE the existing `AgentEvent` discriminated union (i.e. +in the "Agent Status & Events" section, immediately after `AgentStatus` +type definition): + +```ts +/** + * Snapshot of a single tab's live state, sent on WS connect and via + * `GET /status`. Carries enough information for a freshly-loaded + * frontend to reconstruct any in-flight assistant message. + * + * - `status` — always present; mirrors the in-memory `TabAgent.status`. + * - `currentChunks` — the live in-flight `Chunk[]` for the running + * assistant turn. Present iff `status === "running"` AND + * `TabAgent.currentChunks` is non-null. Defensively copied at + * snapshot time; the consumer owns the array. + * - `currentAssistantId` — DB id of the in-flight assistant message + * (the row that the eventual `flushAssistant` call will write/update). + * Present iff `status === "running"` AND `TabAgent.currentAssistantId` + * is set. The frontend uses this to align its local assistant message + * id with the persisted id so subsequent `done` / reload paths line up. + */ +export interface TabStatusSnapshot { + status: AgentStatus; + currentChunks?: Chunk[]; + currentAssistantId?: string; +} +``` + +Then UPDATE the existing `statuses` variant of `AgentEvent` (currently around line 134, but verify with grep on `"statuses"` literal) to use the new shape: + +```ts +// before: +// | { type: "statuses"; statuses: Record } +// after: + | { type: "statuses"; statuses: Record } +``` + +### 5.2 File: `packages/core/src/index.ts` + +If `TabStatusSnapshot` is not already re-exported via a `export * from "./types"` (it almost certainly already is — verify by reading the file), no change needed. Otherwise add it to the explicit type re-export list. + +### 5.3 Verification + +```sh +bun run --cwd packages/core typecheck +bun run --cwd packages/core test +bunx biome check packages/core +``` + +All three must be clean. If they're not, fix the type issue before +proceeding to Phase 1. + +--- + +## 6. Phase 1 — Parallel implementation + +> **CRITICAL FOR FLASH AGENTS**: read your segment in full BEFORE touching +> any file. Every file path, function signature, and code block is exact. +> Do not improvise. If something is ambiguous, leave a `TODO(plan-bg-restore):` +> comment instead of guessing. + +### 6.A SEGMENT A — Backend + +**Files owned (exclusive write access):** +- `packages/api/src/agent-manager.ts` +- `packages/api/tests/agent-manager.test.ts` + +**Files allowed to READ (do not modify):** +- `packages/core/src/types/index.ts` (already has `TabStatusSnapshot` from Phase 0) +- `packages/api/src/app.ts` (already calls `agentManager.getAllStatuses()` — its consumption pattern shows the contract) +- `packages/api/src/index.ts` (already calls `agentManager.getAllStatuses()` in the WS `onOpen` — same contract) + +#### Task A.1 — Add the snapshot import + +At the top of `packages/api/src/agent-manager.ts`, locate the existing import block from `@dispatch/core`. Add `TabStatusSnapshot` to that type-import list. Example: + +```ts +// Before (illustrative; merge with the existing import in place): +import type { AgentEvent, AgentStatus, Chunk, ChatMessage } from "@dispatch/core"; + +// After: +import type { AgentEvent, AgentStatus, Chunk, ChatMessage, TabStatusSnapshot } from "@dispatch/core"; +``` + +#### Task A.2 — Rewrite `getAllStatuses` + +Find the existing method at `packages/api/src/agent-manager.ts:714-720`. It currently reads: + +```ts +getAllStatuses(): Record { + const result: Record = {}; + for (const [tabId, tabAgent] of this.tabAgents.entries()) { + result[tabId] = tabAgent.status; + } + return result; +} +``` + +Replace with: + +```ts +/** + * Snapshot of every tab the manager is currently tracking. Sent on WS + * connect and via GET /status so a freshly-loaded frontend can + * reconstruct any in-flight assistant turn without missing the chunks + * that arrived before its WS handshake completed. + * + * For each running tab, the snapshot includes: + * - status: "running" + * - currentChunks: a defensive shallow copy of `tabAgent.currentChunks` + * (the live chunk array the streaming loop appends to). The + * consumer owns this copy and may mutate it freely. + * - currentAssistantId: the DB id of the in-flight assistant message + * row. The frontend aligns its local assistant message id with + * this so the next `done` event lands on the right message. + * + * For idle/error tabs, only `status` is present. Tabs not in + * `this.tabAgents` (e.g. tabs in the DB that have never been touched + * since server start) are absent from the returned record — the + * caller infers their status from the DB row (always "idle" at rest). + */ +getAllStatuses(): Record { + const result: Record = {}; + for (const [tabId, tabAgent] of this.tabAgents.entries()) { + const snap: TabStatusSnapshot = { status: tabAgent.status }; + if (tabAgent.status === "running") { + if (tabAgent.currentChunks) { + // Defensive shallow copy: callers may serialize/mutate. + snap.currentChunks = [...tabAgent.currentChunks]; + } + if (tabAgent.currentAssistantId) { + snap.currentAssistantId = tabAgent.currentAssistantId; + } + } + result[tabId] = snap; + } + return result; +} +``` + +**DO NOT** add a separate `getTabSnapshot(tabId)` method. The single +`getAllStatuses` covers every consumer. + +#### Task A.3 — Tests + +In `packages/api/tests/agent-manager.test.ts`, add a new `describe` block at the end of the existing top-level `describe("AgentManager", ...)`. Place it as the LAST sub-section, AFTER the existing "done event includes a thinking chunk with metadata in its message" test and AFTER the "History pre-population on Agent (re)construction" tests already in place. Use this exact code: + +```ts + // ─── getAllStatuses snapshot shape (for browser-reopen restore) ──── + // + // The snapshot enriches the legacy `Record` shape + // with per-tab in-flight context so a fresh frontend can render the + // streaming assistant message correctly after a reload. + + it("getAllStatuses returns an empty record when no tabs are tracked", () => { + const manager = new AgentManager(); + expect(manager.getAllStatuses()).toEqual({}); + }); + + it("getAllStatuses returns { status } for an idle tab (no currentChunks/currentAssistantId)", async () => { + const manager = new AgentManager(); + // Drive a full turn so the tab gets registered; default mock run + // settles back to idle by the time `await` resolves. + await manager.processMessage("tab-idle", "hi"); + const snap = manager.getAllStatuses(); + expect(snap["tab-idle"]).toBeDefined(); + expect(snap["tab-idle"]?.status).toBe("idle"); + expect(snap["tab-idle"]).not.toHaveProperty("currentChunks"); + expect(snap["tab-idle"]).not.toHaveProperty("currentAssistantId"); + }); + + it("getAllStatuses includes currentChunks and currentAssistantId for a running tab", () => { + const manager = new AgentManager(); + // Reach into the private map to set up a synthetic running state. + // Justification: there is no public API to enter a sustained + // "running" state without actually streaming, and we want to + // assert the snapshot shape — not the streaming pipeline. + const inner = manager as unknown as { + tabAgents: Map void }; + messageQueue: unknown[]; + queueListeners: unknown[]; + shellStore: unknown; + transcriptStore: unknown; + currentChunks: Array<{ type: string; text?: string }> | null; + currentAssistantId: string | null; + }>; + }; + inner.tabAgents.set("tab-running", { + agent: null, + status: "running", + keyId: null, + modelId: null, + taskList: { onChange: () => {} }, + messageQueue: [], + queueListeners: [], + shellStore: {}, + transcriptStore: {}, + currentChunks: [ + { type: "thinking", text: "let me think" }, + { type: "text", text: "partial answer" }, + ], + currentAssistantId: "assistant-msg-id-7", + }); + + const snap = manager.getAllStatuses(); + expect(snap["tab-running"]).toBeDefined(); + expect(snap["tab-running"]?.status).toBe("running"); + expect(snap["tab-running"]?.currentAssistantId).toBe("assistant-msg-id-7"); + expect(snap["tab-running"]?.currentChunks).toEqual([ + { type: "thinking", text: "let me think" }, + { type: "text", text: "partial answer" }, + ]); + }); + + it("getAllStatuses defensively copies currentChunks (mutating the snapshot doesn't affect the live array)", () => { + const manager = new AgentManager(); + const inner = manager as unknown as { + tabAgents: Map void }; + messageQueue: unknown[]; + queueListeners: unknown[]; + shellStore: unknown; + transcriptStore: unknown; + currentChunks: Array<{ type: string; text?: string }>; + currentAssistantId: string; + }>; + }; + const liveChunks = [{ type: "text", text: "live" }]; + inner.tabAgents.set("tab-copy", { + agent: null, + status: "running", + keyId: null, + modelId: null, + taskList: { onChange: () => {} }, + messageQueue: [], + queueListeners: [], + shellStore: {}, + transcriptStore: {}, + currentChunks: liveChunks, + currentAssistantId: "msg-x", + }); + + const snap = manager.getAllStatuses(); + // Mutate the snapshot's array + snap["tab-copy"]?.currentChunks?.push({ type: "text", text: "polluted" }); + // Live array must be untouched + expect(liveChunks).toEqual([{ type: "text", text: "live" }]); + }); + + it("getAllStatuses omits currentChunks when a running tab has none yet", () => { + const manager = new AgentManager(); + const inner = manager as unknown as { + tabAgents: Map void }; + messageQueue: unknown[]; + queueListeners: unknown[]; + shellStore: unknown; + transcriptStore: unknown; + currentChunks: null; + currentAssistantId: null; + }>; + }; + inner.tabAgents.set("tab-early", { + agent: null, + status: "running", + keyId: null, + modelId: null, + taskList: { onChange: () => {} }, + messageQueue: [], + queueListeners: [], + shellStore: {}, + transcriptStore: {}, + currentChunks: null, + currentAssistantId: null, + }); + + const snap = manager.getAllStatuses(); + expect(snap["tab-early"]?.status).toBe("running"); + expect(snap["tab-early"]).not.toHaveProperty("currentChunks"); + expect(snap["tab-early"]).not.toHaveProperty("currentAssistantId"); + }); +``` + +#### Task A.4 — Verify + +```sh +bun run --cwd packages/api typecheck # must pass +bun run --cwd packages/api test # all tests must pass (existing + new) +bunx biome check packages/api # must be clean (no errors) +``` + +If biome reports formatting issues, run `bunx biome check --write packages/api` and re-verify. + +#### Task A.5 — What NOT to do + +- **DO NOT** modify `packages/api/src/index.ts`. The WS `onOpen` already calls `agentManager.getAllStatuses()` — the wire payload changes automatically when the return type changes. +- **DO NOT** modify `packages/api/src/app.ts`. The `GET /status` route already calls `agentManager.getAllStatuses()` — same automatic propagation. +- **DO NOT** modify the frontend. Segment B owns it. +- **DO NOT** change any existing test cases. Only add the new ones described above. +- **DO NOT** add or change the `getStatus()` (singular, deprecated) method. Leave it as-is. +- **DO NOT** add or change `getTabStatus(tabId)`. Leave it returning `AgentStatus`. + +--- + +### 6.B SEGMENT B — Frontend + +**Files owned (exclusive write access):** +- `packages/frontend/src/lib/types.ts` +- `packages/frontend/src/lib/tabs.svelte.ts` +- `packages/frontend/src/App.svelte` +- `packages/frontend/tests/chat-store.test.ts` + +**Files allowed to READ (do not modify):** +- `packages/core/src/types/index.ts` (already has `TabStatusSnapshot` from Phase 0; you'll mirror the type) +- `packages/frontend/src/lib/config.ts` (gives you `config.apiBase`) +- `packages/frontend/src/lib/ws.svelte.ts` (gives you `wsClient.connect()`) + +#### Task B.1 — Mirror the `TabStatusSnapshot` type in the frontend + +The frontend deliberately mirrors core's type shapes locally (see the existing `Chunk` and `AgentEvent` types in `packages/frontend/src/lib/types.ts:21-127`). Add the snapshot type next to those. + +In `packages/frontend/src/lib/types.ts`, find the existing `AgentEvent` discriminated union (currently at line 79). The `statuses` variant currently reads: + +```ts +| { type: "statuses"; statuses: Record } +``` + +Replace this single variant with: + +```ts +| { type: "statuses"; statuses: Record } +``` + +Then add the `TabStatusSnapshot` interface immediately before the `AgentEvent` union (i.e. between the existing `ConnectionStatus` type and the `AgentEvent` union): + +```ts +/** + * Mirror of core's `TabStatusSnapshot` (see packages/core/src/types/index.ts). + * + * Sent on every WS (re)connect and via `GET /status`. The frontend uses + * this to: + * - reconcile its in-memory `agentStatus` with the backend's truth + * after a disconnect window; + * - reconstruct the in-flight assistant message for any tab the + * backend is currently streaming, so the user sees the partial + * thinking / text without waiting for the next delta. + * + * Wire-format symmetry MUST be kept with core. If you change one, + * change the other. + */ +export interface TabStatusSnapshot { + status: "idle" | "running" | "error"; + currentChunks?: Chunk[]; + currentAssistantId?: string; +} +``` + +#### Task B.2 — Add `hydrateFromBackend()` to the tab store + +In `packages/frontend/src/lib/tabs.svelte.ts`, add a new function `hydrateFromBackend()` and export it from the store. Place the function definition adjacent to the existing `openAgentTab` and `reloadTabMessagesFromApi` (around line 172-400 of the file). + +The function should be defined inside the `createTabStore` factory (or wherever the existing exported functions like `createNewTab`, `closeTab` live). It should be exported by adding it to the returned object literal at the bottom of `createTabStore`. + +Exact behavior (see also Task B.3 and B.4 for the integration): + +```ts +/** + * Hydrate the tab store from the backend on app mount. Restores the + * full list of open tabs (every row with `is_open = 1` in the DB), + * loads each tab's persisted message history, and seeds the in-flight + * assistant message for any tab the backend is currently streaming. + * + * Wire calls: + * - GET /tabs → list of open tabs in `position` order + * - GET /tabs/:id/messages → persisted ChatMessage[] for each + * - GET /status → in-flight TabStatusSnapshot map + * + * Failure modes (all log + continue with whatever was successfully + * hydrated; callers fall back to creating a fresh tab if the final + * `tabs` array is empty): + * - /tabs request fails → no tabs restored + * - /tabs/:id/messages fails → that tab restored with empty messages + * - /status fails → tabs restored, in-flight streaming will be + * lost (will surface as a static "running" status until the next + * event arrives); harmless because the WS will broadcast `statuses` + * on reconnect anyway. + * + * Returns the number of tabs hydrated (0 on total failure, ≥1 on + * partial or full success). Caller uses this to decide whether to + * create a fresh tab. + * + * Idempotency: if `tabs.length > 0` when called, returns 0 without + * touching state — the caller already has tabs from elsewhere (e.g. + * a hot-reload that preserved Svelte state). + */ +async function hydrateFromBackend(): Promise { + if (tabs.length > 0) return 0; + + // 1. Fetch the list of open tabs from the DB. + let tabRows: Array<{ + id: string; + title: string; + keyId?: string | null; + modelId?: string | null; + parentTabId?: string | null; + }> = []; + try { + const res = await fetch(`${config.apiBase}/tabs`); + if (!res.ok) return 0; + const data = (await res.json()) as { tabs?: typeof tabRows }; + tabRows = Array.isArray(data.tabs) ? data.tabs : []; + } catch { + return 0; + } + + if (tabRows.length === 0) return 0; + + // 2. Fetch the in-flight snapshot. Failure is non-fatal. + let statusMap: Record = {}; + try { + const res = await fetch(`${config.apiBase}/status`); + if (res.ok) { + const data = (await res.json()) as { statuses?: Record }; + if (data.statuses && typeof data.statuses === "object") { + statusMap = data.statuses; + } + } + } catch { + // Non-fatal: tabs still restore with idle status. + } + + // 3. For each tab, fetch its persisted messages in parallel. + const messageFetches = tabRows.map(async (row) => { + try { + const res = await fetch(`${config.apiBase}/tabs/${row.id}/messages`); + if (!res.ok) return { id: row.id, messages: [] as ChatMessage[] }; + const data = (await res.json()) as { + messages?: Array<{ id?: string; role: string; chunks?: Chunk[] }>; + }; + const messages: ChatMessage[] = (data.messages ?? []).map((m) => ({ + id: m.id ?? generateId(), + role: m.role as ChatMessage["role"], + chunks: Array.isArray(m.chunks) ? m.chunks : [], + isStreaming: false, + })); + return { id: row.id, messages }; + } catch { + return { id: row.id, messages: [] as ChatMessage[] }; + } + }); + + const messagesByTab = new Map(); + for (const result of await Promise.all(messageFetches)) { + messagesByTab.set(result.id, result.messages); + } + + // 4. Build the Tab objects, splicing in the in-flight snapshot for + // running tabs. + const restored: Tab[] = tabRows.map((row) => { + const snap = statusMap[row.id]; + const messages = messagesByTab.get(row.id) ?? []; + const agentStatus: Tab["agentStatus"] = snap?.status ?? "idle"; + + let currentAssistantId: string | null = null; + let finalMessages = messages; + + if (agentStatus === "running" && snap?.currentAssistantId) { + currentAssistantId = snap.currentAssistantId; + // Find or create the in-flight assistant message. If the DB + // already has a row with this id (the backend appended on + // first flush and we picked it up via /tabs/:id/messages), + // merge the snapshot chunks on top — the snapshot is the + // live source of truth and may have chunks the DB doesn't. + // If there's no matching row, append a new in-flight + // assistant message holding only the snapshot chunks. + const existingIdx = finalMessages.findIndex((m) => m.id === snap.currentAssistantId); + if (existingIdx >= 0) { + finalMessages = finalMessages.map((m, i) => + i === existingIdx + ? { + ...m, + chunks: snap.currentChunks ? [...snap.currentChunks] : m.chunks, + isStreaming: true, + } + : m, + ); + } else { + finalMessages = [ + ...finalMessages, + { + id: snap.currentAssistantId, + role: "assistant", + chunks: snap.currentChunks ? [...snap.currentChunks] : [], + isStreaming: true, + }, + ]; + } + } + + return { + id: row.id, + title: row.title, + messages: finalMessages, + agentStatus, + keyId: row.keyId ?? null, + modelId: row.modelId ?? null, + reasoningEffort: "max", + currentAssistantId, + tasks: [], + injectedSkills: [], + parentTabId: row.parentTabId ?? null, + persistent: true, + agentSlug: null, + agentScope: null, + agentModels: null, + workingDirectory: null, + queuedMessages: [], + }; + }); + + tabs = restored; + // Activate the first restored tab (the list is already ordered by + // `position` from the backend). + activeTabId = restored[0]?.id ?? null; + return restored.length; +} +``` + +Then add `hydrateFromBackend` to the returned object at the bottom of `createTabStore` so it's accessible via `tabStore.hydrateFromBackend()`. Find the existing `return { ... }` block (it lists `createNewTab`, `switchTab`, `closeTab`, etc.) and add `hydrateFromBackend,` to it. **DO NOT** reorder existing keys. + +#### Task B.3 — Update the WS `statuses` handler + +In `packages/frontend/src/lib/tabs.svelte.ts`, find the existing `case "statuses":` block inside `handleEvent` (currently around line 419-446). It currently reads: + +```ts +case "statuses": { + const backend = event.statuses; + for (const t of tabs) { + const backendStatus = backend[t.id] ?? "idle"; + if (t.agentStatus === "running" && backendStatus !== "running") { + void reloadTabMessagesFromApi(t.id); + } + if (t.agentStatus !== backendStatus) { + updateTab(t.id, { agentStatus: backendStatus }); + } + if (backendStatus !== "running" && t.currentAssistantId) { + updateMessages(t.id, (msgs) => + msgs.map((m) => (m.id === t.currentAssistantId ? { ...m, isStreaming: false } : m)), + ); + updateTab(t.id, { currentAssistantId: null }); + } + } + break; +} +``` + +Replace this entire `case "statuses":` block with the following: + +```ts +case "statuses": { + // WS (re)connect snapshot. The shape was widened to + // TabStatusSnapshot (status + optional currentChunks + + // optional currentAssistantId) so the frontend can seed + // in-flight assistant messages on browser reopen. + const backend = event.statuses; + for (const t of tabs) { + const snap = backend[t.id]; + const backendStatus = snap?.status ?? "idle"; + + // Desync case: frontend thought it was streaming, backend + // has already moved on. Pull the persisted chunks so the + // final answer shows up. + if (t.agentStatus === "running" && backendStatus !== "running") { + void reloadTabMessagesFromApi(t.id); + } + + // Status alignment. + if (t.agentStatus !== backendStatus) { + updateTab(t.id, { agentStatus: backendStatus }); + } + + if (backendStatus === "running") { + // Seed the in-flight assistant message from the snapshot. + // This handles the "browser just reopened mid-stream" + // path: the DB only has chunks up to the last + // flushAssistant call, but the snapshot has the live + // in-memory currentChunks. + if (snap?.currentAssistantId) { + const targetId = snap.currentAssistantId; + updateTab(t.id, { currentAssistantId: targetId }); + updateMessages(t.id, (msgs) => { + const idx = msgs.findIndex((m) => m.id === targetId); + if (idx >= 0) { + return msgs.map((m, i) => + i === idx + ? { + ...m, + chunks: snap.currentChunks ? [...snap.currentChunks] : m.chunks, + isStreaming: true, + } + : m, + ); + } + return [ + ...msgs, + { + id: targetId, + role: "assistant", + chunks: snap.currentChunks ? [...snap.currentChunks] : [], + isStreaming: true, + }, + ]; + }); + } + } else if (t.currentAssistantId) { + // Not running: clear streaming flags. + updateMessages(t.id, (msgs) => + msgs.map((m) => + m.id === t.currentAssistantId ? { ...m, isStreaming: false } : m, + ), + ); + updateTab(t.id, { currentAssistantId: null }); + } + } + break; +} +``` + +**DO NOT** modify any other case in the `handleEvent` switch. + +#### Task B.4 — Update `App.svelte` onMount to hydrate + +In `packages/frontend/src/App.svelte`, find the existing `onMount` (currently lines 78-99). It reads: + +```ts +onMount(() => { + // Apply saved theme + const saved = localStorage.getItem(STORAGE_KEY); + if (saved) { + document.documentElement.setAttribute("data-theme", saved); + } + + // Connect WebSocket + wsClient.connect(); + + // Initial models fetch + fetchModels(); + + // Create initial tab + if (tabStore.tabs.length === 0) { + tabStore.createNewTab(); + } + + return () => { + wsClient.disconnect(); + }; +}); +``` + +Replace its body so it (1) hydrates tabs from the backend BEFORE deciding to create a fresh tab, and (2) connects the WS in parallel with hydration (since hydration uses HTTP, the WS connect can race ahead — the WS `statuses` handler is now idempotent for already-restored tabs): + +```ts +onMount(() => { + // Apply saved theme + const saved = localStorage.getItem(STORAGE_KEY); + if (saved) { + document.documentElement.setAttribute("data-theme", saved); + } + + // Connect WebSocket in parallel with hydration. The `statuses` + // snapshot delivered on WS open is idempotent against + // already-hydrated tabs (the handler reconciles per-tab). + wsClient.connect(); + + // Initial models fetch (fire-and-forget; UI tolerates models + // arriving later than tabs). + fetchModels(); + + // Restore tabs from the backend. The user's previous session is + // the source of truth; only fall back to a fresh tab if nothing + // was restored (first-ever load, or DB was wiped, or HTTP failed). + void (async () => { + const restored = await tabStore.hydrateFromBackend(); + if (restored === 0 && tabStore.tabs.length === 0) { + await tabStore.createNewTab(); + } + })(); + + return () => { + wsClient.disconnect(); + }; +}); +``` + +**DO NOT** change the `