From 2f14260bb0f1a51d51e516feda285b68f793ae1b Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Thu, 28 May 2026 07:15:32 +0900 Subject: fix(api): pre-populate Agent.messages from DB on construction so model switches preserve chat history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, swapping the model mid-conversation via the sidebar slider lost all prior turns: the new model saw only the current user message and treated the conversation as brand-new. Root cause: `getOrCreateAgentForTab` invalidates the cached Agent (`tabAgent.agent = null`) whenever the effective keyId, modelId, permission key, or working directory differs from the cached values. The replacement Agent was then constructed with `messages: []` and the post-construction step that loads prior turns from the SQLite `messages` table simply did not exist. `processMessage` had already appended the current turn's user message to the DB (line 960) before calling `getOrCreateAgentForTab` (line 1015), so the DB held the full context — it was just never read. Fix: after every `new Agent(...)` in `getOrCreateAgentForTab`, call `getMessagesForTab(tabId)`, walk backwards to the most recent user-role row, and assign all strictly-prior rows to `tabAgent.agent.messages`. The walk-backwards strategy correctly handles two boundary cases: 1. Simple model switch — last DB row is the current user message; drop it (`Agent.run()` will push it again at line 546). 2. Agent-mode auto-fallback retry — last DB row may be a partial assistant response flushed by the previous failed attempt; we drop both that and the current user message in one pass. System-role rows (config-reload notices, etc.) are preserved verbatim; `toModelMessages` already strips them before the wire payload, so this is safe. The fix covers every Agent-reconstruction trigger, not just the model slider: - Sidebar model/key change (the reported case) - Permission setting change - Working-directory change (`processMessage` line 951) - dispatch.toml config-watcher reload (lines 236–237) - Skills directory watcher reload (lines 249–250) - `stopTab` after user cancellation (line 775) If `getMessagesForTab` throws (e.g. DB locked, schema mismatch), we swallow the error and leave `messages: []` — matching pre-fix behaviour for that case so this commit never regresses. Tests (+6 in `packages/api/tests/agent-manager.test.ts`, total 26): - pre-populates Agent.messages from DB history - leaves messages empty when DB has only the current turn (first msg) - excludes a partial assistant trail from a prior fallback attempt - preserves system-role rows in pre-populated history - survives a getMessagesForTab failure without crashing - reloads history on every Agent reconstruction (simulated slider switch from Opus to DeepSeek across two processMessage calls) The test rig was extended with: - `fakeMessagesByTab` map + `setFakeMessages` helper letting tests inject arbitrary DB rows for the mocked `getMessagesForTab`. - `constructedAgents` array captured at `run()` entry (not at construction) so each test sees the post-pre-populate snapshot; the production code reassigns `agent.messages` after `new Agent()` returns, so capturing at construction yielded a stale empty array. - Pluggable `runImpl` hook for tests that want a custom event stream (not yet exercised; staged for the next round of agent-mode fallback tests). Totals: 229 tests across 3 packages all green; typecheck clean on core + api + frontend; biome clean across 124 files. --- packages/api/src/agent-manager.ts | 49 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) (limited to 'packages/api/src') diff --git a/packages/api/src/agent-manager.ts b/packages/api/src/agent-manager.ts index 6e03adb..efdd732 100644 --- a/packages/api/src/agent-manager.ts +++ b/packages/api/src/agent-manager.ts @@ -654,6 +654,55 @@ export class AgentManager { waitForQueuedMessage: () => this.waitForQueuedMessage(tabId), }, ); + + // Pre-populate the Agent's in-memory message history from the DB + // so prior turns survive Agent recreation. The Agent is + // constructed fresh here in three scenarios that ALL discard + // the previous in-memory `messages` array: + // 1. First call for this tab (no prior Agent existed) + // 2. Model/key/permission/working-directory change — the + // invalidation gate above set `tabAgent.agent = null`. + // This is the model-switcher-slider case: without this + // pre-population, DeepSeek would see zero context after + // switching from Opus mid-conversation. + // 3. Config or skills reload (configWatcher / skillsWatcher + // also null out `tabAgent.agent`). + // + // Boundary semantics: `processMessage` calls `appendMessage` + // for the current turn's user message BEFORE calling this + // function, so the DB ends in `[..., u_current]`. In the + // fallback retry path (agent-mode automatic model fallback), + // the previous attempt may also have flushed a partial + // assistant response, so the DB ends in + // `[..., u_current, partial_a]`. Either way, we walk + // backwards to the most recent user-role row and load only + // strictly-prior rows: `agent.run()` will push the current + // user message itself at agent.ts:546, so including it here + // would duplicate it. + // + // `toModelMessages` already filters out `role === "system"` + // rows and strips `error` / `system` chunks, so it's safe to + // load system messages verbatim. + try { + const rows = getMessagesForTab(tabId); + let cutIdx = rows.length; + for (let i = rows.length - 1; i >= 0; i--) { + const row = rows[i]; + if (row && row.role === "user") { + cutIdx = i; + break; + } + } + if (cutIdx > 0) { + tabAgent.agent.messages = rows + .slice(0, cutIdx) + .map((r) => ({ role: r.role, chunks: r.chunks })); + } + } catch { + // DB read failed — leave `messages: []`. The agent still + // works, just without prior history (matches pre-fix + // behaviour, so this is no worse than what we had before). + } } return tabAgent.agent; } -- cgit v1.2.3