-
Notifications
You must be signed in to change notification settings - Fork 0
feat(session): persist boot context across reconnect and switch #360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -675,6 +675,9 @@ async function _startChatInner( | |
| const mcpAllowed = buildMcpAllowedTools(clientId); | ||
| const extraTools = options.extraTools ? options.extraTools.split(',').map((t) => t.trim()) : []; | ||
|
|
||
| // Resolve agent name early — needed for registration, resume upsert, and boot context. | ||
| const agentName = options.agentName ?? DEFAULT_AGENT_NAME; | ||
|
|
||
| // Streaming-input queue — kept open for the session lifetime. | ||
| const inputQueue = new AsyncQueue<SDKUserMessage>(); | ||
| inputQueue.push(makeUserMessage(fullPrompt, 'now')); | ||
|
|
@@ -687,6 +690,7 @@ async function _startChatInner( | |
| wtId, | ||
| sessionAllowList: new Set<string>(), | ||
| worktreePath, | ||
| agentName, | ||
| // Set sessionId early so pre-assistant events are persisted (iOS reconnect). | ||
| ...(options.resume ? { sessionId: options.resume } : {}), | ||
| ...(options.telosTaskId ? { telosTaskId: options.telosTaskId } : {}), | ||
|
|
@@ -726,6 +730,7 @@ async function _startChatInner( | |
| ...(worktreePath ? { wtId } : {}), | ||
| ...(options.telosTaskId ? { telosTaskId: options.telosTaskId } : {}), | ||
| ...(existingMeta ? { updatedAt: existingMeta.updatedAt } : {}), | ||
| agentName, | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 bugs: The resume upsert at line 724–734 persists |
||
| }); | ||
| } | ||
|
|
||
|
|
@@ -741,7 +746,6 @@ async function _startChatInner( | |
| // Build session env with worktree paths for the agent (all repos including primary) | ||
| const sessionEnv = sdkEnv(); | ||
| sessionEnv.MITZO_SESSION_ID = wtId; | ||
| const agentName = options.agentName ?? DEFAULT_AGENT_NAME; | ||
| sessionEnv.MITZO_AGENT_NAME = agentName; | ||
| for (const [name, { path }] of repoWorktrees) { | ||
| sessionEnv[`MITZO_REPO_${name.toUpperCase()}`] = path; | ||
|
|
@@ -777,16 +781,18 @@ async function _startChatInner( | |
| } catch (importErr: unknown) { | ||
| const msg = importErr instanceof Error ? importErr.message : String(importErr); | ||
| log.info('contexgin not available, using fallback', { error: msg }); | ||
| send(transport, { | ||
| type: 'boot_context', | ||
| source: 'local-fallback', | ||
| const fallback = { | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 style: The fallback boot_context object is duplicated three times (lines 784–792, 830–845, 921–928) with identical shape but verbose inline type annotations (
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 style: The fallback boot_context object is constructed identically in three places (import error at ~784, invalid compile result at ~828, and catch block at ~918), each with verbose inline type annotations like |
||
| source: 'local-fallback' as const, | ||
| sourceCount: 0, | ||
| tokenCount: 0, | ||
| tokenBudget: DEFAULT_TOKEN_BUDGET, | ||
| sources: [], | ||
| included: [], | ||
| trimmed: [], | ||
| }); | ||
| sources: [] as Array<{ path: string; kind: string }>, | ||
| included: [] as Array<{ source: string; heading: string; tokens: number; content: string }>, | ||
| trimmed: [] as Array<{ source: string; heading: string; tokens: number; content: string }>, | ||
| }; | ||
| send(transport, { type: 'boot_context', ...fallback }); | ||
| const s = registry.get(clientId); | ||
| if (s) s.bootContext = fallback; | ||
| return; | ||
| } | ||
|
|
||
|
|
@@ -819,16 +825,28 @@ async function _startChatInner( | |
| // Validate the compiled object shape | ||
| if (!compiled || typeof compiled !== 'object') { | ||
| log.warn('contexgin compile() returned unexpected shape', { compiled }); | ||
| send(transport, { | ||
| type: 'boot_context', | ||
| source: 'local-fallback', | ||
| const fallback = { | ||
| source: 'local-fallback' as const, | ||
| sourceCount: 0, | ||
| tokenCount: 0, | ||
| tokenBudget: tokenBudget, | ||
| sources: [], | ||
| included: [], | ||
| trimmed: [], | ||
| }); | ||
| sources: [] as Array<{ path: string; kind: string }>, | ||
| included: [] as Array<{ | ||
| source: string; | ||
| heading: string; | ||
| tokens: number; | ||
| content: string; | ||
| }>, | ||
| trimmed: [] as Array<{ | ||
| source: string; | ||
| heading: string; | ||
| tokens: number; | ||
| content: string; | ||
| }>, | ||
| }; | ||
| send(transport, { type: 'boot_context', ...fallback }); | ||
| const s = registry.get(clientId); | ||
| if (s) s.bootContext = fallback; | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 style: The pattern |
||
| return; | ||
| } | ||
|
|
||
|
|
@@ -879,30 +897,36 @@ async function _startChatInner( | |
| const trimmed = extractSections(rawTrimmed); | ||
| const fullMarkdown = typeof obj.bootPayload === 'string' ? obj.bootPayload : undefined; | ||
|
|
||
| send(transport, { | ||
| type: 'boot_context', | ||
| source: 'contexgin', | ||
| const bootPayload = { | ||
| source: 'contexgin' as const, | ||
| sourceCount: sources.length, | ||
| tokenCount: bootTokens, | ||
| tokenBudget: tokenBudget, | ||
| sources, | ||
| included, | ||
| trimmed, | ||
| fullMarkdown, | ||
| }); | ||
| }; | ||
| send(transport, { type: 'boot_context', ...bootPayload }); | ||
|
|
||
| // Cache in ManagedSession for replay on reconnect/switch | ||
| const s = registry.get(clientId); | ||
| if (s) s.bootContext = bootPayload; | ||
| } catch (err: unknown) { | ||
| const msg = err instanceof Error ? err.message : String(err); | ||
| log.warn('boot context compilation failed', { error: msg }); | ||
| send(transport, { | ||
| type: 'boot_context', | ||
| source: 'local-fallback', | ||
| const fallbackPayload = { | ||
| source: 'local-fallback' as const, | ||
| sourceCount: 0, | ||
| tokenCount: 0, | ||
| tokenBudget: tokenBudget, | ||
| sources: [], | ||
| included: [], | ||
| trimmed: [], | ||
| }); | ||
| sources: [] as Array<{ path: string; kind: string }>, | ||
| included: [] as Array<{ source: string; heading: string; tokens: number; content: string }>, | ||
| trimmed: [] as Array<{ source: string; heading: string; tokens: number; content: string }>, | ||
| }; | ||
| send(transport, { type: 'boot_context', ...fallbackPayload }); | ||
| const s = registry.get(clientId); | ||
| if (s) s.bootContext = fallbackPayload; | ||
| } | ||
| })(); | ||
| capturePromptComparison(wtId, cwd, systemPromptAppend, repoWorktrees).catch(() => {}); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -280,6 +280,14 @@ export function handleReconnect( | |
| running, | ||
| }); | ||
|
|
||
| // Re-send cached boot_context so pills reappear after reconnect | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 bugs: Reconnect boot_context replay lacks |
||
| if (found && running && found.session?.bootContext) { | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 bugs: handleReconnect only replays boot_context for running sessions (hot path). Non-running sessions have no cold path — unlike handleSwitchSession which falls back to EventStore's serialized bootContext. Since boot_context is sent via raw
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 bugs: handleReconnect only replays boot_context from in-memory SessionRegistry (hot path) when |
||
| ctx.connRegistry.get(connectionId)?.transport.send({ | ||
| type: 'boot_context', | ||
| ...found.session.bootContext, | ||
| }); | ||
| } | ||
|
|
||
| log.info('reconnect replay', { | ||
| connectionId, | ||
| sessionId: entry.sessionId, | ||
|
|
@@ -371,6 +379,27 @@ export async function handleSwitchSession( | |
| }, | ||
| }); | ||
|
|
||
| // Re-send boot_context so pills appear on session switch. | ||
| // Hot path: running session in SessionRegistry (in-memory cache). | ||
| // Cold path: ended session — read serialized JSON from EventStore. | ||
| const found = ctx.sessionRegistry.findBySessionId(msg.sessionId); | ||
| if (found?.session?.bootContext) { | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 bugs: Same sessionId omission on switch_session boot_context replay. Both the hot path (in-memory) and cold path (JSON.parse) send boot_context without a |
||
| ctx.connRegistry.get(connectionId)?.transport.send({ | ||
| type: 'boot_context', | ||
| ...found.session.bootContext, | ||
| }); | ||
| } else if (sessionMeta.bootContext) { | ||
| try { | ||
| const parsed = JSON.parse(sessionMeta.bootContext); | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 unsafe_assumptions: The cold path in handleSwitchSession does
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 unsafe_assumptions: JSON.parse of |
||
| ctx.connRegistry.get(connectionId)?.transport.send({ | ||
| type: 'boot_context', | ||
| ...parsed, | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 unsafe_assumptions: Cold-path JSON.parse has no shape validation. |
||
| }); | ||
| } catch { | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 style: The |
||
| // Invalid JSON — skip | ||
| } | ||
| } | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 style: Silent catch on JSON.parse — project conventions use |
||
|
|
||
| log.info('switch_session', { connectionId, sessionId: msg.sessionId }); | ||
| }, | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 style: Migration method
migrateBootContextalso adds theagent_namecolumn. Consider renaming tomigrateBootContextAndAgentNameormigrateSessionAgentFieldsto accurately reflect its scope.[fixable]