From 7cc84df58ec4ccb04443008a6d59c711587a1262 Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Fri, 9 Oct 2026 19:50:14 -0700 Subject: [PATCH 1/8] fix(sdk): implement f.memory.learn so findings persist across runs f.memory.learn was a hard-coded stub that threw unsupported_verb: ai-hist 0.4.1 has no public trajectory writer, so the write slice was never built. learn() now writes the finding under the flow's script scope, twice: - a compacted trajectory file at /.trajectories/compacted/.json, the format `ai-hist sync` ingests; - the matching trajectories/history rows in the ai-hist SQLite DB, through ai-hist's own sql.js and an atomic file replace, so the next run's recall/why see it without a sync. The id hashes scope + finding, so a resumed body re-running learn (or a later run learning the same thing) upserts one record. Invalid findings refuse with memory_finding_invalid before any write; persistence failures surface as memory_unwritable. Regenerates AUTHORING.md (also picks up drift from #636). Co-Authored-By: Claude Opus 5.5 (1M context) Session-Id: b6cf20aa-bdbb-4248-91ab-d2a4fd2ce1b8 --- packages/sdk/src/authored-flow-error.ts | 4 + packages/sdk/src/authored-memory.ts | 121 +++++++++++++++++++++++- packages/sdk/tests/f-memory.test.ts | 80 +++++++++++++++- packages/surface/AUTHORING.md | 16 +++- packages/surface/src/memory.ts | 2 +- testdata/memory/README.md | 19 ++-- 6 files changed, 225 insertions(+), 17 deletions(-) diff --git a/packages/sdk/src/authored-flow-error.ts b/packages/sdk/src/authored-flow-error.ts index 307343791..91949c560 100644 --- a/packages/sdk/src/authored-flow-error.ts +++ b/packages/sdk/src/authored-flow-error.ts @@ -26,6 +26,10 @@ export type AuthoredFlowExecutionErrorCode = | 'journal_protocol_violation' | 'missing_completion' | 'memory_unreachable' + /** `f.memory.learn` was given a finding without non-empty string fields. */ + | 'memory_finding_invalid' + /** `f.memory.learn` could not persist the finding to script memory. */ + | 'memory_unwritable' | 'operation_after_completion' | 'operation_callback_failed' | 'step_failed' diff --git a/packages/sdk/src/authored-memory.ts b/packages/sdk/src/authored-memory.ts index 62ab491da..d5990d250 100644 --- a/packages/sdk/src/authored-memory.ts +++ b/packages/sdk/src/authored-memory.ts @@ -1,7 +1,9 @@ import type { MemoryHelper } from '@relayflows/surface'; -import { createHash } from 'node:crypto'; -import { access, stat } from 'node:fs/promises'; +import type { MemoryFinding } from '@relayflows/surface'; +import { createHash, randomUUID } from 'node:crypto'; +import { access, mkdir, readFile, rename, stat, writeFile } from 'node:fs/promises'; import { constants } from 'node:fs'; +import { createRequire } from 'node:module'; import { dirname, join, resolve } from 'node:path'; import { AuthoredFlowExecutionError } from './authored-flow-error.js'; import { preflightMemory } from './preflight.js'; @@ -47,9 +49,120 @@ export function authoredMemory( const entry = reader.whyForTask(task); return entry?.projectId === scope ? [entry] : []; }), - async learn() { + async learn(finding) { assertOpen(); - throw new AuthoredFlowExecutionError('unsupported_verb', 'f.memory.learn requires the follow-up journal-backed trajectory writer'); + if (!enabled) throw new AuthoredFlowExecutionError('unsupported_header', 'f.memory requires script memory; memory.script is false'); + const decision = normalizeFinding(finding); + await assertMemoryReachable(); + assertOpen(); + const { defaultDbPath } = await import('ai-hist'); + try { + await persistFinding(defaultDbPath(), scope, decision); + } catch (error) { + throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn could not persist the finding: ${error instanceof Error ? error.message : String(error)}`); + } }, }; } + +interface SqlDatabase { + run(sql: string, params?: unknown[]): void; + export(): Uint8Array; + close(): void; +} +type SqlJs = { Database: new (data?: Uint8Array) => SqlDatabase }; + +/** sql.js is the SQLite build ai-hist ships; resolve it through ai-hist so the two always agree. */ +async function loadSqlJs(): Promise { + const fromAiHist = createRequire(createRequire(import.meta.url).resolve('ai-hist/package.json')); + const init = fromAiHist('sql.js') as (config?: unknown) => Promise; + return init(); +} + +function normalizeFinding(finding: MemoryFinding): Required { + const text = (value: unknown, field: string): string => { + if (typeof value !== 'string' || value.trim() === '') { + throw new AuthoredFlowExecutionError('memory_finding_invalid', `f.memory.learn requires a non-empty string ${field}`); + } + return value; + }; + const alternatives = finding?.alternatives ?? []; + if (!Array.isArray(alternatives) || alternatives.some(item => typeof item !== 'string')) { + throw new AuthoredFlowExecutionError('memory_finding_invalid', 'f.memory.learn alternatives must be an array of strings'); + } + return { + question: text(finding?.question, 'question'), + chosen: text(finding?.chosen, 'chosen'), + reasoning: text(finding?.reasoning, 'reasoning'), + alternatives: [...alternatives], + }; +} + +async function writeAtomic(path: string, data: string | Uint8Array): Promise { + const temp = `${path}.${process.pid}.${randomUUID()}.tmp`; + await writeFile(temp, data); + await rename(temp, path); +} + +/** + * Persist one finding so later runs of the same flow can recall it. + * + * Two idempotent writes, both scoped to the flow's script scope: + * - a compacted trajectory file under `/.trajectories/compacted/`, the + * format ai-hist ingests, so a later `ai-hist sync` re-indexes it; + * - the matching `trajectories` and `history` rows in the ai-hist database, + * mirroring ai-hist's own ingestion, so the next run sees it without a sync. + */ +async function persistFinding(dbPath: string, scope: string, decision: Required): Promise { + const now = new Date(); + const timestampMs = now.getTime(); + // Content-addressed: re-learning the same finding (a resumed body re-running + // its effects, or a later run reaching the same conclusion) upserts one record. + const id = `flows-memory-${createHash('sha256').update(JSON.stringify([scope, decision])).digest('hex').slice(0, 32)}`; + const retrospective = { summary: null, approach: null, learnings: [] as string[], confidence: null }; + const trajectory = { + id, + version: 1, + type: 'compacted', + projectId: scope, + task: { title: decision.question, description: null }, + status: 'completed', + startedAt: now.toISOString(), + completedAt: now.toISOString(), + decisions: [decision], + retrospective, + }; + const file = join(scope, '.trajectories', 'compacted', `${id}.json`); + await mkdir(dirname(file), { recursive: true }); + await writeAtomic(file, `${JSON.stringify(trajectory, null, 2)}\n`); + + // Same field order ai-hist's buildSearchText uses, so search ranks identically after a sync. + const searchText = [ + id, scope, trajectory.status, decision.question, + decision.question, decision.chosen, decision.reasoning, ...decision.alternatives, + ].filter(part => part.length > 0).join('\n'); + const SQL = await loadSqlJs(); + const db = new SQL.Database(await readFile(dbPath)); + try { + db.run(`CREATE TABLE IF NOT EXISTS trajectories ( + id TEXT PRIMARY KEY, version INTEGER, persona_id TEXT, project_id TEXT, + task_title TEXT, task_description TEXT, status TEXT, started_at TEXT, + completed_at TEXT, decisions_json TEXT NOT NULL, retrospective_json TEXT NOT NULL, + search_text TEXT NOT NULL, path TEXT, updated_ms INTEGER NOT NULL, timestamp_ms INTEGER NOT NULL + )`); + db.run(`INSERT OR REPLACE INTO trajectories + (id, version, persona_id, project_id, task_title, task_description, status, + started_at, completed_at, decisions_json, retrospective_json, search_text, + path, updated_ms, timestamp_ms) + VALUES (?, 1, NULL, ?, ?, NULL, 'completed', ?, ?, ?, ?, ?, ?, ?, ?)`, [ + id, scope, decision.question, trajectory.startedAt, trajectory.completedAt, + JSON.stringify([decision]), JSON.stringify(retrospective), searchText, file, timestampMs, timestampMs, + ]); + db.run(`DELETE FROM history WHERE source = 'trajectory' AND session_id = ?`, [id]); + db.run(`INSERT OR IGNORE INTO history (source, session_id, project, prompt, timestamp_ms, git_branch) + VALUES ('trajectory', ?, ?, ?, ?, NULL)`, [id, scope, searchText, timestampMs]); + await writeAtomic(dbPath, db.export()); + } finally { + db.close(); + } +} diff --git a/packages/sdk/tests/f-memory.test.ts b/packages/sdk/tests/f-memory.test.ts index 1c57e3df4..eb7cc7f71 100644 --- a/packages/sdk/tests/f-memory.test.ts +++ b/packages/sdk/tests/f-memory.test.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process'; -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { chmodSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -94,11 +94,83 @@ it('maps an unavailable provider probe to memory_unreachable', async () => { .toMatchObject({ severity: 'refusal', kind: 'memory_unreachable' }); }); -it('fails closed for deferred writes, agent scope, disabled script memory, and reads after done', async () => { - await expect(authoredMemory(scope, () => {}, true).learn({ question: 'q', chosen: 'a', reasoning: 'r' })) - .rejects.toMatchObject({ code: 'unsupported_verb' }); +function completingJournal(): JournalClient { + const journal = new JournalClient('/unused'); + vi.spyOn(journal, 'runStart').mockResolvedValue({ + run_id: 'completion', status: 'completed', completion_reason: 'success', completed_steps: 1, + }); + vi.spyOn(journal, 'journalRead').mockResolvedValue({ entries: [{ + entry_type: 'step.completed', step_id: 'complete-1', payload: { + completionReason: 'success', disposition: 'step_done', + output: { exit_code: 0, stdout_tail: '', stderr_tail: '' }, + }, + }] } as never); + return journal; +} + +it('persists a learned finding that a later run of the same flow recalls', async () => { + const flowPath = join(dir, 'test.flow.ts'); + const finding = { + question: 'how should the webhook retry', chosen: 'exponential backoff', + reasoning: 'the provider rate-limits bursts', alternatives: ['fixed delay'], + }; + await executeAuthoredFlow(flow('memory-example', { memory: { script: true } }, async f => { + expect(await f.memory.why('webhook retry')).toEqual([]); + await f.memory.learn(finding); + f.done('success'); + }), completingJournal(), undefined, { flowPath }); + + let recalled: unknown[] = []; + let why: unknown[] = []; + await executeAuthoredFlow(flow('memory-example', { memory: { script: true } }, async f => { + recalled = await f.memory.recall('exponential backoff'); + why = await f.memory.why('webhook retry'); + f.done('success'); + }), completingJournal(), undefined, { flowPath }); + expect(recalled).toMatchObject([{ source: 'trajectory', project: scope }]); + expect(why).toMatchObject([{ projectId: scope, decisions: [finding] }]); + + // The trajectory file is what a later `ai-hist sync` re-ingests. + const compacted = join(scope, '.trajectories', 'compacted'); + const [file] = readdirSync(compacted); + expect(JSON.parse(readFileSync(join(compacted, file!), 'utf8'))).toMatchObject({ projectId: scope, decisions: [finding] }); +}); + +it('learns idempotently and only into its own script scope', async () => { + const memory = authoredMemory(scope, () => {}, true); + const finding = { question: 'cache invalidation', chosen: 'ttl', reasoning: 'simple' }; + await memory.learn(finding); + await memory.learn(finding); + expect(await memory.recall('cache invalidation')).toHaveLength(1); + expect(readdirSync(join(scope, '.trajectories', 'compacted'))).toHaveLength(1); + expect(await memory.recall('retry safely')).toHaveLength(1); + + const other = authoredMemory(scriptMemoryScope(join(dir, 'test.flow.ts'), 'another-flow'), () => {}, true); + expect(await other.recall('cache invalidation')).toEqual([]); + expect(await other.why('cache invalidation')).toEqual([]); +}); + +it('rejects invalid findings before writing, and unwritable stores', async () => { + const memory = authoredMemory(scope, () => {}, true); + await expect(memory.learn({ question: '', chosen: 'a', reasoning: 'r' })) + .rejects.toMatchObject({ code: 'memory_finding_invalid' }); + await expect(memory.learn({ question: 'q', chosen: 'a', reasoning: 'r', alternatives: [1] } as never)) + .rejects.toMatchObject({ code: 'memory_finding_invalid' }); + expect(existsSync(join(scope, '.trajectories'))).toBe(false); + if (process.getuid?.() !== 0) { + chmodSync(dir, 0o500); + try { + await expect(memory.learn({ question: 'q', chosen: 'a', reasoning: 'r' })) + .rejects.toMatchObject({ code: 'memory_unwritable' }); + } finally { chmodSync(dir, 0o700); } + } +}); + +it('fails closed for agent scope, disabled script memory, and memory use after done', async () => { await expect(authoredMemory(scope, () => {}, false).recall('q')) .rejects.toMatchObject({ code: 'unsupported_header' }); + await expect(authoredMemory(scope, () => {}, false).learn({ question: 'q', chosen: 'a', reasoning: 'r' })) + .rejects.toMatchObject({ code: 'unsupported_header' }); await expect(executeAuthoredFlow(flow('agent', { memory: { agent: true } }, async f => f.done('success')), new JournalClient('/unused'))) .rejects.toMatchObject({ code: 'unsupported_header' }); await expect(executeAuthoredFlow(flow('closed', async f => { diff --git a/packages/surface/AUTHORING.md b/packages/surface/AUTHORING.md index 2f342d720..cd1577a77 100644 --- a/packages/surface/AUTHORING.md +++ b/packages/surface/AUTHORING.md @@ -211,6 +211,14 @@ export interface AgentOptions { * `transport: 'relay'`. See docs/SURFACE.md. */ cwd?: string; + /** + * Continue a CLI session instead of starting a new one: an earlier step's + * `AgentResult.sessionId`. Claude and Codex resume it from their own + * session store (`~/.claude`, `~/.codex`), so it works only where that + * store survives — such as a sandbox reused for the same pull request. + * Other CLIs refuse the step. Not supported with `transport: 'relay'`. + */ + resume?: string; /** * Dispatch transport (flows#385). `'direct'` (default) spawns the CLI as a * local subprocess. `'relay'` posts to agent-relay so the agent registers @@ -253,6 +261,12 @@ export interface AgentResult { * final message is a JSON object (that object is the output, unmodified). */ artifacts: string[]; + /** + * The CLI session this step ran as (Claude's session id, Codex's thread + * id), when the CLI reported one. Pass it as a later step's `resume` to + * continue the same conversation. + */ + sessionId?: string; } ``` @@ -1276,7 +1290,7 @@ export interface MemoryHelper { recall(query: string, options?: MemoryRecallOptions): Promise; /** Best matching decision trajectory, or an empty array. No journal step. */ why(task: string): Promise; - /** Reserved for the journal-backed write slice; currently refuses. */ + /** Persists a finding to this flow's script memory; later runs see it via recall/why. Idempotent per finding; no journal step. */ learn(finding: MemoryFinding): Promise; } ``` diff --git a/packages/surface/src/memory.ts b/packages/surface/src/memory.ts index e6742806c..e9d8ef1e9 100644 --- a/packages/surface/src/memory.ts +++ b/packages/surface/src/memory.ts @@ -15,6 +15,6 @@ export interface MemoryHelper { recall(query: string, options?: MemoryRecallOptions): Promise; /** Best matching decision trajectory, or an empty array. No journal step. */ why(task: string): Promise; - /** Reserved for the journal-backed write slice; currently refuses. */ + /** Persists a finding to this flow's script memory; later runs see it via recall/why. Idempotent per finding; no journal step. */ learn(finding: MemoryFinding): Promise; } diff --git a/testdata/memory/README.md b/testdata/memory/README.md index edcec7dbe..27fa9fd84 100644 --- a/testdata/memory/README.md +++ b/testdata/memory/README.md @@ -1,7 +1,7 @@ # Minimal local script-memory example -This slice implements `f.memory.recall` and `f.memory.why` through ai-hist -0.4.1. Reads have no journal steps. `memory: { script: true }` is already part +This slice implements `f.memory.recall`, `f.memory.why`, and `f.memory.learn` +through ai-hist 0.4.1. None of them journal a step. `memory: { script: true }` is already part of the surface header contract and now works in the internal authored executor. `seed.mjs` creates a new deterministic SQLite database with two script scopes, @@ -42,10 +42,15 @@ than ai-hist's automatic scan of unrelated local JSONL history. ## Deferred acceptance work -- `learn` and journal effect record/confirm idempotency. ai-hist 0.4.1 exposes - reads and tag writes, but no public trajectory writer. The authored executor - also lacks a durable root run ID. `learn` explicitly refuses until both seams - are available; no unjournaled write is substituted. +- Journaled `learn` effects. ai-hist 0.4.1 has no public trajectory writer, + so `learn` writes the finding twice under the script scope: a compacted + trajectory file at `/.trajectories/compacted/.json` (the format + `ai-hist sync` ingests), and the matching `trajectories`/`history` rows in the + ai-hist database (atomic file replace) so the next run recalls it without a + sync. The id is a hash of scope and finding, so a resumed body re-running + `learn`, or a later run learning the same thing, upserts one record. It is + still not a journaled effect, and a concurrent `ai-hist sync` writing the same + database can race the row write; the trajectory file survives either way. - Identity-scoped agent context injection. `memory.agent: true` explicitly refuses; it must not silently read script scope. - CLI-only provider operation and automatic creation of a fresh database. @@ -54,5 +59,5 @@ than ai-hist's automatic scan of unrelated local JSONL history. - Automatic trajectory import and the behavioural Gate 5 acceptance example. - Cloud push and pair mode, per the original exclusions. -This is a read-only proof slice, not completion of Gate 5 or the full original +This is a local proof slice, not completion of Gate 5 or the full original recall/why/learn acceptance matrix. The lead owns PR creation and CI review. From 317a6b904eabc52e7eb6c0d152543305197e06b5 Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Fri, 9 Oct 2026 20:32:17 -0700 Subject: [PATCH 2/8] fix(sdk): harden f.memory.learn persistence per review - Serialize the DB read-modify-replace: per-path promise chain in-process plus an exclusive .flows-memory.lock across processes (stale after 30s). - Stage the trajectory file and DB image as temp files; publish the file only once the DB image is built, and remove it if the DB replace fails, so a rejected learn leaves nothing for ai-hist sync to index. - Preserve the DB's file mode on replace (a 0600 history stays 0600). - Migrate a missing history.git_branch before inserting (pre-handoff DBs). - Report memory_finding_invalid / memory_unwritable as their own run and resume diagnostic kinds instead of protocol_error. - docs/SURFACE.md: learn is supported; describe its guarantees. Co-Authored-By: Claude Opus 5.5 (1M context) Session-Id: b6cf20aa-bdbb-4248-91ab-d2a4fd2ce1b8 --- docs/SURFACE.md | 15 +++- packages/sdk/src/authored-memory.ts | 117 +++++++++++++++++++++------- packages/sdk/src/cli/direct-run.ts | 5 ++ packages/sdk/src/cli/run.ts | 25 ++++++ packages/sdk/src/failure-kinds.ts | 4 + packages/sdk/tests/f-memory.test.ts | 82 ++++++++++++++++++- testdata/memory/README.md | 10 ++- 7 files changed, 222 insertions(+), 36 deletions(-) diff --git a/docs/SURFACE.md b/docs/SURFACE.md index 13e6d03d3..4f665c0cd 100644 --- a/docs/SURFACE.md +++ b/docs/SURFACE.md @@ -128,15 +128,22 @@ No process runs between events: the handler wakes, executes to its next await, p `close-merge-request` in addition to comments and discussions; the earlier comment-only gap was closed by the pinned relay-helpers release. - The initial local memory slice supports `recall` and `why` in authored flows, - with no journal step for either read. Script scope is stable across runs of + The local memory slice supports `recall`, `why`, and `learn` in authored + flows, with no journal step for any of them. Script scope is stable across runs of the same flow file and name; reads cannot widen it to another flow. The existing `memory: { script: true }` header enables an eager reachability check; direct `.memory` use also triggers it. Aliased access is checked at call time. The local Node SDK and an existing readable SQLite DB are required (`AI_HIST_DB` overrides `defaultDbPath()`); JSONL fallback is disabled. - `learn` and `memory: { agent: true }` refuse pending the journal-backed write - and identity-scoped agent follow-ups. CLI-only operation is also deferred. + `learn(finding)` persists the finding under the same script scope: a + compacted trajectory file at `/.trajectories/compacted/.json` + (which `ai-hist sync` ingests) plus the matching rows in the ai-hist DB, so + the next run recalls it. The id hashes scope and finding, so re-learning the + same finding upserts one record. Writes are serialized by a lock file next + to the DB, keep the DB's file mode, and leave nothing behind when they fail + (`memory_finding_invalid`, `memory_unwritable`). `learn` is not yet a + journaled effect. `memory: { agent: true }` refuses pending the + identity-scoped agent follow-up. CLI-only operation is also deferred. 4. **`{{prev}}` / return-value chaining.** Output flows downward implicitly; naming steps is for reaching back, not bookkeeping. 5. **Headers are optional escalation.** identity, memory, budget, tools appear only when used. The empty header is the common case. [Budget headers and spend](BUDGET.md) specifies parsing, prices, journal attribution, and admission limits. diff --git a/packages/sdk/src/authored-memory.ts b/packages/sdk/src/authored-memory.ts index d5990d250..3563ff027 100644 --- a/packages/sdk/src/authored-memory.ts +++ b/packages/sdk/src/authored-memory.ts @@ -1,7 +1,7 @@ import type { MemoryHelper } from '@relayflows/surface'; import type { MemoryFinding } from '@relayflows/surface'; import { createHash, randomUUID } from 'node:crypto'; -import { access, mkdir, readFile, rename, stat, writeFile } from 'node:fs/promises'; +import { access, chmod, mkdir, open, readFile, rename, rm, stat, writeFile } from 'node:fs/promises'; import { constants } from 'node:fs'; import { createRequire } from 'node:module'; import { dirname, join, resolve } from 'node:path'; @@ -67,6 +67,7 @@ export function authoredMemory( interface SqlDatabase { run(sql: string, params?: unknown[]): void; + exec(sql: string): Array<{ columns: string[]; values: unknown[][] }>; export(): Uint8Array; close(): void; } @@ -98,10 +99,42 @@ function normalizeFinding(finding: MemoryFinding): Required { }; } -async function writeAtomic(path: string, data: string | Uint8Array): Promise { - const temp = `${path}.${process.pid}.${randomUUID()}.tmp`; - await writeFile(temp, data); - await rename(temp, path); +const tempPath = (path: string): string => `${path}.${process.pid}.${randomUUID()}.tmp`; + +const LOCK_WAIT_MS = 10_000; +const LOCK_STALE_MS = 30_000; +const inProcess = new Map>(); + +/** + * Serialize read-modify-replace of one database file: a promise chain within + * this process, and an exclusive lock file across flows processes. A lock left + * by a crashed writer is broken once it is older than LOCK_STALE_MS. Writers + * that do not take this lock (`ai-hist sync`) are not serialized against it. + */ +async function withDatabaseLock(dbPath: string, task: () => Promise): Promise { + const previous = inProcess.get(dbPath) ?? Promise.resolve(); + const run = previous.catch(() => {}).then(async () => { + const lock = `${dbPath}.flows-memory.lock`; + const deadline = Date.now() + LOCK_WAIT_MS; + for (;;) { + try { + await (await open(lock, 'wx')).close(); + break; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; + const held = await stat(lock).catch(() => undefined); + if (held !== undefined && Date.now() - held.mtimeMs > LOCK_STALE_MS) { + await rm(lock, { force: true }); + continue; + } + if (Date.now() > deadline) throw new Error(`timed out waiting for ${lock}`); + await new Promise(resolve => setTimeout(resolve, 25)); + } + } + try { return await task(); } finally { await rm(lock, { force: true }); } + }); + inProcess.set(dbPath, run); + try { return await run; } finally { if (inProcess.get(dbPath) === run) inProcess.delete(dbPath); } } /** @@ -112,6 +145,10 @@ async function writeAtomic(path: string, data: string | Uint8Array): Promise): Promise { const now = new Date(); @@ -133,36 +170,60 @@ async function persistFinding(dbPath: string, scope: string, decision: Required< retrospective, }; const file = join(scope, '.trajectories', 'compacted', `${id}.json`); - await mkdir(dirname(file), { recursive: true }); - await writeAtomic(file, `${JSON.stringify(trajectory, null, 2)}\n`); - // Same field order ai-hist's buildSearchText uses, so search ranks identically after a sync. const searchText = [ id, scope, trajectory.status, decision.question, decision.question, decision.chosen, decision.reasoning, ...decision.alternatives, ].filter(part => part.length > 0).join('\n'); const SQL = await loadSqlJs(); - const db = new SQL.Database(await readFile(dbPath)); + + await mkdir(dirname(file), { recursive: true }); + const stagedFile = tempPath(file); + let stagedDb: string | undefined; try { - db.run(`CREATE TABLE IF NOT EXISTS trajectories ( - id TEXT PRIMARY KEY, version INTEGER, persona_id TEXT, project_id TEXT, - task_title TEXT, task_description TEXT, status TEXT, started_at TEXT, - completed_at TEXT, decisions_json TEXT NOT NULL, retrospective_json TEXT NOT NULL, - search_text TEXT NOT NULL, path TEXT, updated_ms INTEGER NOT NULL, timestamp_ms INTEGER NOT NULL - )`); - db.run(`INSERT OR REPLACE INTO trajectories - (id, version, persona_id, project_id, task_title, task_description, status, - started_at, completed_at, decisions_json, retrospective_json, search_text, - path, updated_ms, timestamp_ms) - VALUES (?, 1, NULL, ?, ?, NULL, 'completed', ?, ?, ?, ?, ?, ?, ?, ?)`, [ - id, scope, decision.question, trajectory.startedAt, trajectory.completedAt, - JSON.stringify([decision]), JSON.stringify(retrospective), searchText, file, timestampMs, timestampMs, - ]); - db.run(`DELETE FROM history WHERE source = 'trajectory' AND session_id = ?`, [id]); - db.run(`INSERT OR IGNORE INTO history (source, session_id, project, prompt, timestamp_ms, git_branch) - VALUES ('trajectory', ?, ?, ?, ?, NULL)`, [id, scope, searchText, timestampMs]); - await writeAtomic(dbPath, db.export()); + await writeFile(stagedFile, `${JSON.stringify(trajectory, null, 2)}\n`); + await withDatabaseLock(dbPath, async () => { + // Keep the database's own permissions: a 0600 history file must not become world-readable. + const mode = (await stat(dbPath)).mode & 0o777; + const db = new SQL.Database(await readFile(dbPath)); + try { + db.run(`CREATE TABLE IF NOT EXISTS trajectories ( + id TEXT PRIMARY KEY, version INTEGER, persona_id TEXT, project_id TEXT, + task_title TEXT, task_description TEXT, status TEXT, started_at TEXT, + completed_at TEXT, decisions_json TEXT NOT NULL, retrospective_json TEXT NOT NULL, + search_text TEXT NOT NULL, path TEXT, updated_ms INTEGER NOT NULL, timestamp_ms INTEGER NOT NULL + )`); + // ai-hist adds this column only to its in-memory copy of pre-handoff databases. + const historyColumns = db.exec('PRAGMA table_info(history)')[0]?.values.map(row => row[1]) ?? []; + if (!historyColumns.includes('git_branch')) db.run('ALTER TABLE history ADD COLUMN git_branch TEXT'); + db.run(`INSERT OR REPLACE INTO trajectories + (id, version, persona_id, project_id, task_title, task_description, status, + started_at, completed_at, decisions_json, retrospective_json, search_text, + path, updated_ms, timestamp_ms) + VALUES (?, 1, NULL, ?, ?, NULL, 'completed', ?, ?, ?, ?, ?, ?, ?, ?)`, [ + id, scope, decision.question, trajectory.startedAt, trajectory.completedAt, + JSON.stringify([decision]), JSON.stringify(retrospective), searchText, file, timestampMs, timestampMs, + ]); + db.run(`DELETE FROM history WHERE source = 'trajectory' AND session_id = ?`, [id]); + db.run(`INSERT OR IGNORE INTO history (source, session_id, project, prompt, timestamp_ms, git_branch) + VALUES ('trajectory', ?, ?, ?, ?, NULL)`, [id, scope, searchText, timestampMs]); + stagedDb = tempPath(dbPath); + await writeFile(stagedDb, db.export(), { mode }); + await chmod(stagedDb, mode); + } finally { + db.close(); + } + const existed = await stat(file).then(() => true, () => false); + await rename(stagedFile, file); + try { + await rename(stagedDb, dbPath); + } catch (error) { + if (!existed) await rm(file, { force: true }); + throw error; + } + }); } finally { - db.close(); + await rm(stagedFile, { force: true }); + if (stagedDb !== undefined) await rm(stagedDb, { force: true }); } } diff --git a/packages/sdk/src/cli/direct-run.ts b/packages/sdk/src/cli/direct-run.ts index cca83dedd..3ad797bc4 100644 --- a/packages/sdk/src/cli/direct-run.ts +++ b/packages/sdk/src/cli/direct-run.ts @@ -27,6 +27,7 @@ import { connect, emptyReport, fromCheckReport, + memoryWriteFailure, protocolFailure, suspendedExecution, socketFor, @@ -219,6 +220,10 @@ export async function runDirectFlow( if (error instanceof AuthoredFlowExecutionError && error.code === 'result_unreadable') { return completedResultUnreadableReport('run', base, socketPath, error); } + if (error instanceof AuthoredFlowExecutionError) { + const memoryFailure = memoryWriteFailure('run', base, socketPath, error); + if (memoryFailure !== undefined) return memoryFailure; + } if (isReadInterruption(error)) return daemonUnresponsiveReport('run', base, socketPath, error, base.rootRunId, options, dataDir); if (error instanceof AuthoredFlowExecutionError && error.code === 'root_lease_lost') { return rootLeaseLostReport('run', base, socketPath, error); diff --git a/packages/sdk/src/cli/run.ts b/packages/sdk/src/cli/run.ts index 0a76bc242..4c10483c2 100644 --- a/packages/sdk/src/cli/run.ts +++ b/packages/sdk/src/cli/run.ts @@ -418,6 +418,10 @@ export async function resumeFlow( if (error instanceof AuthoredFlowExecutionError && error.code === 'result_unreadable') { return completedResultUnreadableReport('resume', base, socketPath, error); } + if (error instanceof AuthoredFlowExecutionError) { + const memoryFailure = memoryWriteFailure('resume', base, socketPath, error, runId); + if (memoryFailure !== undefined) return memoryFailure; + } if (isReadInterruption(error)) return daemonUnresponsiveReport('resume', base, socketPath, error, runId, options, dataDir); if (error instanceof AuthoredFlowExecutionError && error.code === 'root_lease_lost') { return rootLeaseLostReport('resume', base, socketPath, error, runId); @@ -1098,6 +1102,27 @@ async function inspectOutOfBandStep( const EXPIRED_LEASE_POLL_MS = 250; +/** `f.memory.learn` failures keep their own kind instead of collapsing into `protocol_error`. */ +export function memoryWriteFailure( + command: RunCommand, + base: CheckReport | RunReport, + socketPath: string, + error: AuthoredFlowExecutionError, + runId?: string, +): RunExecution | undefined { + if (error.code !== 'memory_finding_invalid' && error.code !== 'memory_unwritable') return undefined; + const id = runId ?? error.runId; + return { + exitCode: 1, + report: { + ...fromBase(command, base), + ...(id !== undefined ? { runId: id } : {}), + socketPath, + diagnostics: [...base.diagnostics, { severity: 'failure', kind: error.code, message: error.message }], + }, + }; +} + export function protocolFailure( command: RunCommand, base: CheckReport | RunReport, diff --git a/packages/sdk/src/failure-kinds.ts b/packages/sdk/src/failure-kinds.ts index bb1e23cd7..d5d5c81c4 100644 --- a/packages/sdk/src/failure-kinds.ts +++ b/packages/sdk/src/failure-kinds.ts @@ -138,6 +138,10 @@ export const RUN_FAILURE_KINDS = [ 'gate_failed', /** `flows answer` named a wait the run is not asking: unknown, or already answered. */ 'human_wait_unknown', + /** `f.memory.learn` was given a finding without non-empty string fields. */ + 'memory_finding_invalid', + /** `f.memory.learn` could not persist the finding to script memory. */ + 'memory_unwritable', /** * `--local-agent` cannot be honoured for this invocation, so it is refused * rather than accepted and ignored. A local agent worker is admitted at run diff --git a/packages/sdk/tests/f-memory.test.ts b/packages/sdk/tests/f-memory.test.ts index eb7cc7f71..afcf1041d 100644 --- a/packages/sdk/tests/f-memory.test.ts +++ b/packages/sdk/tests/f-memory.test.ts @@ -1,13 +1,28 @@ import { execFileSync } from 'node:child_process'; -import { chmodSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { chmodSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { flow } from '@relayflows/surface'; import { afterEach, beforeEach, expect, it, vi } from 'vitest'; + +let failRenameTo: string | undefined; +vi.mock('node:fs/promises', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + rename: async (from: string, to: string) => { + if (to === failRenameTo) throw Object.assign(new Error('injected rename failure'), { code: 'EIO' }); + return actual.rename(from, to); + }, + }; +}); import { openAiHist } from 'ai-hist'; import { authoredMemory, scriptMemoryScope } from '../src/authored-memory.js'; import { executeAuthoredFlow } from '../src/authored-flow-executor.js'; +import { AuthoredFlowExecutionError } from '../src/authored-flow-error.js'; +import { emptyReport, memoryWriteFailure } from '../src/cli/run.js'; import { JournalClient } from '../src/journal-client.js'; import { preflightMemory } from '../src/preflight.js'; @@ -166,6 +181,71 @@ it('rejects invalid findings before writing, and unwritable stores', async () => } }); +it('serializes concurrent learns so no finding is lost from the database', async () => { + const memory = authoredMemory(scope, () => {}, true); + await Promise.all(Array.from({ length: 6 }, (_, i) => + memory.learn({ question: `parallel finding ${i}`, chosen: 'c', reasoning: 'r' }))); + expect(await memory.recall('parallel finding')).toHaveLength(6); + expect(existsSync(`${dbPath}.flows-memory.lock`)).toBe(false); +}); + +it('waits for a live lock and breaks a stale one', async () => { + const lock = `${dbPath}.flows-memory.lock`; + writeFileSync(lock, ''); + const memory = authoredMemory(scope, () => {}, true); + let settled = false; + const pending = memory.learn({ question: 'locked finding', chosen: 'c', reasoning: 'r' }).then(() => { settled = true; }); + await new Promise(resolve => setTimeout(resolve, 150)); + expect(settled).toBe(false); + rmSync(lock); + await pending; + expect(await memory.recall('locked finding')).toHaveLength(1); + + writeFileSync(lock, ''); + const { utimesSync } = await import('node:fs'); + utimesSync(lock, new Date(0), new Date(0)); + await memory.learn({ question: 'after stale lock', chosen: 'c', reasoning: 'r' }); + expect(await memory.recall('after stale lock')).toHaveLength(1); +}); + +it.skipIf(process.platform === 'win32')('keeps the database file mode', async () => { + chmodSync(dbPath, 0o600); + await authoredMemory(scope, () => {}, true).learn({ question: 'private', chosen: 'c', reasoning: 'r' }); + expect(statSync(dbPath).mode & 0o777).toBe(0o600); +}); + +it('migrates a pre-handoff history table without git_branch', async () => { + const require = createRequire(import.meta.url); + const SQL = await createRequire(require.resolve('ai-hist/package.json'))('sql.js')(); + const db = new SQL.Database(); + db.run('CREATE TABLE history (id INTEGER PRIMARY KEY, source TEXT, session_id TEXT, project TEXT, prompt TEXT, timestamp_ms INTEGER)'); + writeFileSync(dbPath, Buffer.from(db.export())); + db.close(); + const memory = authoredMemory(scope, () => {}, true); + await memory.learn({ question: 'old database', chosen: 'c', reasoning: 'r' }); + expect(await memory.recall('old database')).toMatchObject([{ source: 'trajectory', gitBranch: null }]); +}); + +it('removes the trajectory file when the database replace fails', async () => { + const memory = authoredMemory(scope, () => {}, true); + failRenameTo = dbPath; + try { + await expect(memory.learn({ question: 'doomed', chosen: 'c', reasoning: 'r' })) + .rejects.toMatchObject({ code: 'memory_unwritable' }); + } finally { failRenameTo = undefined; } + expect(readdirSync(join(scope, '.trajectories', 'compacted'))).toEqual([]); + expect(readdirSync(dir).filter(name => name.endsWith('.tmp') || name.endsWith('.lock'))).toEqual([]); + expect(await memory.recall('doomed')).toEqual([]); +}); + +it('reports learn failures with their own kind, not protocol_error', () => { + for (const code of ['memory_finding_invalid', 'memory_unwritable'] as const) { + const execution = memoryWriteFailure('run', emptyReport('run'), '/sock', new AuthoredFlowExecutionError(code, 'nope'), 'run-1'); + expect(execution).toMatchObject({ exitCode: 1, report: { runId: 'run-1', diagnostics: [{ severity: 'failure', kind: code }] } }); + } + expect(memoryWriteFailure('run', emptyReport('run'), '/sock', new AuthoredFlowExecutionError('step_failed', 'x'))).toBeUndefined(); +}); + it('fails closed for agent scope, disabled script memory, and memory use after done', async () => { await expect(authoredMemory(scope, () => {}, false).recall('q')) .rejects.toMatchObject({ code: 'unsupported_header' }); diff --git a/testdata/memory/README.md b/testdata/memory/README.md index 27fa9fd84..3fafcfe3c 100644 --- a/testdata/memory/README.md +++ b/testdata/memory/README.md @@ -48,9 +48,13 @@ than ai-hist's automatic scan of unrelated local JSONL history. `ai-hist sync` ingests), and the matching `trajectories`/`history` rows in the ai-hist database (atomic file replace) so the next run recalls it without a sync. The id is a hash of scope and finding, so a resumed body re-running - `learn`, or a later run learning the same thing, upserts one record. It is - still not a journaled effect, and a concurrent `ai-hist sync` writing the same - database can race the row write; the trajectory file survives either way. + `learn`, or a later run learning the same thing, upserts one record. Writes + are serialized in-process and across flows processes by + `.flows-memory.lock` (stale after 30s), keep the database's file mode, + migrate a missing `history.git_branch`, and roll back the trajectory file if + the database replace fails. It is still not a journaled effect, and + `ai-hist sync` does not take the lock, so a concurrent sync can race the row + write; the trajectory file survives and the sync re-ingests it. - Identity-scoped agent context injection. `memory.agent: true` explicitly refuses; it must not silently read script scope. - CLI-only provider operation and automatic creation of a fresh database. From fdab097602ec5635728ee28bccfc75579a159356 Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Fri, 9 Oct 2026 20:40:55 -0700 Subject: [PATCH 3/8] fix(sdk): atomic stale-lock takeover; report the root on memory failures - Lock files carry an owner {pid, host, token} and are published with link(), so they appear whole and never replace another lock. A lock is stale only when its owner pid is gone on this host (5-minute age backstop otherwise). Takeover renames the lock aside before removing it, so two breakers cannot both remove it, and a breaker that moved a live lock links it back. The writer re-checks its token before committing and releases only its own lock. - memoryWriteFailure reports the terminalized root (runId/rootRunId, status failed, completionReason step_failed); direct-run passes base.rootRunId so the plain CLI prints its RUN summary. - Lock-wait test synchronizes on the writer's staged lock instead of a delay; add a dead-owner takeover test. - SURFACE.md: say the lock serializes learn writers only, not ai-hist sync. Co-Authored-By: Claude Opus 5.5 (1M context) Session-Id: b6cf20aa-bdbb-4248-91ab-d2a4fd2ce1b8 --- docs/SURFACE.md | 9 ++- packages/sdk/src/authored-memory.ts | 87 +++++++++++++++++++++++------ packages/sdk/src/cli/direct-run.ts | 2 +- packages/sdk/src/cli/run.ts | 17 ++++-- packages/sdk/tests/f-memory.test.ts | 41 ++++++++++---- testdata/memory/README.md | 3 +- 6 files changed, 123 insertions(+), 36 deletions(-) diff --git a/docs/SURFACE.md b/docs/SURFACE.md index 4f665c0cd..107647ffe 100644 --- a/docs/SURFACE.md +++ b/docs/SURFACE.md @@ -139,9 +139,12 @@ No process runs between events: the handler wakes, executes to its next await, p compacted trajectory file at `/.trajectories/compacted/.json` (which `ai-hist sync` ingests) plus the matching rows in the ai-hist DB, so the next run recalls it. The id hashes scope and finding, so re-learning the - same finding upserts one record. Writes are serialized by a lock file next - to the DB, keep the DB's file mode, and leave nothing behind when they fail - (`memory_finding_invalid`, `memory_unwritable`). `learn` is not yet a + same finding upserts one record. `learn` writes are serialized with each + other by a lock file next to the DB; `ai-hist sync` does not take that lock, + so a concurrent sync can race the DB replace (the trajectory file survives + and the next sync re-ingests it). Writes keep the DB's file mode and leave + nothing behind when they fail (`memory_finding_invalid`, + `memory_unwritable`). `learn` is not yet a journaled effect. `memory: { agent: true }` refuses pending the identity-scoped agent follow-up. CLI-only operation is also deferred. diff --git a/packages/sdk/src/authored-memory.ts b/packages/sdk/src/authored-memory.ts index 3563ff027..937741671 100644 --- a/packages/sdk/src/authored-memory.ts +++ b/packages/sdk/src/authored-memory.ts @@ -1,7 +1,8 @@ import type { MemoryHelper } from '@relayflows/surface'; import type { MemoryFinding } from '@relayflows/surface'; import { createHash, randomUUID } from 'node:crypto'; -import { access, chmod, mkdir, open, readFile, rename, rm, stat, writeFile } from 'node:fs/promises'; +import { access, chmod, link, mkdir, readFile, rename, rm, stat, writeFile } from 'node:fs/promises'; +import { hostname } from 'node:os'; import { constants } from 'node:fs'; import { createRequire } from 'node:module'; import { dirname, join, resolve } from 'node:path'; @@ -102,36 +103,87 @@ function normalizeFinding(finding: MemoryFinding): Required { const tempPath = (path: string): string => `${path}.${process.pid}.${randomUUID()}.tmp`; const LOCK_WAIT_MS = 10_000; -const LOCK_STALE_MS = 30_000; +/** Backstop for an owner this host cannot probe (another host, or a reused pid). Writers hold the lock for milliseconds. */ +const LOCK_STALE_MS = 5 * 60_000; const inProcess = new Map>(); +interface LockOwner { pid: number; host: string; token: string } + +async function readLockOwner(path: string): Promise<{ owner?: LockOwner; ageMs: number } | undefined> { + try { + const [raw, info] = await Promise.all([readFile(path, 'utf8'), stat(path)]); + let owner: LockOwner | undefined; + try { owner = JSON.parse(raw) as LockOwner; } catch { owner = undefined; } + return { owner, ageMs: Date.now() - info.mtimeMs }; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined; + throw error; + } +} + +function lockIsStale({ owner, ageMs }: { owner?: LockOwner; ageMs: number }): boolean { + if (owner !== undefined && owner.host === hostname() && Number.isInteger(owner.pid)) { + try { process.kill(owner.pid, 0); } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ESRCH') return true; + } + } + return ageMs > LOCK_STALE_MS; +} + /** * Serialize read-modify-replace of one database file: a promise chain within - * this process, and an exclusive lock file across flows processes. A lock left - * by a crashed writer is broken once it is older than LOCK_STALE_MS. Writers - * that do not take this lock (`ai-hist sync`) are not serialized against it. + * this process, and an exclusive lock file across flows processes. + * + * The lock is published with `link`, so it appears complete or not at all and + * never replaces another lock. It is stale only when its owner process is gone + * (or, as a backstop, after LOCK_STALE_MS). Takeover renames the lock aside + * first, so two breakers cannot both remove it; a breaker that finds it moved + * a live lock links it back. The task gets `assertHeld` to re-check ownership + * right before it commits. Writers that do not take this lock (`ai-hist sync`) + * are not serialized against it. */ -async function withDatabaseLock(dbPath: string, task: () => Promise): Promise { +async function withDatabaseLock(dbPath: string, task: (assertHeld: () => Promise) => Promise): Promise { const previous = inProcess.get(dbPath) ?? Promise.resolve(); const run = previous.catch(() => {}).then(async () => { const lock = `${dbPath}.flows-memory.lock`; + const self: LockOwner = { pid: process.pid, host: hostname(), token: randomUUID() }; + const staged = tempPath(lock); + await writeFile(staged, JSON.stringify(self)); const deadline = Date.now() + LOCK_WAIT_MS; - for (;;) { - try { - await (await open(lock, 'wx')).close(); - break; - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; - const held = await stat(lock).catch(() => undefined); - if (held !== undefined && Date.now() - held.mtimeMs > LOCK_STALE_MS) { - await rm(lock, { force: true }); + try { + for (;;) { + try { + await link(staged, lock); + break; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; + } + const held = await readLockOwner(lock); + if (held !== undefined && lockIsStale(held)) { + const aside = `${lock}.${self.token}.stale`; + try { await rename(lock, aside); } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + continue; + } + const moved = await readLockOwner(aside); + if (moved !== undefined && !lockIsStale(moved)) await link(aside, lock).catch(() => {}); + await rm(aside, { force: true }); continue; } if (Date.now() > deadline) throw new Error(`timed out waiting for ${lock}`); await new Promise(resolve => setTimeout(resolve, 25)); } + } finally { + await rm(staged, { force: true }); + } + const assertHeld = async () => { + if ((await readLockOwner(lock))?.owner?.token !== self.token) throw new Error(`lost ${lock} before commit`); + }; + try { + return await task(assertHeld); + } finally { + if ((await readLockOwner(lock).catch(() => undefined))?.owner?.token === self.token) await rm(lock, { force: true }); } - try { return await task(); } finally { await rm(lock, { force: true }); } }); inProcess.set(dbPath, run); try { return await run; } finally { if (inProcess.get(dbPath) === run) inProcess.delete(dbPath); } @@ -182,7 +234,7 @@ async function persistFinding(dbPath: string, scope: string, decision: Required< let stagedDb: string | undefined; try { await writeFile(stagedFile, `${JSON.stringify(trajectory, null, 2)}\n`); - await withDatabaseLock(dbPath, async () => { + await withDatabaseLock(dbPath, async assertHeld => { // Keep the database's own permissions: a 0600 history file must not become world-readable. const mode = (await stat(dbPath)).mode & 0o777; const db = new SQL.Database(await readFile(dbPath)); @@ -213,6 +265,7 @@ async function persistFinding(dbPath: string, scope: string, decision: Required< } finally { db.close(); } + await assertHeld(); const existed = await stat(file).then(() => true, () => false); await rename(stagedFile, file); try { diff --git a/packages/sdk/src/cli/direct-run.ts b/packages/sdk/src/cli/direct-run.ts index 3ad797bc4..e419d244d 100644 --- a/packages/sdk/src/cli/direct-run.ts +++ b/packages/sdk/src/cli/direct-run.ts @@ -221,7 +221,7 @@ export async function runDirectFlow( return completedResultUnreadableReport('run', base, socketPath, error); } if (error instanceof AuthoredFlowExecutionError) { - const memoryFailure = memoryWriteFailure('run', base, socketPath, error); + const memoryFailure = memoryWriteFailure('run', base, socketPath, error, base.rootRunId); if (memoryFailure !== undefined) return memoryFailure; } if (isReadInterruption(error)) return daemonUnresponsiveReport('run', base, socketPath, error, base.rootRunId, options, dataDir); diff --git a/packages/sdk/src/cli/run.ts b/packages/sdk/src/cli/run.ts index 4c10483c2..f4b2bd1f7 100644 --- a/packages/sdk/src/cli/run.ts +++ b/packages/sdk/src/cli/run.ts @@ -1102,22 +1102,31 @@ async function inspectOutOfBandStep( const EXPIRED_LEASE_POLL_MS = 250; -/** `f.memory.learn` failures keep their own kind instead of collapsing into `protocol_error`. */ +/** + * `f.memory.learn` failures keep their own kind instead of collapsing into + * `protocol_error`. By then the root has been terminalized as failed, so the + * report names it the way a step failure does. + */ export function memoryWriteFailure( command: RunCommand, base: CheckReport | RunReport, socketPath: string, error: AuthoredFlowExecutionError, - runId?: string, + fallbackRunId?: string, ): RunExecution | undefined { if (error.code !== 'memory_finding_invalid' && error.code !== 'memory_unwritable') return undefined; - const id = runId ?? error.runId; + const rootRunId = error.rootRunId ?? fallbackRunId; + const runId = error.runId ?? rootRunId; return { exitCode: 1, report: { ...fromBase(command, base), - ...(id !== undefined ? { runId: id } : {}), + ok: false, + ...(runId === undefined ? {} : { runId }), + ...(rootRunId === undefined ? {} : { rootRunId }), socketPath, + status: 'failed', + completionReason: 'step_failed', diagnostics: [...base.diagnostics, { severity: 'failure', kind: error.code, message: error.message }], }, }; diff --git a/packages/sdk/tests/f-memory.test.ts b/packages/sdk/tests/f-memory.test.ts index afcf1041d..f6832278c 100644 --- a/packages/sdk/tests/f-memory.test.ts +++ b/packages/sdk/tests/f-memory.test.ts @@ -1,7 +1,7 @@ -import { execFileSync } from 'node:child_process'; -import { chmodSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { chmodSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, utimesSync, writeFileSync } from 'node:fs'; import { createRequire } from 'node:module'; -import { tmpdir } from 'node:os'; +import { hostname, tmpdir } from 'node:os'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { flow } from '@relayflows/surface'; @@ -189,23 +189,41 @@ it('serializes concurrent learns so no finding is lost from the database', async expect(existsSync(`${dbPath}.flows-memory.lock`)).toBe(false); }); -it('waits for a live lock and breaks a stale one', async () => { +async function until(predicate: () => boolean): Promise { + for (let i = 0; i < 400 && !predicate(); i += 1) await new Promise(resolve => setTimeout(resolve, 10)); + expect(predicate()).toBe(true); +} + +it('waits on a lock held by a live process, then proceeds once it is released', async () => { const lock = `${dbPath}.flows-memory.lock`; - writeFileSync(lock, ''); + writeFileSync(lock, JSON.stringify({ pid: process.pid, host: hostname(), token: 'another-writer' })); const memory = authoredMemory(scope, () => {}, true); let settled = false; const pending = memory.learn({ question: 'locked finding', chosen: 'c', reasoning: 'r' }).then(() => { settled = true; }); + // The writer stages its own lock file before contending; once it exists, the wait is under way. + await until(() => readdirSync(dir).some(name => name.startsWith('fixture.db.flows-memory.lock.') && name.endsWith('.tmp'))); await new Promise(resolve => setTimeout(resolve, 150)); expect(settled).toBe(false); + expect(JSON.parse(readFileSync(lock, 'utf8'))).toMatchObject({ token: 'another-writer' }); rmSync(lock); await pending; expect(await memory.recall('locked finding')).toHaveLength(1); + expect(existsSync(lock)).toBe(false); +}); + +it('breaks a lock whose owner process is gone, or that is past the age backstop', async () => { + const lock = `${dbPath}.flows-memory.lock`; + const memory = authoredMemory(scope, () => {}, true); + const deadPid = spawnSync(process.execPath, ['-e', '']).pid; + writeFileSync(lock, JSON.stringify({ pid: deadPid, host: hostname(), token: 'crashed-writer' })); + await memory.learn({ question: 'after dead owner', chosen: 'c', reasoning: 'r' }); + expect(await memory.recall('after dead owner')).toHaveLength(1); writeFileSync(lock, ''); - const { utimesSync } = await import('node:fs'); utimesSync(lock, new Date(0), new Date(0)); - await memory.learn({ question: 'after stale lock', chosen: 'c', reasoning: 'r' }); - expect(await memory.recall('after stale lock')).toHaveLength(1); + await memory.learn({ question: 'after aged lock', chosen: 'c', reasoning: 'r' }); + expect(await memory.recall('after aged lock')).toHaveLength(1); + expect(readdirSync(dir).filter(name => name.includes('.flows-memory.lock'))).toEqual([]); }); it.skipIf(process.platform === 'win32')('keeps the database file mode', async () => { @@ -240,8 +258,11 @@ it('removes the trajectory file when the database replace fails', async () => { it('reports learn failures with their own kind, not protocol_error', () => { for (const code of ['memory_finding_invalid', 'memory_unwritable'] as const) { - const execution = memoryWriteFailure('run', emptyReport('run'), '/sock', new AuthoredFlowExecutionError(code, 'nope'), 'run-1'); - expect(execution).toMatchObject({ exitCode: 1, report: { runId: 'run-1', diagnostics: [{ severity: 'failure', kind: code }] } }); + const error = Object.assign(new AuthoredFlowExecutionError(code, 'nope'), { rootRunId: 'root-1' }); + expect(memoryWriteFailure('run', emptyReport('run'), '/sock', error)).toMatchObject({ exitCode: 1, report: { + runId: 'root-1', rootRunId: 'root-1', status: 'failed', completionReason: 'step_failed', + diagnostics: [{ severity: 'failure', kind: code }], + } }); } expect(memoryWriteFailure('run', emptyReport('run'), '/sock', new AuthoredFlowExecutionError('step_failed', 'x'))).toBeUndefined(); }); diff --git a/testdata/memory/README.md b/testdata/memory/README.md index 3fafcfe3c..31690747d 100644 --- a/testdata/memory/README.md +++ b/testdata/memory/README.md @@ -50,7 +50,8 @@ than ai-hist's automatic scan of unrelated local JSONL history. sync. The id is a hash of scope and finding, so a resumed body re-running `learn`, or a later run learning the same thing, upserts one record. Writes are serialized in-process and across flows processes by - `.flows-memory.lock` (stale after 30s), keep the database's file mode, + `.flows-memory.lock` (stale once its owner process is gone, or after 5 + minutes), keep the database's file mode, migrate a missing `history.git_branch`, and roll back the trajectory file if the database replace fails. It is still not a journaled effect, and `ai-hist sync` does not take the lock, so a concurrent sync can race the row From 806963bc482e2544d26f080f5a975ed5172d5f79 Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Fri, 9 Oct 2026 20:46:12 -0700 Subject: [PATCH 4/8] fix(sdk): live local lock owners never expire; trajectory files take the DB's mode - A lock held by a live process on this host is never stale, however old; the 5-minute age backstop applies only to owners this host cannot probe. - The compacted trajectory file is created with the ai-hist DB's mode, so a 0600 history does not leak findings through a 0644 trajectory. Co-Authored-By: Claude Opus 5.5 (1M context) Session-Id: b6cf20aa-bdbb-4248-91ab-d2a4fd2ce1b8 --- packages/sdk/src/authored-memory.ts | 17 ++++++++++++----- packages/sdk/tests/f-memory.test.ts | 6 +++++- testdata/memory/README.md | 4 ++-- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/sdk/src/authored-memory.ts b/packages/sdk/src/authored-memory.ts index 937741671..3b7d1ee93 100644 --- a/packages/sdk/src/authored-memory.ts +++ b/packages/sdk/src/authored-memory.ts @@ -103,7 +103,7 @@ function normalizeFinding(finding: MemoryFinding): Required { const tempPath = (path: string): string => `${path}.${process.pid}.${randomUUID()}.tmp`; const LOCK_WAIT_MS = 10_000; -/** Backstop for an owner this host cannot probe (another host, or a reused pid). Writers hold the lock for milliseconds. */ +/** Backstop for an owner this host cannot probe (another host, or an unreadable lock). Writers hold the lock for milliseconds. */ const LOCK_STALE_MS = 5 * 60_000; const inProcess = new Map>(); @@ -121,10 +121,14 @@ async function readLockOwner(path: string): Promise<{ owner?: LockOwner; ageMs: } } +/** A local owner is judged by whether its process exists, never by age; the age backstop is only for owners this host cannot probe. */ function lockIsStale({ owner, ageMs }: { owner?: LockOwner; ageMs: number }): boolean { - if (owner !== undefined && owner.host === hostname() && Number.isInteger(owner.pid)) { - try { process.kill(owner.pid, 0); } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ESRCH') return true; + if (owner !== undefined && owner.host === hostname() && Number.isInteger(owner.pid) && owner.pid > 0) { + try { + process.kill(owner.pid, 0); + return false; + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'ESRCH'; } } return ageMs > LOCK_STALE_MS; @@ -233,7 +237,10 @@ async function persistFinding(dbPath: string, scope: string, decision: Required< const stagedFile = tempPath(file); let stagedDb: string | undefined; try { - await writeFile(stagedFile, `${JSON.stringify(trajectory, null, 2)}\n`); + // The trajectory holds the same finding as the DB row, so it gets the DB's permissions, not the umask's. + const fileMode = (await stat(dbPath)).mode & 0o777; + await writeFile(stagedFile, `${JSON.stringify(trajectory, null, 2)}\n`, { mode: fileMode }); + await chmod(stagedFile, fileMode); await withDatabaseLock(dbPath, async assertHeld => { // Keep the database's own permissions: a 0600 history file must not become world-readable. const mode = (await stat(dbPath)).mode & 0o777; diff --git a/packages/sdk/tests/f-memory.test.ts b/packages/sdk/tests/f-memory.test.ts index f6832278c..1e4cb6538 100644 --- a/packages/sdk/tests/f-memory.test.ts +++ b/packages/sdk/tests/f-memory.test.ts @@ -197,6 +197,8 @@ async function until(predicate: () => boolean): Promise { it('waits on a lock held by a live process, then proceeds once it is released', async () => { const lock = `${dbPath}.flows-memory.lock`; writeFileSync(lock, JSON.stringify({ pid: process.pid, host: hostname(), token: 'another-writer' })); + // Live local owners never expire by age, however old the lock looks. + utimesSync(lock, new Date(0), new Date(0)); const memory = authoredMemory(scope, () => {}, true); let settled = false; const pending = memory.learn({ question: 'locked finding', chosen: 'c', reasoning: 'r' }).then(() => { settled = true; }); @@ -226,10 +228,12 @@ it('breaks a lock whose owner process is gone, or that is past the age backstop' expect(readdirSync(dir).filter(name => name.includes('.flows-memory.lock'))).toEqual([]); }); -it.skipIf(process.platform === 'win32')('keeps the database file mode', async () => { +it.skipIf(process.platform === 'win32')('keeps the database file mode, and gives the trajectory the same mode', async () => { chmodSync(dbPath, 0o600); await authoredMemory(scope, () => {}, true).learn({ question: 'private', chosen: 'c', reasoning: 'r' }); expect(statSync(dbPath).mode & 0o777).toBe(0o600); + const compacted = join(scope, '.trajectories', 'compacted'); + expect(statSync(join(compacted, readdirSync(compacted)[0]!)).mode & 0o777).toBe(0o600); }); it('migrates a pre-handoff history table without git_branch', async () => { diff --git a/testdata/memory/README.md b/testdata/memory/README.md index 31690747d..56701baa0 100644 --- a/testdata/memory/README.md +++ b/testdata/memory/README.md @@ -50,8 +50,8 @@ than ai-hist's automatic scan of unrelated local JSONL history. sync. The id is a hash of scope and finding, so a resumed body re-running `learn`, or a later run learning the same thing, upserts one record. Writes are serialized in-process and across flows processes by - `.flows-memory.lock` (stale once its owner process is gone, or after 5 - minutes), keep the database's file mode, + `.flows-memory.lock` (stale once its local owner process is gone; the 5-minute age + backstop applies only to owners this host cannot probe), keep the database's file mode (also used for the trajectory file), migrate a missing `history.git_branch`, and roll back the trajectory file if the database replace fails. It is still not a journaled effect, and `ai-hist sync` does not take the lock, so a concurrent sync can race the row From f4b23d6c9ff68da5d680a8ab108f30e9101de69e Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Fri, 9 Oct 2026 20:55:20 -0700 Subject: [PATCH 5/8] refactor(sdk): learn writes immutable trajectory files; reads merge them Review kept finding races in the shared-DB write path (stale-lock takeover, lost updates against ai-hist sync). Remove the shared mutable state instead of locking it: - learn() writes only an immutable, content-addressed compacted trajectory file under the script scope (staged + atomic rename, DB's file mode). The ai-hist DB is never written: no lock, no read-modify-replace, no git_branch migration, no race with ai-hist sync. - recall()/why() read the DB plus the scope's own trajectory files, so the next run sees a finding before any sync; a synced copy is deduplicated in favour of the DB row. recall honours source/tag/beforeMs/limit for unsynced entries (negative id marks one read from disk). - The executor tracks learn() promises; one still pending when the body returns fails the run with unawaited_step instead of reporting success over an unconfirmed write. Co-Authored-By: Claude Opus 5.5 (1M context) Session-Id: b6cf20aa-bdbb-4248-91ab-d2a4fd2ce1b8 --- docs/SURFACE.md | 22 +- packages/sdk/src/authored-flow-executor.ts | 13 +- packages/sdk/src/authored-memory.ts | 355 +++++++++------------ packages/sdk/tests/f-memory.test.ts | 117 ++++--- testdata/memory/README.md | 23 +- 5 files changed, 237 insertions(+), 293 deletions(-) diff --git a/docs/SURFACE.md b/docs/SURFACE.md index 107647ffe..209eecff7 100644 --- a/docs/SURFACE.md +++ b/docs/SURFACE.md @@ -135,16 +135,18 @@ No process runs between events: the handler wakes, executes to its next await, p check; direct `.memory` use also triggers it. Aliased access is checked at call time. The local Node SDK and an existing readable SQLite DB are required (`AI_HIST_DB` overrides `defaultDbPath()`); JSONL fallback is disabled. - `learn(finding)` persists the finding under the same script scope: a - compacted trajectory file at `/.trajectories/compacted/.json` - (which `ai-hist sync` ingests) plus the matching rows in the ai-hist DB, so - the next run recalls it. The id hashes scope and finding, so re-learning the - same finding upserts one record. `learn` writes are serialized with each - other by a lock file next to the DB; `ai-hist sync` does not take that lock, - so a concurrent sync can race the DB replace (the trajectory file survives - and the next sync re-ingests it). Writes keep the DB's file mode and leave - nothing behind when they fail (`memory_finding_invalid`, - `memory_unwritable`). `learn` is not yet a + `learn(finding)` writes the finding as an immutable compacted trajectory + file under the script scope (`/.trajectories/compacted/.json`, + the format `ai-hist sync` ingests). `recall` and `why` read the ai-hist DB + plus the scope's own trajectory files, so the next run sees a finding + before any sync, and a synced copy is deduplicated in favour of the DB row + (an unsynced `recall` entry has a negative `id`). The id hashes scope and + finding, so re-learning the same finding rewrites one file. `learn` never + modifies the ai-hist DB and shares no state between writers, so it needs no + lock; the file takes the DB's mode and is published by atomic rename, so a + failed `learn` (`memory_finding_invalid`, `memory_unwritable`) leaves + nothing behind. A body must await `learn`: one still pending when the body + returns fails the run with `unawaited_step`. `learn` is not yet a journaled effect. `memory: { agent: true }` refuses pending the identity-scoped agent follow-up. CLI-only operation is also deferred. diff --git a/packages/sdk/src/authored-flow-executor.ts b/packages/sdk/src/authored-flow-executor.ts index 8205e261d..a8084781a 100644 --- a/packages/sdk/src/authored-flow-executor.ts +++ b/packages/sdk/src/authored-flow-executor.ts @@ -10,7 +10,7 @@ import type { SlackCall } from './slack-writeback.js'; import { checkMcpHeader, McpPreflightError } from './cli/check-typescript.js'; import { buildMcpProxy, runMcpEffect } from './authored-mcp.js'; import { AuthoredBudget } from './authored-budget.js'; -import { assertMemoryReachable, authoredMemory, scriptMemoryScope } from './authored-memory.js'; +import { assertMemoryReachable, authoredMemory, scriptMemoryScope, type MemoryWriteTracker } from './authored-memory.js'; import { authoredDeterministicRunner, authoredWorkerRunner } from './authored-worker-step.js'; import { isSurfaceFlowCompletionReason, isSurfaceRunCompletionReason } from './authored-step-output.js'; import { @@ -285,6 +285,7 @@ export async function executeAuthoredFlow( const journalSteps: AuthoredFlowJournalStep[] = []; const authoredSteps: AuthoredFlowOperation[] = []; + const memoryWrites: MemoryWriteTracker = { pending: new Set() }; const lifecycle = new AuthoredFlowLifecycle(); const activities = new AuthoredActivities(journal, options.rootRunId); const edgeOverrides = new Map(); @@ -563,6 +564,7 @@ export async function executeAuthoredFlow( scriptMemoryScope(flowPath, definition.name), () => assertOperationAllowed('memory', definition.name, requestedCompletion), definition.header.memory?.script !== false, + memoryWrites, ), run: runOperation, llm: llmOperation, @@ -860,6 +862,15 @@ export async function executeAuthoredFlow( throw missingCompletion; } try { + // A `learn` still running when the body returns was not awaited: it races + // done(), so the run cannot claim the finding was kept. Let it settle, + // then fail the run rather than report success over an unconfirmed write. + if (memoryWrites.pending.size > 0) { + const unsettled = memoryWrites.pending.size; + await Promise.allSettled([...memoryWrites.pending]); + throw new AuthoredFlowExecutionError('unawaited_step', + `flow "${definition.name}" returned with ${unsettled} unawaited f.memory.learn call${unsettled === 1 ? '' : 's'}; await each learn before done()`); + } await verifyAuthoredOperations(definition.name, authoredSteps, lifecycle); } catch (error) { try { diff --git a/packages/sdk/src/authored-memory.ts b/packages/sdk/src/authored-memory.ts index 3b7d1ee93..0629d5ea0 100644 --- a/packages/sdk/src/authored-memory.ts +++ b/packages/sdk/src/authored-memory.ts @@ -1,10 +1,7 @@ -import type { MemoryHelper } from '@relayflows/surface'; -import type { MemoryFinding } from '@relayflows/surface'; +import type { HistoryEntry, MemoryFinding, MemoryHelper, MemoryRecallOptions, TrajectoryEntry } from '@relayflows/surface'; import { createHash, randomUUID } from 'node:crypto'; -import { access, chmod, link, mkdir, readFile, rename, rm, stat, writeFile } from 'node:fs/promises'; -import { hostname } from 'node:os'; +import { access, chmod, mkdir, readFile, readdir, rename, rm, stat, writeFile } from 'node:fs/promises'; import { constants } from 'node:fs'; -import { createRequire } from 'node:module'; import { dirname, join, resolve } from 'node:path'; import { AuthoredFlowExecutionError } from './authored-flow-error.js'; import { preflightMemory } from './preflight.js'; @@ -30,57 +27,65 @@ export async function assertMemoryReachable(): Promise { if (refusal) throw new AuthoredFlowExecutionError('memory_unreachable', refusal.message); } +/** Lets the executor see `learn` calls the body has not awaited. */ +export interface MemoryWriteTracker { + readonly pending: Set>; +} + export function authoredMemory( scope: string, assertOpen: () => void, enabled: boolean, + tracker?: MemoryWriteTracker, ): MemoryHelper { - async function read(action: (reader: import('ai-hist').AiHist) => T): Promise { - assertOpen(); + function assertEnabled(): void { if (!enabled) throw new AuthoredFlowExecutionError('unsupported_header', 'f.memory requires script memory; memory.script is false'); + } + async function read(action: (reader: import('ai-hist').AiHist, learned: LearnedTrajectory[]) => T): Promise { + assertOpen(); + assertEnabled(); await assertMemoryReachable(); assertOpen(); + const learned = await readLearned(scope); const { openAiHist } = await import('ai-hist'); const reader = await openAiHist({ projectScope: scope, fallback: 'error' }); - try { return action(reader); } finally { reader.close(); } + try { return action(reader, learned); } finally { reader.close(); } + } + async function learn(finding: MemoryFinding): Promise { + assertOpen(); + assertEnabled(); + const decision = normalizeFinding(finding); + await assertMemoryReachable(); + assertOpen(); + const { defaultDbPath } = await import('ai-hist'); + try { + await writeLearned(scope, decision, (await stat(defaultDbPath())).mode & 0o777); + } catch (error) { + throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn could not persist the finding: ${error instanceof Error ? error.message : String(error)}`); + } } return { - recall: (query, options) => read(reader => reader.search(query, { ...options, project: scope })), - why: task => read(reader => { - const entry = reader.whyForTask(task); - return entry?.projectId === scope ? [entry] : []; + recall: (query, options) => read((reader, learned) => recallMerged(reader, learned, scope, query, options)), + why: task => read((reader, learned) => { + const synced = reader.whyForTask(task); + const candidates = [ + ...(synced?.projectId === scope ? [synced] : []), + ...learned.map(entry => entry.trajectory).filter(entry => trajectoryMatches(entry, task)), + ]; + const newest = candidates.sort((a, b) => b.timestampMs - a.timestampMs)[0]; + return newest === undefined ? [] : [newest]; }), - async learn(finding) { - assertOpen(); - if (!enabled) throw new AuthoredFlowExecutionError('unsupported_header', 'f.memory requires script memory; memory.script is false'); - const decision = normalizeFinding(finding); - await assertMemoryReachable(); - assertOpen(); - const { defaultDbPath } = await import('ai-hist'); - try { - await persistFinding(defaultDbPath(), scope, decision); - } catch (error) { - throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn could not persist the finding: ${error instanceof Error ? error.message : String(error)}`); + learn(finding) { + const write = learn(finding); + if (tracker !== undefined) { + tracker.pending.add(write); + void write.then(() => tracker.pending.delete(write), () => tracker.pending.delete(write)); } + return write; }, }; } -interface SqlDatabase { - run(sql: string, params?: unknown[]): void; - exec(sql: string): Array<{ columns: string[]; values: unknown[][] }>; - export(): Uint8Array; - close(): void; -} -type SqlJs = { Database: new (data?: Uint8Array) => SqlDatabase }; - -/** sql.js is the SQLite build ai-hist ships; resolve it through ai-hist so the two always agree. */ -async function loadSqlJs(): Promise { - const fromAiHist = createRequire(createRequire(import.meta.url).resolve('ai-hist/package.json')); - const init = fromAiHist('sql.js') as (config?: unknown) => Promise; - return init(); -} - function normalizeFinding(finding: MemoryFinding): Required { const text = (value: unknown, field: string): string => { if (typeof value !== 'string' || value.trim() === '') { @@ -100,119 +105,22 @@ function normalizeFinding(finding: MemoryFinding): Required { }; } -const tempPath = (path: string): string => `${path}.${process.pid}.${randomUUID()}.tmp`; - -const LOCK_WAIT_MS = 10_000; -/** Backstop for an owner this host cannot probe (another host, or an unreadable lock). Writers hold the lock for milliseconds. */ -const LOCK_STALE_MS = 5 * 60_000; -const inProcess = new Map>(); - -interface LockOwner { pid: number; host: string; token: string } - -async function readLockOwner(path: string): Promise<{ owner?: LockOwner; ageMs: number } | undefined> { - try { - const [raw, info] = await Promise.all([readFile(path, 'utf8'), stat(path)]); - let owner: LockOwner | undefined; - try { owner = JSON.parse(raw) as LockOwner; } catch { owner = undefined; } - return { owner, ageMs: Date.now() - info.mtimeMs }; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined; - throw error; - } -} - -/** A local owner is judged by whether its process exists, never by age; the age backstop is only for owners this host cannot probe. */ -function lockIsStale({ owner, ageMs }: { owner?: LockOwner; ageMs: number }): boolean { - if (owner !== undefined && owner.host === hostname() && Number.isInteger(owner.pid) && owner.pid > 0) { - try { - process.kill(owner.pid, 0); - return false; - } catch (error) { - return (error as NodeJS.ErrnoException).code === 'ESRCH'; - } - } - return ageMs > LOCK_STALE_MS; -} - -/** - * Serialize read-modify-replace of one database file: a promise chain within - * this process, and an exclusive lock file across flows processes. - * - * The lock is published with `link`, so it appears complete or not at all and - * never replaces another lock. It is stale only when its owner process is gone - * (or, as a backstop, after LOCK_STALE_MS). Takeover renames the lock aside - * first, so two breakers cannot both remove it; a breaker that finds it moved - * a live lock links it back. The task gets `assertHeld` to re-check ownership - * right before it commits. Writers that do not take this lock (`ai-hist sync`) - * are not serialized against it. - */ -async function withDatabaseLock(dbPath: string, task: (assertHeld: () => Promise) => Promise): Promise { - const previous = inProcess.get(dbPath) ?? Promise.resolve(); - const run = previous.catch(() => {}).then(async () => { - const lock = `${dbPath}.flows-memory.lock`; - const self: LockOwner = { pid: process.pid, host: hostname(), token: randomUUID() }; - const staged = tempPath(lock); - await writeFile(staged, JSON.stringify(self)); - const deadline = Date.now() + LOCK_WAIT_MS; - try { - for (;;) { - try { - await link(staged, lock); - break; - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; - } - const held = await readLockOwner(lock); - if (held !== undefined && lockIsStale(held)) { - const aside = `${lock}.${self.token}.stale`; - try { await rename(lock, aside); } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; - continue; - } - const moved = await readLockOwner(aside); - if (moved !== undefined && !lockIsStale(moved)) await link(aside, lock).catch(() => {}); - await rm(aside, { force: true }); - continue; - } - if (Date.now() > deadline) throw new Error(`timed out waiting for ${lock}`); - await new Promise(resolve => setTimeout(resolve, 25)); - } - } finally { - await rm(staged, { force: true }); - } - const assertHeld = async () => { - if ((await readLockOwner(lock))?.owner?.token !== self.token) throw new Error(`lost ${lock} before commit`); - }; - try { - return await task(assertHeld); - } finally { - if ((await readLockOwner(lock).catch(() => undefined))?.owner?.token === self.token) await rm(lock, { force: true }); - } - }); - inProcess.set(dbPath, run); - try { return await run; } finally { if (inProcess.get(dbPath) === run) inProcess.delete(dbPath); } -} +const learnedDir = (scope: string): string => join(scope, '.trajectories', 'compacted'); +const LEARNED_PREFIX = 'flows-memory-'; /** - * Persist one finding so later runs of the same flow can recall it. - * - * Two idempotent writes, both scoped to the flow's script scope: - * - a compacted trajectory file under `/.trajectories/compacted/`, the - * format ai-hist ingests, so a later `ai-hist sync` re-indexes it; - * - the matching `trajectories` and `history` rows in the ai-hist database, - * mirroring ai-hist's own ingestion, so the next run sees it without a sync. - * - * Both are staged as temp files first and published only once the database - * image is built; if the database replace fails, a newly published trajectory - * file is removed, so a rejected learn leaves nothing for a later sync to index. + * Persist one finding as an immutable compacted trajectory file, the format + * `ai-hist sync` ingests. The file name is a hash of scope and finding, so + * re-learning the same finding (a resumed body re-running `learn`, or a later + * run reaching the same conclusion) rewrites one file rather than adding one. + * The file is staged and renamed into place, so it appears whole or not at + * all, and nothing else is shared between writers: no lock, and the ai-hist + * database is never modified. It takes the database's mode, so a private + * history does not leak findings through a world-readable file. */ -async function persistFinding(dbPath: string, scope: string, decision: Required): Promise { - const now = new Date(); - const timestampMs = now.getTime(); - // Content-addressed: re-learning the same finding (a resumed body re-running - // its effects, or a later run reaching the same conclusion) upserts one record. - const id = `flows-memory-${createHash('sha256').update(JSON.stringify([scope, decision])).digest('hex').slice(0, 32)}`; - const retrospective = { summary: null, approach: null, learnings: [] as string[], confidence: null }; +async function writeLearned(scope: string, decision: Required, mode: number): Promise { + const now = new Date().toISOString(); + const id = `${LEARNED_PREFIX}${createHash('sha256').update(JSON.stringify([scope, decision])).digest('hex').slice(0, 32)}`; const trajectory = { id, version: 1, @@ -220,70 +128,101 @@ async function persistFinding(dbPath: string, scope: string, decision: Required< projectId: scope, task: { title: decision.question, description: null }, status: 'completed', - startedAt: now.toISOString(), - completedAt: now.toISOString(), + startedAt: now, + completedAt: now, decisions: [decision], - retrospective, + retrospective: { summary: null, approach: null, learnings: [], confidence: null }, }; - const file = join(scope, '.trajectories', 'compacted', `${id}.json`); - // Same field order ai-hist's buildSearchText uses, so search ranks identically after a sync. - const searchText = [ - id, scope, trajectory.status, decision.question, - decision.question, decision.chosen, decision.reasoning, ...decision.alternatives, - ].filter(part => part.length > 0).join('\n'); - const SQL = await loadSqlJs(); - + const file = join(learnedDir(scope), `${id}.json`); await mkdir(dirname(file), { recursive: true }); - const stagedFile = tempPath(file); - let stagedDb: string | undefined; + const staged = `${file}.${process.pid}.${randomUUID()}.tmp`; try { - // The trajectory holds the same finding as the DB row, so it gets the DB's permissions, not the umask's. - const fileMode = (await stat(dbPath)).mode & 0o777; - await writeFile(stagedFile, `${JSON.stringify(trajectory, null, 2)}\n`, { mode: fileMode }); - await chmod(stagedFile, fileMode); - await withDatabaseLock(dbPath, async assertHeld => { - // Keep the database's own permissions: a 0600 history file must not become world-readable. - const mode = (await stat(dbPath)).mode & 0o777; - const db = new SQL.Database(await readFile(dbPath)); - try { - db.run(`CREATE TABLE IF NOT EXISTS trajectories ( - id TEXT PRIMARY KEY, version INTEGER, persona_id TEXT, project_id TEXT, - task_title TEXT, task_description TEXT, status TEXT, started_at TEXT, - completed_at TEXT, decisions_json TEXT NOT NULL, retrospective_json TEXT NOT NULL, - search_text TEXT NOT NULL, path TEXT, updated_ms INTEGER NOT NULL, timestamp_ms INTEGER NOT NULL - )`); - // ai-hist adds this column only to its in-memory copy of pre-handoff databases. - const historyColumns = db.exec('PRAGMA table_info(history)')[0]?.values.map(row => row[1]) ?? []; - if (!historyColumns.includes('git_branch')) db.run('ALTER TABLE history ADD COLUMN git_branch TEXT'); - db.run(`INSERT OR REPLACE INTO trajectories - (id, version, persona_id, project_id, task_title, task_description, status, - started_at, completed_at, decisions_json, retrospective_json, search_text, - path, updated_ms, timestamp_ms) - VALUES (?, 1, NULL, ?, ?, NULL, 'completed', ?, ?, ?, ?, ?, ?, ?, ?)`, [ - id, scope, decision.question, trajectory.startedAt, trajectory.completedAt, - JSON.stringify([decision]), JSON.stringify(retrospective), searchText, file, timestampMs, timestampMs, - ]); - db.run(`DELETE FROM history WHERE source = 'trajectory' AND session_id = ?`, [id]); - db.run(`INSERT OR IGNORE INTO history (source, session_id, project, prompt, timestamp_ms, git_branch) - VALUES ('trajectory', ?, ?, ?, ?, NULL)`, [id, scope, searchText, timestampMs]); - stagedDb = tempPath(dbPath); - await writeFile(stagedDb, db.export(), { mode }); - await chmod(stagedDb, mode); - } finally { - db.close(); - } - await assertHeld(); - const existed = await stat(file).then(() => true, () => false); - await rename(stagedFile, file); - try { - await rename(stagedDb, dbPath); - } catch (error) { - if (!existed) await rm(file, { force: true }); - throw error; - } - }); + await writeFile(staged, `${JSON.stringify(trajectory, null, 2)}\n`, { mode }); + await chmod(staged, mode); + await rename(staged, file); } finally { - await rm(stagedFile, { force: true }); - if (stagedDb !== undefined) await rm(stagedDb, { force: true }); + await rm(staged, { force: true }); } } + +interface LearnedTrajectory { + trajectory: TrajectoryEntry; + history: HistoryEntry; +} + +/** This scope's learned findings, read straight from disk so they need no `ai-hist sync`. */ +async function readLearned(scope: string): Promise { + const dir = learnedDir(scope); + let names: string[]; + try { names = await readdir(dir); } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return []; + throw error; + } + const learned: LearnedTrajectory[] = []; + for (const name of names) { + if (!name.startsWith(LEARNED_PREFIX) || !name.endsWith('.json')) continue; + const path = join(dir, name); + let raw: Record; + try { raw = JSON.parse(await readFile(path, 'utf8')) as Record; } catch { continue; } + const decisions = Array.isArray(raw.decisions) ? raw.decisions as TrajectoryEntry['decisions'] : []; + const task = (raw.task ?? {}) as { title?: string | null; description?: string | null }; + const id = typeof raw.id === 'string' ? raw.id : name.slice(0, -'.json'.length); + const completedAt = typeof raw.completedAt === 'string' ? raw.completedAt : null; + const updatedMs = Math.trunc((await stat(path)).mtimeMs); + const timestampMs = completedAt !== null && Number.isFinite(Date.parse(completedAt)) ? Date.parse(completedAt) : updatedMs; + // Same field order ai-hist's buildSearchText uses, so matching agrees with a later sync. + const searchText = [id, scope, 'completed', task.title ?? null, task.description ?? null, + ...decisions.flatMap(d => [d.question, d.chosen, d.reasoning, ...(d.alternatives ?? [])])] + .filter((part): part is string => typeof part === 'string' && part.length > 0).join('\n'); + learned.push({ + trajectory: { + id, version: 1, personaId: null, projectId: scope, + task: { title: task.title ?? null, description: task.description ?? null }, + status: 'completed', startedAt: typeof raw.startedAt === 'string' ? raw.startedAt : null, completedAt, + decisions, retrospective: { summary: null, approach: null, learnings: [], confidence: null }, + searchText, path, updatedMs, timestampMs, + }, + // Not yet in the ai-hist database, so it has no row id; a negative id + // derived from the trajectory id marks it as read from disk. + history: { + id: -Number.parseInt(createHash('sha256').update(id).digest('hex').slice(0, 12), 16), + source: 'trajectory', sessionId: id, project: scope, prompt: searchText, timestampMs, gitBranch: null, + }, + }); + } + return learned; +} + +const includesFolded = (haystack: string | null | undefined, needle: string): boolean => + (haystack ?? '').toLowerCase().includes(needle.toLowerCase()); + +function trajectoryMatches(entry: TrajectoryEntry, query: string): boolean { + const needle = query.trim(); + if (needle === '') return false; + return [entry.searchText, entry.task.title, entry.task.description, entry.personaId, entry.projectId] + .some(field => includesFolded(field, needle)); +} + +/** ai-hist's `search` semantics over the database plus this scope's not-yet-synced findings. */ +function recallMerged( + reader: import('ai-hist').AiHist, + learned: LearnedTrajectory[], + scope: string, + query: string, + options: MemoryRecallOptions | undefined, +): HistoryEntry[] { + const limit = options?.limit ?? 50; + const needle = query.trim(); + const fromDisk = learned.map(entry => entry.history).filter(entry => + (needle === '' || includesFolded(entry.prompt, needle) || includesFolded(entry.project, needle)) + && (options?.source === undefined || options.source === 'trajectory') + && options?.tag === undefined + && (options?.beforeMs === undefined || entry.timestampMs < options.beforeMs)); + // Fetching `limit + fromDisk.length` rows guarantees a synced copy of any + // disk entry that makes the cut is among them, so it dedupes correctly. + const synced = reader.search(query, { ...options, project: scope, limit: limit + fromDisk.length }); + const syncedIds = new Set(synced.filter(entry => entry.source === 'trajectory').map(entry => entry.sessionId)); + return [...synced, ...fromDisk.filter(entry => !syncedIds.has(entry.sessionId))] + .sort((a, b) => b.timestampMs - a.timestampMs) + .slice(0, limit); +} diff --git a/packages/sdk/tests/f-memory.test.ts b/packages/sdk/tests/f-memory.test.ts index 1e4cb6538..ad3dfeb9a 100644 --- a/packages/sdk/tests/f-memory.test.ts +++ b/packages/sdk/tests/f-memory.test.ts @@ -1,19 +1,20 @@ -import { execFileSync, spawnSync } from 'node:child_process'; -import { chmodSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, utimesSync, writeFileSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import { chmodSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { createRequire } from 'node:module'; -import { hostname, tmpdir } from 'node:os'; +import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { flow } from '@relayflows/surface'; import { afterEach, beforeEach, expect, it, vi } from 'vitest'; -let failRenameTo: string | undefined; +let failRename: ((to: string) => boolean) | undefined; vi.mock('node:fs/promises', async importOriginal => { const actual = await importOriginal(); return { ...actual, rename: async (from: string, to: string) => { - if (to === failRenameTo) throw Object.assign(new Error('injected rename failure'), { code: 'EIO' }); + if (failRename?.(to)) throw Object.assign(new Error('injected rename failure'), { code: 'EIO' }); return actual.rename(from, to); }, }; @@ -181,85 +182,79 @@ it('rejects invalid findings before writing, and unwritable stores', async () => } }); -it('serializes concurrent learns so no finding is lost from the database', async () => { +const compactedDir = () => join(scope, '.trajectories', 'compacted'); +const dbDigest = () => createHash('sha256').update(readFileSync(dbPath)).digest('hex'); + +it('keeps concurrent findings without touching the shared database', async () => { + const before = dbDigest(); const memory = authoredMemory(scope, () => {}, true); await Promise.all(Array.from({ length: 6 }, (_, i) => memory.learn({ question: `parallel finding ${i}`, chosen: 'c', reasoning: 'r' }))); expect(await memory.recall('parallel finding')).toHaveLength(6); - expect(existsSync(`${dbPath}.flows-memory.lock`)).toBe(false); + expect(dbDigest()).toBe(before); + expect(readdirSync(compactedDir()).filter(name => !name.endsWith('.json'))).toEqual([]); }); -async function until(predicate: () => boolean): Promise { - for (let i = 0; i < 400 && !predicate(); i += 1) await new Promise(resolve => setTimeout(resolve, 10)); - expect(predicate()).toBe(true); -} - -it('waits on a lock held by a live process, then proceeds once it is released', async () => { - const lock = `${dbPath}.flows-memory.lock`; - writeFileSync(lock, JSON.stringify({ pid: process.pid, host: hostname(), token: 'another-writer' })); - // Live local owners never expire by age, however old the lock looks. - utimesSync(lock, new Date(0), new Date(0)); +it('dedupes a finding once ai-hist sync has indexed it, preferring the synced row', async () => { const memory = authoredMemory(scope, () => {}, true); - let settled = false; - const pending = memory.learn({ question: 'locked finding', chosen: 'c', reasoning: 'r' }).then(() => { settled = true; }); - // The writer stages its own lock file before contending; once it exists, the wait is under way. - await until(() => readdirSync(dir).some(name => name.startsWith('fixture.db.flows-memory.lock.') && name.endsWith('.tmp'))); - await new Promise(resolve => setTimeout(resolve, 150)); - expect(settled).toBe(false); - expect(JSON.parse(readFileSync(lock, 'utf8'))).toMatchObject({ token: 'another-writer' }); - rmSync(lock); - await pending; - expect(await memory.recall('locked finding')).toHaveLength(1); - expect(existsSync(lock)).toBe(false); + await memory.learn({ question: 'synced finding', chosen: 'c', reasoning: 'r' }); + const [diskEntry] = await memory.recall('synced finding'); + expect(diskEntry).toMatchObject({ source: 'trajectory', project: scope }); + expect(diskEntry!.id).toBeLessThan(0); + + // What `ai-hist sync` writes for the trajectory file. + const [name] = readdirSync(compactedDir()); + const raw = JSON.parse(readFileSync(join(compactedDir(), name!), 'utf8')); + const require = createRequire(import.meta.url); + const SQL = await createRequire(require.resolve('ai-hist/package.json'))('sql.js')(); + const db = new SQL.Database(readFileSync(dbPath)); + const ts = Date.parse(raw.completedAt); + db.run('INSERT INTO trajectories VALUES (?, 1, NULL, ?, ?, NULL, ?, ?, ?, ?, ?, ?, ?, ?, ?)', [ + raw.id, scope, raw.task.title, 'completed', raw.startedAt, raw.completedAt, + JSON.stringify(raw.decisions), JSON.stringify(raw.retrospective), diskEntry!.prompt, join(compactedDir(), name!), ts, ts]); + db.run('INSERT INTO history VALUES (?, ?, ?, ?, ?, ?, ?)', [99, 'trajectory', raw.id, scope, diskEntry!.prompt, ts, null]); + writeFileSync(dbPath, Buffer.from(db.export())); + db.close(); + + expect(await memory.recall('synced finding')).toMatchObject([{ id: 99, sessionId: raw.id }]); + expect(await memory.why('synced finding')).toMatchObject([{ id: raw.id }]); }); -it('breaks a lock whose owner process is gone, or that is past the age backstop', async () => { - const lock = `${dbPath}.flows-memory.lock`; +it('applies recall options to findings not yet synced', async () => { const memory = authoredMemory(scope, () => {}, true); - const deadPid = spawnSync(process.execPath, ['-e', '']).pid; - writeFileSync(lock, JSON.stringify({ pid: deadPid, host: hostname(), token: 'crashed-writer' })); - await memory.learn({ question: 'after dead owner', chosen: 'c', reasoning: 'r' }); - expect(await memory.recall('after dead owner')).toHaveLength(1); - - writeFileSync(lock, ''); - utimesSync(lock, new Date(0), new Date(0)); - await memory.learn({ question: 'after aged lock', chosen: 'c', reasoning: 'r' }); - expect(await memory.recall('after aged lock')).toHaveLength(1); - expect(readdirSync(dir).filter(name => name.includes('.flows-memory.lock'))).toEqual([]); + await memory.learn({ question: 'option finding', chosen: 'c', reasoning: 'r' }); + expect(await memory.recall('option finding', { source: 'claude' })).toEqual([]); + expect(await memory.recall('option finding', { tag: 'anything' })).toEqual([]); + expect(await memory.recall('option finding', { beforeMs: 1 })).toEqual([]); + expect(await memory.recall('', { limit: 1 })).toHaveLength(1); + expect(await memory.recall('')).toHaveLength(2); }); -it.skipIf(process.platform === 'win32')('keeps the database file mode, and gives the trajectory the same mode', async () => { +it.skipIf(process.platform === 'win32')('gives the trajectory file the database file mode', async () => { chmodSync(dbPath, 0o600); await authoredMemory(scope, () => {}, true).learn({ question: 'private', chosen: 'c', reasoning: 'r' }); - expect(statSync(dbPath).mode & 0o777).toBe(0o600); - const compacted = join(scope, '.trajectories', 'compacted'); - expect(statSync(join(compacted, readdirSync(compacted)[0]!)).mode & 0o777).toBe(0o600); + expect(statSync(join(compactedDir(), readdirSync(compactedDir())[0]!)).mode & 0o777).toBe(0o600); }); -it('migrates a pre-handoff history table without git_branch', async () => { - const require = createRequire(import.meta.url); - const SQL = await createRequire(require.resolve('ai-hist/package.json'))('sql.js')(); - const db = new SQL.Database(); - db.run('CREATE TABLE history (id INTEGER PRIMARY KEY, source TEXT, session_id TEXT, project TEXT, prompt TEXT, timestamp_ms INTEGER)'); - writeFileSync(dbPath, Buffer.from(db.export())); - db.close(); - const memory = authoredMemory(scope, () => {}, true); - await memory.learn({ question: 'old database', chosen: 'c', reasoning: 'r' }); - expect(await memory.recall('old database')).toMatchObject([{ source: 'trajectory', gitBranch: null }]); -}); - -it('removes the trajectory file when the database replace fails', async () => { +it('leaves nothing behind when the trajectory cannot be published', async () => { const memory = authoredMemory(scope, () => {}, true); - failRenameTo = dbPath; + failRename = to => to.startsWith(compactedDir()) && to.endsWith('.json'); try { await expect(memory.learn({ question: 'doomed', chosen: 'c', reasoning: 'r' })) .rejects.toMatchObject({ code: 'memory_unwritable' }); - } finally { failRenameTo = undefined; } - expect(readdirSync(join(scope, '.trajectories', 'compacted'))).toEqual([]); - expect(readdirSync(dir).filter(name => name.endsWith('.tmp') || name.endsWith('.lock'))).toEqual([]); + } finally { failRename = undefined; } + expect(readdirSync(compactedDir())).toEqual([]); expect(await memory.recall('doomed')).toEqual([]); }); +it('fails the run when the body does not await learn', async () => { + await expect(executeAuthoredFlow(flow('memory-example', { memory: { script: true } }, async f => { + void f.memory.learn({ question: 'unawaited', chosen: 'c', reasoning: 'r' }); + f.done('success'); + }), completingJournal(), undefined, { flowPath: join(dir, 'test.flow.ts') })) + .rejects.toMatchObject({ code: 'unawaited_step' }); +}); + it('reports learn failures with their own kind, not protocol_error', () => { for (const code of ['memory_finding_invalid', 'memory_unwritable'] as const) { const error = Object.assign(new AuthoredFlowExecutionError(code, 'nope'), { rootRunId: 'root-1' }); diff --git a/testdata/memory/README.md b/testdata/memory/README.md index 56701baa0..c916f692c 100644 --- a/testdata/memory/README.md +++ b/testdata/memory/README.md @@ -43,19 +43,16 @@ than ai-hist's automatic scan of unrelated local JSONL history. ## Deferred acceptance work - Journaled `learn` effects. ai-hist 0.4.1 has no public trajectory writer, - so `learn` writes the finding twice under the script scope: a compacted - trajectory file at `/.trajectories/compacted/.json` (the format - `ai-hist sync` ingests), and the matching `trajectories`/`history` rows in the - ai-hist database (atomic file replace) so the next run recalls it without a - sync. The id is a hash of scope and finding, so a resumed body re-running - `learn`, or a later run learning the same thing, upserts one record. Writes - are serialized in-process and across flows processes by - `.flows-memory.lock` (stale once its local owner process is gone; the 5-minute age - backstop applies only to owners this host cannot probe), keep the database's file mode (also used for the trajectory file), - migrate a missing `history.git_branch`, and roll back the trajectory file if - the database replace fails. It is still not a journaled effect, and - `ai-hist sync` does not take the lock, so a concurrent sync can race the row - write; the trajectory file survives and the sync re-ingests it. + so `learn` writes the finding as an immutable compacted trajectory file at + `/.trajectories/compacted/.json` (the format `ai-hist sync` + ingests), staged and atomically renamed, with the database's file mode. + `recall`/`why` read the database plus those files, deduplicating synced + copies, so the next run sees the finding without a sync. The ai-hist + database is never written, so there is no lock and no race with + `ai-hist sync`. The id is a hash of scope and finding, so a resumed body + re-running `learn`, or a later run learning the same thing, rewrites one + file. An unawaited `learn` fails the run with `unawaited_step`. It is still + not a journaled effect. - Identity-scoped agent context injection. `memory.agent: true` explicitly refuses; it must not silently read script scope. - CLI-only provider operation and automatic creation of a fresh database. From 8b887aff566ff83c01a75d8f6d46bddf19ef33e4 Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Fri, 9 Oct 2026 21:07:38 -0700 Subject: [PATCH 6/8] feat(sdk): journal f.memory.learn on the durable root run before recall Per the principal's decision, learn ships journaled: - learn() commits the finding to the root run's `memory-learn` stream (stream.append, journaled under the run lock) BEFORE projecting it as a trajectory file, so nothing is recallable that the journal does not record. The file cites its commit ({runId, stream, offset}) and is derived only from it; a resumed body re-running learn finds the commit and republishes identical bytes without journaling again. Concurrent learns of one finding share one commit. - The durable root-run ID is required, not deferred: every flows run path already drives the body under a kernel root (driveRoot passes dispatch.run_id); learn without one refuses with memory_unjournaled. - Learned files move beside the ai-hist DB (/flows-memory/), so flows deployed from read-only checkouts can learn. - A damaged learned file fails recall/why with memory_unreachable instead of being skipped. - The unawaited-learn guard tracks whether the body consumed each call's outcome (a Promise subclass reached through then), so a fire-and-forget learn that settles before the body returns still fails the run. - Live test through the built CLI and a real relayflowd: the journal record, the citation, and a second run recalling the finding. Co-Authored-By: Claude Opus 5.5 (1M context) Session-Id: b6cf20aa-bdbb-4248-91ab-d2a4fd2ce1b8 --- docs/SURFACE.md | 33 +-- packages/sdk/src/authored-flow-error.ts | 2 + packages/sdk/src/authored-flow-executor.ts | 18 +- packages/sdk/src/authored-memory.ts | 223 ++++++++++++++++--- packages/sdk/src/cli/run.ts | 2 +- packages/sdk/src/failure-kinds.ts | 2 + packages/sdk/tests/f-memory.test.ts | 162 ++++++++++++-- packages/sdk/tests/memory-learn-live.test.ts | 62 ++++++ packages/surface/AUTHORING.md | 2 +- packages/surface/src/memory.ts | 2 +- testdata/memory/README.md | 24 +- 11 files changed, 438 insertions(+), 94 deletions(-) create mode 100644 packages/sdk/tests/memory-learn-live.test.ts diff --git a/docs/SURFACE.md b/docs/SURFACE.md index 209eecff7..6fdfa7524 100644 --- a/docs/SURFACE.md +++ b/docs/SURFACE.md @@ -135,19 +135,26 @@ No process runs between events: the handler wakes, executes to its next await, p check; direct `.memory` use also triggers it. Aliased access is checked at call time. The local Node SDK and an existing readable SQLite DB are required (`AI_HIST_DB` overrides `defaultDbPath()`); JSONL fallback is disabled. - `learn(finding)` writes the finding as an immutable compacted trajectory - file under the script scope (`/.trajectories/compacted/.json`, - the format `ai-hist sync` ingests). `recall` and `why` read the ai-hist DB - plus the scope's own trajectory files, so the next run sees a finding - before any sync, and a synced copy is deduplicated in favour of the DB row - (an unsynced `recall` entry has a negative `id`). The id hashes scope and - finding, so re-learning the same finding rewrites one file. `learn` never - modifies the ai-hist DB and shares no state between writers, so it needs no - lock; the file takes the DB's mode and is published by atomic rename, so a - failed `learn` (`memory_finding_invalid`, `memory_unwritable`) leaves - nothing behind. A body must await `learn`: one still pending when the body - returns fails the run with `unawaited_step`. `learn` is not yet a - journaled effect. `memory: { agent: true }` refuses pending the + `learn(finding)` journals first: the finding is appended to the durable + root run's `memory-learn` stream before it can be recalled. It is then + projected as a compacted trajectory file (the format `ai-hist sync` + ingests) at `/flows-memory//.trajectories/compacted/.json`, + beside the ai-hist DB rather than the flow source, so a flow deployed from a + read-only checkout can still learn. The file cites its commit + (`journal: { runId, stream, offset }`) and is derived only from it, so a + resumed body that re-runs `learn` finds the commit and republishes the same + bytes without journaling again. `learn` without a durable root run refuses + (`memory_unjournaled`). `recall` and `why` read the ai-hist DB plus the + scope's learned files, so the next run sees a finding before any sync; a + synced copy is deduplicated in favour of the DB row (an unsynced `recall` + entry has a negative `id`), and a damaged learned file fails the read + (`memory_unreachable`) rather than being skipped. The id hashes scope and + finding, so re-learning a finding atomically replaces one file. `learn` + never modifies the ai-hist DB and shares no state between writers, so it + needs no lock; the file takes the DB's mode, and a failed `learn` + (`memory_finding_invalid`, `memory_unwritable`) publishes nothing. A body + must await `learn`: a call whose outcome the body never consumed fails the + run with `unawaited_step`. `memory: { agent: true }` refuses pending the identity-scoped agent follow-up. CLI-only operation is also deferred. 4. **`{{prev}}` / return-value chaining.** Output flows downward implicitly; naming steps is for reaching back, not bookkeeping. diff --git a/packages/sdk/src/authored-flow-error.ts b/packages/sdk/src/authored-flow-error.ts index 91949c560..243ca56c3 100644 --- a/packages/sdk/src/authored-flow-error.ts +++ b/packages/sdk/src/authored-flow-error.ts @@ -30,6 +30,8 @@ export type AuthoredFlowExecutionErrorCode = | 'memory_finding_invalid' /** `f.memory.learn` could not persist the finding to script memory. */ | 'memory_unwritable' + /** `f.memory.learn` ran without a durable root run to journal the finding in. */ + | 'memory_unjournaled' | 'operation_after_completion' | 'operation_callback_failed' | 'step_failed' diff --git a/packages/sdk/src/authored-flow-executor.ts b/packages/sdk/src/authored-flow-executor.ts index a8084781a..894758e74 100644 --- a/packages/sdk/src/authored-flow-executor.ts +++ b/packages/sdk/src/authored-flow-executor.ts @@ -10,7 +10,7 @@ import type { SlackCall } from './slack-writeback.js'; import { checkMcpHeader, McpPreflightError } from './cli/check-typescript.js'; import { buildMcpProxy, runMcpEffect } from './authored-mcp.js'; import { AuthoredBudget } from './authored-budget.js'; -import { assertMemoryReachable, authoredMemory, scriptMemoryScope, type MemoryWriteTracker } from './authored-memory.js'; +import { assertMemoryReachable, authoredMemory, journalLearnLedger, MemoryWriteTracker, scriptMemoryScope } from './authored-memory.js'; import { authoredDeterministicRunner, authoredWorkerRunner } from './authored-worker-step.js'; import { isSurfaceFlowCompletionReason, isSurfaceRunCompletionReason } from './authored-step-output.js'; import { @@ -285,7 +285,7 @@ export async function executeAuthoredFlow( const journalSteps: AuthoredFlowJournalStep[] = []; const authoredSteps: AuthoredFlowOperation[] = []; - const memoryWrites: MemoryWriteTracker = { pending: new Set() }; + const memoryWrites = new MemoryWriteTracker(); const lifecycle = new AuthoredFlowLifecycle(); const activities = new AuthoredActivities(journal, options.rootRunId); const edgeOverrides = new Map(); @@ -565,6 +565,7 @@ export async function executeAuthoredFlow( () => assertOperationAllowed('memory', definition.name, requestedCompletion), definition.header.memory?.script !== false, memoryWrites, + options.rootRunId === undefined ? undefined : journalLearnLedger(journal, options.rootRunId), ), run: runOperation, llm: llmOperation, @@ -862,14 +863,13 @@ export async function executeAuthoredFlow( throw missingCompletion; } try { - // A `learn` still running when the body returns was not awaited: it races - // done(), so the run cannot claim the finding was kept. Let it settle, - // then fail the run rather than report success over an unconfirmed write. - if (memoryWrites.pending.size > 0) { - const unsettled = memoryWrites.pending.size; - await Promise.allSettled([...memoryWrites.pending]); + // A `learn` whose outcome the body never awaited may have failed, or may + // still be racing done(), so the run cannot claim the finding was kept. + // Let each settle, then fail the run rather than report success over it. + const unawaitedLearns = await memoryWrites.unawaited(); + if (unawaitedLearns > 0) { throw new AuthoredFlowExecutionError('unawaited_step', - `flow "${definition.name}" returned with ${unsettled} unawaited f.memory.learn call${unsettled === 1 ? '' : 's'}; await each learn before done()`); + `flow "${definition.name}" returned with ${unawaitedLearns} unawaited f.memory.learn call${unawaitedLearns === 1 ? '' : 's'}; await each learn before done()`); } await verifyAuthoredOperations(definition.name, authoredSteps, lifecycle); } catch (error) { diff --git a/packages/sdk/src/authored-memory.ts b/packages/sdk/src/authored-memory.ts index 0629d5ea0..4e4e96b64 100644 --- a/packages/sdk/src/authored-memory.ts +++ b/packages/sdk/src/authored-memory.ts @@ -2,8 +2,9 @@ import type { HistoryEntry, MemoryFinding, MemoryHelper, MemoryRecallOptions, Tr import { createHash, randomUUID } from 'node:crypto'; import { access, chmod, mkdir, readFile, readdir, rename, rm, stat, writeFile } from 'node:fs/promises'; import { constants } from 'node:fs'; -import { dirname, join, resolve } from 'node:path'; +import { basename, dirname, join, resolve } from 'node:path'; import { AuthoredFlowExecutionError } from './authored-flow-error.js'; +import type { JournalClient } from './journal-client.js'; import { preflightMemory } from './preflight.js'; /** Stable across runs; disjoint from other flow files and other named flows. */ @@ -27,9 +28,127 @@ export async function assertMemoryReachable(): Promise { if (refusal) throw new AuthoredFlowExecutionError('memory_unreachable', refusal.message); } -/** Lets the executor see `learn` calls the body has not awaited. */ -export interface MemoryWriteTracker { - readonly pending: Set>; +/** + * The promise `learn` returns. Its constructor is not `Promise`, so `await`, + * `Promise.all` and friends reach it through `then`, which records that the + * body consumed the result. + */ +class LearnCall extends Promise { + static override get [Symbol.species](): PromiseConstructor { return Promise; } + observed = false; + settled = false; + override then( + onFulfilled?: ((value: void) => A | PromiseLike) | null, + onRejected?: ((reason: unknown) => B | PromiseLike) | null, + ): Promise { + this.observed = true; + return super.then(onFulfilled, onRejected); + } +} + +/** Lets the executor find `learn` calls whose outcome the body never consumed. */ +export class MemoryWriteTracker { + private readonly calls: LearnCall[] = []; + track(write: Promise): Promise { + const call = new LearnCall((resolve, reject) => { void write.then(resolve, reject); }); + // Bypass the override: marking the rejection handled must not count as the body consuming it. + void Promise.prototype.then.call(call, () => { call.settled = true; }, () => { call.settled = true; }); + this.calls.push(call); + return call; + } + /** Every call the body did not await, after letting each one settle. */ + async unawaited(): Promise { + const unobserved = this.calls.filter(call => !call.observed || !call.settled); + await Promise.allSettled(unobserved.map(call => Promise.prototype.then.call(call, () => {}, () => {}))); + return unobserved.length; + } +} + +/** Root-run stream holding every finding `learn` committed: the source of truth the trajectory files are projected from. */ +export const MEMORY_LEARN_STREAM = 'memory-learn'; + +export interface LearnRecord { + readonly memory: 'learn'; + readonly id: string; + readonly scope: string; + readonly decision: Required; + readonly learnedAt: string; +} + +export interface CommittedLearn { + readonly record: LearnRecord; + readonly runId: string; + readonly offset: number; +} + +/** Where `learn` journals a finding before it may become recallable. */ +export interface LearnLedger { + /** The recorded commit for `id`, or a new one journaled from `build()`. Concurrent calls for one id share one commit. */ + commit(id: string, build: () => LearnRecord): Promise; +} + +function canonicalLearnRecord(raw: unknown): LearnRecord | undefined { + if (typeof raw !== 'object' || raw === null) return undefined; + const value = raw as Record; + const decision = value.decision as Record | undefined; + if (value.memory !== 'learn' || typeof value.id !== 'string' || typeof value.scope !== 'string' + || typeof value.learnedAt !== 'string' || typeof decision !== 'object' || decision === null) return undefined; + const { question, chosen, reasoning, alternatives } = decision; + if (typeof question !== 'string' || typeof chosen !== 'string' || typeof reasoning !== 'string' + || !Array.isArray(alternatives) || alternatives.some(item => typeof item !== 'string')) return undefined; + // Fixed field order, never the record as it came back: the journal stores + // stream messages with sorted keys, and the projected file must be + // byte-identical whether it is written now or republished on resume. + return { + memory: 'learn', id: value.id, scope: value.scope, + decision: { question, chosen, reasoning, alternatives: [...alternatives] as string[] }, + learnedAt: value.learnedAt, + }; +} + +/** + * A ledger on the durable root run's `memory-learn` stream. The stream is read + * once per execution, so a resumed body finds what an earlier attempt already + * committed and does not journal it twice. + */ +export function journalLearnLedger(journal: Pick, rootRunId: string): LearnLedger { + let loaded: Promise> | undefined; + const inFlight = new Map>(); + async function load(): Promise> { + const commits = new Map(); + let offset = 0; + for (;;) { + const page = await journal.streamRead(rootRunId, MEMORY_LEARN_STREAM, offset, 1000); + page.messages.forEach((message, index) => { + const record = canonicalLearnRecord((message as { message?: unknown }).message ?? message); + // The first commit for an id wins; it is the one its file cites. + if (record !== undefined && !commits.has(record.id)) commits.set(record.id, { record, runId: rootRunId, offset: offset + index }); + }); + if (page.messages.length === 0 || page.next_offset <= offset) break; + offset = page.next_offset; + } + return commits; + } + return { + commit(id, build) { + const pending = inFlight.get(id); + if (pending !== undefined) return pending; + const committing = (async () => { + loaded ??= load(); + const commits = await loaded; + const existing = commits.get(id); + if (existing !== undefined) return existing; + const record = build(); + const { offset } = await journal.streamAppend(rootRunId, MEMORY_LEARN_STREAM, record); + const committed = { record, runId: rootRunId, offset }; + commits.set(id, committed); + return committed; + })(); + inFlight.set(id, committing); + void committing.then(() => inFlight.delete(id), () => inFlight.delete(id)); + return committing; + }, + }; } export function authoredMemory( @@ -37,6 +156,7 @@ export function authoredMemory( assertOpen: () => void, enabled: boolean, tracker?: MemoryWriteTracker, + ledger?: LearnLedger, ): MemoryHelper { function assertEnabled(): void { if (!enabled) throw new AuthoredFlowExecutionError('unsupported_header', 'f.memory requires script memory; memory.script is false'); @@ -46,22 +166,42 @@ export function authoredMemory( assertEnabled(); await assertMemoryReachable(); assertOpen(); - const learned = await readLearned(scope); - const { openAiHist } = await import('ai-hist'); + const { defaultDbPath, openAiHist } = await import('ai-hist'); + const learned = await readLearned(defaultDbPath(), scope); const reader = await openAiHist({ projectScope: scope, fallback: 'error' }); try { return action(reader, learned); } finally { reader.close(); } } + /** + * Journal first, then project. The finding is committed to the root run's + * `memory-learn` stream before its trajectory file exists, so nothing is + * recallable that the journal does not record. A crash between the two + * leaves a journaled finding without its file; the resumed body re-runs + * `learn`, finds the commit, and republishes the identical file from it + * without journaling again. With no durable root run there is nowhere to + * journal, so `learn` refuses rather than write unrecorded memory. + */ async function learn(finding: MemoryFinding): Promise { assertOpen(); assertEnabled(); const decision = normalizeFinding(finding); + if (ledger === undefined) { + throw new AuthoredFlowExecutionError('memory_unjournaled', 'f.memory.learn requires a durable root run to journal the finding; run the flow through flows run'); + } await assertMemoryReachable(); assertOpen(); + const id = learnedId(scope, decision); + let committed: CommittedLearn; + try { + committed = await ledger.commit(id, () => ({ memory: 'learn', id, scope, decision, learnedAt: new Date().toISOString() })); + } catch (error) { + throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn could not journal the finding: ${error instanceof Error ? error.message : String(error)}`); + } const { defaultDbPath } = await import('ai-hist'); + const dbPath = defaultDbPath(); try { - await writeLearned(scope, decision, (await stat(defaultDbPath())).mode & 0o777); + await writeLearned(dbPath, committed, (await stat(dbPath)).mode & 0o777); } catch (error) { - throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn could not persist the finding: ${error instanceof Error ? error.message : String(error)}`); + throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn journaled the finding but could not publish it: ${error instanceof Error ? error.message : String(error)}`); } } return { @@ -77,11 +217,7 @@ export function authoredMemory( }), learn(finding) { const write = learn(finding); - if (tracker !== undefined) { - tracker.pending.add(write); - void write.then(() => tracker.pending.delete(write), () => tracker.pending.delete(write)); - } - return write; + return tracker === undefined ? write : tracker.track(write); }, }; } @@ -105,35 +241,45 @@ function normalizeFinding(finding: MemoryFinding): Required { }; } -const learnedDir = (scope: string): string => join(scope, '.trajectories', 'compacted'); +/** + * Learned findings live beside the ai-hist database, not beside the flow + * source: a flow deployed from a read-only checkout, package or image can + * still learn. The directory is keyed by the scope's hash, and each file + * still names the scope as its `projectId`. `ai-hist sync` indexes them when + * `TRAJECTORY_ROOT` includes `/flows-memory`; reads here do not need it. + */ +const learnedDir = (dbPath: string, scope: string): string => + join(dirname(dbPath), 'flows-memory', basename(scope), '.trajectories', 'compacted'); const LEARNED_PREFIX = 'flows-memory-'; +const learnedId = (scope: string, decision: Required): string => + `${LEARNED_PREFIX}${createHash('sha256').update(JSON.stringify([scope, decision])).digest('hex').slice(0, 32)}`; + /** - * Persist one finding as an immutable compacted trajectory file, the format - * `ai-hist sync` ingests. The file name is a hash of scope and finding, so - * re-learning the same finding (a resumed body re-running `learn`, or a later - * run reaching the same conclusion) rewrites one file rather than adding one. - * The file is staged and renamed into place, so it appears whole or not at - * all, and nothing else is shared between writers: no lock, and the ai-hist - * database is never modified. It takes the database's mode, so a private - * history does not leak findings through a world-readable file. + * Project one committed finding as an immutable compacted trajectory file, the + * format `ai-hist sync` ingests. Its content is derived only from the journal + * commit (including the time it was learned and a citation of the commit), so + * republishing on resume writes the same bytes. The file name is a hash of + * scope and finding, so re-learning the same finding rewrites one file. It is + * staged and renamed into place, so it appears whole or not at all, and the + * ai-hist database is never modified. It takes the database's mode, so a + * private history does not leak findings through a world-readable file. */ -async function writeLearned(scope: string, decision: Required, mode: number): Promise { - const now = new Date().toISOString(); - const id = `${LEARNED_PREFIX}${createHash('sha256').update(JSON.stringify([scope, decision])).digest('hex').slice(0, 32)}`; +async function writeLearned(dbPath: string, { record, runId, offset }: CommittedLearn, mode: number): Promise { const trajectory = { - id, + id: record.id, version: 1, type: 'compacted', - projectId: scope, - task: { title: decision.question, description: null }, + projectId: record.scope, + task: { title: record.decision.question, description: null }, status: 'completed', - startedAt: now, - completedAt: now, - decisions: [decision], + startedAt: record.learnedAt, + completedAt: record.learnedAt, + decisions: [record.decision], retrospective: { summary: null, approach: null, learnings: [], confidence: null }, + journal: { runId, stream: MEMORY_LEARN_STREAM, offset }, }; - const file = join(learnedDir(scope), `${id}.json`); + const file = join(learnedDir(dbPath, record.scope), `${record.id}.json`); await mkdir(dirname(file), { recursive: true }); const staged = `${file}.${process.pid}.${randomUUID()}.tmp`; try { @@ -151,8 +297,8 @@ interface LearnedTrajectory { } /** This scope's learned findings, read straight from disk so they need no `ai-hist sync`. */ -async function readLearned(scope: string): Promise { - const dir = learnedDir(scope); +async function readLearned(dbPath: string, scope: string): Promise { + const dir = learnedDir(dbPath, scope); let names: string[]; try { names = await readdir(dir); } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') return []; @@ -163,7 +309,14 @@ async function readLearned(scope: string): Promise { if (!name.startsWith(LEARNED_PREFIX) || !name.endsWith('.json')) continue; const path = join(dir, name); let raw: Record; - try { raw = JSON.parse(await readFile(path, 'utf8')) as Record; } catch { continue; } + try { + raw = JSON.parse(await readFile(path, 'utf8')) as Record; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue; + // Files here are only ever published whole, so an unreadable one is real + // damage; recalling around it would silently drop journaled memory. + throw new AuthoredFlowExecutionError('memory_unreachable', `learned finding ${path} is unreadable: ${error instanceof Error ? error.message : String(error)}`); + } const decisions = Array.isArray(raw.decisions) ? raw.decisions as TrajectoryEntry['decisions'] : []; const task = (raw.task ?? {}) as { title?: string | null; description?: string | null }; const id = typeof raw.id === 'string' ? raw.id : name.slice(0, -'.json'.length); diff --git a/packages/sdk/src/cli/run.ts b/packages/sdk/src/cli/run.ts index f4b2bd1f7..ac8077291 100644 --- a/packages/sdk/src/cli/run.ts +++ b/packages/sdk/src/cli/run.ts @@ -1114,7 +1114,7 @@ export function memoryWriteFailure( error: AuthoredFlowExecutionError, fallbackRunId?: string, ): RunExecution | undefined { - if (error.code !== 'memory_finding_invalid' && error.code !== 'memory_unwritable') return undefined; + if (error.code !== 'memory_finding_invalid' && error.code !== 'memory_unwritable' && error.code !== 'memory_unjournaled') return undefined; const rootRunId = error.rootRunId ?? fallbackRunId; const runId = error.runId ?? rootRunId; return { diff --git a/packages/sdk/src/failure-kinds.ts b/packages/sdk/src/failure-kinds.ts index d5d5c81c4..8ab0764d2 100644 --- a/packages/sdk/src/failure-kinds.ts +++ b/packages/sdk/src/failure-kinds.ts @@ -142,6 +142,8 @@ export const RUN_FAILURE_KINDS = [ 'memory_finding_invalid', /** `f.memory.learn` could not persist the finding to script memory. */ 'memory_unwritable', + /** `f.memory.learn` ran without a durable root run to journal the finding in. */ + 'memory_unjournaled', /** * `--local-agent` cannot be honoured for this invocation, so it is refused * rather than accepted and ignored. A local agent worker is admitted at run diff --git a/packages/sdk/tests/f-memory.test.ts b/packages/sdk/tests/f-memory.test.ts index ad3dfeb9a..082dd4dee 100644 --- a/packages/sdk/tests/f-memory.test.ts +++ b/packages/sdk/tests/f-memory.test.ts @@ -3,7 +3,7 @@ import { createHash } from 'node:crypto'; import { chmodSync, existsSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { createRequire } from 'node:module'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { basename, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { flow } from '@relayflows/surface'; import { afterEach, beforeEach, expect, it, vi } from 'vitest'; @@ -20,7 +20,7 @@ vi.mock('node:fs/promises', async importOriginal => { }; }); import { openAiHist } from 'ai-hist'; -import { authoredMemory, scriptMemoryScope } from '../src/authored-memory.js'; +import { authoredMemory, journalLearnLedger, MEMORY_LEARN_STREAM, scriptMemoryScope } from '../src/authored-memory.js'; import { executeAuthoredFlow } from '../src/authored-flow-executor.js'; import { AuthoredFlowExecutionError } from '../src/authored-flow-error.js'; import { emptyReport, memoryWriteFailure } from '../src/cli/run.js'; @@ -39,10 +39,43 @@ beforeEach(() => { }); afterEach(() => { vi.unstubAllEnvs(); rmSync(dir, { recursive: true, force: true }); }); +/** The daemon's stream storage, with the kernel's paging contract (`engine/remote.rs` `read_stream`). */ +function streamStore() { + const stored = new Map(); + const key = (runId: string, stream: string): string => `${runId}\u0000${stream}`; + const appended: Array<{ stream: string; message: unknown; fileExisted: boolean }> = []; + return { + stored, appended, + list: (runId: string, stream: string) => stored.get(key(runId, stream)) ?? [], + async streamAppend(runId: string, stream: string, message: unknown) { + const list = stored.get(key(runId, stream)) ?? []; + stored.set(key(runId, stream), list); + appended.push({ stream, message, fileExisted: existsSync(compactedDir()) && readdirSync(compactedDir()).length > 0 }); + // Stored the way the journal stores it: keys sorted. + list.push(JSON.parse(JSON.stringify(message, Object.keys(flatKeys(message)).sort()))); + return { offset: list.length - 1 }; + }, + async streamRead(runId: string, stream: string, fromOffset: number, limit: number) { + const messages = (stored.get(key(runId, stream)) ?? []).slice(fromOffset, fromOffset + limit); + return { messages, next_offset: fromOffset + messages.length }; + }, + }; +} +function flatKeys(value: unknown, keys: Record = {}): Record { + if (typeof value === 'object' && value !== null) { + for (const [k, v] of Object.entries(value)) { keys[k] = true; flatKeys(v, keys); } + } + return keys; +} +let store: ReturnType; +beforeEach(() => { store = streamStore(); }); +const memoryFor = (forScope: string) => authoredMemory(forScope, () => {}, true, undefined, journalLearnLedger(store, 'root-1')); +const compactedDir = (forScope = scope) => join(dir, 'flows-memory', basename(forScope), '.trajectories', 'compacted'); + it('reads the seeded local SQLite database with cloud unused and fallback disabled', async () => { const db = await openAiHist({ dbPath, projectScope: scope, fallback: 'error' }); try { expect(db.search('retry safely')).toHaveLength(1); } finally { db.close(); } - const memory = authoredMemory(scope, () => {}, true); + const memory = memoryFor(scope); expect(await memory.recall('retry safely')).toMatchObject([{ id: 1, project: scope }]); expect(await memory.why('retry safely')).toMatchObject([{ id: 'run-1', decisions: [{ chosen: 'idempotency key', reasoning: 'avoid duplicate writes' }], @@ -52,9 +85,9 @@ it('reads the seeded local SQLite database with cloud unused and fallback disabl }); it('cannot widen script scope using a raw project option or another flow name', async () => { - const memory = authoredMemory(scope, () => {}, true); + const memory = memoryFor(scope); expect(await memory.recall('', { project: `${scope}-other` } as never)).toMatchObject([{ id: 1 }]); - const other = authoredMemory(scriptMemoryScope(join(dir, 'test.flow.ts'), 'another-flow'), () => {}, true); + const other = memoryFor(scriptMemoryScope(join(dir, 'test.flow.ts'), 'another-flow')); expect(await other.recall('retry safely')).toEqual([]); expect(await other.why('retry safely')).toEqual([]); }); @@ -112,6 +145,8 @@ it('maps an unavailable provider probe to memory_unreachable', async () => { function completingJournal(): JournalClient { const journal = new JournalClient('/unused'); + vi.spyOn(journal, 'streamAppend').mockImplementation((runId, stream, message) => store.streamAppend(runId, stream, message)); + vi.spyOn(journal, 'streamRead').mockImplementation((runId, stream, from, limit) => store.streamRead(runId, stream, from, limit ?? 100)); vi.spyOn(journal, 'runStart').mockResolvedValue({ run_id: 'completion', status: 'completed', completion_reason: 'success', completed_steps: 1, }); @@ -134,7 +169,7 @@ it('persists a learned finding that a later run of the same flow recalls', async expect(await f.memory.why('webhook retry')).toEqual([]); await f.memory.learn(finding); f.done('success'); - }), completingJournal(), undefined, { flowPath }); + }), completingJournal(), undefined, { flowPath, rootRunId: 'root-1' }); let recalled: unknown[] = []; let why: unknown[] = []; @@ -142,37 +177,37 @@ it('persists a learned finding that a later run of the same flow recalls', async recalled = await f.memory.recall('exponential backoff'); why = await f.memory.why('webhook retry'); f.done('success'); - }), completingJournal(), undefined, { flowPath }); + }), completingJournal(), undefined, { flowPath, rootRunId: 'root-1' }); expect(recalled).toMatchObject([{ source: 'trajectory', project: scope }]); expect(why).toMatchObject([{ projectId: scope, decisions: [finding] }]); // The trajectory file is what a later `ai-hist sync` re-ingests. - const compacted = join(scope, '.trajectories', 'compacted'); + const compacted = compactedDir(); const [file] = readdirSync(compacted); expect(JSON.parse(readFileSync(join(compacted, file!), 'utf8'))).toMatchObject({ projectId: scope, decisions: [finding] }); }); it('learns idempotently and only into its own script scope', async () => { - const memory = authoredMemory(scope, () => {}, true); + const memory = memoryFor(scope); const finding = { question: 'cache invalidation', chosen: 'ttl', reasoning: 'simple' }; await memory.learn(finding); await memory.learn(finding); expect(await memory.recall('cache invalidation')).toHaveLength(1); - expect(readdirSync(join(scope, '.trajectories', 'compacted'))).toHaveLength(1); + expect(readdirSync(compactedDir())).toHaveLength(1); expect(await memory.recall('retry safely')).toHaveLength(1); - const other = authoredMemory(scriptMemoryScope(join(dir, 'test.flow.ts'), 'another-flow'), () => {}, true); + const other = memoryFor(scriptMemoryScope(join(dir, 'test.flow.ts'), 'another-flow')); expect(await other.recall('cache invalidation')).toEqual([]); expect(await other.why('cache invalidation')).toEqual([]); }); it('rejects invalid findings before writing, and unwritable stores', async () => { - const memory = authoredMemory(scope, () => {}, true); + const memory = memoryFor(scope); await expect(memory.learn({ question: '', chosen: 'a', reasoning: 'r' })) .rejects.toMatchObject({ code: 'memory_finding_invalid' }); await expect(memory.learn({ question: 'q', chosen: 'a', reasoning: 'r', alternatives: [1] } as never)) .rejects.toMatchObject({ code: 'memory_finding_invalid' }); - expect(existsSync(join(scope, '.trajectories'))).toBe(false); + expect(existsSync(join(dir, 'flows-memory'))).toBe(false); if (process.getuid?.() !== 0) { chmodSync(dir, 0o500); try { @@ -182,12 +217,11 @@ it('rejects invalid findings before writing, and unwritable stores', async () => } }); -const compactedDir = () => join(scope, '.trajectories', 'compacted'); const dbDigest = () => createHash('sha256').update(readFileSync(dbPath)).digest('hex'); it('keeps concurrent findings without touching the shared database', async () => { const before = dbDigest(); - const memory = authoredMemory(scope, () => {}, true); + const memory = memoryFor(scope); await Promise.all(Array.from({ length: 6 }, (_, i) => memory.learn({ question: `parallel finding ${i}`, chosen: 'c', reasoning: 'r' }))); expect(await memory.recall('parallel finding')).toHaveLength(6); @@ -196,7 +230,7 @@ it('keeps concurrent findings without touching the shared database', async () => }); it('dedupes a finding once ai-hist sync has indexed it, preferring the synced row', async () => { - const memory = authoredMemory(scope, () => {}, true); + const memory = memoryFor(scope); await memory.learn({ question: 'synced finding', chosen: 'c', reasoning: 'r' }); const [diskEntry] = await memory.recall('synced finding'); expect(diskEntry).toMatchObject({ source: 'trajectory', project: scope }); @@ -221,7 +255,7 @@ it('dedupes a finding once ai-hist sync has indexed it, preferring the synced ro }); it('applies recall options to findings not yet synced', async () => { - const memory = authoredMemory(scope, () => {}, true); + const memory = memoryFor(scope); await memory.learn({ question: 'option finding', chosen: 'c', reasoning: 'r' }); expect(await memory.recall('option finding', { source: 'claude' })).toEqual([]); expect(await memory.recall('option finding', { tag: 'anything' })).toEqual([]); @@ -232,12 +266,12 @@ it('applies recall options to findings not yet synced', async () => { it.skipIf(process.platform === 'win32')('gives the trajectory file the database file mode', async () => { chmodSync(dbPath, 0o600); - await authoredMemory(scope, () => {}, true).learn({ question: 'private', chosen: 'c', reasoning: 'r' }); + await memoryFor(scope).learn({ question: 'private', chosen: 'c', reasoning: 'r' }); expect(statSync(join(compactedDir(), readdirSync(compactedDir())[0]!)).mode & 0o777).toBe(0o600); }); it('leaves nothing behind when the trajectory cannot be published', async () => { - const memory = authoredMemory(scope, () => {}, true); + const memory = memoryFor(scope); failRename = to => to.startsWith(compactedDir()) && to.endsWith('.json'); try { await expect(memory.learn({ question: 'doomed', chosen: 'c', reasoning: 'r' })) @@ -251,12 +285,98 @@ it('fails the run when the body does not await learn', async () => { await expect(executeAuthoredFlow(flow('memory-example', { memory: { script: true } }, async f => { void f.memory.learn({ question: 'unawaited', chosen: 'c', reasoning: 'r' }); f.done('success'); - }), completingJournal(), undefined, { flowPath: join(dir, 'test.flow.ts') })) + }), completingJournal(), undefined, { flowPath: join(dir, 'test.flow.ts'), rootRunId: 'root-1' })) .rejects.toMatchObject({ code: 'unawaited_step' }); }); +it('fails the run for an unawaited learn that already failed before the body returned', async () => { + await expect(executeAuthoredFlow(flow('memory-example', { memory: { script: true } }, async f => { + void f.memory.learn({ question: '', chosen: 'c', reasoning: 'r' }); + await new Promise(resolve => setTimeout(resolve, 50)); + f.done('success'); + }), completingJournal(), undefined, { flowPath: join(dir, 'test.flow.ts'), rootRunId: 'root-1' })) + .rejects.toMatchObject({ code: 'unawaited_step' }); +}); + +it('accepts a learn failure the body awaited and handled', async () => { + const result = await executeAuthoredFlow(flow('memory-example', { memory: { script: true } }, async f => { + await f.memory.learn({ question: '', chosen: 'c', reasoning: 'r' }).catch(() => undefined); + await Promise.all([f.memory.learn({ question: 'all', chosen: 'c', reasoning: 'r' })]); + f.done('success'); + }), completingJournal(), undefined, { flowPath: join(dir, 'test.flow.ts'), rootRunId: 'root-1' }); + expect(result.completionReason).toBe('success'); +}); + +it('journals the finding on the root run before it becomes recallable, and cites the commit', async () => { + const finding = { question: 'journal first', chosen: 'c', reasoning: 'r', alternatives: ['x'] }; + await memoryFor(scope).learn(finding); + expect(store.appended).toEqual([{ stream: MEMORY_LEARN_STREAM, message: expect.objectContaining({ + memory: 'learn', scope, decision: finding, + }), fileExisted: false }]); + const [name] = readdirSync(compactedDir()); + expect(JSON.parse(readFileSync(join(compactedDir(), name!), 'utf8'))) + .toMatchObject({ journal: { runId: 'root-1', stream: MEMORY_LEARN_STREAM, offset: 0 }, decisions: [finding] }); +}); + +it('refuses to learn without a durable root run, writing nothing', async () => { + await expect(authoredMemory(scope, () => {}, true).learn({ question: 'q', chosen: 'c', reasoning: 'r' })) + .rejects.toMatchObject({ code: 'memory_unjournaled' }); + await expect(executeAuthoredFlow(flow('memory-example', { memory: { script: true } }, async f => { + await f.memory.learn({ question: 'q', chosen: 'c', reasoning: 'r' }); + f.done('success'); + }), completingJournal(), undefined, { flowPath: join(dir, 'test.flow.ts') })) + .rejects.toMatchObject({ code: 'memory_unjournaled' }); + expect(existsSync(join(dir, 'flows-memory'))).toBe(false); +}); + +it('publishes nothing when the journal append fails', async () => { + vi.spyOn(store, 'streamAppend').mockRejectedValueOnce(new Error('journal down')); + const memory = memoryFor(scope); + await expect(memory.learn({ question: 'not journaled', chosen: 'c', reasoning: 'r' })) + .rejects.toMatchObject({ code: 'memory_unwritable', message: expect.stringContaining('could not journal') }); + expect(existsSync(join(dir, 'flows-memory'))).toBe(false); + expect(await memory.recall('not journaled')).toEqual([]); +}); + +it('on resume, republishes a journaled finding from its commit without journaling it again', async () => { + const finding = { question: 'resumed finding', chosen: 'c', reasoning: 'r', alternatives: [] }; + await memoryFor(scope).learn(finding); + const [name] = readdirSync(compactedDir()); + const published = readFileSync(join(compactedDir(), name!), 'utf8'); + // The crash window: journaled, but the file never landed. + rmSync(join(compactedDir(), name!)); + // A resumed body is a new execution: a fresh ledger reading the same root stream. + const resumed = memoryFor(scope); + expect(await resumed.recall('resumed finding')).toEqual([]); + await resumed.learn(finding); + expect(store.list('root-1', MEMORY_LEARN_STREAM)).toHaveLength(1); + expect(readFileSync(join(compactedDir(), name!), 'utf8')).toBe(published); + expect(await resumed.recall('resumed finding')).toHaveLength(1); +}); + +it('journals concurrent learns of one finding once', async () => { + const memory = memoryFor(scope); + const finding = { question: 'same finding', chosen: 'c', reasoning: 'r' }; + await Promise.all([memory.learn(finding), memory.learn(finding), memory.learn(finding)]); + expect(store.list('root-1', MEMORY_LEARN_STREAM)).toHaveLength(1); +}); + +it('fails recall closed on a damaged learned file instead of skipping it', async () => { + const memory = memoryFor(scope); + await memory.learn({ question: 'damaged', chosen: 'c', reasoning: 'r' }); + const [name] = readdirSync(compactedDir()); + writeFileSync(join(compactedDir(), name!), '{ not json'); + await expect(memory.recall('damaged')).rejects.toMatchObject({ code: 'memory_unreachable' }); +}); + +it('keeps learned files out of the flow source directory', async () => { + await memoryFor(scope).learn({ question: 'placement', chosen: 'c', reasoning: 'r' }); + expect(existsSync(scope)).toBe(false); + expect(readdirSync(compactedDir())).toHaveLength(1); +}); + it('reports learn failures with their own kind, not protocol_error', () => { - for (const code of ['memory_finding_invalid', 'memory_unwritable'] as const) { + for (const code of ['memory_finding_invalid', 'memory_unwritable', 'memory_unjournaled'] as const) { const error = Object.assign(new AuthoredFlowExecutionError(code, 'nope'), { rootRunId: 'root-1' }); expect(memoryWriteFailure('run', emptyReport('run'), '/sock', error)).toMatchObject({ exitCode: 1, report: { runId: 'root-1', rootRunId: 'root-1', status: 'failed', completionReason: 'step_failed', diff --git a/packages/sdk/tests/memory-learn-live.test.ts b/packages/sdk/tests/memory-learn-live.test.ts new file mode 100644 index 000000000..ce8ede329 --- /dev/null +++ b/packages/sdk/tests/memory-learn-live.test.ts @@ -0,0 +1,62 @@ +import { execFileSync } from 'node:child_process'; +import { existsSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { basename, join, resolve } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { MEMORY_LEARN_STREAM, scriptMemoryScope } from '../src/authored-memory.js'; +import { JournalClient } from '../src/journal-client.js'; +import { socketPathFor } from '../src/daemon-connection.js'; +import { chainFixture } from './flow-chain-fixture.js'; + +const closes: Array<() => Promise> = []; +afterEach(async () => { + for (const close of closes.splice(0).reverse()) await close(); + delete process.env.AI_HIST_DB; +}); + +describe('f.memory.learn through the built CLI and a real kernel', () => { + it('journals the finding on the root run, then a later run of the same flow recalls it', async () => { + const fixture = chainFixture(); + closes.push(() => fixture.close()); + const dbPath = join(fixture.root, 'ai-history.db'); + const scope = scriptMemoryScope(fixture.flowPath, 'learner'); + execFileSync(process.execPath, [resolve('../../testdata/memory/seed.mjs'), dbPath, scope]); + process.env.AI_HIST_DB = dbPath; + + writeFileSync(fixture.flowPath, `import { flow } from '@relayflows/surface'; +export default flow('learner', { memory: { script: true } }, async f => { + await f.memory.learn({ question: 'how should the importer page', chosen: 'cursor pagination', + reasoning: 'offsets skip rows under concurrent inserts', alternatives: ['offset pagination'] }); + f.done('success'); +}); +`); + const learned = fixture.invoke('run', '--no-observer-link', '--json', '--data-dir', fixture.data, fixture.flowPath, '--input', '{}'); + expect(learned.status, learned.stderr + learned.stdout).toBe(0); + const report = JSON.parse(learned.stdout) as { runId: string; completionReason: string }; + expect(report.completionReason).toBe('success'); + + const journal = new JournalClient(socketPathFor(fixture.data)); + await journal.connect(); + await journal.hello('memory-learn-live-test'); + const records = (await journal.streamRead(report.runId, MEMORY_LEARN_STREAM, 0, 1000)).messages + .map(message => (message as { message?: unknown }).message ?? message) as Array>; + journal.close(); + expect(records).toMatchObject([{ memory: 'learn', scope, decision: { chosen: 'cursor pagination' } }]); + + const compacted = join(fixture.root, 'flows-memory', basename(scope), '.trajectories', 'compacted'); + const [file] = readdirSync(compacted); + expect(JSON.parse(readFileSync(join(compacted, file!), 'utf8'))) + .toMatchObject({ id: records[0]!.id, journal: { runId: report.runId, stream: MEMORY_LEARN_STREAM, offset: 0 } }); + expect(existsSync(scope)).toBe(false); + + writeFileSync(fixture.flowPath, `import { flow } from '@relayflows/surface'; +export default flow('learner', { memory: { script: true } }, async f => { + const [why] = await f.memory.why('importer page'); + const hits = await f.memory.recall('cursor pagination'); + f.done(why?.decisions[0]?.chosen === 'cursor pagination' && hits.length === 1 ? 'success' : 'step_failed'); +}); +`); + const recalled = fixture.invoke('run', '--no-observer-link', '--json', '--data-dir', fixture.data, fixture.flowPath, '--input', '{}'); + expect(recalled.status, recalled.stderr + recalled.stdout).toBe(0); + expect(JSON.parse(recalled.stdout)).toMatchObject({ completionReason: 'success' }); + }, 60_000); +}); diff --git a/packages/surface/AUTHORING.md b/packages/surface/AUTHORING.md index cd1577a77..0a3f3c820 100644 --- a/packages/surface/AUTHORING.md +++ b/packages/surface/AUTHORING.md @@ -1290,7 +1290,7 @@ export interface MemoryHelper { recall(query: string, options?: MemoryRecallOptions): Promise; /** Best matching decision trajectory, or an empty array. No journal step. */ why(task: string): Promise; - /** Persists a finding to this flow's script memory; later runs see it via recall/why. Idempotent per finding; no journal step. */ + /** Journals the finding on the durable root run, then persists it to this flow's script memory; later runs see it via recall/why. Idempotent per finding. Must be awaited. */ learn(finding: MemoryFinding): Promise; } ``` diff --git a/packages/surface/src/memory.ts b/packages/surface/src/memory.ts index e9d8ef1e9..2924fba4e 100644 --- a/packages/surface/src/memory.ts +++ b/packages/surface/src/memory.ts @@ -15,6 +15,6 @@ export interface MemoryHelper { recall(query: string, options?: MemoryRecallOptions): Promise; /** Best matching decision trajectory, or an empty array. No journal step. */ why(task: string): Promise; - /** Persists a finding to this flow's script memory; later runs see it via recall/why. Idempotent per finding; no journal step. */ + /** Journals the finding on the durable root run, then persists it to this flow's script memory; later runs see it via recall/why. Idempotent per finding. Must be awaited. */ learn(finding: MemoryFinding): Promise; } diff --git a/testdata/memory/README.md b/testdata/memory/README.md index c916f692c..8ac65b130 100644 --- a/testdata/memory/README.md +++ b/testdata/memory/README.md @@ -1,7 +1,16 @@ # Minimal local script-memory example This slice implements `f.memory.recall`, `f.memory.why`, and `f.memory.learn` -through ai-hist 0.4.1. None of them journal a step. `memory: { script: true }` is already part +through ai-hist 0.4.1. Reads journal nothing. `learn` appends the finding to +the durable root run's `memory-learn` stream first, then projects it as a +compacted trajectory file at +`/flows-memory//.trajectories/compacted/.json` that +cites the commit. ai-hist 0.4.1 has no trajectory writer and its database is +never modified; reads merge the database with the scope's learned files. A +resumed body re-running `learn` republishes from the commit instead of +journaling twice, and `learn` refuses (`memory_unjournaled`) without a root +run. `tests/memory-learn-live.test.ts` exercises it through the built CLI and +a real kernel. `memory: { script: true }` is already part of the surface header contract and now works in the internal authored executor. `seed.mjs` creates a new deterministic SQLite database with two script scopes, @@ -42,17 +51,6 @@ than ai-hist's automatic scan of unrelated local JSONL history. ## Deferred acceptance work -- Journaled `learn` effects. ai-hist 0.4.1 has no public trajectory writer, - so `learn` writes the finding as an immutable compacted trajectory file at - `/.trajectories/compacted/.json` (the format `ai-hist sync` - ingests), staged and atomically renamed, with the database's file mode. - `recall`/`why` read the database plus those files, deduplicating synced - copies, so the next run sees the finding without a sync. The ai-hist - database is never written, so there is no lock and no race with - `ai-hist sync`. The id is a hash of scope and finding, so a resumed body - re-running `learn`, or a later run learning the same thing, rewrites one - file. An unawaited `learn` fails the run with `unawaited_step`. It is still - not a journaled effect. - Identity-scoped agent context injection. `memory.agent: true` explicitly refuses; it must not silently read script scope. - CLI-only provider operation and automatic creation of a fresh database. @@ -61,5 +59,5 @@ than ai-hist's automatic scan of unrelated local JSONL history. - Automatic trajectory import and the behavioural Gate 5 acceptance example. - Cloud push and pair mode, per the original exclusions. -This is a local proof slice, not completion of Gate 5 or the full original +This is a local slice, not completion of Gate 5 or the full original recall/why/learn acceptance matrix. The lead owns PR creation and CI review. From ec8204cdb4faefaf95a84b8211dcc59cc7d25cdb Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Fri, 9 Oct 2026 21:20:33 -0700 Subject: [PATCH 7/8] fix(sdk): keep journal errors resumable; stage before journaling; converge commits From a self-review of the journaled learn: - Journal errors from the memory-learn ledger propagate unchanged instead of becoming memory_unwritable, so a read interruption or lost lease keeps its resumable meaning for driveRoot. - A failed stream read is no longer cached for the execution. - The trajectory file is staged under a temporary name BEFORE the journal append and renamed into place after it, so disk failures (space, permissions) happen before anything is journaled; a journal failure publishes nothing. - After appending, the ledger re-reads the stream and publishes the first commit for the id, so attempts racing on one root converge on the same bytes. The file cites {runId, stream, id}. - A failed body waits for in-flight learns before handing the root back. - learn stats the DB for its mode instead of loading it; reads parse learned files in parallel and reject any malformed shape as memory_unreachable, which the run/resume reporters now classify. - Diagnostics drop the duplicated `: ` prefix. - Docs: learn's stream append is not a journal step; TRAJECTORY_ROOT replaces ai-hist's default scan; the no-lock claim is scoped to the files. The live test restores AI_HIST_DB with vi.stubEnv. Co-Authored-By: Claude Opus 5.5 (1M context) Session-Id: b6cf20aa-bdbb-4248-91ab-d2a4fd2ce1b8 --- docs/SURFACE.md | 44 +-- packages/sdk/src/authored-flow-executor.ts | 4 + packages/sdk/src/authored-memory.ts | 318 ++++++++++++------- packages/sdk/src/cli/run.ts | 11 +- packages/sdk/src/failure-kinds.ts | 2 + packages/sdk/tests/f-memory.test.ts | 71 ++++- packages/sdk/tests/memory-learn-live.test.ts | 8 +- testdata/memory/README.md | 10 +- 8 files changed, 320 insertions(+), 148 deletions(-) diff --git a/docs/SURFACE.md b/docs/SURFACE.md index 6fdfa7524..7d66b7726 100644 --- a/docs/SURFACE.md +++ b/docs/SURFACE.md @@ -129,29 +129,37 @@ No process runs between events: the handler wakes, executes to its next await, p comment-only gap was closed by the pinned relay-helpers release. The local memory slice supports `recall`, `why`, and `learn` in authored - flows, with no journal step for any of them. Script scope is stable across runs of + flows. None of them is a journal step: reads journal nothing, and `learn` + appends a record to the root run's `memory-learn` stream. Script scope is stable across runs of the same flow file and name; reads cannot widen it to another flow. The existing `memory: { script: true }` header enables an eager reachability check; direct `.memory` use also triggers it. Aliased access is checked at call time. The local Node SDK and an existing readable SQLite DB are required (`AI_HIST_DB` overrides `defaultDbPath()`); JSONL fallback is disabled. - `learn(finding)` journals first: the finding is appended to the durable - root run's `memory-learn` stream before it can be recalled. It is then - projected as a compacted trajectory file (the format `ai-hist sync` - ingests) at `/flows-memory//.trajectories/compacted/.json`, - beside the ai-hist DB rather than the flow source, so a flow deployed from a - read-only checkout can still learn. The file cites its commit - (`journal: { runId, stream, offset }`) and is derived only from it, so a - resumed body that re-runs `learn` finds the commit and republishes the same - bytes without journaling again. `learn` without a durable root run refuses - (`memory_unjournaled`). `recall` and `why` read the ai-hist DB plus the - scope's learned files, so the next run sees a finding before any sync; a - synced copy is deduplicated in favour of the DB row (an unsynced `recall` - entry has a negative `id`), and a damaged learned file fails the read - (`memory_unreachable`) rather than being skipped. The id hashes scope and - finding, so re-learning a finding atomically replaces one file. `learn` - never modifies the ai-hist DB and shares no state between writers, so it - needs no lock; the file takes the DB's mode, and a failed `learn` + `learn(finding)` journals first. It stages the finding's compacted + trajectory file (the format `ai-hist sync` ingests) under a temporary name, + appends the finding to the durable root run's `memory-learn` stream, and + only then renames the file into place at + `/flows-memory//.trajectories/compacted/.json`, so + nothing is recallable that the journal does not record, and disk failures + happen before anything is journaled. Files live beside the ai-hist DB, not + the flow source, so a flow deployed from a read-only checkout can still + learn. A file is derived only from its commit, which it cites + (`journal: { runId, stream, id }`); if another attempt on the root + committed the same finding first, that commit wins, so every attempt + publishes the same bytes. A resumed body that re-runs `learn` finds the + commit and publishes it without journaling again. Journal errors propagate + unchanged, so a read interruption or lost lease leaves the root resumable. + `learn` without a durable root run refuses (`memory_unjournaled`). `recall` + and `why` read the ai-hist DB plus the scope's learned files, so the next + run sees a finding before any sync; a synced copy is deduplicated in favour + of the DB row (an unsynced `recall` entry has a negative `id`), and a + damaged learned file fails the read (`memory_unreachable`) rather than + being skipped. The id hashes scope and finding, so re-learning a finding + atomically replaces one file. Trajectory files need no lock: each is + staged privately and published by atomic rename, and `learn` never + modifies the ai-hist DB; ordering between writers comes from the root run's + stream. A file takes the DB's mode, and a failed `learn` (`memory_finding_invalid`, `memory_unwritable`) publishes nothing. A body must await `learn`: a call whose outcome the body never consumed fails the run with `unawaited_step`. `memory: { agent: true }` refuses pending the diff --git a/packages/sdk/src/authored-flow-executor.ts b/packages/sdk/src/authored-flow-executor.ts index 894758e74..badf4b16a 100644 --- a/packages/sdk/src/authored-flow-executor.ts +++ b/packages/sdk/src/authored-flow-executor.ts @@ -821,6 +821,10 @@ export async function executeAuthoredFlow( bodyFailure = error; } if (bodyFailed) { + // An in-flight learn must land or fail before this attempt gives the root + // back: a re-driven attempt reads the memory-learn stream, and must not + // race a commit this one is still making. + await memoryWrites.settle(); try { worker.stop(bodyFailure); await stopAuthoredOperations(authoredSteps, bodyFailure); diff --git a/packages/sdk/src/authored-memory.ts b/packages/sdk/src/authored-memory.ts index 4e4e96b64..f78e079aa 100644 --- a/packages/sdk/src/authored-memory.ts +++ b/packages/sdk/src/authored-memory.ts @@ -56,6 +56,10 @@ export class MemoryWriteTracker { this.calls.push(call); return call; } + /** Wait for every call to settle, so no write outlives the execution that started it. */ + async settle(): Promise { + await Promise.allSettled(this.calls.map(call => Promise.prototype.then.call(call, () => {}, () => {}))); + } /** Every call the body did not await, after letting each one settle. */ async unawaited(): Promise { const unobserved = this.calls.filter(call => !call.observed || !call.settled); @@ -78,13 +82,21 @@ export interface LearnRecord { export interface CommittedLearn { readonly record: LearnRecord; readonly runId: string; - readonly offset: number; } /** Where `learn` journals a finding before it may become recallable. */ export interface LearnLedger { - /** The recorded commit for `id`, or a new one journaled from `build()`. Concurrent calls for one id share one commit. */ - commit(id: string, build: () => LearnRecord): Promise; + /** The durable root run this ledger journals on. */ + readonly runId: string; + /** The commit already recorded for `id`, if any. */ + recorded(id: string): Promise; + /** + * Journal `record`, then return the commit that wins for its id: the first + * one in the stream. Usually that is `record`; if another attempt on this + * root committed the same finding first, it is theirs, so every attempt + * projects the same bytes. + */ + append(record: LearnRecord): Promise; } function canonicalLearnRecord(raw: unknown): LearnRecord | undefined { @@ -108,45 +120,41 @@ function canonicalLearnRecord(raw: unknown): LearnRecord | undefined { /** * A ledger on the durable root run's `memory-learn` stream. The stream is read - * once per execution, so a resumed body finds what an earlier attempt already - * committed and does not journal it twice. + * once per execution and re-read after each append, so a resumed or re-driven + * body finds what any earlier attempt committed. A failed read is not cached: + * the next call reads again. Journal errors propagate unchanged, so a read + * interruption or a lost lease keeps its resumable meaning for the root. */ export function journalLearnLedger(journal: Pick, rootRunId: string): LearnLedger { let loaded: Promise> | undefined; - const inFlight = new Map>(); - async function load(): Promise> { + async function read(): Promise> { const commits = new Map(); let offset = 0; for (;;) { const page = await journal.streamRead(rootRunId, MEMORY_LEARN_STREAM, offset, 1000); - page.messages.forEach((message, index) => { + for (const message of page.messages) { const record = canonicalLearnRecord((message as { message?: unknown }).message ?? message); - // The first commit for an id wins; it is the one its file cites. - if (record !== undefined && !commits.has(record.id)) commits.set(record.id, { record, runId: rootRunId, offset: offset + index }); - }); + // The first commit for an id wins; it is the one its file is built from. + if (record !== undefined && !commits.has(record.id)) commits.set(record.id, { record, runId: rootRunId }); + } if (page.messages.length === 0 || page.next_offset <= offset) break; offset = page.next_offset; } return commits; } + function load(): Promise> { + loaded ??= read().catch(error => { loaded = undefined; throw error; }); + return loaded; + } return { - commit(id, build) { - const pending = inFlight.get(id); - if (pending !== undefined) return pending; - const committing = (async () => { - loaded ??= load(); - const commits = await loaded; - const existing = commits.get(id); - if (existing !== undefined) return existing; - const record = build(); - const { offset } = await journal.streamAppend(rootRunId, MEMORY_LEARN_STREAM, record); - const committed = { record, runId: rootRunId, offset }; - commits.set(id, committed); - return committed; - })(); - inFlight.set(id, committing); - void committing.then(() => inFlight.delete(id), () => inFlight.delete(id)); - return committing; + runId: rootRunId, + async recorded(id) { + return (await load()).get(id); + }, + async append(record) { + await journal.streamAppend(rootRunId, MEMORY_LEARN_STREAM, record); + loaded = undefined; + return (await load()).get(record.id) ?? { record, runId: rootRunId }; }, }; } @@ -172,14 +180,46 @@ export function authoredMemory( try { return action(reader, learned); } finally { reader.close(); } } /** - * Journal first, then project. The finding is committed to the root run's - * `memory-learn` stream before its trajectory file exists, so nothing is - * recallable that the journal does not record. A crash between the two - * leaves a journaled finding without its file; the resumed body re-runs - * `learn`, finds the commit, and republishes the identical file from it - * without journaling again. With no durable root run there is nowhere to - * journal, so `learn` refuses rather than write unrecorded memory. + * Journal first, then publish. The trajectory file is staged under a name + * no read lists, the finding is committed to the root run's `memory-learn` + * stream, and only then is the file renamed into place. Nothing is + * recallable that the journal does not record, and the failures a disk + * write can have (space, permissions) happen while staging, before anything + * is journaled. If the process dies between the commit and the rename, the + * resumed body re-runs `learn`, finds the commit, and publishes the same + * file from it without journaling again. With no durable root run there is + * nowhere to journal, so `learn` refuses rather than write unrecorded memory. */ + const learning = new Map>(); + async function learnOnce(decision: Required, journaled: LearnLedger): Promise { + const { dbPath, mode } = await learnTarget(); + const id = learnedId(scope, decision); + const existing = await journaled.recorded(id); + if (existing !== undefined) { + await publishLearned(dbPath, existing, mode); + return; + } + const record: LearnRecord = { memory: 'learn', id, scope, decision, learnedAt: new Date().toISOString() }; + let staged: StagedLearned; + try { + staged = await stageLearned(dbPath, { record, runId: journaled.runId }, mode); + } catch (error) { + throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn could not stage the finding: ${error instanceof Error ? error.message : String(error)}`); + } + try { + // The last point a body that already called done() can be refused: past + // here the finding is journaled and will be published. + assertOpen(); + const winner = await journaled.append(record); + try { + await staged.publish(winner); + } catch (error) { + throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn journaled the finding (root run ${winner.runId}, stream ${MEMORY_LEARN_STREAM}, id ${id}) but could not publish it: ${error instanceof Error ? error.message : String(error)}`); + } + } finally { + await staged.discard(); + } + } async function learn(finding: MemoryFinding): Promise { assertOpen(); assertEnabled(); @@ -187,22 +227,15 @@ export function authoredMemory( if (ledger === undefined) { throw new AuthoredFlowExecutionError('memory_unjournaled', 'f.memory.learn requires a durable root run to journal the finding; run the flow through flows run'); } - await assertMemoryReachable(); - assertOpen(); const id = learnedId(scope, decision); - let committed: CommittedLearn; - try { - committed = await ledger.commit(id, () => ({ memory: 'learn', id, scope, decision, learnedAt: new Date().toISOString() })); - } catch (error) { - throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn could not journal the finding: ${error instanceof Error ? error.message : String(error)}`); - } - const { defaultDbPath } = await import('ai-hist'); - const dbPath = defaultDbPath(); - try { - await writeLearned(dbPath, committed, (await stat(dbPath)).mode & 0o777); - } catch (error) { - throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn journaled the finding but could not publish it: ${error instanceof Error ? error.message : String(error)}`); + // Concurrent learns of one finding in this execution share one commit. + let pending = learning.get(id); + if (pending === undefined) { + pending = learnOnce(decision, ledger); + learning.set(id, pending); + void pending.then(() => learning.delete(id), () => learning.delete(id)); } + await pending; } return { recall: (query, options) => read((reader, learned) => recallMerged(reader, learned, scope, query, options)), @@ -245,8 +278,10 @@ function normalizeFinding(finding: MemoryFinding): Required { * Learned findings live beside the ai-hist database, not beside the flow * source: a flow deployed from a read-only checkout, package or image can * still learn. The directory is keyed by the scope's hash, and each file - * still names the scope as its `projectId`. `ai-hist sync` indexes them when - * `TRAJECTORY_ROOT` includes `/flows-memory`; reads here do not need it. + * still names the scope as its `projectId`. Reads here do not need + * `ai-hist sync`. For sync to index them too, add `/flows-memory` to + * `TRAJECTORY_ROOT` together with the roots it already scans: setting it + * replaces ai-hist's default `~/Projects` scan rather than adding to it. */ const learnedDir = (dbPath: string, scope: string): string => join(dirname(dbPath), 'flows-memory', basename(scope), '.trajectories', 'compacted'); @@ -256,17 +291,22 @@ const learnedId = (scope: string, decision: Required): string => `${LEARNED_PREFIX}${createHash('sha256').update(JSON.stringify([scope, decision])).digest('hex').slice(0, 32)}`; /** - * Project one committed finding as an immutable compacted trajectory file, the - * format `ai-hist sync` ingests. Its content is derived only from the journal - * commit (including the time it was learned and a citation of the commit), so - * republishing on resume writes the same bytes. The file name is a hash of - * scope and finding, so re-learning the same finding rewrites one file. It is - * staged and renamed into place, so it appears whole or not at all, and the - * ai-hist database is never modified. It takes the database's mode, so a - * private history does not leak findings through a world-readable file. + * `learn` never reads the ai-hist database, so it does not load it: it needs + * only the directory it lives in and its mode. A missing database is the same + * refusal reads give. */ -async function writeLearned(dbPath: string, { record, runId, offset }: CommittedLearn, mode: number): Promise { - const trajectory = { +async function learnTarget(): Promise<{ dbPath: string; mode: number }> { + const { defaultDbPath } = await import('ai-hist'); + const dbPath = defaultDbPath(); + const info = await stat(dbPath).catch(() => undefined); + if (info === undefined || !info.isFile()) { + throw new AuthoredFlowExecutionError('memory_unreachable', `memory database ${dbPath} is missing or not a file`); + } + return { dbPath, mode: info.mode & 0o777 }; +} + +function learnedContent({ record, runId }: CommittedLearn): string { + return `${JSON.stringify({ id: record.id, version: 1, type: 'compacted', @@ -277,17 +317,66 @@ async function writeLearned(dbPath: string, { record, runId, offset }: Committed completedAt: record.learnedAt, decisions: [record.decision], retrospective: { summary: null, approach: null, learnings: [], confidence: null }, - journal: { runId, stream: MEMORY_LEARN_STREAM, offset }, - }; - const file = join(learnedDir(dbPath, record.scope), `${record.id}.json`); + journal: { runId, stream: MEMORY_LEARN_STREAM, id: record.id }, + }, null, 2)}\n`; +} + +interface StagedLearned { + /** Rename into place, rewriting first if the winning commit is not the one staged. */ + publish(winner: CommittedLearn): Promise; + discard(): Promise; +} + +/** + * Write a committed finding's compacted trajectory file under a temporary + * name. Its content is derived only from the commit (including when it was + * learned and which root run journaled it), so every publication of one + * commit writes the same bytes. The final name is a hash of scope and + * finding, so re-learning a finding atomically replaces one file. The file + * takes the database's mode, so a private history does not leak findings + * through a world-readable file. The ai-hist database is never modified. + */ +async function stageLearned(dbPath: string, staging: CommittedLearn, mode: number): Promise { + const file = join(learnedDir(dbPath, staging.record.scope), `${staging.record.id}.json`); await mkdir(dirname(file), { recursive: true }); const staged = `${file}.${process.pid}.${randomUUID()}.tmp`; - try { - await writeFile(staged, `${JSON.stringify(trajectory, null, 2)}\n`, { mode }); + let content = learnedContent(staging); + const write = async () => { + await writeFile(staged, content, { mode }); await chmod(staged, mode); - await rename(staged, file); - } finally { + }; + try { + await write(); + } catch (error) { await rm(staged, { force: true }); + throw error; + } + return { + async publish(winner) { + const winning = learnedContent(winner); + if (winning !== content) { + content = winning; + await write(); + } + await rename(staged, file); + }, + discard: () => rm(staged, { force: true }), + }; +} + +async function publishLearned(dbPath: string, committed: CommittedLearn, mode: number): Promise { + let staged: StagedLearned; + try { + staged = await stageLearned(dbPath, committed, mode); + } catch (error) { + throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn could not publish journaled finding ${committed.record.id}: ${error instanceof Error ? error.message : String(error)}`); + } + try { + await staged.publish(committed); + } catch (error) { + throw new AuthoredFlowExecutionError('memory_unwritable', `f.memory.learn could not publish journaled finding ${committed.record.id}: ${error instanceof Error ? error.message : String(error)}`); + } finally { + await staged.discard(); } } @@ -304,46 +393,61 @@ async function readLearned(dbPath: string, scope: string): Promise; - try { - raw = JSON.parse(await readFile(path, 'utf8')) as Record; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') continue; - // Files here are only ever published whole, so an unreadable one is real - // damage; recalling around it would silently drop journaled memory. - throw new AuthoredFlowExecutionError('memory_unreachable', `learned finding ${path} is unreadable: ${error instanceof Error ? error.message : String(error)}`); + const learned = await Promise.all(names + .filter(name => name.startsWith(LEARNED_PREFIX) && name.endsWith('.json')) + .map(async name => { + const path = join(dir, name); + try { + return learnedEntry(scope, path, await readFile(path, 'utf8'), Math.trunc((await stat(path)).mtimeMs)); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined; + // Files here are only ever published whole, so an unreadable one is + // real damage; recalling around it would silently drop journaled memory. + throw new AuthoredFlowExecutionError('memory_unreachable', `learned finding ${path} is unreadable: ${error instanceof Error ? error.message : String(error)}`); + } + })); + return learned.filter((entry): entry is LearnedTrajectory => entry !== undefined); +} + +function learnedEntry(scope: string, path: string, text: string, updatedMs: number): LearnedTrajectory { + const raw: unknown = JSON.parse(text); + if (typeof raw !== 'object' || raw === null || Array.isArray(raw)) throw new Error('not a trajectory object'); + const value = raw as Record; + const decisions = value.decisions; + if (typeof value.id !== 'string' || !Array.isArray(decisions)) throw new Error('missing id or decisions'); + const parsed = decisions.map(decision => { + const d = decision as Record | null; + if (typeof d !== 'object' || d === null || typeof d.question !== 'string' || typeof d.chosen !== 'string' + || typeof d.reasoning !== 'string' || !Array.isArray(d.alternatives) || d.alternatives.some(a => typeof a !== 'string')) { + throw new Error('malformed decision'); } - const decisions = Array.isArray(raw.decisions) ? raw.decisions as TrajectoryEntry['decisions'] : []; - const task = (raw.task ?? {}) as { title?: string | null; description?: string | null }; - const id = typeof raw.id === 'string' ? raw.id : name.slice(0, -'.json'.length); - const completedAt = typeof raw.completedAt === 'string' ? raw.completedAt : null; - const updatedMs = Math.trunc((await stat(path)).mtimeMs); - const timestampMs = completedAt !== null && Number.isFinite(Date.parse(completedAt)) ? Date.parse(completedAt) : updatedMs; - // Same field order ai-hist's buildSearchText uses, so matching agrees with a later sync. - const searchText = [id, scope, 'completed', task.title ?? null, task.description ?? null, - ...decisions.flatMap(d => [d.question, d.chosen, d.reasoning, ...(d.alternatives ?? [])])] - .filter((part): part is string => typeof part === 'string' && part.length > 0).join('\n'); - learned.push({ - trajectory: { - id, version: 1, personaId: null, projectId: scope, - task: { title: task.title ?? null, description: task.description ?? null }, - status: 'completed', startedAt: typeof raw.startedAt === 'string' ? raw.startedAt : null, completedAt, - decisions, retrospective: { summary: null, approach: null, learnings: [], confidence: null }, - searchText, path, updatedMs, timestampMs, - }, - // Not yet in the ai-hist database, so it has no row id; a negative id - // derived from the trajectory id marks it as read from disk. - history: { - id: -Number.parseInt(createHash('sha256').update(id).digest('hex').slice(0, 12), 16), - source: 'trajectory', sessionId: id, project: scope, prompt: searchText, timestampMs, gitBranch: null, - }, - }); - } - return learned; + return { question: d.question, chosen: d.chosen, reasoning: d.reasoning, alternatives: d.alternatives as string[] }; + }); + const task = (typeof value.task === 'object' && value.task !== null ? value.task : {}) as Record; + const title = typeof task.title === 'string' ? task.title : null; + const description = typeof task.description === 'string' ? task.description : null; + const id = value.id; + const completedAt = typeof value.completedAt === 'string' ? value.completedAt : null; + const timestampMs = completedAt !== null && Number.isFinite(Date.parse(completedAt)) ? Date.parse(completedAt) : updatedMs; + // Same field order ai-hist's buildSearchText uses, so matching agrees with a later sync. + const searchText = [id, scope, 'completed', title, description, + ...parsed.flatMap(d => [d.question, d.chosen, d.reasoning, ...d.alternatives])] + .filter((part): part is string => typeof part === 'string' && part.length > 0).join('\n'); + return { + trajectory: { + id, version: 1, personaId: null, projectId: scope, + task: { title, description }, + status: 'completed', startedAt: typeof value.startedAt === 'string' ? value.startedAt : null, completedAt, + decisions: parsed, retrospective: { summary: null, approach: null, learnings: [], confidence: null }, + searchText, path, updatedMs, timestampMs, + }, + // Not yet in the ai-hist database, so it has no row id; a negative id + // derived from the trajectory id marks it as read from disk. + history: { + id: -Number.parseInt(createHash('sha256').update(id).digest('hex').slice(0, 12), 16), + source: 'trajectory', sessionId: id, project: scope, prompt: searchText, timestampMs, gitBranch: null, + }, + }; } const includesFolded = (haystack: string | null | undefined, needle: string): boolean => diff --git a/packages/sdk/src/cli/run.ts b/packages/sdk/src/cli/run.ts index ac8077291..2a1348b40 100644 --- a/packages/sdk/src/cli/run.ts +++ b/packages/sdk/src/cli/run.ts @@ -1103,7 +1103,7 @@ async function inspectOutOfBandStep( const EXPIRED_LEASE_POLL_MS = 250; /** - * `f.memory.learn` failures keep their own kind instead of collapsing into + * `f.memory` failures keep their own kind instead of collapsing into * `protocol_error`. By then the root has been terminalized as failed, so the * report names it the way a step failure does. */ @@ -1114,7 +1114,8 @@ export function memoryWriteFailure( error: AuthoredFlowExecutionError, fallbackRunId?: string, ): RunExecution | undefined { - if (error.code !== 'memory_finding_invalid' && error.code !== 'memory_unwritable' && error.code !== 'memory_unjournaled') return undefined; + if (error.code !== 'memory_finding_invalid' && error.code !== 'memory_unwritable' + && error.code !== 'memory_unjournaled' && error.code !== 'memory_unreachable') return undefined; const rootRunId = error.rootRunId ?? fallbackRunId; const runId = error.runId ?? rootRunId; return { @@ -1127,7 +1128,11 @@ export function memoryWriteFailure( socketPath, status: 'failed', completionReason: 'step_failed', - diagnostics: [...base.diagnostics, { severity: 'failure', kind: error.code, message: error.message }], + diagnostics: [...base.diagnostics, { + severity: 'failure', kind: error.code, + // The `: ` prefix `AuthoredFlowExecutionError` adds is redundant once the diagnostic is labelled. + message: error.message.startsWith(`${error.code}: `) ? error.message.slice(error.code.length + 2) : error.message, + }], }, }; } diff --git a/packages/sdk/src/failure-kinds.ts b/packages/sdk/src/failure-kinds.ts index 8ab0764d2..9c0259590 100644 --- a/packages/sdk/src/failure-kinds.ts +++ b/packages/sdk/src/failure-kinds.ts @@ -144,6 +144,8 @@ export const RUN_FAILURE_KINDS = [ 'memory_unwritable', /** `f.memory.learn` ran without a durable root run to journal the finding in. */ 'memory_unjournaled', + /** `f.memory` lost its database, or found a damaged learned finding, after the run started. */ + 'memory_unreachable', /** * `--local-agent` cannot be honoured for this invocation, so it is refused * rather than accepted and ignored. A local agent worker is admitted at run diff --git a/packages/sdk/tests/f-memory.test.ts b/packages/sdk/tests/f-memory.test.ts index 082dd4dee..3c53ddcd7 100644 --- a/packages/sdk/tests/f-memory.test.ts +++ b/packages/sdk/tests/f-memory.test.ts @@ -50,7 +50,7 @@ function streamStore() { async streamAppend(runId: string, stream: string, message: unknown) { const list = stored.get(key(runId, stream)) ?? []; stored.set(key(runId, stream), list); - appended.push({ stream, message, fileExisted: existsSync(compactedDir()) && readdirSync(compactedDir()).length > 0 }); + appended.push({ stream, message, fileExisted: existsSync(compactedDir()) && readdirSync(compactedDir()).some(name => name.endsWith('.json')) }); // Stored the way the journal stores it: keys sorted. list.push(JSON.parse(JSON.stringify(message, Object.keys(flatKeys(message)).sort()))); return { offset: list.length - 1 }; @@ -315,7 +315,7 @@ it('journals the finding on the root run before it becomes recallable, and cites }), fileExisted: false }]); const [name] = readdirSync(compactedDir()); expect(JSON.parse(readFileSync(join(compactedDir(), name!), 'utf8'))) - .toMatchObject({ journal: { runId: 'root-1', stream: MEMORY_LEARN_STREAM, offset: 0 }, decisions: [finding] }); + .toMatchObject({ journal: { runId: 'root-1', stream: MEMORY_LEARN_STREAM, id: expect.stringMatching(/^flows-memory-/) }, decisions: [finding] }); }); it('refuses to learn without a durable root run, writing nothing', async () => { @@ -329,15 +329,61 @@ it('refuses to learn without a durable root run, writing nothing', async () => { expect(existsSync(join(dir, 'flows-memory'))).toBe(false); }); -it('publishes nothing when the journal append fails', async () => { - vi.spyOn(store, 'streamAppend').mockRejectedValueOnce(new Error('journal down')); +it('publishes nothing when the journal append fails, and passes the journal error through unchanged', async () => { + const journalDown = new Error('journal down'); + vi.spyOn(store, 'streamAppend').mockRejectedValueOnce(journalDown); const memory = memoryFor(scope); - await expect(memory.learn({ question: 'not journaled', chosen: 'c', reasoning: 'r' })) - .rejects.toMatchObject({ code: 'memory_unwritable', message: expect.stringContaining('could not journal') }); - expect(existsSync(join(dir, 'flows-memory'))).toBe(false); + await expect(memory.learn({ question: 'not journaled', chosen: 'c', reasoning: 'r' })).rejects.toBe(journalDown); + expect(readdirSync(compactedDir())).toEqual([]); expect(await memory.recall('not journaled')).toEqual([]); }); +it('reads the stream again after a failed read instead of caching the failure', async () => { + const interrupted = new Error('read interrupted'); + vi.spyOn(store, 'streamRead').mockRejectedValueOnce(interrupted); + const memory = memoryFor(scope); + const finding = { question: 'retry after blip', chosen: 'c', reasoning: 'r' }; + await expect(memory.learn(finding)).rejects.toBe(interrupted); + await memory.learn(finding); + expect(store.list('root-1', MEMORY_LEARN_STREAM)).toHaveLength(1); + expect(await memory.recall('retry after blip')).toHaveLength(1); +}); + +it('publishes the first commit when another attempt on the root committed the same finding first', async () => { + const realAppend = store.streamAppend.bind(store); + vi.spyOn(store, 'streamAppend').mockImplementationOnce(async (runId, stream, message) => { + // A re-driven attempt's append lands first, with its own learnedAt. + await realAppend(runId, stream, { ...(message as object), learnedAt: '2020-01-01T00:00:00.000Z' }); + return realAppend(runId, stream, message); + }); + await memoryFor(scope).learn({ question: 'raced finding', chosen: 'c', reasoning: 'r' }); + const [name] = readdirSync(compactedDir()); + expect(JSON.parse(readFileSync(join(compactedDir(), name!), 'utf8'))).toMatchObject({ completedAt: '2020-01-01T00:00:00.000Z' }); +}); + +it('lets an in-flight learn land before a failed body hands the root back', async () => { + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const realAppend = store.streamAppend.bind(store); + vi.spyOn(store, 'streamAppend').mockImplementation(async (runId, stream, message) => { + if (stream === MEMORY_LEARN_STREAM) await gate; + return realAppend(runId, stream, message); + }); + const failing = executeAuthoredFlow(flow('memory-example', { memory: { script: true } }, async f => { + void f.memory.learn({ question: 'in flight', chosen: 'c', reasoning: 'r' }); + await new Promise(resolve => setTimeout(resolve, 20)); + throw new Error('body failed'); + }), completingJournal(), undefined, { flowPath: join(dir, 'test.flow.ts'), rootRunId: 'root-1' }); + let settled = false; + void failing.catch(() => { settled = true; }); + await new Promise(resolve => setTimeout(resolve, 100)); + expect(settled).toBe(false); + release(); + await expect(failing).rejects.toThrow('body failed'); + expect(store.list('root-1', MEMORY_LEARN_STREAM)).toHaveLength(1); + expect(readdirSync(compactedDir()).filter(name => name.endsWith('.json'))).toHaveLength(1); +}); + it('on resume, republishes a journaled finding from its commit without journaling it again', async () => { const finding = { question: 'resumed finding', chosen: 'c', reasoning: 'r', alternatives: [] }; await memoryFor(scope).learn(finding); @@ -365,8 +411,11 @@ it('fails recall closed on a damaged learned file instead of skipping it', async const memory = memoryFor(scope); await memory.learn({ question: 'damaged', chosen: 'c', reasoning: 'r' }); const [name] = readdirSync(compactedDir()); - writeFileSync(join(compactedDir(), name!), '{ not json'); - await expect(memory.recall('damaged')).rejects.toMatchObject({ code: 'memory_unreachable' }); + for (const damage of ['{ not json', 'null', '[]', '{"id":"x","decisions":[null]}']) { + writeFileSync(join(compactedDir(), name!), damage); + await expect(memory.recall('damaged')).rejects.toMatchObject({ code: 'memory_unreachable' }); + await expect(memory.why('damaged')).rejects.toMatchObject({ code: 'memory_unreachable' }); + } }); it('keeps learned files out of the flow source directory', async () => { @@ -376,11 +425,11 @@ it('keeps learned files out of the flow source directory', async () => { }); it('reports learn failures with their own kind, not protocol_error', () => { - for (const code of ['memory_finding_invalid', 'memory_unwritable', 'memory_unjournaled'] as const) { + for (const code of ['memory_finding_invalid', 'memory_unwritable', 'memory_unjournaled', 'memory_unreachable'] as const) { const error = Object.assign(new AuthoredFlowExecutionError(code, 'nope'), { rootRunId: 'root-1' }); expect(memoryWriteFailure('run', emptyReport('run'), '/sock', error)).toMatchObject({ exitCode: 1, report: { runId: 'root-1', rootRunId: 'root-1', status: 'failed', completionReason: 'step_failed', - diagnostics: [{ severity: 'failure', kind: code }], + diagnostics: [{ severity: 'failure', kind: code, message: 'nope' }], } }); } expect(memoryWriteFailure('run', emptyReport('run'), '/sock', new AuthoredFlowExecutionError('step_failed', 'x'))).toBeUndefined(); diff --git a/packages/sdk/tests/memory-learn-live.test.ts b/packages/sdk/tests/memory-learn-live.test.ts index ce8ede329..482f9e710 100644 --- a/packages/sdk/tests/memory-learn-live.test.ts +++ b/packages/sdk/tests/memory-learn-live.test.ts @@ -1,7 +1,7 @@ import { execFileSync } from 'node:child_process'; import { existsSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; import { basename, join, resolve } from 'node:path'; -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { MEMORY_LEARN_STREAM, scriptMemoryScope } from '../src/authored-memory.js'; import { JournalClient } from '../src/journal-client.js'; import { socketPathFor } from '../src/daemon-connection.js'; @@ -10,7 +10,7 @@ import { chainFixture } from './flow-chain-fixture.js'; const closes: Array<() => Promise> = []; afterEach(async () => { for (const close of closes.splice(0).reverse()) await close(); - delete process.env.AI_HIST_DB; + vi.unstubAllEnvs(); }); describe('f.memory.learn through the built CLI and a real kernel', () => { @@ -20,7 +20,7 @@ describe('f.memory.learn through the built CLI and a real kernel', () => { const dbPath = join(fixture.root, 'ai-history.db'); const scope = scriptMemoryScope(fixture.flowPath, 'learner'); execFileSync(process.execPath, [resolve('../../testdata/memory/seed.mjs'), dbPath, scope]); - process.env.AI_HIST_DB = dbPath; + vi.stubEnv('AI_HIST_DB', dbPath); writeFileSync(fixture.flowPath, `import { flow } from '@relayflows/surface'; export default flow('learner', { memory: { script: true } }, async f => { @@ -45,7 +45,7 @@ export default flow('learner', { memory: { script: true } }, async f => { const compacted = join(fixture.root, 'flows-memory', basename(scope), '.trajectories', 'compacted'); const [file] = readdirSync(compacted); expect(JSON.parse(readFileSync(join(compacted, file!), 'utf8'))) - .toMatchObject({ id: records[0]!.id, journal: { runId: report.runId, stream: MEMORY_LEARN_STREAM, offset: 0 } }); + .toMatchObject({ id: records[0]!.id, journal: { runId: report.runId, stream: MEMORY_LEARN_STREAM, id: records[0]!.id } }); expect(existsSync(scope)).toBe(false); writeFileSync(fixture.flowPath, `import { flow } from '@relayflows/surface'; diff --git a/testdata/memory/README.md b/testdata/memory/README.md index 8ac65b130..ee653e24d 100644 --- a/testdata/memory/README.md +++ b/testdata/memory/README.md @@ -1,11 +1,11 @@ # Minimal local script-memory example This slice implements `f.memory.recall`, `f.memory.why`, and `f.memory.learn` -through ai-hist 0.4.1. Reads journal nothing. `learn` appends the finding to -the durable root run's `memory-learn` stream first, then projects it as a -compacted trajectory file at -`/flows-memory//.trajectories/compacted/.json` that -cites the commit. ai-hist 0.4.1 has no trajectory writer and its database is +through ai-hist 0.4.1. Reads journal nothing. `learn` stages a compacted +trajectory file, appends the finding to the durable root run's +`memory-learn` stream, then renames the file into place at +`/flows-memory//.trajectories/compacted/.json`; the +file cites the commit. ai-hist 0.4.1 has no trajectory writer and its database is never modified; reads merge the database with the scope's learned files. A resumed body re-running `learn` republishes from the commit instead of journaling twice, and `learn` refuses (`memory_unjournaled`) without a root From 73806b22a8bb6b310f594d8e94a78ca311556c4d Mon Sep 17 00:00:00 2001 From: khaliqgant Date: Fri, 9 Oct 2026 21:30:02 -0700 Subject: [PATCH 8/8] fix(sdk): settle in-flight learns on the missing_completion path too Co-Authored-By: Claude Opus 5.5 (1M context) Session-Id: b6cf20aa-bdbb-4248-91ab-d2a4fd2ce1b8 --- packages/sdk/src/authored-flow-executor.ts | 2 ++ packages/sdk/tests/f-memory.test.ts | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/packages/sdk/src/authored-flow-executor.ts b/packages/sdk/src/authored-flow-executor.ts index badf4b16a..9ba457558 100644 --- a/packages/sdk/src/authored-flow-executor.ts +++ b/packages/sdk/src/authored-flow-executor.ts @@ -857,6 +857,8 @@ export async function executeAuthoredFlow( 'missing_completion', `flow "${definition.name}" returned without done()`, ); + // Same as a failed body: no learn may outlive the attempt that started it. + await memoryWrites.settle(); try { worker.stop(missingCompletion); await stopAuthoredOperations(authoredSteps, missingCompletion); diff --git a/packages/sdk/tests/f-memory.test.ts b/packages/sdk/tests/f-memory.test.ts index 3c53ddcd7..4d15431a2 100644 --- a/packages/sdk/tests/f-memory.test.ts +++ b/packages/sdk/tests/f-memory.test.ts @@ -361,6 +361,27 @@ it('publishes the first commit when another attempt on the root committed the sa expect(JSON.parse(readFileSync(join(compactedDir(), name!), 'utf8'))).toMatchObject({ completedAt: '2020-01-01T00:00:00.000Z' }); }); +it('lets an in-flight learn land before a body without done() hands the root back', async () => { + let release!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const realAppend = store.streamAppend.bind(store); + vi.spyOn(store, 'streamAppend').mockImplementation(async (runId, stream, message) => { + if (stream === MEMORY_LEARN_STREAM) await gate; + return realAppend(runId, stream, message); + }); + const incomplete = executeAuthoredFlow(flow('memory-example', { memory: { script: true } }, async f => { + void f.memory.learn({ question: 'no done', chosen: 'c', reasoning: 'r' }); + await new Promise(resolve => setTimeout(resolve, 20)); + }), completingJournal(), undefined, { flowPath: join(dir, 'test.flow.ts'), rootRunId: 'root-1' }); + let settled = false; + void incomplete.catch(() => { settled = true; }); + await new Promise(resolve => setTimeout(resolve, 100)); + expect(settled).toBe(false); + release(); + await expect(incomplete).rejects.toMatchObject({ code: 'missing_completion' }); + expect(store.list('root-1', MEMORY_LEARN_STREAM)).toHaveLength(1); +}); + it('lets an in-flight learn land before a failed body hands the root back', async () => { let release!: () => void; const gate = new Promise(resolve => { release = resolve; });