From 4380d8c4af079224972735c3df65232b8bec7b93 Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 09:35:57 -0400 Subject: [PATCH 01/11] fix(boot): a state create() can't read, or can't take, fails it naming the state, not an empty state At boot, these reads took a failed read of the framework's own state, or a stored value in a form the code doesn't read, as "nothing stored", and the framework served without it: - Host mode (readHostMode): a quiesced host booted serving, and the writes it had deferred during the maintenance landed at that boot. A host-mode document that isn't a record, or whose quiesced isn't a boolean, read as not quiesced. - Deferred writes (restorePersistedDeferredWrites): the queue restored nothing, and the next deferral replaced it. A context that couldn't be scanned for writes that landed before their ack was counted as holding none, so such a write landed a second time at resume. - Usage totals (restoreUsageState): they started at zero, and the next inference's persistUsageState stored zeros over them. - An agent's persisted runtime settings (createAgent): the agent booted at its recipe's settings, and its next settings change stored over the record. - MCPL coalescing (initializePushCoalescer): an acknowledged deferred acceptance or a pending batch was never delivered, and the next receipt or flush replaced the stored state. A snapshot of a version this code doesn't read was ignored and replaced. Each now throws an error that names the state (or its recovery file), says why the framework doesn't start without it, says it's unchanged, and says what to do. In create(), that fails create(). The coalescing state is read before the coalescer is built; on a framework started with no MCPL servers that read runs at the first connectMcplServer, which fails instead and leaves the subsystem uninitialized, so the next connect reads again. Nothing stored still starts empty, and entries inside a value these reads take (one queued write, one agent's usage) are checked as before. The persisted tool_result_inline_max_chars caps lose nothing when their read fails, since persistToolResultInlineCap reads them again before it writes one, so that read is logged and boot goes on, as before. usage-tracker.ts exports isPersistedUsageState, the check its restore applied, so the boot and the tracker agree on what stored totals are. jsonKind names what a form check found without its content. The writers of these states, from the release each first appears in through main, write only forms this code reads, so none of the form checks fires on a store a release left. Not taken: reading these early and closing an owned store (a failed create() that releases what it acquired covers every such failure); booting quiesced on an unreadable host-mode record (it would hold every data plane on a host that may never have been quiesced); an error class (nothing branches on one). Tests: test/boot-state-read-errors.test.ts, one or more per site: a boot with that read failing, or its state in a form this code doesn't read, fails naming it and leaves the state as it was, and the next boot that can read it restores it; the caps' log line; the runtime connect; and a control that nothing stored starts empty. agent-runtime-settings' "a record the boot can't read is logged, and the agent boots at the recipe's" turns round, and a record that isn't an object is added. test/helpers/ failed-create.ts stops what a failed create() built, so a test can boot again on its store while a failed create() still keeps it open. Co-Authored-By: Claude Opus 5.5 --- changelog.d/boot-state-read-errors.fixed.md | 7 + src/framework.ts | 360 ++++++++++++------ src/usage/usage-tracker.ts | 8 + test/agent-runtime-settings.test.ts | 24 +- test/boot-state-read-errors.test.ts | 400 ++++++++++++++++++++ test/helpers/failed-create.ts | 34 ++ 6 files changed, 707 insertions(+), 126 deletions(-) create mode 100644 changelog.d/boot-state-read-errors.fixed.md create mode 100644 test/boot-state-read-errors.test.ts create mode 100644 test/helpers/failed-create.ts diff --git a/changelog.d/boot-state-read-errors.fixed.md b/changelog.d/boot-state-read-errors.fixed.md new file mode 100644 index 000000000..ee8609f61 --- /dev/null +++ b/changelog.d/boot-state-read-errors.fixed.md @@ -0,0 +1,7 @@ +- `AgentFramework.create()` now fails when a state it restores at boot can't be read, or reads as something this version can't take for that state. The error names the state, or its recovery file, and says what to do; nothing it names is changed. Before, each was taken as empty, the framework served without it, and the next write replaced what was stored. Nothing stored still starts empty, and a store any release wrote boots as before unless one of these reads fails. + - **Host mode** (`recovery/host-mode.json`, or `framework/state` without one): a quiesced host booted serving, and landed the writes it had deferred during the maintenance. + - **Deferred writes** (`recovery/deferred-writes.json`, or `framework/deferred-writes` without one): the queue restored nothing, and the next deferral replaced it. An agent's context that can't be checked for writes that landed before their ack now fails `create()` too, where the write was landed a second time. + - **Usage totals** (`framework/state`): they started at zero, and the next inference stored zeros over them. + - **An agent's persisted runtime settings** (`framework/state`): the agent booted at its recipe's settings, a `private` think-text policy as `public`, and its next settings change stored over the record. 0.22.2's notes said such a read is logged and the agent boots at the recipe's; it now fails `create()`. + - **MCPL coalescing** (`mcpl/coalescing`, `mcpl/coalescing-recent`): an acknowledged deferred acceptance or a pending batch was never delivered, and the next receipt or flush replaced the stored ones. A snapshot of a version this one doesn't read was ignored and replaced; it now fails `create()` and is kept for the version that wrote it. On a framework started with no MCPL servers, these are read at the first `connectMcplServer`, which fails instead, and the next connect reads them again. +- A failed read of the persisted `tool_result_inline_max_chars` caps at boot is now logged. As before, they don't apply until a restart, and setting one keeps the others stored. diff --git a/src/framework.ts b/src/framework.ts index 5b4f3785f..0e77b6182 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -401,7 +401,7 @@ function validateUtilityArgs( import { CheckpointManager } from './mcpl/checkpoint-manager.js'; import { isToolAllowed } from './mcpl/tool-policy.js'; import { EventGate, formatShadowWarning } from './gate/event-gate.js'; -import { UsageTracker, type PersistedUsageState } from './usage/usage-tracker.js'; +import { UsageTracker, isPersistedUsageState } from './usage/usage-tracker.js'; import type { SessionUsageSnapshot, UsageUpdatedEvent } from './usage/types.js'; import { stderrTailOf, withStderr, type McplConnectionCloseInfo, type McplServerConnection } from './mcpl/server-connection.js'; import { isWebSocketTransport } from './mcpl/transport.js'; @@ -1552,6 +1552,15 @@ function thrownText(error: unknown): string { return text.length <= THROWN_TEXT_LIMIT ? text : `${text.slice(0, THROWN_TEXT_LIMIT)}… (${text.length} characters)`; } +/** A stored JSON value's kind, for a refusal that names what it found + * without its content. */ +function jsonKind(value: unknown): string { + return value === null ? 'null' + : Array.isArray(value) ? 'an array' + : typeof value === 'object' ? 'an object' + : `a ${typeof value}`; +} + /** * A failed handler's reason, never throwing: an Error's message, else "the * handler failed" and what it threw (nothing, for `undefined`). Telling an @@ -5022,7 +5031,9 @@ export class AgentFramework { * Which of `ids` already exist in some agent's store as a landed deferred * write (`metadata.deferredWriteId`). Scans each context manager's recent * tail blob-free — an interrupted flush is always within the last few - * hundred slots — and tolerates facades without windowed reads. + * hundred slots — and tolerates facades without windowed reads. A context + * that can't be read throws, naming its agent: counted as holding none, a + * write that landed there before its ack would land a second time. */ private landedDeferredWriteIds(ids: Set, scanFrom: Map): Set { const landed = new Set(); @@ -5052,8 +5063,8 @@ export class AgentFramework { } else { for (const m of cm.getAllMessages()) consider(m); } - } catch (err) { - console.error(`[host-mode] could not scan ${agent.name} for landed deferred writes:`, err); + } catch (error) { + throw new Error(`${agent.name}'s context could not be read: ${thrownText(error)}`, { cause: error }); } } return landed; @@ -5134,61 +5145,91 @@ export class AgentFramework { } } + /** + * Restore the deferred-write queue a crash or a stop left durable. Nothing + * stored is an empty queue. A queue that can't be read, or that reads as + * anything but a queue, throws, since taken as empty the next deferral + * would replace the stored one; so does a context that can't be checked + * for writes that already landed (see landedDeferredWriteIds). + */ private restorePersistedDeferredWrites(): number { + // Stores that predate the recovery file (or store-only configs) keep the + // queue in the store. + const file = this.deferredWritesPath && existsSync(this.deferredWritesPath) ? this.deferredWritesPath : undefined; + const where = file ?? DEFERRED_WRITES_ID; + const refuse = (problem: string, cause?: unknown): Error => new Error( + `the deferred context writes (${where}) ${problem}. The framework doesn't start without them, ` + + `since the next deferral would replace the stored queue. ` + + (file + ? `The file is unchanged; the framework starts once it can be read, or without those writes once it's moved aside.` + : `They're unchanged, and the framework starts once they can be read.`), + cause === undefined ? undefined : { cause }, + ); + let raw: unknown; try { - let raw: unknown; - if (this.deferredWritesPath && existsSync(this.deferredWritesPath)) { - raw = this.readRecoveryFile(this.deferredWritesPath); - } else { - // Stores that predate the recovery file (or store-only configs). - raw = this.store.getStateJson(DEFERRED_WRITES_ID); - } - // v1 slot payloads were a bare array; v1/v2 files and v2 slots are - // `{ pending, scanFrom? }`. - const doc = Array.isArray(raw) ? { pending: raw } : (raw as { pending?: unknown; scanFrom?: unknown } | undefined); - const data = doc?.pending; - if (!Array.isArray(data) || data.length === 0) return 0; - const scanFrom = new Map(); - if (doc?.scanFrom && typeof doc.scanFrom === 'object') { - for (const [k, v] of Object.entries(doc.scanFrom as Record)) { - if (typeof v === 'number' && Number.isFinite(v)) scanFrom.set(k, Math.max(0, Math.floor(v))); - } - } - const candidates = data - .filter((m): m is Omit & { id?: string; seq?: number } => - !!m && typeof m === 'object' && typeof (m as { participant?: unknown }).participant === 'string' - && Array.isArray((m as { content?: unknown }).content)) - .map((m, i) => ({ - ...m, - id: typeof m.id === 'string' ? m.id : randomUUID(), - // Files that predate `seq` are in write order already. - seq: typeof m.seq === 'number' ? m.seq : i + 1, - })) - .sort(bySeq); - this.deferredSeq = Math.max(this.deferredSeq, ...candidates.map((m) => m.seq)); - // Exactly-once: a message that landed before its ack was written is - // already in the store under its deferredWriteId — do not replay it. - // The scan covers every slot the interrupted batch could occupy (from - // the receipt's per-agent position), or the whole store when the - // receipt predates the position field. - const landed = this.landedDeferredWriteIds(new Set(candidates.map((m) => m.id)), scanFrom); - const restored = candidates.filter((m) => !landed.has(m.id)); - if (landed.size > 0) { - console.error( - `[host-mode] ${landed.size} deferred context write(s) had already landed before their ack ` + - `(interrupted flush) — skipped, not replayed`, - ); - } - this.deferredMessages.push(...restored); - this.deferredWritesPersisted = true; - // Rewrite the durable queue without the already-landed entries so a - // second interrupted boot does not re-scan them. - if (landed.size > 0) this.persistDeferredWrites(); - return restored.length; - } catch (err) { - console.error('[host-mode] failed to restore persisted deferred context writes:', err); - return 0; + raw = file ? this.readRecoveryFile(file) : this.store.getStateJson(DEFERRED_WRITES_ID); + } catch (error) { + throw refuse(`could not be read: ${thrownText(error)}`, error); + } + if (raw === undefined || raw === null) return 0; + // v1 slot payloads were a bare array; v1/v2 files and v2 slots are + // `{ pending, scanFrom? }`. + const doc = (Array.isArray(raw) ? { pending: raw } : typeof raw === 'object' ? raw : {}) as { pending?: unknown; scanFrom?: unknown }; + if (!Array.isArray(doc.pending)) { + throw refuse(typeof raw === 'object' + ? `hold an object whose pending is ${doc.pending === undefined ? 'missing' : jsonKind(doc.pending)}, not a list of writes` + : `hold ${jsonKind(raw)}, not a queue of writes`); + } + const data = doc.pending; + if (data.length === 0) return 0; + const scanFrom = new Map(); + if (doc.scanFrom && typeof doc.scanFrom === 'object') { + for (const [k, v] of Object.entries(doc.scanFrom as Record)) { + if (typeof v === 'number' && Number.isFinite(v)) scanFrom.set(k, Math.max(0, Math.floor(v))); + } + } + const candidates = data + .filter((m): m is Omit & { id?: string; seq?: number } => + !!m && typeof m === 'object' && typeof (m as { participant?: unknown }).participant === 'string' + && Array.isArray((m as { content?: unknown }).content)) + .map((m, i) => ({ + ...m, + id: typeof m.id === 'string' ? m.id : randomUUID(), + // Files that predate `seq` are in write order already. + seq: typeof m.seq === 'number' ? m.seq : i + 1, + })) + .sort(bySeq); + this.deferredSeq = Math.max(this.deferredSeq, ...candidates.map((m) => m.seq)); + // Exactly-once: a message that landed before its ack was written is + // already in the store under its deferredWriteId — do not replay it. + // The scan covers every slot the interrupted batch could occupy (from + // the receipt's per-agent position), or the whole store when the + // receipt predates the position field. + let landed: Set; + try { + landed = this.landedDeferredWriteIds(new Set(candidates.map((m) => m.id)), scanFrom); + } catch (error) { + throw new Error( + `the deferred context writes (${where}) can't be checked for writes that already landed: ` + + `${thrownText(error)}. The framework doesn't start without that check, since a write that landed ` + + `before its ack would land a second time. The queue is unchanged, and the framework starts once ` + + `that context can be read.`, + { cause: error }, + ); + } + const restored = candidates.filter((m) => !landed.has(m.id)); + if (landed.size > 0) { + console.error( + `[host-mode] ${landed.size} deferred context write(s) had already landed before their ack ` + + `(interrupted flush) — skipped, not replayed`, + ); } + this.deferredMessages.push(...restored); + this.deferredWritesPersisted = true; + // Rewrite the durable queue without the already-landed entries so a + // second interrupted boot does not re-scan them. + if (landed.size > 0) this.persistDeferredWrites(); + return restored.length; } /** @@ -5587,19 +5628,33 @@ export class AgentFramework { return this.usageTracker.getSnapshot(); } + /** Restore the usage totals a prior session stored. None stored starts + * fresh (the constructor's tracker). Totals that can't be read, or that + * read as anything but totals, throw: taken as none, the next inference's + * persistUsageState would store fresh totals over them. */ private restoreUsageState(): void { + const refuse = (problem: string, cause?: unknown): Error => new Error( + `the usage totals (${FRAMEWORK_STATE_ID}, usage) ${problem}. The framework doesn't start without them, ` + + `since the next inference would store fresh totals over them. They're unchanged, and the framework ` + + `starts once they can be read.`, + cause === undefined ? undefined : { cause }, + ); + let usage: unknown; try { const data = this.store.getStateJson(FRAMEWORK_STATE_ID); - if (data && typeof data === 'object' && (data as any).usage) { - const restored = (data as any).usage as PersistedUsageState; - this.usageTracker = new UsageTracker({ - emitTrace: (e: UsageUpdatedEvent) => this.emitTrace({ ...e }), - restored, - }); - } - } catch { - // No prior state or corrupt — start fresh (already initialized in constructor) + usage = data && typeof data === 'object' ? (data as Record).usage : undefined; + } catch (error) { + throw refuse(`could not be read: ${thrownText(error)}`, error); + } + if (usage === undefined || usage === null) return; + if (!isPersistedUsageState(usage)) { + const kind = jsonKind(usage); + throw refuse(kind === 'an object' ? 'are an object without token counts' : `are ${kind}, not token counts`); } + this.usageTracker = new UsageTracker({ + emitTrace: (e: UsageUpdatedEvent) => this.emitTrace({ ...e }), + restored: usage, + }); } private persistUsageState(): void { @@ -5613,13 +5668,18 @@ export class AgentFramework { } } - /** An agent's persisted record, or null when it has none; throws when the store can't be read. */ + /** An agent's persisted record, or null when it has none; throws when the + * store can't be read, or when what's stored for the agent isn't a record. */ private readAgentRuntimeSettings(agentName: string): AgentRuntimeSettingsOverrides | null { const data = this.store.getStateJson(FRAMEWORK_STATE_ID) as { - agentRuntimeSettings?: Record; + agentRuntimeSettings?: Record; } | null; const stored = data?.agentRuntimeSettings?.[agentName]; - return stored ? { ...stored } : null; + if (stored === undefined || stored === null) return null; + if (typeof stored !== 'object' || Array.isArray(stored)) { + throw new Error(`what's stored for the agent is ${jsonKind(stored)}, not a settings record`); + } + return { ...(stored as AgentRuntimeSettingsOverrides) }; } private validatePersistedAgentRuntimeSettings( @@ -5661,27 +5721,33 @@ export class AgentFramework { } /** Load resident-set inline caps from framework state, dropping (loudly) - * anything that fails the same validation the live update enforces. */ + * anything that fails the same validation the live update enforces. A + * failed read is logged and boot goes on without them: nothing is lost, + * since persistToolResultInlineCap reads the stored caps again before it + * writes one. */ private readPersistedToolResultInlineCaps(): Record { + let data: { toolResultInlineCaps?: Record } | null; try { - const data = this.store.getStateJson(FRAMEWORK_STATE_ID) as { - toolResultInlineCaps?: Record; - } | null; - const out: Record = {}; - for (const [name, value] of Object.entries(data?.toolResultInlineCaps ?? {})) { - const n = Number(value); - if (Number.isFinite(n) && n >= 1000) { - out[name] = Math.floor(n); - } else { - console.error( - `[agent-settings] dropping invalid persisted tool_result_inline_max_chars for '${name}': ${JSON.stringify(value)}`, - ); - } - } - return out; - } catch { + data = this.store.getStateJson(FRAMEWORK_STATE_ID) as typeof data; + } catch (error) { + console.error( + `[agent-settings] the persisted tool_result_inline_max_chars caps (${FRAMEWORK_STATE_ID}) could not be read: ` + + `${thrownText(error)}; they don't apply until a restart, and setting one keeps the others stored`, + ); return {}; } + const out: Record = {}; + for (const [name, value] of Object.entries(data?.toolResultInlineCaps ?? {})) { + const n = Number(value); + if (Number.isFinite(n) && n >= 1000) { + out[name] = Math.floor(n); + } else { + console.error( + `[agent-settings] dropping invalid persisted tool_result_inline_max_chars for '${name}': ${JSON.stringify(value)}`, + ); + } + } + return out; } /** Write (cap) or clear (null) one agent's resident-set inline cap. */ @@ -5889,29 +5955,45 @@ export class AgentFramework { } /** The recovery file wins whenever it exists; the branch-local slot is - * read only for stores that predate it (or store-only configs). */ + * read only for stores that predate it (or store-only configs). No record + * is a host that was never quiesced. A record that can't be read, or that + * reads as anything but a host-mode record, throws: taken as "not + * quiesced", a host that stopped mid-maintenance would boot serving. */ private readHostMode(): { quiesced: boolean; reason?: string; since?: number } | null { + const file = this.hostModePath && existsSync(this.hostModePath) ? this.hostModePath : undefined; + const refuse = (problem: string, cause?: unknown): Error => new Error( + `the host mode (${file ?? `${FRAMEWORK_STATE_ID}, hostMode`}) ${problem}. The framework doesn't start without it, ` + + `since a host quiesced for maintenance would boot serving. ` + + (file + ? `The file is unchanged; the framework starts once it can be read, or without it once it's moved aside.` + : `It's unchanged, and the framework starts once it can be read.`), + cause === undefined ? undefined : { cause }, + ); + let record: unknown; try { - let record: Record | null = null; - if (this.hostModePath && existsSync(this.hostModePath)) { - const doc = this.readRecoveryFile(this.hostModePath); - if (doc && typeof doc === 'object') record = doc as Record; + if (file) { + record = this.readRecoveryFile(file); } else { const data = this.store.getStateJson(FRAMEWORK_STATE_ID); - if (!data || typeof data !== 'object') return null; - const mode = (data as Record).hostMode; - if (!mode || typeof mode !== 'object') return null; - record = mode as Record; + record = data && typeof data === 'object' ? (data as Record).hostMode : undefined; } - if (!record) return null; - return { - quiesced: record.quiesced === true, - ...(typeof record.reason === 'string' ? { reason: record.reason } : {}), - ...(typeof record.since === 'number' ? { since: record.since } : {}), - }; - } catch { - return null; + } catch (error) { + throw refuse(`could not be read: ${thrownText(error)}`, error); } + // A file removed between the check and the read is no record, as no file is. + if (record === undefined) return null; + const isRecord = !!record && typeof record === 'object' && !Array.isArray(record); + const { quiesced, reason, since } = (isRecord ? record : {}) as Record; + if (typeof quiesced !== 'boolean') { + throw refuse(isRecord + ? `is a record whose quiesced is ${quiesced === undefined ? 'missing' : jsonKind(quiesced)}, not true or false` + : `holds ${jsonKind(record)}, not a host-mode record`); + } + return { + quiesced, + ...(typeof reason === 'string' ? { reason } : {}), + ...(typeof since === 'number' ? { since } : {}), + }; } /** @@ -8660,13 +8742,19 @@ export class AgentFramework { this.adoptToolResultGuard(agent); agent.markContextConsumed(); // RFC-006: nothing stored before now is replaceable this.sharedSlotAgents.add(agent); - let restoredSettings: AgentRuntimeSettingsOverrides | null = null; + let restoredSettings: AgentRuntimeSettingsOverrides | null; try { restoredSettings = this.readAgentRuntimeSettings(config.name); } catch (error) { - console.error( - `[agent-settings] ${config.name}: its persisted settings can't be read ` + - `(${error instanceof Error ? error.message : String(error)}); it boots at the recipe's`, + // Booted at the recipe's, the agent would serve settings it didn't + // choose (a private think-text policy as public), and its next + // persisting change would store over the record this read skipped. + throw new Error( + `the persisted settings of agent "${config.name}" (${FRAMEWORK_STATE_ID}, agentRuntimeSettings) can't be ` + + `restored: ${thrownText(error)}. The framework doesn't start without them, since the agent would boot at ` + + `its recipe's settings and the next settings change would store over them. They're unchanged, and the ` + + `framework starts once they can be read.`, + { cause: error }, ); } if (restoredSettings) { @@ -9940,6 +10028,43 @@ export class AgentFramework { } private initializePushCoalescer(): void { + // The stored receipts and batches are read before the coalescer is built. + // Nothing stored is a coalescer with no history. What can't be read, or + // reads as anything but this version's snapshot and receipts, throws, so + // MCPL doesn't start: taken as no history, the next acceptance's receipt + // and the next flush would replace what's stored. + const refuse = (stateId: string, problem: string, remedy: string, cause?: unknown): Error => new Error( + `the coalescing ${stateId === COALESCING_STATE_ID ? 'receipts and batches' : 'receipts since the last snapshot'} ` + + `(${stateId}) ${problem}. MCPL doesn't start without them, since the next coalesced push or message ` + + `would replace them. ${remedy}`, + cause === undefined ? undefined : { cause }, + ); + const read = (stateId: string): unknown => { + try { + return this.store.getStateJson(stateId); + } catch (error) { + throw refuse(stateId, `could not be read: ${thrownText(error)}`, `They're unchanged, and MCPL starts once they can be read.`, error); + } + }; + const snapshot = read(COALESCING_STATE_ID); + const recent = read(COALESCING_RECENT_ID); + if (snapshot !== null && snapshot !== undefined) { + const version = typeof snapshot === 'object' && !Array.isArray(snapshot) + ? (snapshot as { version?: unknown }).version + : undefined; + if (version !== 1) { + throw version === undefined + ? refuse(COALESCING_STATE_ID, `are ${jsonKind(snapshot)} with no snapshot version`, `They're unchanged.`) + : refuse( + COALESCING_STATE_ID, + `are a version ${typeof version === 'number' ? version : jsonKind(version)} snapshot, which this version of agent-framework doesn't read`, + `They're unchanged, and the version that wrote them reads them.`, + ); + } + } + if (recent !== null && recent !== undefined && !Array.isArray(recent)) { + throw refuse(COALESCING_RECENT_ID, `are ${jsonKind(recent)}, not a list of receipts`, `They're unchanged.`); + } const coalescer = new PushCoalescer({ isUnread: (p) => this.isUnreadPlacement(p), remove: (p) => this.removePlacement(p), @@ -9997,17 +10122,20 @@ export class AgentFramework { }, }); try { - coalescer.restore(this.store.getStateJson(COALESCING_STATE_ID) as CoalescingSnapshot | null); - const recent = this.store.getStateJson(COALESCING_RECENT_ID) as CoalescingReceiptRecord[] | null; - if (Array.isArray(recent)) { - coalescer.restoreReceipts(recent); - // Until the next snapshot flush checkpoints the recovered state, these - // records are its only durable form: keep them in the writable buffer - // so the next acceptance appends to them instead of replacing them. - this.coalescingRecentReceipts = recent; + coalescer.restore(snapshot as CoalescingSnapshot | null | undefined); + } catch (error) { + throw refuse(COALESCING_STATE_ID, `could not be restored: ${thrownText(error)}`, `They're unchanged.`, error); + } + if (Array.isArray(recent)) { + try { + coalescer.restoreReceipts(recent as CoalescingReceiptRecord[]); + } catch (error) { + throw refuse(COALESCING_RECENT_ID, `could not be restored: ${thrownText(error)}`, `They're unchanged.`, error); } - } catch (err) { - console.error('[coalescing] could not restore coalescing state; starting with unknown history:', err); + // Until the next snapshot flush checkpoints the recovered state, these + // records are its only durable form: keep them in the writable buffer + // so the next acceptance appends to them instead of replacing them. + this.coalescingRecentReceipts = recent as CoalescingReceiptRecord[]; } this.pushCoalescer = coalescer; // Batches restored from a snapshot still owe the agent a wake (§14 v41). diff --git a/src/usage/usage-tracker.ts b/src/usage/usage-tracker.ts index d5abf09be..7cf8a422f 100644 --- a/src/usage/usage-tracker.ts +++ b/src/usage/usage-tracker.ts @@ -49,6 +49,14 @@ export interface PersistedUsageState { inferenceCount: number; } +/** Whether a stored usage record is one the tracker restores: an object whose + * totals are token counts. Its per-agent entries are checked one by one as + * they restore. */ +export function isPersistedUsageState(value: unknown): value is PersistedUsageState { + return !!value && typeof value === 'object' && !Array.isArray(value) + && parseUsage((value as { totals?: unknown }).totals) !== null; +} + export class UsageTracker { private totals: SessionUsage = { inputTokens: 0, outputTokens: 0, cacheCreationTokens: 0, cacheReadTokens: 0 }; private byAgent: Map = new Map(); diff --git a/test/agent-runtime-settings.test.ts b/test/agent-runtime-settings.test.ts index 4eed3e766..25455f7f4 100644 --- a/test/agent-runtime-settings.test.ts +++ b/test/agent-runtime-settings.test.ts @@ -7,6 +7,7 @@ import { JsStore } from '@animalabs/chronicle'; import { AutobiographicalStrategy, ContextManager } from '@animalabs/context-manager'; import { Agent, AgentFramework, BudgetPreflightError, type AgentRuntimeSettingsPatch } from '../src/index.js'; +import { rejectsBuild } from './helpers/failed-create.js'; const membrane = {} as any; @@ -853,7 +854,7 @@ describe('a reached budget across a restart', () => { }); }); - it("a record the boot can't read is logged, and the agent boots at the recipe's", async () => { + it("a record the boot can't read fails create(), naming it, and the next boot restores it", async () => { await run(async ({ framework, restart }) => { framework().updateAgentRuntimeSettings('agent', { contextBudgetTokens: 60_000, immediate: true }); // Only the boot's read of the record fails. @@ -863,19 +864,22 @@ describe('a reached budget across a restart', () => { if (new Error().stack?.includes('readAgentRuntimeSettings')) throw new Error('store unreadable'); return original.apply(this, args); }; - const log = quietErrors(); - let restarted: AgentFramework; try { - restarted = await restart(); + // Booted at the recipe's, its next settings change would store over the record. + await rejectsBuild(restart, /^the persisted settings of agent "agent" \(framework\/state, agentRuntimeSettings\) can't be restored: store unreadable\. The framework doesn't start without them, since the agent would boot at its recipe's settings and the next settings change would store over them\. They're unchanged, and the framework starts once they can be read\.$/); } finally { proto.getStateJson = original; - log.restore(); } - assert.deepEqual(budgets(restarted), { target: 100_000, effective: 100_000, transition: 'stable' }); - assert.ok( - log.lines.some((l) => l.includes("agent: its persisted settings can't be read (store unreadable); it boots at the recipe's")), - log.lines.join('\n'), - ); + const restarted = await restart(); + assert.equal(budgets(restarted).target, 60_000); + assert.deepEqual(persisted(restarted), { contextBudgetTokens: 60_000 }); + }); + }); + + it("what's stored for an agent that isn't a settings record fails create(), naming what it is", async () => { + await run(async ({ framework, restart }) => { + writeRecord(framework(), ['contextBudgetTokens', 60_000] as never); + await rejectsBuild(restart, /^the persisted settings of agent "agent" \(framework\/state, agentRuntimeSettings\) can't be restored: what's stored for the agent is an array, not a settings record\. The framework doesn't start without them/); }); }); diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts new file mode 100644 index 000000000..d63a33dbe --- /dev/null +++ b/test/boot-state-read-errors.test.ts @@ -0,0 +1,400 @@ +/** + * At boot, a stored state that can't be read, or that reads as something this + * code doesn't read as that state, is a refusal that names it, not an empty + * state. Taken as empty, each was lost or ignored: a quiesced host booted + * serving and landed its deferred writes mid-surgery; a deferred-write queue + * restored nothing and the next deferral replaced it; a deferred write that + * had landed before its ack landed again when an agent's context couldn't be + * scanned; usage totals started at zero and the next inference stored zeros + * over them; an agent's settings booted at its recipe's and the next change + * stored over the record; coalesced work acknowledged before a crash was never + * delivered, and the next receipt or flush replaced what was stored. Each + * refusal leaves the state as it was, so the next boot that can read it + * restores it. The inline caps' failed read loses nothing, since setting a cap + * reads them again, so it is logged and boot goes on. Nothing stored still + * reads as empty. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { JsStore } from '@animalabs/chronicle'; +import { ContextManager } from '@animalabs/context-manager'; +import { AgentFramework } from '../src/index.js'; +import type { FrameworkConfig } from '../src/types/framework.js'; +import type { McplServerConfig } from '../src/mcpl/types.js'; +import { MockMembrane } from './helpers/mock-membrane.js'; +import { fixture, crash } from './helpers/coalescing-fixture.js'; +import { rejectsBuild } from './helpers/failed-create.js'; +import { isStateExistsError } from '../src/module-registry.js'; + +/** An I/O error from a store read of `stateId` by the framework's `reader`, + * `times` times, as a failing disk would give. */ +function failStoreRead(stateId: string, reader: string, times = 1): () => void { + const proto = JsStore.prototype as unknown as { getStateJson(this: JsStore, id: string): unknown }; + const original = proto.getStateJson; + let left = times; + proto.getStateJson = function (this: JsStore, id: string) { + if (id === stateId && left > 0 && new Error().stack?.includes(reader)) { + left--; + throw Object.assign(new Error(`injected EIO reading ${id}`), { code: 'EIO' }); + } + return original.call(this, id); + }; + return () => { proto.getStateJson = original; }; +} + +/** An I/O error from the next read of the recovery file whose path ends in `name`. */ +function failRecoveryRead(name: string): () => void { + const proto = AgentFramework.prototype as unknown as { readRecoveryFile(path: string): unknown }; + const original = proto.readRecoveryFile; + let left = 1; + proto.readRecoveryFile = function (this: unknown, path: string) { + if (path.endsWith(name) && left > 0) { + left--; + throw Object.assign(new Error(`injected EIO reading ${name}`), { code: 'EIO' }); + } + return original.call(this, path); + }; + return () => { proto.readRecoveryFile = original; }; +} + +function quietErrors(): { lines: string[]; restore: () => void } { + const lines: string[] = []; + const original = console.error; + console.error = (...args: unknown[]) => { lines.push(args.map(String).join(' ')); }; + return { lines, restore: () => { console.error = original; } }; +} + +const agents = [{ name: 'agent', model: 'test-model', systemPrompt: 'test' }]; +const config = (storePath: string): FrameworkConfig => + ({ storePath, membrane: new MockMembrane().asMembrane(), agents, modules: [] }) as unknown as FrameworkConfig; +const texts = (f: AgentFramework): string[] => + f.getAgent('agent')!.getContextManager().getAllMessages().map((m) => (m.content[0] as { text?: string } | undefined)?.text ?? ''); + +/** A store directory removed after `body`, which receives its storePath. */ +function inStoreDir(prefix: string, body: (storePath: string) => Promise): () => Promise { + return async () => { + const dir = mkdtempSync(join(tmpdir(), prefix)); + try { await body(join(dir, 'store')); } finally { rmSync(dir, { recursive: true, force: true }); } + }; +} + +/** Write `framework/state` while no framework holds the store. */ +function seedFrameworkState(storePath: string, state: Record): void { + const store = JsStore.openOrCreate({ path: storePath }); + try { + try { + store.registerState({ id: 'framework/state', strategy: 'snapshot' }); + } catch (error) { + if (!isStateExistsError(error)) throw error; + } + store.setStateJson('framework/state', state); + } finally { store.close(); } +} +function readState(storePath: string, stateId: string): unknown { + const store = JsStore.openOrCreate({ path: storePath }); + try { return store.getStateJson(stateId); } finally { store.close(); } +} + +/** Quiesce with one write deferred, and stop. */ +async function quiescedWithAWrite(storePath: string, text = 'said during surgery'): Promise { + const f = await AgentFramework.create(config(storePath)); + try { + await f.quiesce({ reason: 'surgery' }); + (f as unknown as { addMessage(p: string, c: unknown[]): unknown }).addMessage('user', [{ type: 'text', text }]); + } finally { await f.stop(); } +} + +// --------------------------------------------------------------------------- +// Host mode +// --------------------------------------------------------------------------- + +test("a host-mode file that can't be read fails create(), and the next boot is quiesced with its writes", inStoreDir('boot-read-hostmode-', async (storePath) => { + await quiescedWithAWrite(storePath); + const file = join(storePath, 'recovery', 'host-mode.json'); + const before = readFileSync(file, 'utf8'); + const restore = failRecoveryRead('host-mode.json'); + try { + await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( + `^the host mode \\(${file.replace(/[.*+?^${}()|[\]\\/]/g, '\\$&')}\\) could not be read: injected EIO reading host-mode\\.json\\. ` + + `The framework doesn't start without it, since a host quiesced for maintenance would boot serving\\. ` + + `The file is unchanged; the framework starts once it can be read, or without it once it's moved aside\\.$`, + )); + } finally { restore(); } + assert.equal(readFileSync(file, 'utf8'), before, 'the file is as it was'); + const f = await AgentFramework.create(config(storePath)); + try { + assert.equal(f.getHostModeStatus().quiesced, true); + assert.equal(f.getHostModeStatus().deferredWrites, 1); + assert.ok(!texts(f).includes('said during surgery'), 'the deferred write waits for resume'); + } finally { await f.stop(); } +})); + +test('a host-mode file that holds no record, or one whose quiesced is not a boolean, fails create(), naming what it holds', inStoreDir('boot-read-hostmode-form-', async (storePath) => { + await quiescedWithAWrite(storePath); + const file = join(storePath, 'recovery', 'host-mode.json'); + for (const [content, holds] of [ + ['["quiesced"]', 'holds an array, not a host-mode record'], + ['null', 'holds null, not a host-mode record'], + ['{"version":1,"quiesced":"yes"}', 'is a record whose quiesced is a string, not true or false'], + ['{"version":1}', 'is a record whose quiesced is missing, not true or false'], + ] as const) { + writeFileSync(file, content); + await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( + `^the host mode \\(.*host-mode\\.json\\) ${holds.replace(/[.*+?^${}()|[\]\\/]/g, '\\$&')}\\. The framework doesn't start without it`, + ), content); + assert.equal(readFileSync(file, 'utf8'), content, `${content}: the file is as it was`); + } +})); + +test("with no recovery file, a host mode that can't be read from framework/state fails create()", async () => { + const dir = mkdtempSync(join(tmpdir(), 'boot-read-hostmode-slot-')); + const store = JsStore.openOrCreate({ path: join(dir, 'store') }); + // A store passed in, with no recovery paths, keeps host mode in framework/state. + const slotConfig = () => ({ store, membrane: new MockMembrane().asMembrane(), agents, modules: [] }) as unknown as FrameworkConfig; + try { + const first = await AgentFramework.create(slotConfig()); + await first.quiesce({ reason: 'surgery' }); + await first.stop(); + const restore = failStoreRead('framework/state', 'readHostMode'); + try { + await rejectsBuild(() => AgentFramework.create(slotConfig()), /^the host mode \(framework\/state, hostMode\) could not be read: injected EIO reading framework\/state\. The framework doesn't start without it, since a host quiesced for maintenance would boot serving\. It's unchanged, and the framework starts once it can be read\.$/); + } finally { restore(); } + const f = await AgentFramework.create(slotConfig()); + try { assert.equal(f.getHostModeStatus().quiesced, true); } finally { await f.stop(); } + store.setStateJson('framework/state', { ...(store.getStateJson('framework/state') as object), hostMode: 'quiesced' }); + await rejectsBuild(() => AgentFramework.create(slotConfig()), /^the host mode \(framework\/state, hostMode\) holds a string, not a host-mode record\./); + } finally { + store.close(); + rmSync(dir, { recursive: true, force: true }); + } +}); + +// --------------------------------------------------------------------------- +// Deferred writes +// --------------------------------------------------------------------------- + +test("a deferred-write queue that can't be read fails create(), and the next boot lands its writes at resume", inStoreDir('boot-read-deferred-', async (storePath) => { + await quiescedWithAWrite(storePath); + const file = join(storePath, 'recovery', 'deferred-writes.json'); + const before = readFileSync(file, 'utf8'); + const restore = failRecoveryRead('deferred-writes.json'); + try { + await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the deferred context writes \(.*deferred-writes\.json\) could not be read: injected EIO reading deferred-writes\.json\. The framework doesn't start without them, since the next deferral would replace the stored queue\. The file is unchanged; the framework starts once it can be read, or without those writes once it's moved aside\.$/); + } finally { restore(); } + assert.equal(readFileSync(file, 'utf8'), before, 'the queue is as it was'); + const f = await AgentFramework.create(config(storePath)); + try { + assert.equal(f.getHostModeStatus().deferredWrites, 1); + await f.resume(); + assert.deepEqual(texts(f).filter((t) => t === 'said during surgery').length, 1); + } finally { await f.stop(); } +})); + +test("a deferred-write queue that isn't a queue fails create(), naming what it holds", inStoreDir('boot-read-deferred-form-', async (storePath) => { + await quiescedWithAWrite(storePath); + const file = join(storePath, 'recovery', 'deferred-writes.json'); + for (const [content, holds] of [ + ['"pending"', 'hold a string, not a queue of writes'], + ['{"version":2,"pending":{"0":{}}}', 'hold an object whose pending is an object, not a list of writes'], + ['{"version":2}', 'hold an object whose pending is missing, not a list of writes'], + ] as const) { + writeFileSync(file, content); + await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( + `^the deferred context writes \\(.*deferred-writes\\.json\\) ${holds}\\. The framework doesn't start without them`, + ), content); + assert.equal(readFileSync(file, 'utf8'), content, `${content}: the file is as it was`); + } +})); + +test("a deferred write whose agent's context can't be scanned fails create(), and the next boot doesn't land it twice", inStoreDir('boot-read-deferred-landed-', async (storePath) => { + // A write that landed before its ack: in the queue, and in the context with its id. + await quiescedWithAWrite(storePath); + const [queued] = JSON.parse(readFileSync(join(storePath, 'recovery', 'deferred-writes.json'), 'utf8')).pending as Array<{ id: string }>; + const staging = await AgentFramework.create(config(storePath)); + try { + staging.getAgent('agent')!.getContextManager() + .addMessage('user', [{ type: 'text', text: 'said during surgery' }], { deferredWriteId: queued!.id } as never); + } finally { await staging.stop(); } + const proto = ContextManager.prototype as unknown as { getMessageCount(this: ContextManager): number }; + const original = proto.getMessageCount; + proto.getMessageCount = function (this: ContextManager) { + if (new Error().stack?.includes('landedDeferredWriteIds')) throw new Error('injected EIO scanning the context'); + return original.call(this); + }; + try { + await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the deferred context writes \(.*deferred-writes\.json\) can't be checked for writes that already landed: agent's context could not be read: injected EIO scanning the context\. The framework doesn't start without that check, since a write that landed before its ack would land a second time\. The queue is unchanged, and the framework starts once that context can be read\.$/); + } finally { proto.getMessageCount = original; } + const f = await AgentFramework.create(config(storePath)); + try { + assert.equal(f.getHostModeStatus().deferredWrites, 0, 'the landed write is skipped'); + await f.resume(); + assert.equal(texts(f).filter((t) => t === 'said during surgery').length, 1, 'it is in the context once'); + } finally { await f.stop(); } +})); + +// --------------------------------------------------------------------------- +// Usage totals +// --------------------------------------------------------------------------- + +const storedUsage = { + totals: { inputTokens: 1000, outputTokens: 500 }, + inferenceCount: 7, + byAgent: [{ agentName: 'agent', usage: { inputTokens: 1000, outputTokens: 500 }, inferenceCount: 7 }], +}; + +test("usage totals that can't be read fail create(), and the next boot restores them", inStoreDir('boot-read-usage-', async (storePath) => { + seedFrameworkState(storePath, { usage: storedUsage }); + const restore = failStoreRead('framework/state', 'restoreUsageState'); + try { + await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the usage totals \(framework\/state, usage\) could not be read: injected EIO reading framework\/state\. The framework doesn't start without them, since the next inference would store fresh totals over them\. They're unchanged, and the framework starts once they can be read\.$/); + } finally { restore(); } + assert.deepEqual((readState(storePath, 'framework/state') as { usage: unknown }).usage, storedUsage); + const f = await AgentFramework.create(config(storePath)); + try { + assert.equal(f.getSessionUsage().totals.inputTokens, 1000); + assert.equal(f.getSessionUsage().totals.outputTokens, 500); + } finally { await f.stop(); } +})); + +test("usage totals that aren't token counts fail create(), naming what they are", inStoreDir('boot-read-usage-form-', async (storePath) => { + for (const [usage, are] of [ + [{ totals: 'lots' }, 'are an object without token counts'], + [[storedUsage], 'are an array, not token counts'], + ] as const) { + seedFrameworkState(storePath, { usage }); + await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp(`^the usage totals \\(framework/state, usage\\) ${are}\\. The framework doesn't start without them`), JSON.stringify(usage)); + assert.deepEqual((readState(storePath, 'framework/state') as { usage: unknown }).usage, usage); + } +})); + +// --------------------------------------------------------------------------- +// Inline caps +// --------------------------------------------------------------------------- + +test("inline caps that can't be read are logged, boot goes on, and setting one keeps the others stored", inStoreDir('boot-read-caps-', async (storePath) => { + seedFrameworkState(storePath, { toolResultInlineCaps: { agent: 5000 } }); + const twoAgents = { ...config(storePath), agents: [...agents, { name: 'other', model: 'test-model', systemPrompt: 'test' }] } as FrameworkConfig; + const restore = failStoreRead('framework/state', 'readPersistedToolResultInlineCaps'); + const log = quietErrors(); + let f: AgentFramework; + try { + f = await AgentFramework.create(twoAgents); + } finally { restore(); log.restore(); } + try { + assert.ok( + log.lines.some((l) => l.includes( + "[agent-settings] the persisted tool_result_inline_max_chars caps (framework/state) could not be read: injected EIO reading framework/state; they don't apply until a restart, and setting one keeps the others stored", + )), + log.lines.join('\n'), + ); + const settings = (f as unknown as { + collectAgentSettingsExtensions(): Map; + update(agentName: string, patch: Record): unknown; + }>; + }).collectAgentSettingsExtensions().get('_framework')!; + assert.equal(settings.get('agent').tool_result_inline_max_chars, null, "the stored cap doesn't apply"); + settings.update('other', { tool_result_inline_max_chars: 2000 }); + } finally { await f.stop(); } + assert.deepEqual((readState(storePath, 'framework/state') as { toolResultInlineCaps: unknown }).toolResultInlineCaps, { agent: 5000, other: 2000 }); +})); + +// --------------------------------------------------------------------------- +// Coalescing +// --------------------------------------------------------------------------- + +test("coalescing receipts that can't be read fail create(), and the next boot delivers the acknowledged work", async (t) => { + const f = await fixture(); t.after(f.close); + const params = f.params('1', 'accepted_before_crash', { deferred: true, data: { k: 'v' } }); + const original = await f.send('push/event', params); + await crash(f); + const restore = failStoreRead('mcpl/coalescing-recent', 'initializePushCoalescer'); + try { + await rejectsBuild(() => f.create(), /^the coalescing receipts since the last snapshot \(mcpl\/coalescing-recent\) could not be read: injected EIO reading mcpl\/coalescing-recent\. MCPL doesn't start without them, since the next coalesced push or message would replace them\. They're unchanged, and MCPL starts once they can be read\.$/); + } finally { restore(); } + await f.create(); + const retry = await f.send('push/event', params); + assert.deepEqual(retry.result, original.result, 'the receipt survived'); + await f.framework.runUntilIdle(); + assert.equal(f.membrane.calls.length, 1, 'the accepted work was delivered'); + assert.ok(f.lastRequest().includes('accepted_before_crash')); +}); + +test("a coalescing snapshot that can't be read fails create(), and the next boot renders its batch", async (t) => { + const f = await fixture(); t.after(f.close); + await f.send('push/event', f.params('1', 'pending_before_crash', { deferred: true })); + (f.framework as unknown as { flushCoalescingSnapshot(): void }).flushCoalescingSnapshot(); + await crash(f); + const restore = failStoreRead('mcpl/coalescing', 'initializePushCoalescer'); + try { + await rejectsBuild(() => f.create(), /^the coalescing receipts and batches \(mcpl\/coalescing\) could not be read: injected EIO reading mcpl\/coalescing\. MCPL doesn't start without them/); + } finally { restore(); } + await f.create(); + await f.framework.runUntilIdle(); + assert.equal(f.renders.length, 1, 'the restored batch was rendered'); + assert.ok(f.lastRequest().includes('document_diff')); +}); + +test("coalescing state this code doesn't read fails create(), naming what it is, and is left as it was", async (t) => { + const f = await fixture(); t.after(f.close); + await f.send('push/event', f.params('1', 'pending', { deferred: true })); + (f.framework as unknown as { flushCoalescingSnapshot(): void }).flushCoalescingSnapshot(); + await crash(f); + const storePath = join(f.dir, 'store'); + const snapshot = readState(storePath, 'mcpl/coalescing') as Record; + const cases: Array<[string, string, unknown, string]> = [ + ['a version 2 snapshot', 'mcpl/coalescing', { ...snapshot, version: 2 }, "receipts and batches \\(mcpl/coalescing\\) are a version 2 snapshot, which this version of agent-framework doesn't read\\. MCPL doesn't start without them, since the next coalesced push or message would replace them\\. They're unchanged, and the version that wrote them reads them\\.$"], + ['a snapshot with no version', 'mcpl/coalescing', { receipts: [], subjects: [] }, 'receipts and batches \\(mcpl/coalescing\\) are an object with no snapshot version\\.'], + ['a snapshot that is an array', 'mcpl/coalescing', [snapshot], 'receipts and batches \\(mcpl/coalescing\\) are an array with no snapshot version\\.'], + ['a snapshot restore throws on', 'mcpl/coalescing', { ...snapshot, subjects: 5 }, 'receipts and batches \\(mcpl/coalescing\\) could not be restored: .*\\. MCPL doesn\'t start'], + ['receipts that are an object', 'mcpl/coalescing-recent', { 0: 'receipt' }, 'receipts since the last snapshot \\(mcpl/coalescing-recent\\) are an object, not a list of receipts\\.'], + ['receipts restoreReceipts throws on', 'mcpl/coalescing-recent', [null], 'receipts since the last snapshot \\(mcpl/coalescing-recent\\) could not be restored: .*\\. MCPL doesn\'t start'], + ]; + for (const [name, stateId, value, says] of cases) { + const store = JsStore.openOrCreate({ path: storePath }); + const previous = store.getStateJson(stateId); + store.setStateJson(stateId, value); + store.close(); + await rejectsBuild(() => f.create(), new RegExp(`^the coalescing ${says}`), name); + assert.deepEqual(readState(storePath, stateId), value, `${name}: left as it was`); + const reset = JsStore.openOrCreate({ path: storePath }); + reset.setStateJson(stateId, previous); + reset.close(); + } +}); + +test("at a runtime connect, coalescing receipts that can't be read fail the connect, and the next connect restores them", async (t) => { + const f = await fixture(); t.after(f.close); + const params = f.params('1', 'accepted_before_crash', { deferred: true, data: { k: 'v' } }); + const original = await f.send('push/event', params); + await crash(f); + // With no MCPL servers configured, the subsystem comes up at the first connect. + const framework = await AgentFramework.create({ + storePath: join(f.dir, 'store'), membrane: f.membrane.asMembrane(), + agents: [{ name: 'agent', model: 'test', systemPrompt: 'test' }], modules: [], + } as unknown as FrameworkConfig); + try { + const server = { id: 'editor', url: `ws://127.0.0.1:${f.port}`, enabledFeatureSets: ['doc'] } as McplServerConfig; + const restore = failStoreRead('mcpl/coalescing-recent', 'initializePushCoalescer'); + try { + await assert.rejects(framework.connectMcplServer(server), { + message: /^the coalescing receipts since the last snapshot \(mcpl\/coalescing-recent\) could not be read: injected EIO reading mcpl\/coalescing-recent\. MCPL doesn't start without them/, + }); + } finally { restore(); } + await framework.connectMcplServer(server); + const retry = await f.send('push/event', params); + assert.deepEqual(retry.result, original.result, 'the receipt survived the refused connect'); + } finally { await framework.stop(); } +}); + +test('nothing stored reads as empty: a first boot starts serving, with no usage, deferred writes or coalescing history', async (t) => { + const f = await fixture(); t.after(f.close); + assert.equal(f.framework.getHostModeStatus().quiesced, false); + assert.equal(f.framework.getHostModeStatus().deferredWrites, 0); + assert.equal(f.framework.getSessionUsage().totals.inputTokens, 0); + const first = await f.send('push/event', f.params('1', 'first', { deferred: true })); + assert.ok(first.result, 'a push is accepted'); +}); diff --git a/test/helpers/failed-create.ts b/test/helpers/failed-create.ts new file mode 100644 index 000000000..f16125f9f --- /dev/null +++ b/test/helpers/failed-create.ts @@ -0,0 +1,34 @@ +/** + * A build of the framework that must fail: `AgentFramework.create()`, or a + * fixture's restart through it. Until a failed create() releases what it + * acquired (agent-framework #389), the framework it built keeps its store open, + * so a test that boots again on that store first stops it here, as that + * release would. stop() is idempotent, so this stop is harmless once it does. + */ +import assert from 'node:assert/strict'; +import { AgentFramework } from '../../src/index.js'; + +export async function rejectsBuild(build: () => Promise, message: RegExp, what?: string): Promise { + // restoreUsageState runs early in every create(), on the framework it builds. + const proto = AgentFramework.prototype as unknown as { restoreUsageState(this: AgentFramework): void }; + const original = proto.restoreUsageState; + let built: AgentFramework | undefined; + proto.restoreUsageState = function (this: AgentFramework) { + built = this; + return original.call(this); + }; + let caught: unknown; + let made: unknown; + try { + made = await build(); + } catch (error) { + caught = error; + } finally { + proto.restoreUsageState = original; + await built?.stop(); + } + if (made instanceof AgentFramework) await made.stop(); + assert.ok(caught instanceof Error, `the build should have failed${what ? `: ${what}` : ''}`); + assert.match(caught.message, message, what); + return caught; +} From 5e94fa491e9b09194eab9bb5d1925066f4dd6bf3 Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 10:44:26 -0400 Subject: [PATCH 02/11] fix(boot): refuse the rest of what boot took as empty, and read host-managed MCPL state after a restart The first commit's boot reads still took some states as empty, or refused in ways their text didn't describe, and three more reads of restored state did the same. This round: Recovery files. readRecoveryFile asked existsSync first, which answers false for any error (EACCES, ENOTDIR, EIO) as for a missing file, so an unreachable host-mode or deferred-writes file read as absent and the store's slot was read in its place. It now reads the file and takes only ENOENT as no file; any other error throws. Text that isn't JSON throws a StoredFormError, so the refusal says the file starts the framework once it holds a record, since reading it again won't change it. Both readers call it without an existsSync gate, and fall back to the slot only when the file is missing, which also covers a file removed between a check and the read. Stored nulls. Nothing stored is no file, no key in framework/state, or a state the store holds nothing for (getStateJson's null). A null someone stored in a file or under a key is a present value no release writes: the deferred-writes file holding null, usage: null, and an agent's settings record of null now refuse, as the host-mode read already did. framework/state itself. A framework/state that isn't an object read as holding nothing at every reader, and the first write replaced it. restoreUsageState is create()'s first read of it, so it refuses there, naming framework/state. Usage totals. persistedUsageProblem (replacing isPersistedUsageState) says what keeps a stored record from restoring: not a record, totals that aren't token counts, or a byAgent that isn't a list (which was skipped, and the next persist stored an empty list over it). A byAgent entry that isn't an object is skipped, as invalid entries already were, where a null one threw an unnamed TypeError. A provider's usage can lack a count (membrane 0.5.87's anthropic and bedrock adapters pass a whole response's input_tokens through as reported), which made the totals NaN, stored as null; accumulateUsage now adds 0 for a count that isn't a finite number, and a count stored as null restores as 0, so totals an earlier release stored that way don't refuse the boot. Agent settings. A map of agents' settings that isn't an object refuses, like a record that isn't one (the next settings write spread it and stored over it). The record is read by own key only, so an agent named "constructor" has no inherited record. Form problems throw a StoredFormError, so the refusal can say whether starting again helps. Tool-result guards. adoptToolResultGuard read toolResultGuards through optional chaining, so a container that isn't an object read as no setting, and the next guard change spread it and stored over it; and once any guard was stored, an agent named "constructor" read Object.prototype.constructor and failed create() as an invalid guard. A container that can't be read or isn't an object now refuses, naming it, when an agent is created (at boot or later, as for a fork), and the setting is read by the agent's own name. MCPL coalescing. restorePushCoalescer reads, checks, builds and restores the coalescer, and initializeMcpl calls it before it assigns anything else; installPushCoalescer installs it where the old initializer ran. Before, a refusal left the registry, feature-set manager, hook orchestrator and emitter assigned, and guards that test only the registry then took MCPL as up after a refused runtime connect. The coalescer's wasPublished caught a context it couldn't read and answered "not delivered", so a published batch or a receipt's occurrence was delivered again; restore is its only caller, so it now throws, and the restore refuses naming the agent. A snapshot version that isn't a number reads as "a snapshot whose version is , not 1". MCPL checkpoint trees. CheckpointManager.loadFromStore returned at !data?.trees, so a stored mcpl/checkpoints without trees read as no trees, and the next feature set registered stored over it; a failed read threw from the middle of initializeMcpl. Trees that can't be read, or a stored value that isn't an object whose trees is an object, now refuse, naming the state, and initializeMcpl builds the checkpoint manager right after the coalescer, before anything else, so either refusal leaves nothing of MCPL built. Every release writes { trees }. Host-managed MCPL state. loadFromStore restores each tree with currentState undefined, to be read "during registerFeatureSet", but registerFeatureSet returned early for a tree it already held, before its state read. So after a restart a host-managed feature set's state read as empty, and its next delta checkpoint applied to nothing and stored over it (shown on main with a real store opened again). A stored tree's state is now read when its feature set registers, through loadHostState, which a new tree uses too; a read that fails throws, naming the state, so its server's connect fails. A first-time tree is held only once its state is read, so a registration that fails is made whole, stored tree and trace included, by the next one. checkpoint-manager.ts gets its own thrownText and storedKind, as the repository's modules keep such helpers. Texts. A failed read keeps "starts once it can be read". A form problem says the framework starts once the file holds a record, or, for a store state, "starting again won't help until it's repaired". A recovery file's text now says that moved aside, the framework reads the store's slot instead, since that's what it does. The coalescing and checkpoint texts say that create() with MCPL servers configured, or a first connectMcplServer(), fails with MCPL. The deferred-writes check's text names the slot when there's no file. Tests: boot-state-read-errors grows to 26 tests: an unreachable host-mode file (ENOTDIR) fails create() and the next reachable boot is quiesced; both recovery files holding text that isn't JSON; the deferred-writes slot's failed read and form without a recovery file; usage forms (null, byAgent) and a null count and a null byAgent entry restoring; a framework/state that isn't an object; the tool-result guards' failed read and forms; a context that can't confirm a receipt's delivery; checkpoint trees that can't be read or aren't trees; a host-managed feature set's state read after a restart, and its failed read for a stored tree and a first-time one; the coalescing states and the checkpoint trees at a runtime connect, with nothing of MCPL built after the refusal; the forms releases wrote and a store from before the recovery files booting as before; and an agent named "constructor" with settings and guards stored for another. agent-runtime-settings adds a null record and a map that isn't an object. usage-tracker adds a usage without a count. failed-create.ts's comment names the pull request that fixes the issue it cites. Co-Authored-By: Claude Opus 5.5 --- changelog.d/boot-state-read-errors.fixed.md | 14 +- src/framework.ts | 309 +++++++++++------ src/mcpl/checkpoint-manager.ts | 114 +++++-- src/usage/usage-tracker.ts | 43 ++- test/agent-runtime-settings.test.ts | 19 +- test/boot-state-read-errors.test.ts | 355 ++++++++++++++++++-- test/helpers/failed-create.ts | 2 +- test/usage-tracker.test.ts | 11 + 8 files changed, 701 insertions(+), 166 deletions(-) diff --git a/changelog.d/boot-state-read-errors.fixed.md b/changelog.d/boot-state-read-errors.fixed.md index ee8609f61..0e0f2719e 100644 --- a/changelog.d/boot-state-read-errors.fixed.md +++ b/changelog.d/boot-state-read-errors.fixed.md @@ -1,7 +1,13 @@ -- `AgentFramework.create()` now fails when a state it restores at boot can't be read, or reads as something this version can't take for that state. The error names the state, or its recovery file, and says what to do; nothing it names is changed. Before, each was taken as empty, the framework served without it, and the next write replaced what was stored. Nothing stored still starts empty, and a store any release wrote boots as before unless one of these reads fails. +- Restored state is no longer taken as empty at boot. `AgentFramework.create()` now fails when a state it restores can't be read, or reads as something this version can't take for that state. The error names the state, or its recovery file, and says what to do; nothing it names is changed. Before, each was taken as empty, the framework served without it, and the next write replaced what was stored. Nothing stored still starts empty, and a store any release wrote boots as before unless one of these reads fails. - **Host mode** (`recovery/host-mode.json`, or `framework/state` without one): a quiesced host booted serving, and landed the writes it had deferred during the maintenance. - - **Deferred writes** (`recovery/deferred-writes.json`, or `framework/deferred-writes` without one): the queue restored nothing, and the next deferral replaced it. An agent's context that can't be checked for writes that landed before their ack now fails `create()` too, where the write was landed a second time. - - **Usage totals** (`framework/state`): they started at zero, and the next inference stored zeros over them. + - **Deferred writes** (`recovery/deferred-writes.json`, or `framework/deferred-writes` without one): the queue restored nothing, and the next deferral replaced it. An agent's context that can't be checked for writes that landed before their ack now fails `create()` too, where such a write landed a second time. + - **A recovery file that can't be reached** (an error other than its absence, such as `EACCES` on its directory) fails `create()` too, where it read as absent. Moved aside, either file is read from the store's slot instead, as on a store from before the recovery files. + - **Usage totals** (`framework/state`): they started at zero, and the next inference stored zeros over them. A `framework/state` that isn't an object now fails `create()`, where every reader took it as empty. - **An agent's persisted runtime settings** (`framework/state`): the agent booted at its recipe's settings, a `private` think-text policy as `public`, and its next settings change stored over the record. 0.22.2's notes said such a read is logged and the agent boots at the recipe's; it now fails `create()`. - - **MCPL coalescing** (`mcpl/coalescing`, `mcpl/coalescing-recent`): an acknowledged deferred acceptance or a pending batch was never delivered, and the next receipt or flush replaced the stored ones. A snapshot of a version this one doesn't read was ignored and replaced; it now fails `create()` and is kept for the version that wrote it. On a framework started with no MCPL servers, these are read at the first `connectMcplServer`, which fails instead, and the next connect reads them again. + - **Tool-result guards** (`framework/state`): a `toolResultGuards` that isn't an object read as no setting, and the next guard change stored over it. It's now read by each agent's own name, so an agent named `constructor` no longer fails `create()` once any guard is stored. + - **MCPL coalescing** (`mcpl/coalescing`, `mcpl/coalescing-recent`): an acknowledged deferred acceptance or a pending batch was never delivered, and the next receipt or flush replaced the stored ones. A snapshot of a version this one doesn't read was ignored and replaced; it now fails `create()` and is kept for the version that wrote it. An agent's context that can't be checked for an occurrence already delivered fails `create()` too, where the occurrence was delivered again. + - **MCPL checkpoint trees** (`mcpl/checkpoints`): a stored value without `trees` read as no checkpoints, and the next feature set registered stored over it. + - On a framework started with no MCPL servers, the coalescing state and the checkpoint trees are read at the first `connectMcplServer()`, which fails instead, with nothing of MCPL built, and the next connect reads them again. +- A host-managed MCPL feature set's state (`mcpl/state//`) is now read when its server connects after a restart. It wasn't, so the state read as empty, and the feature set's next delta checkpoint applied to nothing and stored over it. A failed read of it fails the server's connect, naming it. +- A usage total stored as `null`, which a provider's usage without a count left as `NaN`, restores as 0, and a usage without a count no longer makes the totals `NaN`. - A failed read of the persisted `tool_result_inline_max_chars` caps at boot is now logged. As before, they don't apply until a restart, and setting one keeps the others stored. diff --git a/src/framework.ts b/src/framework.ts index 0e77b6182..fcaeba21a 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -401,7 +401,7 @@ function validateUtilityArgs( import { CheckpointManager } from './mcpl/checkpoint-manager.js'; import { isToolAllowed } from './mcpl/tool-policy.js'; import { EventGate, formatShadowWarning } from './gate/event-gate.js'; -import { UsageTracker, isPersistedUsageState } from './usage/usage-tracker.js'; +import { UsageTracker, persistedUsageProblem, type PersistedUsageState } from './usage/usage-tracker.js'; import type { SessionUsageSnapshot, UsageUpdatedEvent } from './usage/types.js'; import { stderrTailOf, withStderr, type McplConnectionCloseInfo, type McplServerConnection } from './mcpl/server-connection.js'; import { isWebSocketTransport } from './mcpl/transport.js'; @@ -1552,6 +1552,11 @@ function thrownText(error: unknown): string { return text.length <= THROWN_TEXT_LIMIT ? text : `${text.slice(0, THROWN_TEXT_LIMIT)}… (${text.length} characters)`; } +/** A stored value that read but isn't in a form this code takes, as against + * a read that failed: starting again changes nothing until it's repaired, + * so the refusal that quotes it says so instead of "once it can be read". */ +class StoredFormError extends Error {} + /** A stored JSON value's kind, for a refusal that names what it found * without its content. */ function jsonKind(value: unknown): string { @@ -4227,11 +4232,34 @@ export class AgentFramework { /** A new agent's guard: its persisted setting, the tool-results spill its * withheld originals are written to, and the set stop() waits on - * (agent-framework #277). */ + * (agent-framework #277). The setting is read from framework/state's + * toolResultGuards by the agent's own name. A read that fails, or a + * toolResultGuards that isn't an object, throws, naming the state: taken + * as no setting, the agent would take its recipe's guard, and the next + * guard change would store over a toolResultGuards that isn't an object. */ private adoptToolResultGuard(agent: Agent): void { - const enabled = (this.store.getStateJson(FRAMEWORK_STATE_ID) as { - toolResultGuards?: Record; - } | null)?.toolResultGuards?.[agent.name]; + const refuse = (problem: string, cause?: unknown): Error => new Error( + `the tool-result guards (${FRAMEWORK_STATE_ID}, toolResultGuards) ${problem}. Agent "${agent.name}" isn't ` + + `created without them, since it would take its recipe's guard` + + (cause === undefined + ? ` and the next guard change would store over them. They're unchanged, and creating it again won't help until they're repaired.` + : `. They're unchanged, and it's created once they can be read.`), + cause === undefined ? undefined : { cause }, + ); + let data: unknown; + try { + data = this.store.getStateJson(FRAMEWORK_STATE_ID); + } catch (error) { + throw refuse(`could not be read: ${thrownText(error)}`, error); + } + const guards = data && typeof data === 'object' ? (data as Record).toolResultGuards : undefined; + if (guards !== undefined && (!guards || typeof guards !== 'object' || Array.isArray(guards))) { + throw refuse(`are ${jsonKind(guards)}, not an object of agents' guards`); + } + // Own keys only: an agent named "constructor" has no inherited override. + const enabled = guards !== undefined && Object.prototype.hasOwnProperty.call(guards, agent.name) + ? (guards as Record)[agent.name] + : undefined; if (enabled !== undefined && typeof enabled !== 'boolean') { throw new Error(`Invalid persisted tool_result_guard for ${agent.name}: expected a boolean`); } @@ -5079,9 +5107,24 @@ export class AgentFramework { renameSync(temporary, path); } + /** A recovery file's document, or undefined when there's no file. Only a + * missing file means none was written: any other failure to read it + * (EACCES, ENOTDIR, EIO, which existsSync would answer false for) + * throws, and text that isn't JSON throws a StoredFormError carrying + * the parser's reason, since reading it again won't change it. */ private readRecoveryFile(path: string): unknown { - if (!existsSync(path)) return undefined; - return JSON.parse(readFileSync(path, 'utf8')); + let text: string; + try { + text = readFileSync(path, 'utf8'); + } catch (error) { + if ((error as { code?: unknown } | null)?.code === 'ENOENT') return undefined; + throw error; + } + try { + return JSON.parse(text); + } catch (error) { + throw new StoredFormError(thrownText(error), { cause: error }); + } } private warnRecoveryFallbackOnce(): void { @@ -5153,30 +5196,41 @@ export class AgentFramework { * for writes that already landed (see landedDeferredWriteIds). */ private restorePersistedDeferredWrites(): number { - // Stores that predate the recovery file (or store-only configs) keep the - // queue in the store. - const file = this.deferredWritesPath && existsSync(this.deferredWritesPath) ? this.deferredWritesPath : undefined; - const where = file ?? DEFERRED_WRITES_ID; + // The recovery file wins whenever it exists. Stores that predate it (or + // store-only configs) keep the queue in the store's slot. + let file = this.deferredWritesPath; const refuse = (problem: string, cause?: unknown): Error => new Error( - `the deferred context writes (${where}) ${problem}. The framework doesn't start without them, ` + + `the deferred context writes (${file ?? DEFERRED_WRITES_ID}) ${problem}. The framework doesn't start without them, ` + `since the next deferral would replace the stored queue. ` + (file - ? `The file is unchanged; the framework starts once it can be read, or without those writes once it's moved aside.` - : `They're unchanged, and the framework starts once they can be read.`), + ? `The file is unchanged, and the framework starts once ${cause === undefined ? 'it holds a queue of writes' : 'it can be read'}; ` + + `moved aside, the framework reads the queue from ${DEFERRED_WRITES_ID} instead.` + : cause === undefined + ? `They're unchanged, and starting again won't help until they're repaired.` + : `They're unchanged, and the framework starts once they can be read.`), cause === undefined ? undefined : { cause }, ); let raw: unknown; try { - raw = file ? this.readRecoveryFile(file) : this.store.getStateJson(DEFERRED_WRITES_ID); + raw = file ? this.readRecoveryFile(file) : undefined; + if (raw === undefined) { + file = undefined; + raw = this.store.getStateJson(DEFERRED_WRITES_ID); + } } catch (error) { - throw refuse(`could not be read: ${thrownText(error)}`, error); + throw error instanceof StoredFormError + ? refuse(`hold text that isn't JSON: ${error.message}`) + : refuse(`could not be read: ${thrownText(error)}`, error); } - if (raw === undefined || raw === null) return 0; + // Nothing stored is an empty queue: no file, and nothing in the slot. A + // file that holds null isn't a queue (no release writes one). + if (!file && (raw === null || raw === undefined)) return 0; // v1 slot payloads were a bare array; v1/v2 files and v2 slots are // `{ pending, scanFrom? }`. - const doc = (Array.isArray(raw) ? { pending: raw } : typeof raw === 'object' ? raw : {}) as { pending?: unknown; scanFrom?: unknown }; - if (!Array.isArray(doc.pending)) { - throw refuse(typeof raw === 'object' + const doc = (Array.isArray(raw) ? { pending: raw } : raw !== null && typeof raw === 'object' ? raw : undefined) as + { pending?: unknown; scanFrom?: unknown } | undefined; + if (!doc || !Array.isArray(doc.pending)) { + throw refuse(doc ? `hold an object whose pending is ${doc.pending === undefined ? 'missing' : jsonKind(doc.pending)}, not a list of writes` : `hold ${jsonKind(raw)}, not a queue of writes`); } @@ -5210,7 +5264,7 @@ export class AgentFramework { landed = this.landedDeferredWriteIds(new Set(candidates.map((m) => m.id)), scanFrom); } catch (error) { throw new Error( - `the deferred context writes (${where}) can't be checked for writes that already landed: ` + + `the deferred context writes (${file ?? DEFERRED_WRITES_ID}) can't be checked for writes that already landed: ` + `${thrownText(error)}. The framework doesn't start without that check, since a write that landed ` + `before its ack would land a second time. The queue is unchanged, and the framework starts once ` + `that context can be read.`, @@ -5631,29 +5685,43 @@ export class AgentFramework { /** Restore the usage totals a prior session stored. None stored starts * fresh (the constructor's tracker). Totals that can't be read, or that * read as anything but totals, throw: taken as none, the next inference's - * persistUsageState would store fresh totals over them. */ + * persistUsageState would store fresh totals over them. + * + * This is create()'s first read of framework/state, so it also refuses a + * framework/state that isn't an object: every reader of that state would + * take it as holding nothing, and its first write would replace it. */ private restoreUsageState(): void { const refuse = (problem: string, cause?: unknown): Error => new Error( `the usage totals (${FRAMEWORK_STATE_ID}, usage) ${problem}. The framework doesn't start without them, ` + - `since the next inference would store fresh totals over them. They're unchanged, and the framework ` + - `starts once they can be read.`, + `since the next inference would store fresh totals over them. ` + + (cause === undefined + ? `They're unchanged, and starting again won't help until they're repaired.` + : `They're unchanged, and the framework starts once they can be read.`), cause === undefined ? undefined : { cause }, ); - let usage: unknown; + let data: unknown; try { - const data = this.store.getStateJson(FRAMEWORK_STATE_ID); - usage = data && typeof data === 'object' ? (data as Record).usage : undefined; + data = this.store.getStateJson(FRAMEWORK_STATE_ID); } catch (error) { throw refuse(`could not be read: ${thrownText(error)}`, error); } - if (usage === undefined || usage === null) return; - if (!isPersistedUsageState(usage)) { - const kind = jsonKind(usage); - throw refuse(kind === 'an object' ? 'are an object without token counts' : `are ${kind}, not token counts`); + if (data === null || data === undefined) return; + if (typeof data !== 'object' || Array.isArray(data)) { + throw new Error( + `${FRAMEWORK_STATE_ID} holds ${jsonKind(data)}, not an object. The framework doesn't start on it, since its ` + + `first write would replace it and everything it keeps, such as the usage totals and each agent's settings. ` + + `It's unchanged, and starting again won't help until it's repaired.`, + ); } + const usage = (data as Record).usage; + if (usage === undefined) return; + const problem = persistedUsageProblem(usage); + if (problem === 'not a record') throw refuse(`are ${jsonKind(usage)}, not a usage record`); + if (problem === 'totals') throw refuse(`are a record whose totals aren't token counts`); + if (problem === 'byAgent') throw refuse(`are a record whose byAgent isn't a list`); this.usageTracker = new UsageTracker({ emitTrace: (e: UsageUpdatedEvent) => this.emitTrace({ ...e }), - restored: usage, + restored: usage as PersistedUsageState, }); } @@ -5668,16 +5736,23 @@ export class AgentFramework { } } - /** An agent's persisted record, or null when it has none; throws when the - * store can't be read, or when what's stored for the agent isn't a record. */ + /** An agent's persisted record, or null when it has none. Throws the + * store's error when it can't be read, and a StoredFormError when what's + * stored for the agent, or the map that holds every agent's, isn't a record. */ private readAgentRuntimeSettings(agentName: string): AgentRuntimeSettingsOverrides | null { - const data = this.store.getStateJson(FRAMEWORK_STATE_ID) as { - agentRuntimeSettings?: Record; - } | null; - const stored = data?.agentRuntimeSettings?.[agentName]; - if (stored === undefined || stored === null) return null; - if (typeof stored !== 'object' || Array.isArray(stored)) { - throw new Error(`what's stored for the agent is ${jsonKind(stored)}, not a settings record`); + const data = this.store.getStateJson(FRAMEWORK_STATE_ID); + const map = data && typeof data === 'object' ? (data as Record).agentRuntimeSettings : undefined; + if (map === undefined) return null; + if (!map || typeof map !== 'object' || Array.isArray(map)) { + throw new StoredFormError(`the map that holds every agent's settings is ${jsonKind(map)}, not an object`); + } + // Own keys only: an agent named "constructor" has no inherited record. + const stored = Object.prototype.hasOwnProperty.call(map, agentName) + ? (map as Record)[agentName] + : undefined; + if (stored === undefined) return null; + if (!stored || typeof stored !== 'object' || Array.isArray(stored)) { + throw new StoredFormError(`what's stored for the agent is ${jsonKind(stored)}, not a settings record`); } return { ...(stored as AgentRuntimeSettingsOverrides) }; } @@ -5960,27 +6035,31 @@ export class AgentFramework { * reads as anything but a host-mode record, throws: taken as "not * quiesced", a host that stopped mid-maintenance would boot serving. */ private readHostMode(): { quiesced: boolean; reason?: string; since?: number } | null { - const file = this.hostModePath && existsSync(this.hostModePath) ? this.hostModePath : undefined; + let file = this.hostModePath; const refuse = (problem: string, cause?: unknown): Error => new Error( `the host mode (${file ?? `${FRAMEWORK_STATE_ID}, hostMode`}) ${problem}. The framework doesn't start without it, ` + `since a host quiesced for maintenance would boot serving. ` + (file - ? `The file is unchanged; the framework starts once it can be read, or without it once it's moved aside.` - : `It's unchanged, and the framework starts once it can be read.`), + ? `The file is unchanged, and the framework starts once ${cause === undefined ? 'it holds a host-mode record' : 'it can be read'}; ` + + `moved aside, the framework reads host mode from ${FRAMEWORK_STATE_ID} instead.` + : cause === undefined + ? `It's unchanged, and starting again won't help until it's repaired.` + : `It's unchanged, and the framework starts once it can be read.`), cause === undefined ? undefined : { cause }, ); let record: unknown; try { - if (file) { - record = this.readRecoveryFile(file); - } else { + record = file ? this.readRecoveryFile(file) : undefined; + if (record === undefined) { + file = undefined; const data = this.store.getStateJson(FRAMEWORK_STATE_ID); record = data && typeof data === 'object' ? (data as Record).hostMode : undefined; } } catch (error) { - throw refuse(`could not be read: ${thrownText(error)}`, error); + throw error instanceof StoredFormError + ? refuse(`holds text that isn't JSON: ${error.message}`) + : refuse(`could not be read: ${thrownText(error)}`, error); } - // A file removed between the check and the read is no record, as no file is. if (record === undefined) return null; const isRecord = !!record && typeof record === 'object' && !Array.isArray(record); const { quiesced, reason, since } = (isRecord ? record : {}) as Record; @@ -8752,8 +8831,10 @@ export class AgentFramework { throw new Error( `the persisted settings of agent "${config.name}" (${FRAMEWORK_STATE_ID}, agentRuntimeSettings) can't be ` + `restored: ${thrownText(error)}. The framework doesn't start without them, since the agent would boot at ` + - `its recipe's settings and the next settings change would store over them. They're unchanged, and the ` + - `framework starts once they can be read.`, + `its recipe's settings and the next settings change would store over them. ` + + (error instanceof StoredFormError + ? `They're unchanged, and starting again won't help until they're repaired.` + : `They're unchanged, and the framework starts once they can be read.`), { cause: error }, ); } @@ -10027,18 +10108,25 @@ export class AgentFramework { return createHash('sha256').update(JSON.stringify([serverId, config?.url ?? null, config?.command ?? null, config?.args ?? null])).digest('hex').slice(0, 16); } - private initializePushCoalescer(): void { - // The stored receipts and batches are read before the coalescer is built. - // Nothing stored is a coalescer with no history. What can't be read, or - // reads as anything but this version's snapshot and receipts, throws, so - // MCPL doesn't start: taken as no history, the next acceptance's receipt - // and the next flush would replace what's stored. - const refuse = (stateId: string, problem: string, remedy: string, cause?: unknown): Error => new Error( - `the coalescing ${stateId === COALESCING_STATE_ID ? 'receipts and batches' : 'receipts since the last snapshot'} ` + - `(${stateId}) ${problem}. MCPL doesn't start without them, since the next coalesced push or message ` + - `would replace them. ${remedy}`, - cause === undefined ? undefined : { cause }, - ); + /** + * The push coalescer, built and restored from its stored receipts and + * batches. initializeMcpl calls this before it builds anything else, so a + * refusal here leaves the MCPL subsystem as it was. Nothing stored is a + * coalescer with no history. What can't be read, reads as anything but + * this version's snapshot and receipts, or can't be restored, throws, and + * so does an agent's context that can't be checked for an occurrence + * already delivered (wasPublished): taken as no history, the next + * acceptance's receipt and the next flush would replace what's stored, and + * taken as undelivered, an occurrence would be delivered again. + */ + private restorePushCoalescer(): { coalescer: PushCoalescer; recent?: CoalescingReceiptRecord[] } { + const what = (stateId: string): string => + `the coalescing ${stateId === COALESCING_STATE_ID ? 'receipts and batches' : 'receipts since the last snapshot'} (${stateId})`; + const stops = `MCPL doesn't start without them, and neither does create() with MCPL servers configured, nor a ` + + `first connectMcplServer(), since the next coalesced push or message would replace them.`; + const unchanged = `They're unchanged, and starting again won't help until they're repaired.`; + const refuse = (stateId: string, problem: string, remedy: string, cause?: unknown): Error => + new Error(`${what(stateId)} ${problem}. ${stops} ${remedy}`, cause === undefined ? undefined : { cause }); const read = (stateId: string): unknown => { try { return this.store.getStateJson(stateId); @@ -10053,18 +10141,27 @@ export class AgentFramework { ? (snapshot as { version?: unknown }).version : undefined; if (version !== 1) { - throw version === undefined - ? refuse(COALESCING_STATE_ID, `are ${jsonKind(snapshot)} with no snapshot version`, `They're unchanged.`) - : refuse( + throw typeof version === 'number' + ? refuse( COALESCING_STATE_ID, - `are a version ${typeof version === 'number' ? version : jsonKind(version)} snapshot, which this version of agent-framework doesn't read`, + `are a version ${version} snapshot, which this version of agent-framework doesn't read`, `They're unchanged, and the version that wrote them reads them.`, + ) + : refuse( + COALESCING_STATE_ID, + version === undefined + ? `are ${jsonKind(snapshot)} with no snapshot version` + : `are a snapshot whose version is ${jsonKind(version)}, not 1`, + unchanged, ); } } if (recent !== null && recent !== undefined && !Array.isArray(recent)) { - throw refuse(COALESCING_RECENT_ID, `are ${jsonKind(recent)}, not a list of receipts`, `They're unchanged.`); + throw refuse(COALESCING_RECENT_ID, `are ${jsonKind(recent)}, not a list of receipts`, unchanged); } + // Set when wasPublished can't read an agent's context, so the restore that + // asked refuses with what couldn't be checked rather than as malformed. + let unreadable: { agent: string; error: unknown } | undefined; const coalescer = new PushCoalescer({ isUnread: (p) => this.isUnreadPlacement(p), remove: (p) => this.removePlacement(p), @@ -10102,13 +10199,18 @@ export class AgentFramework { this.store.setStateJson(COALESCING_RECENT_ID, this.coalescingRecentReceipts); }, wasPublished: (subject, eventId) => { - // Boot-time only: the occurrence's durable delivery identity - // (subject + eventId in message metadata) in any agent's context. + // Restore-time only: the occurrence's durable delivery identity + // (subject + eventId in message metadata) in any agent's context. A + // context that can't be read can't say, so the restore refuses. for (const agent of this.agents.values()) { + let messages: ReturnType; try { - if (agent.getContextManager().getAllMessages().some((m) => - m.metadata?.coalescingSubject === subject && m.metadata?.eventId === eventId)) return true; - } catch { /* a context that cannot be read cannot prove publication */ } + messages = agent.getContextManager().getAllMessages(); + } catch (error) { + unreadable = { agent: agent.name, error }; + throw error; + } + if (messages.some((m) => m.metadata?.coalescingSubject === subject && m.metadata?.eventId === eventId)) return true; } return false; }, @@ -10121,25 +10223,38 @@ export class AgentFramework { this.store.setStateJson(COALESCING_RECENT_ID, []); }, }); - try { - coalescer.restore(snapshot as CoalescingSnapshot | null | undefined); - } catch (error) { - throw refuse(COALESCING_STATE_ID, `could not be restored: ${thrownText(error)}`, `They're unchanged.`, error); - } - if (Array.isArray(recent)) { + const restore = (stateId: string, run: () => void): void => { try { - coalescer.restoreReceipts(recent as CoalescingReceiptRecord[]); + run(); } catch (error) { - throw refuse(COALESCING_RECENT_ID, `could not be restored: ${thrownText(error)}`, `They're unchanged.`, error); + if (unreadable) { + throw new Error( + `${what(stateId)} can't be checked for occurrences already delivered: ${unreadable.agent}'s context ` + + `could not be read: ${thrownText(unreadable.error)}. MCPL doesn't start without that check, and neither ` + + `does create() with MCPL servers configured, nor a first connectMcplServer(), since an occurrence ` + + `delivered before the restart would be delivered again. They're unchanged, and MCPL starts once that ` + + `context can be read.`, + { cause: unreadable.error }, + ); + } + throw refuse(stateId, `could not be restored: ${thrownText(error)}`, unchanged, error); } - // Until the next snapshot flush checkpoints the recovered state, these - // records are its only durable form: keep them in the writable buffer - // so the next acceptance appends to them instead of replacing them. - this.coalescingRecentReceipts = recent as CoalescingReceiptRecord[]; - } - this.pushCoalescer = coalescer; + }; + restore(COALESCING_STATE_ID, () => coalescer.restore(snapshot as CoalescingSnapshot | null | undefined)); + if (!Array.isArray(recent)) return { coalescer }; + restore(COALESCING_RECENT_ID, () => coalescer.restoreReceipts(recent as CoalescingReceiptRecord[])); + return { coalescer, recent: recent as CoalescingReceiptRecord[] }; + } + + /** Install a coalescer restorePushCoalescer built: none of this can fail. */ + private installPushCoalescer(restored: { coalescer: PushCoalescer; recent?: CoalescingReceiptRecord[] }): void { + // Until the next snapshot flush checkpoints the recovered state, these + // records are its only durable form: keep them in the writable buffer + // so the next acceptance appends to them instead of replacing them. + if (restored.recent) this.coalescingRecentReceipts = restored.recent; + this.pushCoalescer = restored.coalescer; // Batches restored from a snapshot still owe the agent a wake (§14 v41). - for (const occ of coalescer.pendingBatchOccurrences()) void this.wakeForCoalescedBatch(occ); + for (const occ of restored.coalescer.pendingBatchOccurrences()) void this.wakeForCoalescedBatch(occ); } /** @@ -16395,6 +16510,15 @@ export class AgentFramework { serverConfigs: McplServerConfig[], inferenceRouting?: import('./mcpl/types.js').InferenceRoutingPolicy, ): Promise { + // First, before anything is built: a coalescing state or checkpoint trees + // that can't be taken refuse here and leave the subsystem as it was, so a + // refused runtime connect leaves nothing for the guards that test + // mcplServerRegistry. + const coalescing = this.restorePushCoalescer(); + const checkpoints = new CheckpointManager( + this.store, + (event) => this.emitTrace(event as { type: TraceEvent['type']; [key: string]: unknown }), + ); this.mcplServerRegistry = new McplServerRegistry(); this.featureSetManager = new FeatureSetManager( (event) => this.emitTrace(event as { type: TraceEvent['type']; [key: string]: unknown }), @@ -16419,7 +16543,7 @@ export class AgentFramework { ?? (this.eventGate ? this.eventGate.asShouldTriggerCallback() : undefined); // Push events handler (Step 6) - this.initializePushCoalescer(); + this.installPushCoalescer(coalescing); this.pushHandler = new PushHandler( this.featureSetManager, (event) => this.pushEvent(event as unknown as ProcessEvent), @@ -16441,11 +16565,8 @@ export class AgentFramework { }, ); - // Checkpoint manager (Step 8) - this.checkpointManager = new CheckpointManager( - this.store, - (event) => this.emitTrace(event as { type: TraceEvent['type']; [key: string]: unknown }), - ); + // Checkpoint manager (Step 8), built and loaded at the top + this.checkpointManager = checkpoints; // Channel registry (Step 7) this.channelRegistry = new ChannelRegistry( diff --git a/src/mcpl/checkpoint-manager.ts b/src/mcpl/checkpoint-manager.ts index 4c7a26b42..664dcd78f 100644 --- a/src/mcpl/checkpoint-manager.ts +++ b/src/mcpl/checkpoint-manager.ts @@ -26,6 +26,34 @@ const CHECKPOINTS_STATE_ID = 'mcpl/checkpoints'; /** Prefix for per-feature-set host-managed state. */ const STATE_PREFIX = 'mcpl/state'; +/** What the refusals below say about MCPL when stored trees or state can't be taken. */ +const MCPL_STOPS = `MCPL doesn't start without them, and neither does create() with MCPL servers configured, ` + + `nor a first connectMcplServer()`; + +/** A thrown value's text, bounded and never throwing, as framework.ts's thrownText. */ +function thrownText(error: unknown): string { + let text: string; + try { + text = String(error instanceof Error ? error.message : error); + } catch { + text = '(a value that could not be read)'; + } + return text.length <= 500 ? text : `${text.slice(0, 500)}… (${text.length} characters)`; +} + +/** A stored JSON value's kind, for a refusal that names what it found without its content. */ +function storedKind(value: unknown): string { + return value === undefined ? 'missing' + : value === null ? 'null' + : Array.isArray(value) ? 'an array' + : typeof value === 'object' ? 'an object' + : `a ${typeof value}`; +} + +function isObject(value: unknown): value is Record { + return !!value && typeof value === 'object' && !Array.isArray(value); +} + // ============================================================================ // Internal types // ============================================================================ @@ -214,31 +242,26 @@ export class CheckpointManager { opts: { hostState: boolean; rollback: boolean }, ): void { const key = this.key(serverId, featureSet); - if (this.trees.has(key)) return; // Already registered (e.g. from loaded state) + const held = this.trees.get(key); + if (held) { + // Already registered, or restored from the store by loadFromStore, + // which leaves a host-managed tree's state to be read here: unread, + // the next patch would apply to nothing and store over it. + if (held.hostState && held.currentState === undefined) this.loadHostState(serverId, featureSet, held); + return; + } - this.trees.set(key, { + const tree: FeatureSetState = { hostState: opts.hostState, rollback: opts.rollback, currentCheckpoint: null, currentState: undefined, nodes: new Map(), - }); - - // Register per-feature-set state slot for host-managed state - if (opts.hostState) { - const stateId = this.stateId(serverId, featureSet); - try { - this.store.registerState({ id: stateId, strategy: 'snapshot' }); - } catch { - // Already registered - } - // Load existing state if any - const existing = this.store.getStateJson(stateId); - if (existing !== null && existing !== undefined) { - const tree = this.trees.get(key)!; - tree.currentState = existing; - } - } + }; + // Read before the tree is held, so a registration that fails here is + // made whole by the next one. + if (opts.hostState) this.loadHostState(serverId, featureSet, tree); + this.trees.set(key, tree); this.emitTraceFn({ type: 'mcpl:state_registered', @@ -540,12 +563,57 @@ export class CheckpointManager { this.store.setStateJson(CHECKPOINTS_STATE_ID, serialized); } - /** Load checkpoint tree from Chronicle on startup. */ + /** A host-managed feature set's stored state, read into its tree; nothing + * stored leaves it unset. A read that fails throws, naming the state: + * taken as nothing stored, the next patch would apply to nothing and + * store over it. */ + private loadHostState(serverId: string, featureSet: string, tree: FeatureSetState): void { + const stateId = this.stateId(serverId, featureSet); + try { + this.store.registerState({ id: stateId, strategy: 'snapshot' }); + } catch { + // Already registered + } + let existing: unknown; + try { + existing = this.store.getStateJson(stateId); + } catch (error) { + throw new Error( + `the host-managed state of feature set "${featureSet}" on MCPL server "${serverId}" (${stateId}) could not ` + + `be read: ${thrownText(error)}. The server's connect fails without it, since the feature set's next ` + + `checkpoint would store over it. It's unchanged, and the connect succeeds once it can be read.`, + { cause: error }, + ); + } + if (existing !== null && existing !== undefined) tree.currentState = existing; + } + + /** Load checkpoint trees from Chronicle on startup. Nothing stored is no + * trees. Trees that can't be read, or a stored value that isn't an object + * whose trees is an object, throw: taken as no trees, the next feature set + * registered would store over them. */ private loadFromStore(): void { - const data = this.store.getStateJson(CHECKPOINTS_STATE_ID) as SerializedTrees | null; - if (!data?.trees) return; + const what = `the MCPL checkpoint trees (${CHECKPOINTS_STATE_ID})`; + const stops = `${MCPL_STOPS}, since the next feature set registered would store over them.`; + let data: unknown; + try { + data = this.store.getStateJson(CHECKPOINTS_STATE_ID); + } catch (error) { + throw new Error( + `${what} could not be read: ${thrownText(error)}. ${stops} They're unchanged, and MCPL starts once they can be read.`, + { cause: error }, + ); + } + if (data === null || data === undefined) return; + const trees = isObject(data) ? data.trees : undefined; + if (!isObject(trees)) { + throw new Error( + `${what} are ${isObject(data) ? `an object whose trees is ${storedKind(trees)}` : storedKind(data)}, not ` + + `checkpoint trees. ${stops} They're unchanged, and starting again won't help until they're repaired.`, + ); + } - for (const [key, entry] of Object.entries(data.trees)) { + for (const [key, entry] of Object.entries(trees as SerializedTrees['trees'])) { const nodes = new Map(); for (const [id, nodeData] of Object.entries(entry.nodes)) { nodes.set(id, { diff --git a/src/usage/usage-tracker.ts b/src/usage/usage-tracker.ts index 7cf8a422f..fbedb6080 100644 --- a/src/usage/usage-tracker.ts +++ b/src/usage/usage-tracker.ts @@ -12,10 +12,13 @@ function accumulateUsage( tokens: { inputTokens: number; outputTokens: number; cacheCreationTokens: number; cacheReadTokens: number }, estimatedCost?: { total: number; currency: string } ): void { - target.inputTokens += tokens.inputTokens; - target.outputTokens += tokens.outputTokens; - target.cacheCreationTokens += tokens.cacheCreationTokens; - target.cacheReadTokens += tokens.cacheReadTokens; + // A provider's usage can lack a count (membrane passes some through as + // reported); adding it would make the total NaN, which is stored as null. + const count = (n: number): number => (Number.isFinite(n) ? n : 0); + target.inputTokens += count(tokens.inputTokens); + target.outputTokens += count(tokens.outputTokens); + target.cacheCreationTokens += count(tokens.cacheCreationTokens); + target.cacheReadTokens += count(tokens.cacheReadTokens); if (estimatedCost) { if (!target.estimatedCost) { target.estimatedCost = { total: 0, currency: estimatedCost.currency }; @@ -31,10 +34,16 @@ function accumulateUsage( function parseUsage(raw: unknown): SessionUsage | null { if (!raw || typeof raw !== 'object') return null; const o = raw as Record; - if (typeof o.inputTokens !== 'number' || typeof o.outputTokens !== 'number') return null; + // A count stored as null is a NaN total, as JSON stores one: an earlier + // release could write it when a provider's usage lacked a count. It reads + // as 0, the count that survived it. + const count = (v: unknown): number | undefined => (v === null ? 0 : typeof v === 'number' ? v : undefined); + const inputTokens = count(o.inputTokens); + const outputTokens = count(o.outputTokens); + if (inputTokens === undefined || outputTokens === undefined) return null; return { - inputTokens: o.inputTokens, - outputTokens: o.outputTokens as number, + inputTokens, + outputTokens, cacheCreationTokens: typeof o.cacheCreationTokens === 'number' ? o.cacheCreationTokens : 0, cacheReadTokens: typeof o.cacheReadTokens === 'number' ? o.cacheReadTokens : 0, estimatedCost: o.estimatedCost && typeof o.estimatedCost === 'object' @@ -49,12 +58,16 @@ export interface PersistedUsageState { inferenceCount: number; } -/** Whether a stored usage record is one the tracker restores: an object whose - * totals are token counts. Its per-agent entries are checked one by one as - * they restore. */ -export function isPersistedUsageState(value: unknown): value is PersistedUsageState { - return !!value && typeof value === 'object' && !Array.isArray(value) - && parseUsage((value as { totals?: unknown }).totals) !== null; +/** What keeps a stored usage record from restoring, or undefined when it + * restores: it must be an object whose totals are token counts and whose + * byAgent, if it has one, is a list. Its per-agent entries are checked one + * by one as they restore, and one that isn't an agent's usage is skipped. */ +export function persistedUsageProblem(value: unknown): 'not a record' | 'totals' | 'byAgent' | undefined { + if (!value || typeof value !== 'object' || Array.isArray(value)) return 'not a record'; + const { totals, byAgent } = value as { totals?: unknown; byAgent?: unknown }; + if (parseUsage(totals) === null) return 'totals'; + if (byAgent !== undefined && !Array.isArray(byAgent)) return 'byAgent'; + return undefined; } export class UsageTracker { @@ -79,7 +92,9 @@ export class UsageTracker { this.totals = totals; this.inferenceCount = typeof state.inferenceCount === 'number' ? state.inferenceCount : 0; if (Array.isArray(state.byAgent)) { - for (const entry of state.byAgent) { + for (const item of state.byAgent as unknown[]) { + if (!item || typeof item !== 'object') continue; + const entry = item as { agentName?: unknown; usage?: unknown; inferenceCount?: unknown }; const usage = parseUsage(entry.usage); if (usage && typeof entry.agentName === 'string') { this.byAgent.set(entry.agentName, { diff --git a/test/agent-runtime-settings.test.ts b/test/agent-runtime-settings.test.ts index 25455f7f4..1da7c4f6f 100644 --- a/test/agent-runtime-settings.test.ts +++ b/test/agent-runtime-settings.test.ts @@ -877,9 +877,24 @@ describe('a reached budget across a restart', () => { }); it("what's stored for an agent that isn't a settings record fails create(), naming what it is", async () => { - await run(async ({ framework, restart }) => { + await run(async ({ framework, restart, config }) => { writeRecord(framework(), ['contextBudgetTokens', 60_000] as never); - await rejectsBuild(restart, /^the persisted settings of agent "agent" \(framework\/state, agentRuntimeSettings\) can't be restored: what's stored for the agent is an array, not a settings record\. The framework doesn't start without them/); + await rejectsBuild(restart, /^the persisted settings of agent "agent" \(framework\/state, agentRuntimeSettings\) can't be restored: what's stored for the agent is an array, not a settings record\. The framework doesn't start without them, since the agent would boot at its recipe's settings and the next settings change would store over them\. They're unchanged, and starting again won't help until they're repaired\.$/); + // The refused boots closed their stores, so write the next form through one of the test's own. + const store = JsStore.openOrCreate({ path: config().storePath }); + try { + const state = store.getStateJson('framework/state') as { agentRuntimeSettings?: object }; + store.setStateJson('framework/state', { ...state, agentRuntimeSettings: { ...state.agentRuntimeSettings, agent: null } }); + } finally { store.close(); } + await rejectsBuild(restart, /can't be restored: what's stored for the agent is null, not a settings record\./); + }); + }); + + it("a map of agents' settings that isn't an object fails create(), naming what it is", async () => { + await run(async ({ framework, restart }) => { + const store = framework().getStore(); + store.setStateJson('framework/state', { ...(store.getStateJson('framework/state') as object), agentRuntimeSettings: 'agent' }); + await rejectsBuild(restart, /^the persisted settings of agent "agent" \(framework\/state, agentRuntimeSettings\) can't be restored: the map that holds every agent's settings is a string, not an object\. /); }); }); diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts index d63a33dbe..85cf4d46e 100644 --- a/test/boot-state-read-errors.test.ts +++ b/test/boot-state-read-errors.test.ts @@ -16,7 +16,7 @@ */ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdirSync, mkdtempSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { JsStore } from '@animalabs/chronicle'; @@ -28,6 +28,7 @@ import { MockMembrane } from './helpers/mock-membrane.js'; import { fixture, crash } from './helpers/coalescing-fixture.js'; import { rejectsBuild } from './helpers/failed-create.js'; import { isStateExistsError } from '../src/module-registry.js'; +import { CheckpointManager } from '../src/mcpl/checkpoint-manager.js'; /** An I/O error from a store read of `stateId` by the framework's `reader`, * `times` times, as a failing disk would give. */ @@ -120,7 +121,7 @@ test("a host-mode file that can't be read fails create(), and the next boot is q await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( `^the host mode \\(${file.replace(/[.*+?^${}()|[\]\\/]/g, '\\$&')}\\) could not be read: injected EIO reading host-mode\\.json\\. ` + `The framework doesn't start without it, since a host quiesced for maintenance would boot serving\\. ` + - `The file is unchanged; the framework starts once it can be read, or without it once it's moved aside\\.$`, + `The file is unchanged, and the framework starts once it can be read; moved aside, the framework reads host mode from framework/state instead\\.$`, )); } finally { restore(); } assert.equal(readFileSync(file, 'utf8'), before, 'the file is as it was'); @@ -132,23 +133,47 @@ test("a host-mode file that can't be read fails create(), and the next boot is q } finally { await f.stop(); } })); -test('a host-mode file that holds no record, or one whose quiesced is not a boolean, fails create(), naming what it holds', inStoreDir('boot-read-hostmode-form-', async (storePath) => { +test('a host-mode file that holds no record, text that isn\'t JSON, or a record whose quiesced is not a boolean, fails create(), naming what it holds', inStoreDir('boot-read-hostmode-form-', async (storePath) => { await quiescedWithAWrite(storePath); const file = join(storePath, 'recovery', 'host-mode.json'); + const literal = (s: string) => s.replace(/[.*+?^${}()|[\]\\/]/g, '\\$&'); for (const [content, holds] of [ - ['["quiesced"]', 'holds an array, not a host-mode record'], - ['null', 'holds null, not a host-mode record'], - ['{"version":1,"quiesced":"yes"}', 'is a record whose quiesced is a string, not true or false'], - ['{"version":1}', 'is a record whose quiesced is missing, not true or false'], - ] as const) { + ['["quiesced"]', literal('holds an array, not a host-mode record')], + ['null', literal('holds null, not a host-mode record')], + ['{"version":1,"quiesced":"yes"}', literal('is a record whose quiesced is a string, not true or false')], + ['{"version":1}', literal('is a record whose quiesced is missing, not true or false')], + // The parser's own words differ between Node versions. + ['{"version":1,"quiesced":tr', "holds text that isn't JSON: .+"], + ]) { writeFileSync(file, content); await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( - `^the host mode \\(.*host-mode\\.json\\) ${holds.replace(/[.*+?^${}()|[\]\\/]/g, '\\$&')}\\. The framework doesn't start without it`, + `^the host mode \\(.*host-mode\\.json\\) ${holds}\\. The framework doesn't start without it, ` + + `since a host quiesced for maintenance would boot serving\\. The file is unchanged, and the framework starts once it holds a ` + + `host-mode record; moved aside, the framework reads host mode from framework/state instead\\.$`, ), content); assert.equal(readFileSync(file, 'utf8'), content, `${content}: the file is as it was`); } })); +test("a recovery file that can't be reached, not only one that won't read, fails create(), and doesn't read as absent", inStoreDir('boot-read-hostmode-enotdir-', async (storePath) => { + // A file where the host-mode file's directory should be makes its read fail + // with ENOTDIR, an error existsSync answers false for, as it does EACCES. + const dir = join(storePath, '..', 'host-mode-dir'); + const withPath = (): FrameworkConfig => ({ ...config(storePath), hostModePath: join(dir, 'host-mode.json') }) as FrameworkConfig; + const first = await AgentFramework.create(withPath()); + try { await first.quiesce({ reason: 'surgery' }); } finally { await first.stop(); } + renameSync(dir, `${dir}.aside`); + writeFileSync(dir, 'not a directory'); + try { + await rejectsBuild(() => AgentFramework.create(withPath()), /^the host mode \(.*host-mode-dir\/host-mode\.json\) could not be read: ENOTDIR\b.*The file is unchanged, and the framework starts once it can be read;/); + } finally { + rmSync(dir); + renameSync(`${dir}.aside`, dir); + } + const f = await AgentFramework.create(withPath()); + try { assert.equal(f.getHostModeStatus().quiesced, true); } finally { await f.stop(); } +})); + test("with no recovery file, a host mode that can't be read from framework/state fails create()", async () => { const dir = mkdtempSync(join(tmpdir(), 'boot-read-hostmode-slot-')); const store = JsStore.openOrCreate({ path: join(dir, 'store') }); @@ -165,7 +190,7 @@ test("with no recovery file, a host mode that can't be read from framework/state const f = await AgentFramework.create(slotConfig()); try { assert.equal(f.getHostModeStatus().quiesced, true); } finally { await f.stop(); } store.setStateJson('framework/state', { ...(store.getStateJson('framework/state') as object), hostMode: 'quiesced' }); - await rejectsBuild(() => AgentFramework.create(slotConfig()), /^the host mode \(framework\/state, hostMode\) holds a string, not a host-mode record\./); + await rejectsBuild(() => AgentFramework.create(slotConfig()), /^the host mode \(framework\/state, hostMode\) holds a string, not a host-mode record\. The framework doesn't start without it, since a host quiesced for maintenance would boot serving\. It's unchanged, and starting again won't help until it's repaired\.$/); } finally { store.close(); rmSync(dir, { recursive: true, force: true }); @@ -182,7 +207,7 @@ test("a deferred-write queue that can't be read fails create(), and the next boo const before = readFileSync(file, 'utf8'); const restore = failRecoveryRead('deferred-writes.json'); try { - await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the deferred context writes \(.*deferred-writes\.json\) could not be read: injected EIO reading deferred-writes\.json\. The framework doesn't start without them, since the next deferral would replace the stored queue\. The file is unchanged; the framework starts once it can be read, or without those writes once it's moved aside\.$/); + await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the deferred context writes \(.*deferred-writes\.json\) could not be read: injected EIO reading deferred-writes\.json\. The framework doesn't start without them, since the next deferral would replace the stored queue\. The file is unchanged, and the framework starts once it can be read; moved aside, the framework reads the queue from framework\/deferred-writes instead\.$/); } finally { restore(); } assert.equal(readFileSync(file, 'utf8'), before, 'the queue is as it was'); const f = await AgentFramework.create(config(storePath)); @@ -193,22 +218,53 @@ test("a deferred-write queue that can't be read fails create(), and the next boo } finally { await f.stop(); } })); -test("a deferred-write queue that isn't a queue fails create(), naming what it holds", inStoreDir('boot-read-deferred-form-', async (storePath) => { +test("a deferred-write queue that isn't a queue, or isn't JSON, fails create(), naming what it holds", inStoreDir('boot-read-deferred-form-', async (storePath) => { await quiescedWithAWrite(storePath); const file = join(storePath, 'recovery', 'deferred-writes.json'); for (const [content, holds] of [ ['"pending"', 'hold a string, not a queue of writes'], + ['null', 'hold null, not a queue of writes'], ['{"version":2,"pending":{"0":{}}}', 'hold an object whose pending is an object, not a list of writes'], ['{"version":2}', 'hold an object whose pending is missing, not a list of writes'], + // The parser's own words differ between Node versions. + ['{"version":2,', "hold text that isn't JSON: .+"], ] as const) { writeFileSync(file, content); await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( - `^the deferred context writes \\(.*deferred-writes\\.json\\) ${holds}\\. The framework doesn't start without them`, + `^the deferred context writes \\(.*deferred-writes\\.json\\) ${holds}\\. The framework doesn't start without them, since the next ` + + `deferral would replace the stored queue\\. The file is unchanged, and the framework starts once it holds a queue of writes; ` + + `moved aside, the framework reads the queue from framework/deferred-writes instead\\.$`, ), content); assert.equal(readFileSync(file, 'utf8'), content, `${content}: the file is as it was`); } })); +test("with no recovery file, a deferred-write queue that can't be read, or isn't a queue, fails create()", async () => { + const dir = mkdtempSync(join(tmpdir(), 'boot-read-deferred-slot-')); + const store = JsStore.openOrCreate({ path: join(dir, 'store') }); + // A store passed in, with no recovery paths, keeps the queue in framework/deferred-writes. + const slotConfig = () => ({ store, membrane: new MockMembrane().asMembrane(), agents, modules: [] }) as unknown as FrameworkConfig; + try { + const first = await AgentFramework.create(slotConfig()); + await first.quiesce({ reason: 'surgery' }); + (first as unknown as { addMessage(p: string, c: unknown[]): unknown }).addMessage('user', [{ type: 'text', text: 'said during surgery' }]); + await first.stop(); + const queue = store.getStateJson('framework/deferred-writes'); + const restore = failStoreRead('framework/deferred-writes', 'restorePersistedDeferredWrites'); + try { + await rejectsBuild(() => AgentFramework.create(slotConfig()), /^the deferred context writes \(framework\/deferred-writes\) could not be read: injected EIO reading framework\/deferred-writes\. The framework doesn't start without them, since the next deferral would replace the stored queue\. They're unchanged, and the framework starts once they can be read\.$/); + } finally { restore(); } + assert.deepEqual(store.getStateJson('framework/deferred-writes'), queue, 'the queue is as it was'); + const f = await AgentFramework.create(slotConfig()); + try { assert.equal(f.getHostModeStatus().deferredWrites, 1); } finally { await f.stop(); } + store.setStateJson('framework/deferred-writes', 'pending'); + await rejectsBuild(() => AgentFramework.create(slotConfig()), /^the deferred context writes \(framework\/deferred-writes\) hold a string, not a queue of writes\. The framework doesn't start without them, since the next deferral would replace the stored queue\. They're unchanged, and starting again won't help until they're repaired\.$/); + } finally { + store.close(); + rmSync(dir, { recursive: true, force: true }); + } +}); + test("a deferred write whose agent's context can't be scanned fails create(), and the next boot doesn't land it twice", inStoreDir('boot-read-deferred-landed-', async (storePath) => { // A write that landed before its ack: in the queue, and in the context with its id. await quiescedWithAWrite(storePath); @@ -259,17 +315,48 @@ test("usage totals that can't be read fail create(), and the next boot restores } finally { await f.stop(); } })); -test("usage totals that aren't token counts fail create(), naming what they are", inStoreDir('boot-read-usage-form-', async (storePath) => { +test("usage totals that aren't a usage record fail create(), naming what they are", inStoreDir('boot-read-usage-form-', async (storePath) => { for (const [usage, are] of [ - [{ totals: 'lots' }, 'are an object without token counts'], - [[storedUsage], 'are an array, not token counts'], + [{ totals: 'lots' }, "are a record whose totals aren't token counts"], + [[storedUsage], 'are an array, not a usage record'], + [null, 'are null, not a usage record'], + [{ ...storedUsage, byAgent: 'agent' }, "are a record whose byAgent isn't a list"], ] as const) { seedFrameworkState(storePath, { usage }); - await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp(`^the usage totals \\(framework/state, usage\\) ${are}\\. The framework doesn't start without them`), JSON.stringify(usage)); + await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( + `^the usage totals \\(framework/state, usage\\) ${are}\\. The framework doesn't start without them, since the next inference ` + + `would store fresh totals over them\\. They're unchanged, and starting again won't help until they're repaired\\.$`, + ), JSON.stringify(usage)); assert.deepEqual((readState(storePath, 'framework/state') as { usage: unknown }).usage, usage); } })); +test('usage totals an earlier release wrote with a null count, or with a byAgent entry that is not an agent, restore', inStoreDir('boot-read-usage-null-', async (storePath) => { + // A NaN total, which a provider's usage without a count made, is stored as null. + seedFrameworkState(storePath, { usage: { + totals: { inputTokens: null, outputTokens: 500 }, + inferenceCount: 7, + byAgent: [null, { agentName: 'agent', usage: { inputTokens: 1000, outputTokens: 500 }, inferenceCount: 7 }], + } }); + const f = await AgentFramework.create(config(storePath)); + try { + const usage = f.getSessionUsage(); + assert.equal(usage.totals.inputTokens, 0); + assert.equal(usage.totals.outputTokens, 500); + assert.deepEqual(usage.byAgent.map((a) => [a.agentName, a.usage.inputTokens]), [['agent', 1000]]); + } finally { await f.stop(); } +})); + +test('a framework/state that is not an object fails create(), naming it, and is left as it was', inStoreDir('boot-read-state-root-', async (storePath) => { + const seed = JsStore.openOrCreate({ path: storePath }); + try { + seed.registerState({ id: 'framework/state', strategy: 'snapshot' }); + seed.setStateJson('framework/state', 'usage'); + } finally { seed.close(); } + await rejectsBuild(() => AgentFramework.create(config(storePath)), /^framework\/state holds a string, not an object\. The framework doesn't start on it, since its first write would replace it and everything it keeps, such as the usage totals and each agent's settings\. It's unchanged, and starting again won't help until it's repaired\.$/); + assert.equal(readState(storePath, 'framework/state'), 'usage'); +})); + // --------------------------------------------------------------------------- // Inline caps // --------------------------------------------------------------------------- @@ -302,6 +389,34 @@ test("inline caps that can't be read are logged, boot goes on, and setting one k assert.deepEqual((readState(storePath, 'framework/state') as { toolResultInlineCaps: unknown }).toolResultInlineCaps, { agent: 5000, other: 2000 }); })); +// --------------------------------------------------------------------------- +// Tool-result guards +// --------------------------------------------------------------------------- + +const guardOf = (f: AgentFramework, name: string): boolean | undefined => + (f.getAgent(name) as unknown as { toolResultGuard: { settingOverride?: boolean } }).toolResultGuard.settingOverride; + +test("tool-result guards that can't be read, or that aren't an object, fail create(), naming them, and are left as they were", inStoreDir('boot-read-guards-', async (storePath) => { + const first = await AgentFramework.create(config(storePath)); + await first.stop(); + seedFrameworkState(storePath, { toolResultGuards: { agent: true } }); + const restore = failStoreRead('framework/state', 'adoptToolResultGuard'); + try { + await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the tool-result guards \(framework\/state, toolResultGuards\) could not be read: injected EIO reading framework\/state\. Agent "agent" isn't created without them, since it would take its recipe's guard\. They're unchanged, and it's created once they can be read\.$/); + } finally { restore(); } + const f = await AgentFramework.create(config(storePath)); + try { assert.equal(guardOf(f, 'agent'), true, 'the next boot restores the guard'); } finally { await f.stop(); } + for (const [guards, are] of [['agent', 'a string'], [null, 'null'], [[true], 'an array']] as const) { + seedFrameworkState(storePath, { toolResultGuards: guards }); + await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( + `^the tool-result guards \\(framework/state, toolResultGuards\\) are ${are}, not an object of agents' guards\\. Agent "agent" isn't ` + + `created without them, since it would take its recipe's guard and the next guard change would store over them\\. They're ` + + `unchanged, and creating it again won't help until they're repaired\\.$`, + ), JSON.stringify(guards)); + assert.deepEqual((readState(storePath, 'framework/state') as { toolResultGuards: unknown }).toolResultGuards, guards); + } +})); + // --------------------------------------------------------------------------- // Coalescing // --------------------------------------------------------------------------- @@ -311,9 +426,9 @@ test("coalescing receipts that can't be read fail create(), and the next boot de const params = f.params('1', 'accepted_before_crash', { deferred: true, data: { k: 'v' } }); const original = await f.send('push/event', params); await crash(f); - const restore = failStoreRead('mcpl/coalescing-recent', 'initializePushCoalescer'); + const restore = failStoreRead('mcpl/coalescing-recent', 'restorePushCoalescer'); try { - await rejectsBuild(() => f.create(), /^the coalescing receipts since the last snapshot \(mcpl\/coalescing-recent\) could not be read: injected EIO reading mcpl\/coalescing-recent\. MCPL doesn't start without them, since the next coalesced push or message would replace them\. They're unchanged, and MCPL starts once they can be read\.$/); + await rejectsBuild(() => f.create(), /^the coalescing receipts since the last snapshot \(mcpl\/coalescing-recent\) could not be read: injected EIO reading mcpl\/coalescing-recent\. MCPL doesn't start without them, and neither does create\(\) with MCPL servers configured, nor a first connectMcplServer\(\), since the next coalesced push or message would replace them\. They're unchanged, and MCPL starts once they can be read\.$/); } finally { restore(); } await f.create(); const retry = await f.send('push/event', params); @@ -328,7 +443,7 @@ test("a coalescing snapshot that can't be read fails create(), and the next boot await f.send('push/event', f.params('1', 'pending_before_crash', { deferred: true })); (f.framework as unknown as { flushCoalescingSnapshot(): void }).flushCoalescingSnapshot(); await crash(f); - const restore = failStoreRead('mcpl/coalescing', 'initializePushCoalescer'); + const restore = failStoreRead('mcpl/coalescing', 'restorePushCoalescer'); try { await rejectsBuild(() => f.create(), /^the coalescing receipts and batches \(mcpl\/coalescing\) could not be read: injected EIO reading mcpl\/coalescing\. MCPL doesn't start without them/); } finally { restore(); } @@ -338,6 +453,28 @@ test("a coalescing snapshot that can't be read fails create(), and the next boot assert.ok(f.lastRequest().includes('document_diff')); }); +test("coalescing receipts whose delivery an agent's context can't confirm fail create(), and the next boot delivers once", async (t) => { + const f = await fixture(); t.after(f.close); + const params = f.params('1', 'accepted_before_crash', { deferred: true, data: { k: 'v' } }); + const original = await f.send('push/event', params); + await crash(f); + // Restoring a receipt asks every agent's context whether its occurrence was delivered. + const proto = ContextManager.prototype as unknown as { getAllMessages(this: ContextManager): unknown[] }; + const real = proto.getAllMessages; + proto.getAllMessages = function (this: ContextManager) { + if (new Error().stack?.includes('wasPublished')) throw new Error('injected EIO reading the context'); + return real.call(this); + }; + try { + await rejectsBuild(() => f.create(), /^the coalescing receipts since the last snapshot \(mcpl\/coalescing-recent\) can't be checked for occurrences already delivered: agent's context could not be read: injected EIO reading the context\. MCPL doesn't start without that check, and neither does create\(\) with MCPL servers configured, nor a first connectMcplServer\(\), since an occurrence delivered before the restart would be delivered again\. They're unchanged, and MCPL starts once that context can be read\.$/); + } finally { proto.getAllMessages = real; } + await f.create(); + const retry = await f.send('push/event', params); + assert.deepEqual(retry.result, original.result, 'the receipt survived'); + await f.framework.runUntilIdle(); + assert.equal(f.membrane.calls.length, 1, 'the accepted work was delivered once'); +}); + test("coalescing state this code doesn't read fails create(), naming what it is, and is left as it was", async (t) => { const f = await fixture(); t.after(f.close); await f.send('push/event', f.params('1', 'pending', { deferred: true })); @@ -346,7 +483,8 @@ test("coalescing state this code doesn't read fails create(), naming what it is, const storePath = join(f.dir, 'store'); const snapshot = readState(storePath, 'mcpl/coalescing') as Record; const cases: Array<[string, string, unknown, string]> = [ - ['a version 2 snapshot', 'mcpl/coalescing', { ...snapshot, version: 2 }, "receipts and batches \\(mcpl/coalescing\\) are a version 2 snapshot, which this version of agent-framework doesn't read\\. MCPL doesn't start without them, since the next coalesced push or message would replace them\\. They're unchanged, and the version that wrote them reads them\\.$"], + ['a version 2 snapshot', 'mcpl/coalescing', { ...snapshot, version: 2 }, "receipts and batches \\(mcpl/coalescing\\) are a version 2 snapshot, which this version of agent-framework doesn't read\\. MCPL doesn't start without them, and neither does create\\(\\) with MCPL servers configured, nor a first connectMcplServer\\(\\), since the next coalesced push or message would replace them\\. They're unchanged, and the version that wrote them reads them\\.$"], + ['a snapshot whose version is a string', 'mcpl/coalescing', { ...snapshot, version: '1' }, "receipts and batches \\(mcpl/coalescing\\) are a snapshot whose version is a string, not 1\\. .* They're unchanged, and starting again won't help until they're repaired\\.$"], ['a snapshot with no version', 'mcpl/coalescing', { receipts: [], subjects: [] }, 'receipts and batches \\(mcpl/coalescing\\) are an object with no snapshot version\\.'], ['a snapshot that is an array', 'mcpl/coalescing', [snapshot], 'receipts and batches \\(mcpl/coalescing\\) are an array with no snapshot version\\.'], ['a snapshot restore throws on', 'mcpl/coalescing', { ...snapshot, subjects: 5 }, 'receipts and batches \\(mcpl/coalescing\\) could not be restored: .*\\. MCPL doesn\'t start'], @@ -366,6 +504,94 @@ test("coalescing state this code doesn't read fails create(), naming what it is, } }); +// --------------------------------------------------------------------------- +// MCPL checkpoints +// --------------------------------------------------------------------------- + +test("MCPL checkpoint trees that can't be read, or that aren't checkpoint trees, fail create(), and are left as they were", async (t) => { + const f = await fixture(); t.after(f.close); + await crash(f); + const storePath = join(f.dir, 'store'); + const restore = failStoreRead('mcpl/checkpoints', 'loadFromStore'); + try { + await rejectsBuild(() => f.create(), /^the MCPL checkpoint trees \(mcpl\/checkpoints\) could not be read: injected EIO reading mcpl\/checkpoints\. MCPL doesn't start without them, and neither does create\(\) with MCPL servers configured, nor a first connectMcplServer\(\), since the next feature set registered would store over them\. They're unchanged, and MCPL starts once they can be read\.$/); + } finally { restore(); } + const before = readState(storePath, 'mcpl/checkpoints'); + for (const [name, value, are] of [ + ['an object without trees', { tree: {} }, 'an object whose trees is missing'], + ['an array', [{ trees: {} }], 'an array'], + ['trees that are a string', { trees: 'srv:doc' }, 'an object whose trees is a string'], + ] as const) { + const store = JsStore.openOrCreate({ path: storePath }); + store.setStateJson('mcpl/checkpoints', value); + store.close(); + await rejectsBuild(() => f.create(), new RegExp( + `^the MCPL checkpoint trees \\(mcpl/checkpoints\\) are ${are}, not checkpoint trees\\. MCPL doesn't start without them, .*` + + `They're unchanged, and starting again won't help until they're repaired\\.$`, + ), name); + assert.deepEqual(readState(storePath, 'mcpl/checkpoints'), value, `${name}: left as it was`); + } + const reset = JsStore.openOrCreate({ path: storePath }); + reset.setStateJson('mcpl/checkpoints', before); + reset.close(); + await f.create(); +}); + +test('a host-managed feature set restored from the store reads its state, so its next patch applies to it', inStoreDir('boot-read-host-state-', async (storePath) => { + const noop = () => {}; + const first = JsStore.openOrCreate({ path: storePath }); + try { + const checkpoints = new CheckpointManager(first, noop); + checkpoints.registerFeatureSet('srv', 'doc', { hostState: true, rollback: false }); + checkpoints.recordCheckpoint('srv', 'doc', { checkpoint: 'c1', data: { count: 1 } } as never); + } finally { first.close(); } + // A restart: the store opened again, its trees restored, the feature set registered again. + const store = JsStore.openOrCreate({ path: storePath }); + try { + const checkpoints = new CheckpointManager(store, noop); + checkpoints.registerFeatureSet('srv', 'doc', { hostState: true, rollback: false }); + assert.deepEqual(checkpoints.getCurrentState('srv', 'doc'), { count: 1 }); + checkpoints.recordCheckpoint('srv', 'doc', { checkpoint: 'c2', parent: 'c1', patch: [{ op: 'add', path: '/other', value: 2 }] } as never); + assert.deepEqual(store.getStateJson('mcpl/state/srv/doc'), { count: 1, other: 2 }); + } finally { store.close(); } +})); + +test("a host-managed feature set's state that can't be read fails its registration, naming it, and the next registration reads it", inStoreDir('boot-read-host-state-read-', async (storePath) => { + const noop = () => {}; + const first = JsStore.openOrCreate({ path: storePath }); + try { + const checkpoints = new CheckpointManager(first, noop); + checkpoints.registerFeatureSet('srv', 'doc', { hostState: true, rollback: false }); + checkpoints.recordCheckpoint('srv', 'doc', { checkpoint: 'c1', data: { count: 1 } } as never); + // A second feature set's state, stored with no tree: its first registration reads it. + first.registerState({ id: 'mcpl/state/srv/notes', strategy: 'snapshot' }); + first.setStateJson('mcpl/state/srv/notes', { count: 3 }); + } finally { first.close(); } + const store = JsStore.openOrCreate({ path: storePath }); + try { + const checkpoints = new CheckpointManager(store, noop); + // `doc` is a tree restored from the store; `notes` is registered for the first time. + for (const [featureSet, state] of [['doc', { count: 1 }], ['notes', { count: 3 }]] as const) { + const stateId = `mcpl/state/srv/${featureSet}`; + const restore = failStoreRead(stateId, 'loadHostState'); + try { + assert.throws(() => checkpoints.registerFeatureSet('srv', featureSet, { hostState: true, rollback: false }), { + message: new RegExp( + `^the host-managed state of feature set "${featureSet}" on MCPL server "srv" \\(mcpl/state/srv/${featureSet}\\) could not ` + + `be read: injected EIO reading mcpl/state/srv/${featureSet}\\. The server's connect fails without it, since the feature ` + + `set's next checkpoint would store over it\\. It's unchanged, and the connect succeeds once it can be read\\.$`, + ), + }); + } finally { restore(); } + checkpoints.registerFeatureSet('srv', featureSet, { hostState: true, rollback: false }); + assert.deepEqual(checkpoints.getCurrentState('srv', featureSet), state, `${featureSet}: the next registration reads it`); + } + // The registration that failed held no tree, so the next one registered it whole, stored tree included. + const trees = (store.getStateJson('mcpl/checkpoints') as { trees: Record }).trees; + assert.ok(trees['srv:notes'], 'the first-time feature set is in the stored trees'); + } finally { store.close(); } +})); + test("at a runtime connect, coalescing receipts that can't be read fail the connect, and the next connect restores them", async (t) => { const f = await fixture(); t.after(f.close); const params = f.params('1', 'accepted_before_crash', { deferred: true, data: { k: 'v' } }); @@ -378,18 +604,91 @@ test("at a runtime connect, coalescing receipts that can't be read fail the conn } as unknown as FrameworkConfig); try { const server = { id: 'editor', url: `ws://127.0.0.1:${f.port}`, enabledFeatureSets: ['doc'] } as McplServerConfig; - const restore = failStoreRead('mcpl/coalescing-recent', 'initializePushCoalescer'); - try { - await assert.rejects(framework.connectMcplServer(server), { - message: /^the coalescing receipts since the last snapshot \(mcpl\/coalescing-recent\) could not be read: injected EIO reading mcpl\/coalescing-recent\. MCPL doesn't start without them/, - }); - } finally { restore(); } + const store = framework.getStore() as unknown as JsStore; + const stored = () => ['mcpl/coalescing', 'mcpl/coalescing-recent', 'mcpl/checkpoints'].map((id) => store.getStateJson(id)); + const before = stored(); + const internals = framework as unknown as { mcplServerRegistry: unknown; pushCoalescer: unknown; checkpointManager: unknown }; + for (const [stateId, reader, says] of [ + ['mcpl/coalescing-recent', 'restorePushCoalescer', 'the coalescing receipts since the last snapshot'], + ['mcpl/coalescing', 'restorePushCoalescer', 'the coalescing receipts and batches'], + ['mcpl/checkpoints', 'loadFromStore', 'the MCPL checkpoint trees'], + ] as const) { + const restore = failStoreRead(stateId, reader); + try { + await assert.rejects(framework.connectMcplServer(server), { + message: new RegExp(`^${says} \\(${stateId}\\) could not be read: injected EIO reading ${stateId}\\. MCPL doesn't start without them`), + }); + } finally { restore(); } + assert.equal(internals.mcplServerRegistry, null, `${stateId}: nothing of the MCPL subsystem is built`); + assert.equal(internals.pushCoalescer, null, `${stateId}: no coalescer is installed`); + assert.equal(internals.checkpointManager, null, `${stateId}: no checkpoint manager is installed`); + assert.deepEqual(stored(), before, `${stateId}: the stored state is as it was`); + } await framework.connectMcplServer(server); const retry = await f.send('push/event', params); - assert.deepEqual(retry.result, original.result, 'the receipt survived the refused connect'); + assert.deepEqual(retry.result, original.result, 'the receipt survived the refused connects'); } finally { await framework.stop(); } }); +test('the forms a release wrote boot as before', inStoreDir('boot-read-release-forms-', async (storePath) => { + // As every release from 0.16.0 writes them, with the head's readers: a + // resumed host's record, and the queue's cleared and pending forms. + const recovery = join(storePath, 'recovery'); + const first = await AgentFramework.create(config(storePath)); + await first.stop(); + mkdirSync(recovery, { recursive: true }); + const write = (name: string, doc: unknown) => writeFileSync(join(recovery, name), `${JSON.stringify(doc, null, 2)}\n`); + const boot = async (check: (f: AgentFramework) => void) => { + const f = await AgentFramework.create(config(storePath)); + try { check(f); } finally { await f.stop(); } + }; + write('host-mode.json', { version: 1, quiesced: false, clearedAt: 1 }); + write('deferred-writes.json', { version: 1, pending: [] }); + await boot((f) => { + assert.equal(f.getHostModeStatus().quiesced, false); + assert.equal(f.getHostModeStatus().deferredWrites, 0); + }); + write('host-mode.json', { version: 1, quiesced: true, reason: 'surgery', since: 1 }); + write('deferred-writes.json', { + version: 2, pending: [{ participant: 'user', content: [{ type: 'text', text: 'queued' }], id: 'w1', seq: 1 }], scanFrom: { agent: 0 }, + }); + await boot((f) => { + assert.equal(f.getHostModeStatus().quiesced, true); + assert.equal(f.getHostModeStatus().reason, 'surgery'); + assert.equal(f.getHostModeStatus().deferredWrites, 1); + }); +})); + +test('a store from before the recovery files boots from its slots, as before', inStoreDir('boot-read-pre-file-', async (storePath) => { + // A store from before host-mode.json and deferred-writes.json existed keeps + // both in the store: framework/state's hostMode, and a bare list of writes. + const first = await AgentFramework.create(config(storePath)); + await first.stop(); + const store = JsStore.openOrCreate({ path: storePath }); + try { + store.setStateJson('framework/state', { hostMode: { quiesced: true, reason: 'surgery', since: 1 } }); + store.setStateJson('framework/deferred-writes', [{ participant: 'user', content: [{ type: 'text', text: 'queued' }] }]); + } finally { store.close(); } + const f = await AgentFramework.create(config(storePath)); + try { + assert.equal(f.getHostModeStatus().quiesced, true); + assert.equal(f.getHostModeStatus().deferredWrites, 1); + } finally { await f.stop(); } +})); + +test('an agent named like an object property has no settings record or guard unless one is stored for it', inStoreDir('boot-read-settings-own-', async (storePath) => { + seedFrameworkState(storePath, { + agentRuntimeSettings: { other: { sameRoundThinkTextPolicy: 'private' } }, + toolResultGuards: { other: true }, + }); + const named = { ...config(storePath), agents: [{ name: 'constructor', model: 'test-model', systemPrompt: 'test' }] } as FrameworkConfig; + const f = await AgentFramework.create(named); + try { + assert.ok(f.getAgent('constructor')); + assert.equal(guardOf(f, 'constructor'), undefined, 'no inherited guard'); + } finally { await f.stop(); } +})); + test('nothing stored reads as empty: a first boot starts serving, with no usage, deferred writes or coalescing history', async (t) => { const f = await fixture(); t.after(f.close); assert.equal(f.framework.getHostModeStatus().quiesced, false); diff --git a/test/helpers/failed-create.ts b/test/helpers/failed-create.ts index f16125f9f..a386bec4a 100644 --- a/test/helpers/failed-create.ts +++ b/test/helpers/failed-create.ts @@ -1,7 +1,7 @@ /** * A build of the framework that must fail: `AgentFramework.create()`, or a * fixture's restart through it. Until a failed create() releases what it - * acquired (agent-framework #389), the framework it built keeps its store open, + * acquired (agent-framework #389, which #426 fixes), the framework it built keeps its store open, * so a test that boots again on that store first stops it here, as that * release would. stop() is idempotent, so this stop is harmless once it does. */ diff --git a/test/usage-tracker.test.ts b/test/usage-tracker.test.ts index a5a8db880..a18395942 100644 --- a/test/usage-tracker.test.ts +++ b/test/usage-tracker.test.ts @@ -154,4 +154,15 @@ describe('UsageTracker', () => { assert.ok(snap.totals.estimatedCost); assert.ok(Math.abs(snap.totals.estimatedCost.total - 0.03) < 1e-10); }); + + it("a usage without a count adds nothing for it, so the totals stay numbers and are stored as numbers", () => { + // membrane passes some providers' counts through as reported, so one can be missing. + const { tracker } = createTracker(); + tracker.onInferenceCompleted('agent', { inputTokens: 100, outputTokens: 50 }); + tracker.onInferenceCompleted('agent', { inputTokens: undefined as unknown as number, outputTokens: 25 }); + const stored = JSON.parse(JSON.stringify(tracker.toJSON())); + assert.equal(stored.totals.inputTokens, 100); + assert.equal(stored.totals.outputTokens, 75); + assert.equal(stored.byAgent[0].usage.inputTokens, 100); + }); }); From 3c84bc9b48a3cd375848e8dfe3c7f0e5de0af31b Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 12:08:28 -0400 Subject: [PATCH 03/11] fix(boot): read restored host-managed MCPL state before any server connects, and keep NaN out of the usage cost The fix round's review found that a restored host-managed tree's state was still read only when its feature set registered at a connect, after the server was added. A connect that returned before registering, a feature set now declared with neither flag, or a read that failed with the connection left up all left a tree unread while checkpoints could still be recorded over its state; and at boot the read's failure was logged by initializeMcpl's fail-open per-server loop, so create() went on without the server. CheckpointManager.loadHostStates(serverId) reads the stored state of every host-managed tree held for a server, whatever its feature sets declare now, finding the server's trees by their key prefix as removeServer does. initializeMcpl calls it for each configured server right after the checkpoint manager loads the trees, before anything else is built, so a failure fails create() before any server connects; a runtime connectMcplServer() calls it before the server is added, so a failure leaves nothing connected. registerFeatureSet returns for a held tree, as on main, since loadHostStates has read it before any connect registers it; the round's read there was a second site that no path needed, and missed a tree registered by no declaration. It still reads a first-time tree's state. The host-state refusal now says the server doesn't connect, and neither does create() with it configured, until the state can be read; loadFromStore's comment says where a restored tree's state is read. Usage. accumulateUsage adds 0 for a cost total that isn't a finite number, as it does for a count: membrane computes the cost from the counts, so a missing count made the cost NaN. parseUsage reads a cost total stored as null as 0, as it does a count. Texts. The recovery files' refusals no longer say that moved aside the framework reads the store's slot: with a recovery path the slot doesn't hold the file's record, so following that would boot a quiesced host serving or drop the queue. The tool-result guards' refusal says the agent isn't created, since without them it would take its recipe's guard. The usage tracker's comment names the reachable missing count, Bedrock's streaming message_delta in membrane 0.5.87; the framework streams every inference, so a whole response's usage never reaches it. The changelog fragment says when host-managed state is read, and covers the cost. Tests: at boot, a configured server's restored host-managed state that can't be read fails create(), and the next boot reads it, with the fixture's feature set declared with neither flag; at a runtime connect, it fails the connect before the server is added, and the next connect reads it. The CheckpointManager tests read a restored tree's state through loadHostStates, and its failed read fails loadHostStates. A deferred-writes file behind ENOTDIR fails create(). A null hostMode in framework/state refuses. A stored null cost total restores as 0, and a NaN cost or NaN cache counts add nothing. The runtime connect test is renamed for what it covers. Texts in the existing tests follow the refusals. Co-Authored-By: Claude Opus 5.5 --- changelog.d/boot-state-read-errors.fixed.md | 6 +- src/framework.ts | 21 +-- src/mcpl/checkpoint-manager.ts | 31 +++-- src/usage/usage-tracker.ts | 19 ++- test/boot-state-read-errors.test.ts | 144 +++++++++++++++++--- test/usage-tracker.test.ts | 17 ++- 6 files changed, 183 insertions(+), 55 deletions(-) diff --git a/changelog.d/boot-state-read-errors.fixed.md b/changelog.d/boot-state-read-errors.fixed.md index 0e0f2719e..086ac90f7 100644 --- a/changelog.d/boot-state-read-errors.fixed.md +++ b/changelog.d/boot-state-read-errors.fixed.md @@ -1,13 +1,13 @@ - Restored state is no longer taken as empty at boot. `AgentFramework.create()` now fails when a state it restores can't be read, or reads as something this version can't take for that state. The error names the state, or its recovery file, and says what to do; nothing it names is changed. Before, each was taken as empty, the framework served without it, and the next write replaced what was stored. Nothing stored still starts empty, and a store any release wrote boots as before unless one of these reads fails. - **Host mode** (`recovery/host-mode.json`, or `framework/state` without one): a quiesced host booted serving, and landed the writes it had deferred during the maintenance. - **Deferred writes** (`recovery/deferred-writes.json`, or `framework/deferred-writes` without one): the queue restored nothing, and the next deferral replaced it. An agent's context that can't be checked for writes that landed before their ack now fails `create()` too, where such a write landed a second time. - - **A recovery file that can't be reached** (an error other than its absence, such as `EACCES` on its directory) fails `create()` too, where it read as absent. Moved aside, either file is read from the store's slot instead, as on a store from before the recovery files. + - **A recovery file that can't be reached** (an error other than its absence, such as `EACCES` on its directory) fails `create()` too, where it read as absent. - **Usage totals** (`framework/state`): they started at zero, and the next inference stored zeros over them. A `framework/state` that isn't an object now fails `create()`, where every reader took it as empty. - **An agent's persisted runtime settings** (`framework/state`): the agent booted at its recipe's settings, a `private` think-text policy as `public`, and its next settings change stored over the record. 0.22.2's notes said such a read is logged and the agent boots at the recipe's; it now fails `create()`. - **Tool-result guards** (`framework/state`): a `toolResultGuards` that isn't an object read as no setting, and the next guard change stored over it. It's now read by each agent's own name, so an agent named `constructor` no longer fails `create()` once any guard is stored. - **MCPL coalescing** (`mcpl/coalescing`, `mcpl/coalescing-recent`): an acknowledged deferred acceptance or a pending batch was never delivered, and the next receipt or flush replaced the stored ones. A snapshot of a version this one doesn't read was ignored and replaced; it now fails `create()` and is kept for the version that wrote it. An agent's context that can't be checked for an occurrence already delivered fails `create()` too, where the occurrence was delivered again. - **MCPL checkpoint trees** (`mcpl/checkpoints`): a stored value without `trees` read as no checkpoints, and the next feature set registered stored over it. - On a framework started with no MCPL servers, the coalescing state and the checkpoint trees are read at the first `connectMcplServer()`, which fails instead, with nothing of MCPL built, and the next connect reads them again. -- A host-managed MCPL feature set's state (`mcpl/state//`) is now read when its server connects after a restart. It wasn't, so the state read as empty, and the feature set's next delta checkpoint applied to nothing and stored over it. A failed read of it fails the server's connect, naming it. -- A usage total stored as `null`, which a provider's usage without a count left as `NaN`, restores as 0, and a usage without a count no longer makes the totals `NaN`. +- A host-managed MCPL feature set's state (`mcpl/state//`) is now read after a restart: when MCPL starts, for each configured server, and before `connectMcplServer()` adds a server. It wasn't, so the state read as empty, and the feature set's next delta checkpoint applied to nothing and stored over it. A failed read of it fails `create()`, or that `connectMcplServer()`, naming it, before the server connects. +- A usage count or cost total stored as `null`, which a provider's usage without a count left as `NaN`, restores as 0, and a usage without a count no longer makes the totals or the cost `NaN`. - A failed read of the persisted `tool_result_inline_max_chars` caps at boot is now logged. As before, they don't apply until a restart, and setting one keeps the others stored. diff --git a/src/framework.ts b/src/framework.ts index fcaeba21a..d3af98d9f 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -4240,7 +4240,7 @@ export class AgentFramework { private adoptToolResultGuard(agent: Agent): void { const refuse = (problem: string, cause?: unknown): Error => new Error( `the tool-result guards (${FRAMEWORK_STATE_ID}, toolResultGuards) ${problem}. Agent "${agent.name}" isn't ` + - `created without them, since it would take its recipe's guard` + + `created, since without them it would take its recipe's guard` + (cause === undefined ? ` and the next guard change would store over them. They're unchanged, and creating it again won't help until they're repaired.` : `. They're unchanged, and it's created once they can be read.`), @@ -5203,8 +5203,7 @@ export class AgentFramework { `the deferred context writes (${file ?? DEFERRED_WRITES_ID}) ${problem}. The framework doesn't start without them, ` + `since the next deferral would replace the stored queue. ` + (file - ? `The file is unchanged, and the framework starts once ${cause === undefined ? 'it holds a queue of writes' : 'it can be read'}; ` + - `moved aside, the framework reads the queue from ${DEFERRED_WRITES_ID} instead.` + ? `The file is unchanged, and the framework starts once ${cause === undefined ? 'it holds a queue of writes' : 'it can be read'}.` : cause === undefined ? `They're unchanged, and starting again won't help until they're repaired.` : `They're unchanged, and the framework starts once they can be read.`), @@ -6040,8 +6039,7 @@ export class AgentFramework { `the host mode (${file ?? `${FRAMEWORK_STATE_ID}, hostMode`}) ${problem}. The framework doesn't start without it, ` + `since a host quiesced for maintenance would boot serving. ` + (file - ? `The file is unchanged, and the framework starts once ${cause === undefined ? 'it holds a host-mode record' : 'it can be read'}; ` + - `moved aside, the framework reads host mode from ${FRAMEWORK_STATE_ID} instead.` + ? `The file is unchanged, and the framework starts once ${cause === undefined ? 'it holds a host-mode record' : 'it can be read'}.` : cause === undefined ? `It's unchanged, and starting again won't help until it's repaired.` : `It's unchanged, and the framework starts once it can be read.`), @@ -16510,15 +16508,17 @@ export class AgentFramework { serverConfigs: McplServerConfig[], inferenceRouting?: import('./mcpl/types.js').InferenceRoutingPolicy, ): Promise { - // First, before anything is built: a coalescing state or checkpoint trees - // that can't be taken refuse here and leave the subsystem as it was, so a - // refused runtime connect leaves nothing for the guards that test - // mcplServerRegistry. + // First, before anything is built: a coalescing state, checkpoint trees + // or a configured server's restored host-managed state that can't be + // taken refuses here and leaves the subsystem as it was, so a refused + // runtime connect leaves nothing for the guards that test + // mcplServerRegistry, and a refused boot connects no server. const coalescing = this.restorePushCoalescer(); const checkpoints = new CheckpointManager( this.store, (event) => this.emitTrace(event as { type: TraceEvent['type']; [key: string]: unknown }), ); + for (const config of serverConfigs) checkpoints.loadHostStates(config.id); this.mcplServerRegistry = new McplServerRegistry(); this.featureSetManager = new FeatureSetManager( (event) => this.emitTrace(event as { type: TraceEvent['type']; [key: string]: unknown }), @@ -17681,6 +17681,9 @@ export class AgentFramework { ); } + // The server's restored host-managed state is read before the server is + // added, so a read that fails leaves nothing connected. + this.checkpointManager?.loadHostStates(config.id); const oldToolNames = new Set(this.mcplTools.map(t => t.name)); await this.connectMcplServerInternal(config); await this.refreshMcplTools(); diff --git a/src/mcpl/checkpoint-manager.ts b/src/mcpl/checkpoint-manager.ts index 664dcd78f..88d9c8eab 100644 --- a/src/mcpl/checkpoint-manager.ts +++ b/src/mcpl/checkpoint-manager.ts @@ -242,14 +242,9 @@ export class CheckpointManager { opts: { hostState: boolean; rollback: boolean }, ): void { const key = this.key(serverId, featureSet); - const held = this.trees.get(key); - if (held) { - // Already registered, or restored from the store by loadFromStore, - // which leaves a host-managed tree's state to be read here: unread, - // the next patch would apply to nothing and store over it. - if (held.hostState && held.currentState === undefined) this.loadHostState(serverId, featureSet, held); - return; - } + // Already registered, or restored from the store by loadFromStore, whose + // host-managed state loadHostStates reads before its server connects. + if (this.trees.has(key)) return; const tree: FeatureSetState = { hostState: opts.hostState, @@ -580,14 +575,28 @@ export class CheckpointManager { } catch (error) { throw new Error( `the host-managed state of feature set "${featureSet}" on MCPL server "${serverId}" (${stateId}) could not ` + - `be read: ${thrownText(error)}. The server's connect fails without it, since the feature set's next ` + - `checkpoint would store over it. It's unchanged, and the connect succeeds once it can be read.`, + `be read: ${thrownText(error)}. The server doesn't connect without it, and neither does create() with it ` + + `configured, since the feature set's next checkpoint would store over it. It's unchanged, and the server ` + + `connects once it can be read.`, { cause: error }, ); } if (existing !== null && existing !== undefined) tree.currentState = existing; } + /** Read the stored state of every host-managed tree held for `serverId`, + * whatever its feature sets declare now. The framework calls it at MCPL's + * start for each configured server and before a runtime connect adds its + * server, so no server connects with a restored tree whose state its next + * checkpoint would store over unread. A server's trees are the keys + * `${serverId}:`, as removeServer finds them. */ + loadHostStates(serverId: string): void { + const prefix = `${serverId}:`; + for (const [key, tree] of this.trees) { + if (tree.hostState && key.startsWith(prefix)) this.loadHostState(serverId, key.slice(prefix.length), tree); + } + } + /** Load checkpoint trees from Chronicle on startup. Nothing stored is no * trees. Trees that can't be read, or a stored value that isn't an object * whose trees is an object, throw: taken as no trees, the next feature set @@ -629,7 +638,7 @@ export class CheckpointManager { hostState: entry.hostState, rollback: entry.rollback, currentCheckpoint: entry.current, - currentState: undefined, // Will be loaded per-feature-set during registerFeatureSet + currentState: undefined, // a host-managed tree's state is read by loadHostStates, before its server connects nodes, }); } diff --git a/src/usage/usage-tracker.ts b/src/usage/usage-tracker.ts index fbedb6080..668a33244 100644 --- a/src/usage/usage-tracker.ts +++ b/src/usage/usage-tracker.ts @@ -12,8 +12,10 @@ function accumulateUsage( tokens: { inputTokens: number; outputTokens: number; cacheCreationTokens: number; cacheReadTokens: number }, estimatedCost?: { total: number; currency: string } ): void { - // A provider's usage can lack a count (membrane passes some through as - // reported); adding it would make the total NaN, which is stored as null. + // A provider's usage can lack a count (membrane 0.5.87's Bedrock stream + // takes a message_delta's output count as reported), and the cost membrane + // computes from it is then NaN; adding either would make the total NaN, + // which is stored as null. const count = (n: number): number => (Number.isFinite(n) ? n : 0); target.inputTokens += count(tokens.inputTokens); target.outputTokens += count(tokens.outputTokens); @@ -27,16 +29,16 @@ function accumulateUsage( target.estimatedCost = undefined; return; } - target.estimatedCost.total += estimatedCost.total; + target.estimatedCost.total += count(estimatedCost.total); } } function parseUsage(raw: unknown): SessionUsage | null { if (!raw || typeof raw !== 'object') return null; const o = raw as Record; - // A count stored as null is a NaN total, as JSON stores one: an earlier - // release could write it when a provider's usage lacked a count. It reads - // as 0, the count that survived it. + // A count or a cost total stored as null is a NaN total, as JSON stores + // one: an earlier release could write it when a provider's usage lacked a + // count. It reads as 0, the count that survived it. const count = (v: unknown): number | undefined => (v === null ? 0 : typeof v === 'number' ? v : undefined); const inputTokens = count(o.inputTokens); const outputTokens = count(o.outputTokens); @@ -47,7 +49,10 @@ function parseUsage(raw: unknown): SessionUsage | null { cacheCreationTokens: typeof o.cacheCreationTokens === 'number' ? o.cacheCreationTokens : 0, cacheReadTokens: typeof o.cacheReadTokens === 'number' ? o.cacheReadTokens : 0, estimatedCost: o.estimatedCost && typeof o.estimatedCost === 'object' - ? { total: (o.estimatedCost as any).total, currency: (o.estimatedCost as any).currency } + ? { + total: (o.estimatedCost as any).total === null ? 0 : (o.estimatedCost as any).total, + currency: (o.estimatedCost as any).currency, + } : undefined, }; } diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts index 85cf4d46e..6626a4b5c 100644 --- a/test/boot-state-read-errors.test.ts +++ b/test/boot-state-read-errors.test.ts @@ -121,7 +121,7 @@ test("a host-mode file that can't be read fails create(), and the next boot is q await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( `^the host mode \\(${file.replace(/[.*+?^${}()|[\]\\/]/g, '\\$&')}\\) could not be read: injected EIO reading host-mode\\.json\\. ` + `The framework doesn't start without it, since a host quiesced for maintenance would boot serving\\. ` + - `The file is unchanged, and the framework starts once it can be read; moved aside, the framework reads host mode from framework/state instead\\.$`, + `The file is unchanged, and the framework starts once it can be read\\.$`, )); } finally { restore(); } assert.equal(readFileSync(file, 'utf8'), before, 'the file is as it was'); @@ -149,7 +149,7 @@ test('a host-mode file that holds no record, text that isn\'t JSON, or a record await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( `^the host mode \\(.*host-mode\\.json\\) ${holds}\\. The framework doesn't start without it, ` + `since a host quiesced for maintenance would boot serving\\. The file is unchanged, and the framework starts once it holds a ` + - `host-mode record; moved aside, the framework reads host mode from framework/state instead\\.$`, + `host-mode record\\.$`, ), content); assert.equal(readFileSync(file, 'utf8'), content, `${content}: the file is as it was`); } @@ -165,7 +165,7 @@ test("a recovery file that can't be reached, not only one that won't read, fails renameSync(dir, `${dir}.aside`); writeFileSync(dir, 'not a directory'); try { - await rejectsBuild(() => AgentFramework.create(withPath()), /^the host mode \(.*host-mode-dir\/host-mode\.json\) could not be read: ENOTDIR\b.*The file is unchanged, and the framework starts once it can be read;/); + await rejectsBuild(() => AgentFramework.create(withPath()), /^the host mode \(.*host-mode-dir\/host-mode\.json\) could not be read: ENOTDIR\b.*The file is unchanged, and the framework starts once it can be read\.$/); } finally { rmSync(dir); renameSync(`${dir}.aside`, dir); @@ -174,6 +174,27 @@ test("a recovery file that can't be reached, not only one that won't read, fails try { assert.equal(f.getHostModeStatus().quiesced, true); } finally { await f.stop(); } })); +test("a deferred-writes file that can't be reached fails create(), and doesn't read as absent", inStoreDir('boot-read-deferred-enotdir-', async (storePath) => { + // As for the host-mode file: ENOTDIR from a file where the queue file's directory should be. + const dir = join(storePath, '..', 'deferred-dir'); + const withPath = (): FrameworkConfig => ({ ...config(storePath), deferredWritesPath: join(dir, 'deferred-writes.json') }) as FrameworkConfig; + const first = await AgentFramework.create(withPath()); + try { + await first.quiesce({ reason: 'surgery' }); + (first as unknown as { addMessage(p: string, c: unknown[]): unknown }).addMessage('user', [{ type: 'text', text: 'said during surgery' }]); + } finally { await first.stop(); } + renameSync(dir, `${dir}.aside`); + writeFileSync(dir, 'not a directory'); + try { + await rejectsBuild(() => AgentFramework.create(withPath()), /^the deferred context writes \(.*deferred-dir\/deferred-writes\.json\) could not be read: ENOTDIR\b.*The file is unchanged, and the framework starts once it can be read\.$/); + } finally { + rmSync(dir); + renameSync(`${dir}.aside`, dir); + } + const f = await AgentFramework.create(withPath()); + try { assert.equal(f.getHostModeStatus().deferredWrites, 1, 'the next reachable boot restores the queue'); } finally { await f.stop(); } +})); + test("with no recovery file, a host mode that can't be read from framework/state fails create()", async () => { const dir = mkdtempSync(join(tmpdir(), 'boot-read-hostmode-slot-')); const store = JsStore.openOrCreate({ path: join(dir, 'store') }); @@ -189,8 +210,13 @@ test("with no recovery file, a host mode that can't be read from framework/state } finally { restore(); } const f = await AgentFramework.create(slotConfig()); try { assert.equal(f.getHostModeStatus().quiesced, true); } finally { await f.stop(); } - store.setStateJson('framework/state', { ...(store.getStateJson('framework/state') as object), hostMode: 'quiesced' }); - await rejectsBuild(() => AgentFramework.create(slotConfig()), /^the host mode \(framework\/state, hostMode\) holds a string, not a host-mode record\. The framework doesn't start without it, since a host quiesced for maintenance would boot serving\. It's unchanged, and starting again won't help until it's repaired\.$/); + for (const [hostMode, holds] of [['quiesced', 'holds a string'], [null, 'holds null']] as const) { + store.setStateJson('framework/state', { ...(store.getStateJson('framework/state') as object), hostMode }); + await rejectsBuild(() => AgentFramework.create(slotConfig()), new RegExp( + `^the host mode \\(framework/state, hostMode\\) ${holds}, not a host-mode record\\. The framework doesn't start without it, ` + + `since a host quiesced for maintenance would boot serving\\. It's unchanged, and starting again won't help until it's repaired\\.$`, + ), String(hostMode)); + } } finally { store.close(); rmSync(dir, { recursive: true, force: true }); @@ -207,7 +233,7 @@ test("a deferred-write queue that can't be read fails create(), and the next boo const before = readFileSync(file, 'utf8'); const restore = failRecoveryRead('deferred-writes.json'); try { - await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the deferred context writes \(.*deferred-writes\.json\) could not be read: injected EIO reading deferred-writes\.json\. The framework doesn't start without them, since the next deferral would replace the stored queue\. The file is unchanged, and the framework starts once it can be read; moved aside, the framework reads the queue from framework\/deferred-writes instead\.$/); + await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the deferred context writes \(.*deferred-writes\.json\) could not be read: injected EIO reading deferred-writes\.json\. The framework doesn't start without them, since the next deferral would replace the stored queue\. The file is unchanged, and the framework starts once it can be read\.$/); } finally { restore(); } assert.equal(readFileSync(file, 'utf8'), before, 'the queue is as it was'); const f = await AgentFramework.create(config(storePath)); @@ -232,8 +258,7 @@ test("a deferred-write queue that isn't a queue, or isn't JSON, fails create(), writeFileSync(file, content); await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( `^the deferred context writes \\(.*deferred-writes\\.json\\) ${holds}\\. The framework doesn't start without them, since the next ` + - `deferral would replace the stored queue\\. The file is unchanged, and the framework starts once it holds a queue of writes; ` + - `moved aside, the framework reads the queue from framework/deferred-writes instead\\.$`, + `deferral would replace the stored queue\\. The file is unchanged, and the framework starts once it holds a queue of writes\\.$`, ), content); assert.equal(readFileSync(file, 'utf8'), content, `${content}: the file is as it was`); } @@ -332,9 +357,9 @@ test("usage totals that aren't a usage record fail create(), naming what they ar })); test('usage totals an earlier release wrote with a null count, or with a byAgent entry that is not an agent, restore', inStoreDir('boot-read-usage-null-', async (storePath) => { - // A NaN total, which a provider's usage without a count made, is stored as null. + // A NaN total, which a provider's usage without a count made, is stored as null, and so is a NaN cost. seedFrameworkState(storePath, { usage: { - totals: { inputTokens: null, outputTokens: 500 }, + totals: { inputTokens: null, outputTokens: 500, estimatedCost: { total: null, currency: 'USD' } }, inferenceCount: 7, byAgent: [null, { agentName: 'agent', usage: { inputTokens: 1000, outputTokens: 500 }, inferenceCount: 7 }], } }); @@ -343,6 +368,7 @@ test('usage totals an earlier release wrote with a null count, or with a byAgent const usage = f.getSessionUsage(); assert.equal(usage.totals.inputTokens, 0); assert.equal(usage.totals.outputTokens, 500); + assert.deepEqual(usage.totals.estimatedCost, { total: 0, currency: 'USD' }); assert.deepEqual(usage.byAgent.map((a) => [a.agentName, a.usage.inputTokens]), [['agent', 1000]]); } finally { await f.stop(); } })); @@ -402,7 +428,7 @@ test("tool-result guards that can't be read, or that aren't an object, fail crea seedFrameworkState(storePath, { toolResultGuards: { agent: true } }); const restore = failStoreRead('framework/state', 'adoptToolResultGuard'); try { - await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the tool-result guards \(framework\/state, toolResultGuards\) could not be read: injected EIO reading framework\/state\. Agent "agent" isn't created without them, since it would take its recipe's guard\. They're unchanged, and it's created once they can be read\.$/); + await rejectsBuild(() => AgentFramework.create(config(storePath)), /^the tool-result guards \(framework\/state, toolResultGuards\) could not be read: injected EIO reading framework\/state\. Agent "agent" isn't created, since without them it would take its recipe's guard\. They're unchanged, and it's created once they can be read\.$/); } finally { restore(); } const f = await AgentFramework.create(config(storePath)); try { assert.equal(guardOf(f, 'agent'), true, 'the next boot restores the guard'); } finally { await f.stop(); } @@ -410,7 +436,7 @@ test("tool-result guards that can't be read, or that aren't an object, fail crea seedFrameworkState(storePath, { toolResultGuards: guards }); await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( `^the tool-result guards \\(framework/state, toolResultGuards\\) are ${are}, not an object of agents' guards\\. Agent "agent" isn't ` + - `created without them, since it would take its recipe's guard and the next guard change would store over them\\. They're ` + + `created, since without them it would take its recipe's guard and the next guard change would store over them\\. They're ` + `unchanged, and creating it again won't help until they're repaired\\.$`, ), JSON.stringify(guards)); assert.deepEqual((readState(storePath, 'framework/state') as { toolResultGuards: unknown }).toolResultGuards, guards); @@ -537,7 +563,73 @@ test("MCPL checkpoint trees that can't be read, or that aren't checkpoint trees, await f.create(); }); -test('a host-managed feature set restored from the store reads its state, so its next patch applies to it', inStoreDir('boot-read-host-state-', async (storePath) => { +/** A stored host-managed checkpoint tree for `server`'s `featureSet`, holding `state`, as the checkpoint manager stores them. */ +function seedHostManaged(storePath: string, server: string, featureSet: string, state: unknown): void { + const store = JsStore.openOrCreate({ path: storePath }); + try { + for (const id of ['mcpl/checkpoints', `mcpl/state/${server}/${featureSet}`]) { + try { store.registerState({ id, strategy: 'snapshot' }); } catch (error) { if (!isStateExistsError(error)) throw error; } + } + store.setStateJson('mcpl/checkpoints', { trees: { [`${server}:${featureSet}`]: { + hostState: true, rollback: false, current: 'c1', nodes: { c1: { parent: null, children: [], data: state } }, + } } }); + store.setStateJson(`mcpl/state/${server}/${featureSet}`, state); + } finally { store.close(); } +} + +const hostStateUnreadable = (server: string, featureSet: string): RegExp => new RegExp( + `^the host-managed state of feature set "${featureSet}" on MCPL server "${server}" \\(mcpl/state/${server}/${featureSet}\\) could not ` + + `be read: injected EIO reading mcpl/state/${server}/${featureSet}\\. The server doesn't connect without it, and neither does ` + + `create\\(\\) with it configured, since the feature set's next checkpoint would store over it\\. It's unchanged, and the ` + + `server connects once it can be read\\.$`, +); + +test("at boot, a configured server's restored host-managed state that can't be read fails create() before any server connects, and the next boot reads it", async (t) => { + // The fixture declares `doc` with neither hostState nor rollback, so it never + // registers with the checkpoint manager: its stored tree is read at MCPL's start. + const f = await fixture(); t.after(f.close); + await crash(f); + const storePath = join(f.dir, 'store'); + seedHostManaged(storePath, 'editor', 'doc', { count: 1 }); + const restore = failStoreRead('mcpl/state/editor/doc', 'loadHostState'); + try { + await rejectsBuild(() => f.create(), hostStateUnreadable('editor', 'doc')); + } finally { restore(); } + assert.deepEqual(readState(storePath, 'mcpl/state/editor/doc'), { count: 1 }, 'the state is as it was'); + await f.create(); + const checkpoints = (f.framework as unknown as { checkpointManager: CheckpointManager }).checkpointManager; + assert.deepEqual(checkpoints.getCurrentState('editor', 'doc'), { count: 1 }, 'the next boot reads it'); +}); + +test("at a runtime connect, a restored host-managed state that can't be read fails the connect before the server is added, and the next connect reads it", async (t) => { + const f = await fixture(); t.after(f.close); + await crash(f); + const storePath = join(f.dir, 'store'); + seedHostManaged(storePath, 'editor', 'doc', { count: 1 }); + // With no MCPL servers configured, the subsystem comes up at the first connect. + const framework = await AgentFramework.create({ + storePath, membrane: f.membrane.asMembrane(), + agents: [{ name: 'agent', model: 'test', systemPrompt: 'test' }], modules: [], + } as unknown as FrameworkConfig); + try { + const server = { id: 'editor', url: `ws://127.0.0.1:${f.port}`, enabledFeatureSets: ['doc'] } as McplServerConfig; + const internals = framework as unknown as { + mcplServerRegistry: { getServer(id: string): unknown } | null; + checkpointManager: CheckpointManager | null; + }; + const restore = failStoreRead('mcpl/state/editor/doc', 'loadHostState'); + try { + await assert.rejects(framework.connectMcplServer(server), { message: hostStateUnreadable('editor', 'doc') }); + } finally { restore(); } + // The first connect brought MCPL up, and the read failed before the server was added. + assert.equal(internals.mcplServerRegistry?.getServer('editor'), null, 'the server was not added'); + await framework.connectMcplServer(server); + assert.ok(internals.mcplServerRegistry?.getServer('editor'), 'the next connect adds it'); + assert.deepEqual(internals.checkpointManager?.getCurrentState('editor', 'doc'), { count: 1 }, 'and reads its state'); + } finally { await framework.stop(); } +}); + +test('a host-managed feature set restored from the store has its state read by loadHostStates, so its next patch applies to it', inStoreDir('boot-read-host-state-', async (storePath) => { const noop = () => {}; const first = JsStore.openOrCreate({ path: storePath }); try { @@ -545,10 +637,12 @@ test('a host-managed feature set restored from the store reads its state, so its checkpoints.registerFeatureSet('srv', 'doc', { hostState: true, rollback: false }); checkpoints.recordCheckpoint('srv', 'doc', { checkpoint: 'c1', data: { count: 1 } } as never); } finally { first.close(); } - // A restart: the store opened again, its trees restored, the feature set registered again. + // A restart: the store opened again, its trees restored, their states read before the server connects, and the feature set registered again. const store = JsStore.openOrCreate({ path: storePath }); try { const checkpoints = new CheckpointManager(store, noop); + assert.equal(checkpoints.getCurrentState('srv', 'doc'), undefined, 'restored, the tree holds no state until it is read'); + checkpoints.loadHostStates('srv'); checkpoints.registerFeatureSet('srv', 'doc', { hostState: true, rollback: false }); assert.deepEqual(checkpoints.getCurrentState('srv', 'doc'), { count: 1 }); checkpoints.recordCheckpoint('srv', 'doc', { checkpoint: 'c2', parent: 'c1', patch: [{ op: 'add', path: '/other', value: 2 }] } as never); @@ -556,7 +650,7 @@ test('a host-managed feature set restored from the store reads its state, so its } finally { store.close(); } })); -test("a host-managed feature set's state that can't be read fails its registration, naming it, and the next registration reads it", inStoreDir('boot-read-host-state-read-', async (storePath) => { +test("a host-managed feature set's state that can't be read fails loadHostStates for a restored tree, or the registration for a first-time one, naming it, and the next one reads it", inStoreDir('boot-read-host-state-read-', async (storePath) => { const noop = () => {}; const first = JsStore.openOrCreate({ path: storePath }); try { @@ -570,21 +664,27 @@ test("a host-managed feature set's state that can't be read fails its registrati const store = JsStore.openOrCreate({ path: storePath }); try { const checkpoints = new CheckpointManager(store, noop); - // `doc` is a tree restored from the store; `notes` is registered for the first time. + // `doc` is a tree restored from the store, read by loadHostStates; `notes` + // is registered for the first time, and read by its registration. + const reads = { + doc: () => checkpoints.loadHostStates('srv'), + notes: () => checkpoints.registerFeatureSet('srv', 'notes', { hostState: true, rollback: false }), + }; for (const [featureSet, state] of [['doc', { count: 1 }], ['notes', { count: 3 }]] as const) { const stateId = `mcpl/state/srv/${featureSet}`; const restore = failStoreRead(stateId, 'loadHostState'); try { - assert.throws(() => checkpoints.registerFeatureSet('srv', featureSet, { hostState: true, rollback: false }), { + assert.throws(reads[featureSet], { message: new RegExp( `^the host-managed state of feature set "${featureSet}" on MCPL server "srv" \\(mcpl/state/srv/${featureSet}\\) could not ` + - `be read: injected EIO reading mcpl/state/srv/${featureSet}\\. The server's connect fails without it, since the feature ` + - `set's next checkpoint would store over it\\. It's unchanged, and the connect succeeds once it can be read\\.$`, + `be read: injected EIO reading mcpl/state/srv/${featureSet}\\. The server doesn't connect without it, and neither does ` + + `create\\(\\) with it configured, since the feature set's next checkpoint would store over it\\. It's unchanged, and the ` + + `server connects once it can be read\\.$`, ), }); } finally { restore(); } - checkpoints.registerFeatureSet('srv', featureSet, { hostState: true, rollback: false }); - assert.deepEqual(checkpoints.getCurrentState('srv', featureSet), state, `${featureSet}: the next registration reads it`); + reads[featureSet](); + assert.deepEqual(checkpoints.getCurrentState('srv', featureSet), state, `${featureSet}: the next one reads it`); } // The registration that failed held no tree, so the next one registered it whole, stored tree included. const trees = (store.getStateJson('mcpl/checkpoints') as { trees: Record }).trees; @@ -592,7 +692,7 @@ test("a host-managed feature set's state that can't be read fails its registrati } finally { store.close(); } })); -test("at a runtime connect, coalescing receipts that can't be read fail the connect, and the next connect restores them", async (t) => { +test("at a runtime connect, coalescing state or checkpoint trees that can't be read fail the connect with nothing of MCPL built, and the next connect restores them", async (t) => { const f = await fixture(); t.after(f.close); const params = f.params('1', 'accepted_before_crash', { deferred: true, data: { k: 'v' } }); const original = await f.send('push/event', params); diff --git a/test/usage-tracker.test.ts b/test/usage-tracker.test.ts index a18395942..7dc8ba7dc 100644 --- a/test/usage-tracker.test.ts +++ b/test/usage-tracker.test.ts @@ -156,13 +156,24 @@ describe('UsageTracker', () => { }); it("a usage without a count adds nothing for it, so the totals stay numbers and are stored as numbers", () => { - // membrane passes some providers' counts through as reported, so one can be missing. + // membrane passes a Bedrock stream's output count through as reported, so + // one can be missing, and the cost membrane computes from it is then NaN. const { tracker } = createTracker(); - tracker.onInferenceCompleted('agent', { inputTokens: 100, outputTokens: 50 }); - tracker.onInferenceCompleted('agent', { inputTokens: undefined as unknown as number, outputTokens: 25 }); + const cost = (total: number) => ({ total, currency: 'USD' }); + tracker.onInferenceCompleted('agent', { inputTokens: 100, outputTokens: 50, cacheCreationTokens: 10, cacheReadTokens: 20 }, cost(0.5)); + tracker.onInferenceCompleted('agent', { + inputTokens: undefined as unknown as number, + outputTokens: 25, + cacheCreationTokens: NaN, + cacheReadTokens: NaN, + }, cost(NaN)); const stored = JSON.parse(JSON.stringify(tracker.toJSON())); assert.equal(stored.totals.inputTokens, 100); assert.equal(stored.totals.outputTokens, 75); + assert.equal(stored.totals.cacheCreationTokens, 10); + assert.equal(stored.totals.cacheReadTokens, 20); + assert.equal(stored.totals.estimatedCost.total, 0.5); assert.equal(stored.byAgent[0].usage.inputTokens, 100); + assert.equal(stored.byAgent[0].usage.estimatedCost.total, 0.5); }); }); From 2bf5b03d38b71974db4e3ef194a72288e0779b9e Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 13:17:48 -0400 Subject: [PATCH 04/11] fix(boot): say where each host-managed read refuses, and keep an unknown cost unknown Round 2's review found the text claimed more than the code does for a host-managed feature set with no stored tree. loadHostStates reads the trees restored from mcpl/checkpoints before a server connects, but a feature set with no tree, registering for the first time or after a disconnect's removeServer dropped its tree, is read when it registers, once its server has connected, as on main. A failed read there fails that server's connect; at boot initializeMcpl logs the connect's failure and goes on, as for any server's, and the connection stays in the registry, which is a registration failure's own handling, outside this change. Its state isn't taken as empty: registerFeatureSet holds the tree only once the read succeeds, so the next connect reads again. So loadHostState now takes where it's read from. A restored tree's refusal says neither create() with the server configured nor a connectMcplServer() of it goes on without the state, and that they go on once it can be read; a registration's says the feature set isn't registered, so the server's connect fails, and the next connect reads it again. The comments at initializeMcpl and runConnectMcplServer say the same, including that a runtime connect reads its server's trees once MCPL is up, so a refused one leaves MCPL up, as a first connect brought it up. loadFromStore's comment and the changelog follow. loadHostStates leaves a tree whose state is in memory alone: connecting a server again read its trees again, and could replace a connected server's live state with the stored one, or refuse with a read error where the duplicate connect's own refusal belonged. Usage cost. accumulateUsage added 0 for a NaN cost, so an inference whose cost membrane couldn't compute made the session's cost a known zero when it came first. A cost that isn't a finite number is now an unknown cost, left out as an absent cost already is. parseUsage keeps a stored cost only as a finite total with a string currency; anything else, such as the null a NaN total was stored as, or a string, which the next add concatenated, restores as no cost. The count comment no longer calls a null count's 0 the count that survived: the sum before it was lost. The tool-result guards' form refusal says the host's operator repairs them, since it also reaches agents created later, by callers that can't. Tests: the boot refusal for a restored tree asserts the fixture's server saw no initialize. The CheckpointManager tests take each read's own text. A new test pins which trees loadHostStates reads: only its own server's host-managed ones, not another server's or a rollback-only tree, and none whose state is in memory. The usage test drops the output count, as Bedrock's message_delta can, and checks a NaN cost leaves the session's cost unknown when it comes first; the restore test puts a null and a string cost in agents' entries. Co-Authored-By: Claude Opus 5.5 --- changelog.d/boot-state-read-errors.fixed.md | 4 +- src/framework.ts | 21 ++++--- src/mcpl/checkpoint-manager.ts | 39 ++++++++---- src/usage/usage-tracker.ts | 32 +++++----- test/boot-state-read-errors.test.ts | 70 +++++++++++++++++---- test/usage-tracker.test.ts | 22 ++++--- 6 files changed, 129 insertions(+), 59 deletions(-) diff --git a/changelog.d/boot-state-read-errors.fixed.md b/changelog.d/boot-state-read-errors.fixed.md index 086ac90f7..5ee4329e8 100644 --- a/changelog.d/boot-state-read-errors.fixed.md +++ b/changelog.d/boot-state-read-errors.fixed.md @@ -8,6 +8,6 @@ - **MCPL coalescing** (`mcpl/coalescing`, `mcpl/coalescing-recent`): an acknowledged deferred acceptance or a pending batch was never delivered, and the next receipt or flush replaced the stored ones. A snapshot of a version this one doesn't read was ignored and replaced; it now fails `create()` and is kept for the version that wrote it. An agent's context that can't be checked for an occurrence already delivered fails `create()` too, where the occurrence was delivered again. - **MCPL checkpoint trees** (`mcpl/checkpoints`): a stored value without `trees` read as no checkpoints, and the next feature set registered stored over it. - On a framework started with no MCPL servers, the coalescing state and the checkpoint trees are read at the first `connectMcplServer()`, which fails instead, with nothing of MCPL built, and the next connect reads them again. -- A host-managed MCPL feature set's state (`mcpl/state//`) is now read after a restart: when MCPL starts, for each configured server, and before `connectMcplServer()` adds a server. It wasn't, so the state read as empty, and the feature set's next delta checkpoint applied to nothing and stored over it. A failed read of it fails `create()`, or that `connectMcplServer()`, naming it, before the server connects. -- A usage count or cost total stored as `null`, which a provider's usage without a count left as `NaN`, restores as 0, and a usage without a count no longer makes the totals or the cost `NaN`. +- A host-managed MCPL feature set's state (`mcpl/state//`) is now read after a restart: when MCPL starts, for each configured server, and before `connectMcplServer()` adds a server. It wasn't, so the state read as empty, and the feature set's next delta checkpoint applied to nothing and stored over it. A failed read of it fails `create()`, or that `connectMcplServer()`, naming it, before the server connects. A feature set with no stored tree, such as one registering for the first time or after its server was disconnected, has its state read when it registers, as before. A failed read there fails that server's connect, as before, and `create()` logs that and goes on, as for any server's failed connect; the next connect now reads the state again, where it took it as empty. +- A usage count stored as `null`, which a provider's usage without a count left as `NaN`, restores as 0, and a usage without a count no longer makes the totals `NaN`. Its cost, which membrane computes as `NaN`, is now an unknown cost, left out of the total as a cost membrane couldn't estimate is, and a stored cost that isn't a number with its currency, such as `null`, restores as unknown. - A failed read of the persisted `tool_result_inline_max_chars` caps at boot is now logged. As before, they don't apply until a restart, and setting one keeps the others stored. diff --git a/src/framework.ts b/src/framework.ts index d3af98d9f..26b02c5c3 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -4242,7 +4242,7 @@ export class AgentFramework { `the tool-result guards (${FRAMEWORK_STATE_ID}, toolResultGuards) ${problem}. Agent "${agent.name}" isn't ` + `created, since without them it would take its recipe's guard` + (cause === undefined - ? ` and the next guard change would store over them. They're unchanged, and creating it again won't help until they're repaired.` + ? ` and the next guard change would store over them. They're unchanged, and creating it again won't help until the host's operator repairs them.` : `. They're unchanged, and it's created once they can be read.`), cause === undefined ? undefined : { cause }, ); @@ -16508,11 +16508,13 @@ export class AgentFramework { serverConfigs: McplServerConfig[], inferenceRouting?: import('./mcpl/types.js').InferenceRoutingPolicy, ): Promise { - // First, before anything is built: a coalescing state, checkpoint trees - // or a configured server's restored host-managed state that can't be - // taken refuses here and leaves the subsystem as it was, so a refused - // runtime connect leaves nothing for the guards that test - // mcplServerRegistry, and a refused boot connects no server. + // First, before anything is built: a coalescing state or checkpoint trees + // that can't be taken refuse here and leave the subsystem as it was, so a + // first connectMcplServer() refused for them leaves nothing for the guards + // that test mcplServerRegistry. Each configured server's restored + // host-managed state is read here too, so a boot refused for one connects + // no server; a runtime connect reads its server's in + // runConnectMcplServer, once MCPL is up. const coalescing = this.restorePushCoalescer(); const checkpoints = new CheckpointManager( this.store, @@ -17681,8 +17683,11 @@ export class AgentFramework { ); } - // The server's restored host-managed state is read before the server is - // added, so a read that fails leaves nothing connected. + // The server's restored host-managed trees are read before the server is + // added, so a read that fails leaves it unconnected; MCPL itself stays + // up, even when this connect brought it up. A feature set with no + // restored tree, as after a disconnect removed it, is read when it + // registers. this.checkpointManager?.loadHostStates(config.id); const oldToolNames = new Set(this.mcplTools.map(t => t.name)); await this.connectMcplServerInternal(config); diff --git a/src/mcpl/checkpoint-manager.ts b/src/mcpl/checkpoint-manager.ts index 88d9c8eab..93748b027 100644 --- a/src/mcpl/checkpoint-manager.ts +++ b/src/mcpl/checkpoint-manager.ts @@ -255,7 +255,7 @@ export class CheckpointManager { }; // Read before the tree is held, so a registration that fails here is // made whole by the next one. - if (opts.hostState) this.loadHostState(serverId, featureSet, tree); + if (opts.hostState) this.loadHostState(serverId, featureSet, tree, 'registration'); this.trees.set(key, tree); this.emitTraceFn({ @@ -561,8 +561,15 @@ export class CheckpointManager { /** A host-managed feature set's stored state, read into its tree; nothing * stored leaves it unset. A read that fails throws, naming the state: * taken as nothing stored, the next patch would apply to nothing and - * store over it. */ - private loadHostState(serverId: string, featureSet: string, tree: FeatureSetState): void { + * store over it. `when` says what the refusal stops: a restored tree's + * read comes before its server connects, and a feature set with no tree + * is read when it registers, once its server has connected. */ + private loadHostState( + serverId: string, + featureSet: string, + tree: FeatureSetState, + when: 'before-connect' | 'registration', + ): void { const stateId = this.stateId(serverId, featureSet); try { this.store.registerState({ id: stateId, strategy: 'snapshot' }); @@ -575,25 +582,31 @@ export class CheckpointManager { } catch (error) { throw new Error( `the host-managed state of feature set "${featureSet}" on MCPL server "${serverId}" (${stateId}) could not ` + - `be read: ${thrownText(error)}. The server doesn't connect without it, and neither does create() with it ` + - `configured, since the feature set's next checkpoint would store over it. It's unchanged, and the server ` + - `connects once it can be read.`, + `be read: ${thrownText(error)}. ` + + (when === 'before-connect' + ? `Neither create() with the server configured nor a connectMcplServer() of it goes on without it, since ` + + `the feature set's next checkpoint would store over it. It's unchanged, and they go on once it can be read.` + : `The feature set isn't registered without it, since its next checkpoint would store over it, so the ` + + `server's connect fails. It's unchanged, and the server's next connect reads it again.`), { cause: error }, ); } if (existing !== null && existing !== undefined) tree.currentState = existing; } - /** Read the stored state of every host-managed tree held for `serverId`, - * whatever its feature sets declare now. The framework calls it at MCPL's - * start for each configured server and before a runtime connect adds its - * server, so no server connects with a restored tree whose state its next - * checkpoint would store over unread. A server's trees are the keys - * `${serverId}:`, as removeServer finds them. */ + /** Read the stored state of every host-managed tree held for `serverId` + * with none in memory, whatever its feature sets declare now. The + * framework calls it at MCPL's start for each configured server and + * before a runtime connect adds its server, so no server connects with a + * restored tree whose state its next checkpoint would store over unread. + * A tree whose state is in memory, as a connected server's is, keeps it. + * A server's trees are the keys `${serverId}:`, as removeServer finds + * them. */ loadHostStates(serverId: string): void { const prefix = `${serverId}:`; for (const [key, tree] of this.trees) { - if (tree.hostState && key.startsWith(prefix)) this.loadHostState(serverId, key.slice(prefix.length), tree); + if (!tree.hostState || tree.currentState !== undefined || !key.startsWith(prefix)) continue; + this.loadHostState(serverId, key.slice(prefix.length), tree, 'before-connect'); } } diff --git a/src/usage/usage-tracker.ts b/src/usage/usage-tracker.ts index 668a33244..07e8cfe3c 100644 --- a/src/usage/usage-tracker.ts +++ b/src/usage/usage-tracker.ts @@ -13,15 +13,16 @@ function accumulateUsage( estimatedCost?: { total: number; currency: string } ): void { // A provider's usage can lack a count (membrane 0.5.87's Bedrock stream - // takes a message_delta's output count as reported), and the cost membrane - // computes from it is then NaN; adding either would make the total NaN, - // which is stored as null. + // takes a message_delta's output count as reported); adding it would make + // the total NaN, which is stored as null. const count = (n: number): number => (Number.isFinite(n) ? n : 0); target.inputTokens += count(tokens.inputTokens); target.outputTokens += count(tokens.outputTokens); target.cacheCreationTokens += count(tokens.cacheCreationTokens); target.cacheReadTokens += count(tokens.cacheReadTokens); - if (estimatedCost) { + // The cost membrane computes from such a count is NaN: an unknown cost, + // which is left out as an absent one is, rather than added as zero. + if (estimatedCost && Number.isFinite(estimatedCost.total)) { if (!target.estimatedCost) { target.estimatedCost = { total: 0, currency: estimatedCost.currency }; } else if (target.estimatedCost.currency !== estimatedCost.currency) { @@ -29,17 +30,25 @@ function accumulateUsage( target.estimatedCost = undefined; return; } - target.estimatedCost.total += count(estimatedCost.total); + target.estimatedCost.total += estimatedCost.total; } } function parseUsage(raw: unknown): SessionUsage | null { if (!raw || typeof raw !== 'object') return null; const o = raw as Record; - // A count or a cost total stored as null is a NaN total, as JSON stores - // one: an earlier release could write it when a provider's usage lacked a - // count. It reads as 0, the count that survived it. + // A count stored as null is a NaN total, as JSON stores one: an earlier + // release could write it when a provider's usage lacked a count. The sum + // before it was lost when it became NaN, so it reads as 0. const count = (v: unknown): number | undefined => (v === null ? 0 : typeof v === 'number' ? v : undefined); + // A cost is kept as a finite total with its currency. Anything else, such + // as the null JSON stores for a NaN total, is an unknown cost, as an + // absent one is. + const cost = o.estimatedCost as { total?: unknown; currency?: unknown } | null | undefined; + const estimatedCost = cost && typeof cost === 'object' && typeof cost.total === 'number' + && Number.isFinite(cost.total) && typeof cost.currency === 'string' + ? { total: cost.total, currency: cost.currency } + : undefined; const inputTokens = count(o.inputTokens); const outputTokens = count(o.outputTokens); if (inputTokens === undefined || outputTokens === undefined) return null; @@ -48,12 +57,7 @@ function parseUsage(raw: unknown): SessionUsage | null { outputTokens, cacheCreationTokens: typeof o.cacheCreationTokens === 'number' ? o.cacheCreationTokens : 0, cacheReadTokens: typeof o.cacheReadTokens === 'number' ? o.cacheReadTokens : 0, - estimatedCost: o.estimatedCost && typeof o.estimatedCost === 'object' - ? { - total: (o.estimatedCost as any).total === null ? 0 : (o.estimatedCost as any).total, - currency: (o.estimatedCost as any).currency, - } - : undefined, + estimatedCost, }; } diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts index 6626a4b5c..fd7d8c5e1 100644 --- a/test/boot-state-read-errors.test.ts +++ b/test/boot-state-read-errors.test.ts @@ -361,15 +361,22 @@ test('usage totals an earlier release wrote with a null count, or with a byAgent seedFrameworkState(storePath, { usage: { totals: { inputTokens: null, outputTokens: 500, estimatedCost: { total: null, currency: 'USD' } }, inferenceCount: 7, - byAgent: [null, { agentName: 'agent', usage: { inputTokens: 1000, outputTokens: 500 }, inferenceCount: 7 }], + byAgent: [ + null, + { agentName: 'agent', usage: { inputTokens: 1000, outputTokens: 500, estimatedCost: { total: null, currency: 'USD' } }, inferenceCount: 7 }, + { agentName: 'other', usage: { inputTokens: 10, outputTokens: 5, estimatedCost: { total: '0.25', currency: 'USD' } }, inferenceCount: 1 }, + ], } }); const f = await AgentFramework.create(config(storePath)); try { const usage = f.getSessionUsage(); assert.equal(usage.totals.inputTokens, 0); assert.equal(usage.totals.outputTokens, 500); - assert.deepEqual(usage.totals.estimatedCost, { total: 0, currency: 'USD' }); - assert.deepEqual(usage.byAgent.map((a) => [a.agentName, a.usage.inputTokens]), [['agent', 1000]]); + assert.equal(usage.totals.estimatedCost, undefined, 'a null cost total is an unknown cost'); + assert.deepEqual(usage.byAgent.map((a) => [a.agentName, a.usage.inputTokens, a.usage.estimatedCost]), [ + ['agent', 1000, undefined], + ['other', 10, undefined], + ], "an agent's cost that isn't a finite total is unknown too"); } finally { await f.stop(); } })); @@ -577,11 +584,15 @@ function seedHostManaged(storePath: string, server: string, featureSet: string, } finally { store.close(); } } -const hostStateUnreadable = (server: string, featureSet: string): RegExp => new RegExp( +/** The refusal of a host-managed state read before its server connects, or, with `registration`, when its feature set registers. */ +const hostStateUnreadable = (server: string, featureSet: string, when: 'before-connect' | 'registration' = 'before-connect'): RegExp => new RegExp( `^the host-managed state of feature set "${featureSet}" on MCPL server "${server}" \\(mcpl/state/${server}/${featureSet}\\) could not ` + - `be read: injected EIO reading mcpl/state/${server}/${featureSet}\\. The server doesn't connect without it, and neither does ` + - `create\\(\\) with it configured, since the feature set's next checkpoint would store over it\\. It's unchanged, and the ` + - `server connects once it can be read\\.$`, + `be read: injected EIO reading mcpl/state/${server}/${featureSet}\\. ` + + (when === 'before-connect' + ? `Neither create\\(\\) with the server configured nor a connectMcplServer\\(\\) of it goes on without it, since the feature set's ` + + `next checkpoint would store over it\\. It's unchanged, and they go on once it can be read\\.$` + : `The feature set isn't registered without it, since its next checkpoint would store over it, so the server's connect ` + + `fails\\. It's unchanged, and the server's next connect reads it again\\.$`), ); test("at boot, a configured server's restored host-managed state that can't be read fails create() before any server connects, and the next boot reads it", async (t) => { @@ -591,10 +602,12 @@ test("at boot, a configured server's restored host-managed state that can't be r await crash(f); const storePath = join(f.dir, 'store'); seedHostManaged(storePath, 'editor', 'doc', { count: 1 }); + const initializes = f.hostCaps.length; const restore = failStoreRead('mcpl/state/editor/doc', 'loadHostState'); try { await rejectsBuild(() => f.create(), hostStateUnreadable('editor', 'doc')); } finally { restore(); } + assert.equal(f.hostCaps.length, initializes, 'no server connected: the fixture saw no initialize'); assert.deepEqual(readState(storePath, 'mcpl/state/editor/doc'), { count: 1 }, 'the state is as it was'); await f.create(); const checkpoints = (f.framework as unknown as { checkpointManager: CheckpointManager }).checkpointManager; @@ -650,6 +663,42 @@ test('a host-managed feature set restored from the store has its state read by l } finally { store.close(); } })); +test("loadHostStates reads only its own server's host-managed trees, and only those with no state in memory", inStoreDir('boot-read-host-state-scope-', async (storePath) => { + const noop = () => {}; + const first = JsStore.openOrCreate({ path: storePath }); + try { + const checkpoints = new CheckpointManager(first, noop); + checkpoints.registerFeatureSet('srv', 'doc', { hostState: true, rollback: false }); + checkpoints.recordCheckpoint('srv', 'doc', { checkpoint: 'c1', data: { count: 1 } } as never); + checkpoints.registerFeatureSet('other', 'doc', { hostState: true, rollback: false }); + checkpoints.recordCheckpoint('other', 'doc', { checkpoint: 'c1', data: { count: 2 } } as never); + // A rollback-only tree keeps no host-managed state. + checkpoints.registerFeatureSet('srv', 'undo', { hostState: false, rollback: true }); + } finally { first.close(); } + const store = JsStore.openOrCreate({ path: storePath }); + // The states loadHostStates reads. + const proto = JsStore.prototype as unknown as { getStateJson(this: JsStore, id: string): unknown }; + const original = proto.getStateJson; + const read: string[] = []; + proto.getStateJson = function (this: JsStore, id: string) { + if (new Error().stack?.includes('loadHostState')) read.push(id); + return original.call(this, id); + }; + try { + const checkpoints = new CheckpointManager(store, noop); + checkpoints.loadHostStates('srv'); + assert.deepEqual(read, ['mcpl/state/srv/doc'], "only srv's host-managed tree is read"); + assert.deepEqual(checkpoints.getCurrentState('srv', 'doc'), { count: 1 }); + assert.equal(checkpoints.getCurrentState('other', 'doc'), undefined, "another server's tree waits for its own connect"); + // A state already in memory, as a connected server's is, isn't read again. + checkpoints.loadHostStates('srv'); + assert.deepEqual(read, ['mcpl/state/srv/doc'], 'nothing is read a second time'); + } finally { + proto.getStateJson = original; + store.close(); + } +})); + test("a host-managed feature set's state that can't be read fails loadHostStates for a restored tree, or the registration for a first-time one, naming it, and the next one reads it", inStoreDir('boot-read-host-state-read-', async (storePath) => { const noop = () => {}; const first = JsStore.openOrCreate({ path: storePath }); @@ -675,12 +724,7 @@ test("a host-managed feature set's state that can't be read fails loadHostStates const restore = failStoreRead(stateId, 'loadHostState'); try { assert.throws(reads[featureSet], { - message: new RegExp( - `^the host-managed state of feature set "${featureSet}" on MCPL server "srv" \\(mcpl/state/srv/${featureSet}\\) could not ` + - `be read: injected EIO reading mcpl/state/srv/${featureSet}\\. The server doesn't connect without it, and neither does ` + - `create\\(\\) with it configured, since the feature set's next checkpoint would store over it\\. It's unchanged, and the ` + - `server connects once it can be read\\.$`, - ), + message: hostStateUnreadable('srv', featureSet, featureSet === 'doc' ? 'before-connect' : 'registration'), }); } finally { restore(); } reads[featureSet](); diff --git a/test/usage-tracker.test.ts b/test/usage-tracker.test.ts index 7dc8ba7dc..629cec445 100644 --- a/test/usage-tracker.test.ts +++ b/test/usage-tracker.test.ts @@ -155,25 +155,29 @@ describe('UsageTracker', () => { assert.ok(Math.abs(snap.totals.estimatedCost.total - 0.03) < 1e-10); }); - it("a usage without a count adds nothing for it, so the totals stay numbers and are stored as numbers", () => { + it("a usage without a count adds nothing for it, and its NaN cost is an unknown one, so the totals stay numbers and are stored as numbers", () => { // membrane passes a Bedrock stream's output count through as reported, so // one can be missing, and the cost membrane computes from it is then NaN. const { tracker } = createTracker(); const cost = (total: number) => ({ total, currency: 'USD' }); - tracker.onInferenceCompleted('agent', { inputTokens: 100, outputTokens: 50, cacheCreationTokens: 10, cacheReadTokens: 20 }, cost(0.5)); - tracker.onInferenceCompleted('agent', { - inputTokens: undefined as unknown as number, - outputTokens: 25, + const withoutOutput = { + inputTokens: 25, + outputTokens: undefined as unknown as number, cacheCreationTokens: NaN, cacheReadTokens: NaN, - }, cost(NaN)); + }; + // An unknown cost first: the session's cost stays unknown, not a known zero. + tracker.onInferenceCompleted('agent', withoutOutput, cost(NaN)); + assert.equal(tracker.getSnapshot().totals.estimatedCost, undefined); + tracker.onInferenceCompleted('agent', { inputTokens: 100, outputTokens: 50, cacheCreationTokens: 10, cacheReadTokens: 20 }, cost(0.5)); + tracker.onInferenceCompleted('agent', withoutOutput, cost(NaN)); const stored = JSON.parse(JSON.stringify(tracker.toJSON())); - assert.equal(stored.totals.inputTokens, 100); - assert.equal(stored.totals.outputTokens, 75); + assert.equal(stored.totals.inputTokens, 150); + assert.equal(stored.totals.outputTokens, 50); assert.equal(stored.totals.cacheCreationTokens, 10); assert.equal(stored.totals.cacheReadTokens, 20); assert.equal(stored.totals.estimatedCost.total, 0.5); - assert.equal(stored.byAgent[0].usage.inputTokens, 100); + assert.equal(stored.byAgent[0].usage.inputTokens, 150); assert.equal(stored.byAgent[0].usage.estimatedCost.total, 0.5); }); }); From 246073e01031d1083c2d841b0e14847a31b505ca Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 13:43:53 -0400 Subject: [PATCH 05/11] fix(boot): a registration's refusal names its retry, a duplicate connect isn't read, and the cost is left as main has it Round 3's review found: The guards' new remedy text wasn't followed into its test, so boot-state-read-errors failed at round 3's head. The test now expects "won't help until the host's operator repairs them". A registration's refusal said the server's next connect reads the state again. A failed registration leaves the connection in the registry, so a plain connectMcplServer() of it is refused as already registered until it's disconnected; a reconnect or a restart registers the feature set again. The refusal now says the state is read again when the feature set next registers, and the changelog and the description say what re-reads it. The changelog's comparison with main names main's order: it held the tree before the read, so after a failed read the state was taken as empty. A runtime connect of a server already connected still read its trees whose state wasn't in memory, such as one with nothing stored, so a failed read replaced addServer's "already registered" refusal with a read error whose text promised the connect would go on. runConnectMcplServer now reads only for a server the registry doesn't hold. The cost. Rounds 2 and 3 changed how an unknown cost reads: first adding 0 for a NaN cost, then leaving it out and restoring a cost that isn't a finite total with its currency as none. Each answer left a partial sum reading as a known one, since the tracker has no way to say a session's cost is unknown for good, and that's the same question as the cost after a currency mismatch, which the owner placed as its own item. This change doesn't need either: the boot's usage check never looked at the cost. So accumulateUsage and parseUsage handle the cost as main does, and the description's Not covered names the question. Tests: a connect of a connected server whose tree holds nothing is refused as already registered while a read of that tree would fail. The registration text's test follows the new wording. The usage tests no longer set or check a cost; the restore test puts a null count in an agent's entry. Co-Authored-By: Claude Opus 5.5 --- changelog.d/boot-state-read-errors.fixed.md | 4 +-- src/framework.ts | 5 ++-- src/mcpl/checkpoint-manager.ts | 2 +- src/usage/usage-tracker.ts | 16 +++------- test/boot-state-read-errors.test.ts | 33 ++++++++++++--------- test/usage-tracker.test.ts | 23 +++++--------- 6 files changed, 37 insertions(+), 46 deletions(-) diff --git a/changelog.d/boot-state-read-errors.fixed.md b/changelog.d/boot-state-read-errors.fixed.md index 5ee4329e8..baaa8a9ee 100644 --- a/changelog.d/boot-state-read-errors.fixed.md +++ b/changelog.d/boot-state-read-errors.fixed.md @@ -8,6 +8,6 @@ - **MCPL coalescing** (`mcpl/coalescing`, `mcpl/coalescing-recent`): an acknowledged deferred acceptance or a pending batch was never delivered, and the next receipt or flush replaced the stored ones. A snapshot of a version this one doesn't read was ignored and replaced; it now fails `create()` and is kept for the version that wrote it. An agent's context that can't be checked for an occurrence already delivered fails `create()` too, where the occurrence was delivered again. - **MCPL checkpoint trees** (`mcpl/checkpoints`): a stored value without `trees` read as no checkpoints, and the next feature set registered stored over it. - On a framework started with no MCPL servers, the coalescing state and the checkpoint trees are read at the first `connectMcplServer()`, which fails instead, with nothing of MCPL built, and the next connect reads them again. -- A host-managed MCPL feature set's state (`mcpl/state//`) is now read after a restart: when MCPL starts, for each configured server, and before `connectMcplServer()` adds a server. It wasn't, so the state read as empty, and the feature set's next delta checkpoint applied to nothing and stored over it. A failed read of it fails `create()`, or that `connectMcplServer()`, naming it, before the server connects. A feature set with no stored tree, such as one registering for the first time or after its server was disconnected, has its state read when it registers, as before. A failed read there fails that server's connect, as before, and `create()` logs that and goes on, as for any server's failed connect; the next connect now reads the state again, where it took it as empty. -- A usage count stored as `null`, which a provider's usage without a count left as `NaN`, restores as 0, and a usage without a count no longer makes the totals `NaN`. Its cost, which membrane computes as `NaN`, is now an unknown cost, left out of the total as a cost membrane couldn't estimate is, and a stored cost that isn't a number with its currency, such as `null`, restores as unknown. +- A host-managed MCPL feature set's state (`mcpl/state//`) is now read after a restart: when MCPL starts, for each configured server, and before `connectMcplServer()` adds a server. It wasn't, so the state read as empty, and the feature set's next delta checkpoint applied to nothing and stored over it. A failed read of it fails `create()`, or that `connectMcplServer()`, naming it, before the server connects. A feature set with no stored tree, such as one registering for the first time or after its server was disconnected, has its state read when it registers, as before. A failed read there fails that server's connect, as before, and `create()` logs that and goes on, as for any server's failed connect. The state is now read again the next time the feature set registers; before, its tree was held after the failed read, so the state was taken as empty. +- A usage count stored as `null`, which a provider's usage without a count left as `NaN`, restores as 0, and a usage without a count no longer makes the totals `NaN`. - A failed read of the persisted `tool_result_inline_max_chars` caps at boot is now logged. As before, they don't apply until a restart, and setting one keeps the others stored. diff --git a/src/framework.ts b/src/framework.ts index 26b02c5c3..df7298d47 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -17687,8 +17687,9 @@ export class AgentFramework { // added, so a read that fails leaves it unconnected; MCPL itself stays // up, even when this connect brought it up. A feature set with no // restored tree, as after a disconnect removed it, is read when it - // registers. - this.checkpointManager?.loadHostStates(config.id); + // registers. A server already connected is refused by addServer below, + // so its trees are left as they are. + if (!this.mcplServerRegistry?.getServer(config.id)) this.checkpointManager?.loadHostStates(config.id); const oldToolNames = new Set(this.mcplTools.map(t => t.name)); await this.connectMcplServerInternal(config); await this.refreshMcplTools(); diff --git a/src/mcpl/checkpoint-manager.ts b/src/mcpl/checkpoint-manager.ts index 93748b027..111063bf4 100644 --- a/src/mcpl/checkpoint-manager.ts +++ b/src/mcpl/checkpoint-manager.ts @@ -587,7 +587,7 @@ export class CheckpointManager { ? `Neither create() with the server configured nor a connectMcplServer() of it goes on without it, since ` + `the feature set's next checkpoint would store over it. It's unchanged, and they go on once it can be read.` : `The feature set isn't registered without it, since its next checkpoint would store over it, so the ` + - `server's connect fails. It's unchanged, and the server's next connect reads it again.`), + `server's connect fails. It's unchanged, and it's read again when the feature set next registers.`), { cause: error }, ); } diff --git a/src/usage/usage-tracker.ts b/src/usage/usage-tracker.ts index 07e8cfe3c..dae6c69da 100644 --- a/src/usage/usage-tracker.ts +++ b/src/usage/usage-tracker.ts @@ -20,9 +20,7 @@ function accumulateUsage( target.outputTokens += count(tokens.outputTokens); target.cacheCreationTokens += count(tokens.cacheCreationTokens); target.cacheReadTokens += count(tokens.cacheReadTokens); - // The cost membrane computes from such a count is NaN: an unknown cost, - // which is left out as an absent one is, rather than added as zero. - if (estimatedCost && Number.isFinite(estimatedCost.total)) { + if (estimatedCost) { if (!target.estimatedCost) { target.estimatedCost = { total: 0, currency: estimatedCost.currency }; } else if (target.estimatedCost.currency !== estimatedCost.currency) { @@ -41,14 +39,6 @@ function parseUsage(raw: unknown): SessionUsage | null { // release could write it when a provider's usage lacked a count. The sum // before it was lost when it became NaN, so it reads as 0. const count = (v: unknown): number | undefined => (v === null ? 0 : typeof v === 'number' ? v : undefined); - // A cost is kept as a finite total with its currency. Anything else, such - // as the null JSON stores for a NaN total, is an unknown cost, as an - // absent one is. - const cost = o.estimatedCost as { total?: unknown; currency?: unknown } | null | undefined; - const estimatedCost = cost && typeof cost === 'object' && typeof cost.total === 'number' - && Number.isFinite(cost.total) && typeof cost.currency === 'string' - ? { total: cost.total, currency: cost.currency } - : undefined; const inputTokens = count(o.inputTokens); const outputTokens = count(o.outputTokens); if (inputTokens === undefined || outputTokens === undefined) return null; @@ -57,7 +47,9 @@ function parseUsage(raw: unknown): SessionUsage | null { outputTokens, cacheCreationTokens: typeof o.cacheCreationTokens === 'number' ? o.cacheCreationTokens : 0, cacheReadTokens: typeof o.cacheReadTokens === 'number' ? o.cacheReadTokens : 0, - estimatedCost, + estimatedCost: o.estimatedCost && typeof o.estimatedCost === 'object' + ? { total: (o.estimatedCost as any).total, currency: (o.estimatedCost as any).currency } + : undefined, }; } diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts index fd7d8c5e1..472fa05e4 100644 --- a/test/boot-state-read-errors.test.ts +++ b/test/boot-state-read-errors.test.ts @@ -357,26 +357,18 @@ test("usage totals that aren't a usage record fail create(), naming what they ar })); test('usage totals an earlier release wrote with a null count, or with a byAgent entry that is not an agent, restore', inStoreDir('boot-read-usage-null-', async (storePath) => { - // A NaN total, which a provider's usage without a count made, is stored as null, and so is a NaN cost. + // A NaN total, which a provider's usage without a count made, is stored as null. seedFrameworkState(storePath, { usage: { - totals: { inputTokens: null, outputTokens: 500, estimatedCost: { total: null, currency: 'USD' } }, + totals: { inputTokens: null, outputTokens: 500 }, inferenceCount: 7, - byAgent: [ - null, - { agentName: 'agent', usage: { inputTokens: 1000, outputTokens: 500, estimatedCost: { total: null, currency: 'USD' } }, inferenceCount: 7 }, - { agentName: 'other', usage: { inputTokens: 10, outputTokens: 5, estimatedCost: { total: '0.25', currency: 'USD' } }, inferenceCount: 1 }, - ], + byAgent: [null, { agentName: 'agent', usage: { inputTokens: 1000, outputTokens: null }, inferenceCount: 7 }], } }); const f = await AgentFramework.create(config(storePath)); try { const usage = f.getSessionUsage(); assert.equal(usage.totals.inputTokens, 0); assert.equal(usage.totals.outputTokens, 500); - assert.equal(usage.totals.estimatedCost, undefined, 'a null cost total is an unknown cost'); - assert.deepEqual(usage.byAgent.map((a) => [a.agentName, a.usage.inputTokens, a.usage.estimatedCost]), [ - ['agent', 1000, undefined], - ['other', 10, undefined], - ], "an agent's cost that isn't a finite total is unknown too"); + assert.deepEqual(usage.byAgent.map((a) => [a.agentName, a.usage.inputTokens, a.usage.outputTokens]), [['agent', 1000, 0]]); } finally { await f.stop(); } })); @@ -444,7 +436,7 @@ test("tool-result guards that can't be read, or that aren't an object, fail crea await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( `^the tool-result guards \\(framework/state, toolResultGuards\\) are ${are}, not an object of agents' guards\\. Agent "agent" isn't ` + `created, since without them it would take its recipe's guard and the next guard change would store over them\\. They're ` + - `unchanged, and creating it again won't help until they're repaired\\.$`, + `unchanged, and creating it again won't help until the host's operator repairs them\\.$`, ), JSON.stringify(guards)); assert.deepEqual((readState(storePath, 'framework/state') as { toolResultGuards: unknown }).toolResultGuards, guards); } @@ -592,7 +584,7 @@ const hostStateUnreadable = (server: string, featureSet: string, when: 'before-c ? `Neither create\\(\\) with the server configured nor a connectMcplServer\\(\\) of it goes on without it, since the feature set's ` + `next checkpoint would store over it\\. It's unchanged, and they go on once it can be read\\.$` : `The feature set isn't registered without it, since its next checkpoint would store over it, so the server's connect ` + - `fails\\. It's unchanged, and the server's next connect reads it again\\.$`), + `fails\\. It's unchanged, and it's read again when the feature set next registers\\.$`), ); test("at boot, a configured server's restored host-managed state that can't be read fails create() before any server connects, and the next boot reads it", async (t) => { @@ -642,6 +634,19 @@ test("at a runtime connect, a restored host-managed state that can't be read fai } finally { await framework.stop(); } }); +test("a connect of a server already connected is refused as before, without reading its trees again", async (t) => { + const f = await fixture(); t.after(f.close); + await crash(f); + // A host-managed tree whose stored state holds nothing, so it holds no state in memory once read. + seedHostManaged(join(f.dir, 'store'), 'editor', 'notes', null); + await f.create(); + const server = { id: 'editor', url: `ws://127.0.0.1:${f.port}`, enabledFeatureSets: ['doc'] } as McplServerConfig; + const restore = failStoreRead('mcpl/state/editor/notes', 'loadHostState'); + try { + await assert.rejects(f.framework.connectMcplServer(server), { message: 'MCPL server "editor" is already registered' }); + } finally { restore(); } +}); + test('a host-managed feature set restored from the store has its state read by loadHostStates, so its next patch applies to it', inStoreDir('boot-read-host-state-', async (storePath) => { const noop = () => {}; const first = JsStore.openOrCreate({ path: storePath }); diff --git a/test/usage-tracker.test.ts b/test/usage-tracker.test.ts index 629cec445..6406087ef 100644 --- a/test/usage-tracker.test.ts +++ b/test/usage-tracker.test.ts @@ -155,29 +155,22 @@ describe('UsageTracker', () => { assert.ok(Math.abs(snap.totals.estimatedCost.total - 0.03) < 1e-10); }); - it("a usage without a count adds nothing for it, and its NaN cost is an unknown one, so the totals stay numbers and are stored as numbers", () => { - // membrane passes a Bedrock stream's output count through as reported, so - // one can be missing, and the cost membrane computes from it is then NaN. + it("a usage without a count adds nothing for it, so the totals stay numbers and are stored as numbers", () => { + // membrane passes a Bedrock stream's output count through as reported, so one can be missing. const { tracker } = createTracker(); - const cost = (total: number) => ({ total, currency: 'USD' }); - const withoutOutput = { + tracker.onInferenceCompleted('agent', { inputTokens: 100, outputTokens: 50, cacheCreationTokens: 10, cacheReadTokens: 20 }); + tracker.onInferenceCompleted('agent', { inputTokens: 25, outputTokens: undefined as unknown as number, cacheCreationTokens: NaN, cacheReadTokens: NaN, - }; - // An unknown cost first: the session's cost stays unknown, not a known zero. - tracker.onInferenceCompleted('agent', withoutOutput, cost(NaN)); - assert.equal(tracker.getSnapshot().totals.estimatedCost, undefined); - tracker.onInferenceCompleted('agent', { inputTokens: 100, outputTokens: 50, cacheCreationTokens: 10, cacheReadTokens: 20 }, cost(0.5)); - tracker.onInferenceCompleted('agent', withoutOutput, cost(NaN)); + }); const stored = JSON.parse(JSON.stringify(tracker.toJSON())); - assert.equal(stored.totals.inputTokens, 150); + assert.equal(stored.totals.inputTokens, 125); assert.equal(stored.totals.outputTokens, 50); assert.equal(stored.totals.cacheCreationTokens, 10); assert.equal(stored.totals.cacheReadTokens, 20); - assert.equal(stored.totals.estimatedCost.total, 0.5); - assert.equal(stored.byAgent[0].usage.inputTokens, 150); - assert.equal(stored.byAgent[0].usage.estimatedCost.total, 0.5); + assert.equal(stored.byAgent[0].usage.inputTokens, 125); + assert.equal(stored.byAgent[0].usage.outputTokens, 50); }); }); From 6974aebd70f41588138b14f4693b14c0cd537337 Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 13:58:30 -0400 Subject: [PATCH 06/11] fix(boot): a torn recovery file's refusal names the record to write, not removal The likeliest way an operator meets these refusals is a recovery file a power loss left empty or torn, and its refusal said only that the framework starts once the file holds a record. The obvious repair, removing the file, is the wrong one: without the file the store's slot is read, and with a recovery path the file's writes don't keep that slot. quiesce() writes only the file, so a host quiesced mid-maintenance would boot serving. So the host-mode file's form refusal names the record to write, {"version":1,"quiesced":true} to boot quiesced or {"version":1,"quiesced":false} to serve, and says removing it doesn't help and why. The deferred-writes file's names {"version":1,"pending":[]} for none, and says the same of its slot. (Ezra-2423's read, which Ada-1017 asked be taken.) Tests: the two form tests expect the new texts. Co-Authored-By: Claude Opus 5.5 --- src/framework.ts | 11 +++++++++-- test/boot-state-read-errors.test.ts | 8 ++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/framework.ts b/src/framework.ts index df7298d47..eb0cb0503 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -5203,7 +5203,10 @@ export class AgentFramework { `the deferred context writes (${file ?? DEFERRED_WRITES_ID}) ${problem}. The framework doesn't start without them, ` + `since the next deferral would replace the stored queue. ` + (file - ? `The file is unchanged, and the framework starts once ${cause === undefined ? 'it holds a queue of writes' : 'it can be read'}.` + ? `The file is unchanged, and the framework starts once ${cause === undefined + ? 'it holds a queue of writes, such as {"version":1,"pending":[]} for none. Removing it doesn\'t help: ' + + `without the file, the queue is read from ${DEFERRED_WRITES_ID}, which the file's writes don't keep` + : 'it can be read'}.` : cause === undefined ? `They're unchanged, and starting again won't help until they're repaired.` : `They're unchanged, and the framework starts once they can be read.`), @@ -6039,7 +6042,11 @@ export class AgentFramework { `the host mode (${file ?? `${FRAMEWORK_STATE_ID}, hostMode`}) ${problem}. The framework doesn't start without it, ` + `since a host quiesced for maintenance would boot serving. ` + (file - ? `The file is unchanged, and the framework starts once ${cause === undefined ? 'it holds a host-mode record' : 'it can be read'}.` + ? `The file is unchanged, and the framework starts once ${cause === undefined + ? 'it holds a host-mode record, such as {"version":1,"quiesced":true} to boot quiesced or ' + + `{"version":1,"quiesced":false} to serve. Removing it doesn't help: without the file, host mode is read ` + + `from ${FRAMEWORK_STATE_ID}, which a quiesce doesn't write, so a quiesced host would boot serving` + : 'it can be read'}.` : cause === undefined ? `It's unchanged, and starting again won't help until it's repaired.` : `It's unchanged, and the framework starts once it can be read.`), diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts index 472fa05e4..ccd336762 100644 --- a/test/boot-state-read-errors.test.ts +++ b/test/boot-state-read-errors.test.ts @@ -149,7 +149,9 @@ test('a host-mode file that holds no record, text that isn\'t JSON, or a record await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( `^the host mode \\(.*host-mode\\.json\\) ${holds}\\. The framework doesn't start without it, ` + `since a host quiesced for maintenance would boot serving\\. The file is unchanged, and the framework starts once it holds a ` + - `host-mode record\\.$`, + `host-mode record, such as \\{"version":1,"quiesced":true\\} to boot quiesced or \\{"version":1,"quiesced":false\\} to serve\\. ` + + `Removing it doesn't help: without the file, host mode is read from framework/state, which a quiesce doesn't write, so a ` + + `quiesced host would boot serving\\.$`, ), content); assert.equal(readFileSync(file, 'utf8'), content, `${content}: the file is as it was`); } @@ -258,7 +260,9 @@ test("a deferred-write queue that isn't a queue, or isn't JSON, fails create(), writeFileSync(file, content); await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( `^the deferred context writes \\(.*deferred-writes\\.json\\) ${holds}\\. The framework doesn't start without them, since the next ` + - `deferral would replace the stored queue\\. The file is unchanged, and the framework starts once it holds a queue of writes\\.$`, + `deferral would replace the stored queue\\. The file is unchanged, and the framework starts once it holds a queue of writes, ` + + `such as \\{"version":1,"pending":\\[\\]\\} for none\\. Removing it doesn't help: without the file, the queue is read from ` + + `framework/deferred-writes, which the file's writes don't keep\\.$`, ), content); assert.equal(readFileSync(file, 'utf8'), content, `${content}: the file is as it was`); } From cb2a26c7055a47e60736eb2ae88de74af0f6b6ab Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 14:06:17 -0400 Subject: [PATCH 07/11] fix(boot): the queue file's refusal says what removing it does The deferred-writes file's form refusal said removing the file doesn't help. Removing it does let the framework start: it reads the queue from framework/deferred-writes instead, which the file's writes don't keep, so it restores whatever older queue is there, or none. An operator who removed it would see the text's prediction fail, and the text didn't say what the harm is. It now says what removing it does. (Ezra-2423's read of 86b18032.) Co-Authored-By: Claude Opus 5.5 --- src/framework.ts | 5 +++-- test/boot-state-read-errors.test.ts | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/framework.ts b/src/framework.ts index eb0cb0503..50def7340 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -5204,8 +5204,9 @@ export class AgentFramework { `since the next deferral would replace the stored queue. ` + (file ? `The file is unchanged, and the framework starts once ${cause === undefined - ? 'it holds a queue of writes, such as {"version":1,"pending":[]} for none. Removing it doesn\'t help: ' + - `without the file, the queue is read from ${DEFERRED_WRITES_ID}, which the file's writes don't keep` + ? 'it holds a queue of writes, such as {"version":1,"pending":[]} for none. Removing it reads the queue ' + + `from ${DEFERRED_WRITES_ID} instead, which the file's writes don't keep, so it restores whatever older ` + + `queue is there, or none` : 'it can be read'}.` : cause === undefined ? `They're unchanged, and starting again won't help until they're repaired.` diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts index ccd336762..45ce7d2f6 100644 --- a/test/boot-state-read-errors.test.ts +++ b/test/boot-state-read-errors.test.ts @@ -261,8 +261,8 @@ test("a deferred-write queue that isn't a queue, or isn't JSON, fails create(), await rejectsBuild(() => AgentFramework.create(config(storePath)), new RegExp( `^the deferred context writes \\(.*deferred-writes\\.json\\) ${holds}\\. The framework doesn't start without them, since the next ` + `deferral would replace the stored queue\\. The file is unchanged, and the framework starts once it holds a queue of writes, ` + - `such as \\{"version":1,"pending":\\[\\]\\} for none\\. Removing it doesn't help: without the file, the queue is read from ` + - `framework/deferred-writes, which the file's writes don't keep\\.$`, + `such as \\{"version":1,"pending":\\[\\]\\} for none\\. Removing it reads the queue from framework/deferred-writes instead, ` + + `which the file's writes don't keep, so it restores whatever older queue is there, or none\\.$`, ), content); assert.equal(readFileSync(file, 'utf8'), content, `${content}: the file is as it was`); } From eea1d3f4c14aedf1ab59c7548d504d1baad30bb3 Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 14:57:47 -0400 Subject: [PATCH 08/11] fix(boot): a stored inline cap that can't become a number is dropped, not thrown from create() From Greptile's review of #454. Narrowing the caps read's catch to the read itself left each entry's Number() outside it, and Number() throws for an object it can't make a primitive of, such as {"toString":null}. Such an entry then failed create() with an unnamed TypeError, where main's catch around the whole read dropped every cap silently. The conversion now has its own catch, so that entry is invalid like any other: dropped, loudly, while the other caps apply, as the read's rule says. Test: a stored caps record holding {"toString":null} for one agent and 5000 for another boots, logs the drop, and applies the other's cap. Co-Authored-By: Claude Opus 5.5 --- src/framework.ts | 7 ++++++- test/boot-state-read-errors.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/framework.ts b/src/framework.ts index 50def7340..e51859d9e 100644 --- a/src/framework.ts +++ b/src/framework.ts @@ -5816,7 +5816,12 @@ export class AgentFramework { } const out: Record = {}; for (const [name, value] of Object.entries(data?.toolResultInlineCaps ?? {})) { - const n = Number(value); + // Number() throws for an object it can't make a primitive of, such as + // {"toString":null}: that entry is invalid like any other, and dropped. + let n = Number.NaN; + try { + n = Number(value); + } catch { /* not a number: dropped below */ } if (Number.isFinite(n) && n >= 1000) { out[name] = Math.floor(n); } else { diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts index 45ce7d2f6..895c03d85 100644 --- a/test/boot-state-read-errors.test.ts +++ b/test/boot-state-read-errors.test.ts @@ -418,6 +418,28 @@ test("inline caps that can't be read are logged, boot goes on, and setting one k assert.deepEqual((readState(storePath, 'framework/state') as { toolResultInlineCaps: unknown }).toolResultInlineCaps, { agent: 5000, other: 2000 }); })); +test("a stored inline cap that can't become a number is dropped, loudly, and the others apply", inStoreDir('boot-read-caps-entry-', async (storePath) => { + // Number() throws for an object it can't make a primitive of. + seedFrameworkState(storePath, { toolResultInlineCaps: { agent: { toString: null }, other: 5000 } }); + const twoAgents = { ...config(storePath), agents: [...agents, { name: 'other', model: 'test-model', systemPrompt: 'test' }] } as FrameworkConfig; + const log = quietErrors(); + let f: AgentFramework; + try { + f = await AgentFramework.create(twoAgents); + } finally { log.restore(); } + try { + assert.ok( + log.lines.some((l) => l.includes(`[agent-settings] dropping invalid persisted tool_result_inline_max_chars for 'agent': {"toString":null}`)), + log.lines.join('\n'), + ); + const settings = (f as unknown as { + collectAgentSettingsExtensions(): Map }>; + }).collectAgentSettingsExtensions().get('_framework')!; + assert.equal(settings.get('agent').tool_result_inline_max_chars, null); + assert.equal(settings.get('other').tool_result_inline_max_chars, 5000); + } finally { await f.stop(); } +})); + // --------------------------------------------------------------------------- // Tool-result guards // --------------------------------------------------------------------------- From 20d1c0b56cedf44b1d11e991e7e89b95037f925e Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 15:09:19 -0400 Subject: [PATCH 09/11] test(stop): the MCPL stop() tests' fake registry answers getServer CI on #454 failed two tests in stop-with-host-operations at cb2a26c7 and eea1d3f4: "stop() waits for an MCPL connect already running before it closes the connections", and the same for a restart. Since 246073e0, runConnectMcplServer asks the registry whether the server is connected already before it reads the server's stored trees. Those tests' fake registry had no getServer, so the connect threw "this.mcplServerRegistry ?.getServer is not a function", and the connect test then timed out waiting for the operation it starts. The fake now answers null, as McplServerRegistry.getServer does for an unknown server. The source is unchanged. Co-Authored-By: Claude Opus 5.5 --- test/stop-with-host-operations.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/stop-with-host-operations.test.ts b/test/stop-with-host-operations.test.ts index 1685b13de..1132961b2 100644 --- a/test/stop-with-host-operations.test.ts +++ b/test/stop-with-host-operations.test.ts @@ -944,6 +944,9 @@ describe('stop() with an operator operation in flight', () => { fw.emitMcplToolDiff = () => {}; fw.mcplServerRegistry = { getAllServers: () => [], + // A connect asks whether the server is connected already, as the + // registry answers for an unknown server. + getServer: () => null, removeServer: async () => { entered = true; await held; From e836b4698df5d662a6ecc14ae2c1263834e296d9 Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 18:44:47 -0400 Subject: [PATCH 10/11] test(boot): the failed-build helper leaves the release to create() With #426 in main, a failed create() stops the framework it built and closes a store it opened, so a test can boot again on that store. The helper no longer needs to find the framework through restoreUsageState and stop it: it asserts that the build fails with the expected message, and stops a build that succeeds, as before. Co-Authored-By: Claude Opus 5.5 --- test/helpers/failed-create.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/test/helpers/failed-create.ts b/test/helpers/failed-create.ts index a386bec4a..f5283b4e8 100644 --- a/test/helpers/failed-create.ts +++ b/test/helpers/failed-create.ts @@ -1,31 +1,20 @@ /** * A build of the framework that must fail: `AgentFramework.create()`, or a - * fixture's restart through it. Until a failed create() releases what it - * acquired (agent-framework #389, which #426 fixes), the framework it built keeps its store open, - * so a test that boots again on that store first stops it here, as that - * release would. stop() is idempotent, so this stop is harmless once it does. + * fixture's restart through it. A failed create() releases what it acquired + * (agent-framework #389), so a test can boot again on the same store. A build + * that succeeds is stopped before the assertion fails, so its store is + * released too. */ import assert from 'node:assert/strict'; import { AgentFramework } from '../../src/index.js'; export async function rejectsBuild(build: () => Promise, message: RegExp, what?: string): Promise { - // restoreUsageState runs early in every create(), on the framework it builds. - const proto = AgentFramework.prototype as unknown as { restoreUsageState(this: AgentFramework): void }; - const original = proto.restoreUsageState; - let built: AgentFramework | undefined; - proto.restoreUsageState = function (this: AgentFramework) { - built = this; - return original.call(this); - }; let caught: unknown; let made: unknown; try { made = await build(); } catch (error) { caught = error; - } finally { - proto.restoreUsageState = original; - await built?.stop(); } if (made instanceof AgentFramework) await made.stop(); assert.ok(caught instanceof Error, `the build should have failed${what ? `: ${what}` : ''}`); From ddddcbc34c41df87a362ec390b88e5b526e26e70 Mon Sep 17 00:00:00 2001 From: Hilda-2681 Date: Sat, 17 Oct 2026 18:54:31 -0400 Subject: [PATCH 11/11] test(boot): the landed-write test puts back the receipt a crash would leave Since main 86a36834 merged in, stop() drains every agent's deferred writes and, on a quiesced host, defers them again, handing them off at the agent's current position. The landed-write test builds "a write that landed before its ack" by adding the copy through a staging framework and stopping it, so staging's stop now moved the receipt's scanFrom past the copy ({"agent":1}), and the next boot's check for landed writes started after it and restored the write again (the test saw one deferred write where it expects none). A crash between a write landing and its ack leaves the receipt its hand-off recorded, from before the copy landed, so the test now writes that receipt back (scanFrom {"agent":0}) after staging stops. The refusal it checks, and the next boot's skip, are unchanged. Co-Authored-By: Claude Opus 5.5 --- test/boot-state-read-errors.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/boot-state-read-errors.test.ts b/test/boot-state-read-errors.test.ts index 895c03d85..c532cdd23 100644 --- a/test/boot-state-read-errors.test.ts +++ b/test/boot-state-read-errors.test.ts @@ -303,6 +303,12 @@ test("a deferred write whose agent's context can't be scanned fails create(), an staging.getAgent('agent')!.getContextManager() .addMessage('user', [{ type: 'text', text: 'said during surgery' }], { deferredWriteId: queued!.id } as never); } finally { await staging.stop(); } + // A crash between a write landing and its ack leaves the receipt its + // hand-off recorded, from before the copy landed. Staging's stop handed the + // queue off again from after the copy, so the receipt goes back to where + // the agent's context began. + const queueFile = join(storePath, 'recovery', 'deferred-writes.json'); + writeFileSync(queueFile, JSON.stringify({ ...JSON.parse(readFileSync(queueFile, 'utf8')), scanFrom: { agent: 0 } })); const proto = ContextManager.prototype as unknown as { getMessageCount(this: ContextManager): number }; const original = proto.getMessageCount; proto.getMessageCount = function (this: ContextManager) {