From 4a43442dc1023a2bd390bf8309c53748db49ffce Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:03:34 -0400 Subject: [PATCH 01/17] docs(handoff): record measured session.idle frequency and the data loss it exposes Phase 4 was marked complete, but its e2e synthesizes the session.idle payload instead of observing OpenCode emit it, and the frequency the design hedged on was never measured. Measured against a live OpenCode 1.18.5 server: session.idle fires once per assistant turn (3 turns -> 3 events). Because the adapter's tracker suppresses a session permanently after the first idle, every turn after the first is silently discarded. --- ...25-opencode-phase4-runtime-verification.md | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 .agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md diff --git a/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md new file mode 100644 index 0000000..d0c2e43 --- /dev/null +++ b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md @@ -0,0 +1,77 @@ +# Recall OpenCode Phase 4 — Runtime Verification Handoff + +Status: In progress + +Phase: OpenCode integration — Phase 4 acceptance verification (real runtime) + +Branch: `fm/recall-opencode-phase4-o1` + +Supersedes as the current handoff: `.agents/atlas/handoffs/2026-07-22-opencode-phase4-testing-polish.md` +(that handoff is accurate about what PR #248 shipped; it is not the authority for +the runtime claims this pass re-tests). + +## Why this pass exists + +PR #248 and #249 landed the Phase 4 test surface and marked +`docs/OPENCODE_INTEGRATION.md` Phase 4 "complete". Two of its recorded claims were +not actually established by the code that claims them: + +1. `scripts/e2e-opencode.ts:260-266` reaches the adapter with + `await import(...opencode/RecallExtract.ts)` and then **synthesizes** the event + payload. It proves Recall's handler *accepts* `{type:'session.idle', + properties:{sessionID}}`; it does not prove OpenCode *emits* it, and it does not + prove OpenCode ever *loads* the plugin. +2. `docs/OPENCODE_INTEGRATION.md:133-137` deferred the `session.idle` frequency + question ("add time-based debouncing only if future runtime evidence shows + repeated idle events"). The frequency was never measured, and the hedge points + at the wrong remedy. + +## Measured runtime facts (OpenCode 1.18.5, macOS) + +| Question | Answer | How | +|---|---|---| +| Does OpenCode load Recall's plugin from the installer's path? | **Yes** — `$XDG_CONFIG_HOME/opencode/plugins/` (and `plugin/` singular also works) | real `opencode serve`, plugin factory side effect observed | +| `session.idle` frequency | **1.00 per assistant turn** (3 turns -> 3 events, delta 1 each) | one session, one long-lived server process, `GET /event` SSE | +| `session.idle` payload | `properties.sessionID` | captured from the live stream | +| Does `opencode export ` return the whole conversation? | **Yes** — all 6 messages after 3 turns | direct `opencode export` | + +## Defect this measurement exposed + +`opencode/RecallExtract.ts:140` gates on a **permanent** tracker +(`tracker.has(sessionId)`). Because idle fires once per turn, the first idle wins +and every later turn is discarded. + +Reproduced end to end against a real 3-turn session with Recall's real plugin +loaded by a real OpenCode server: the drop file froze at 154 bytes containing only +turn 1; `BETAMARKERTWO` and `GAMMAMARKERTHREE` never reached it; `.extracted.json` +held the session id, permanently suppressing re-export. + +This is silent data loss for every multi-turn OpenCode session — the normal case. + +## Scope + +In scope: the four Phase 4 acceptance items exercised for real, the tracker defect +the measurement exposed, #243 characterisation, and reconciling +`docs/OPENCODE_INTEGRATION.md` + `README.md` with what is proven. + +Out of scope (unchanged from #248): installation reconciliation, the semantic +#240/#241/#226 wave, release/version bump, and #236/#237/#238/#174. + +## Plan + +1. Handoff (this file). +2. Fix the tracker so later idles re-export; keep retry-on-failure and + `.extracted.json`. +3. Unit regression at the plugin boundary: repeat idle with new content re-exports. +4. Committed real-runtime harness: plugin discovery + measured idle frequency + + multi-turn drop completeness. +5. Concurrent **Claude + OpenCode** writers (the existing e2e used two OpenCode + writers). +6. Installer backup/restore round-trip (existing coverage is surgical removal, not + restore). +7. Docs: correct the overclaims, record the measured number, reconcile README. +8. #243: verify precisely, close or characterise. + +## Holds + +No captain product decision outstanding. From eed66c3c33549726c03fc578213eb3eb69acf5e8 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:05:43 -0400 Subject: [PATCH 02/17] fix(opencode): stop discarding every turn after the first in a session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit session.idle fires once per assistant turn, not once per session (measured against OpenCode 1.18.5: three turns produced three events, one per turn). The adapter gated on a permanent tracker, so the first idle wrote a drop containing only turn 1 and every later idle returned early. Reproduced end to end with a real three-turn session and the real plugin loaded by a real OpenCode server: the drop froze at 154 bytes and two of three markers never reached it. The tracker now records a content digest per session instead of 'this session is done', so a grown session overwrites its earlier, shorter drop while an unchanged one still costs no write. RecallBatchExtract already re-extracts a drop that grows, so the fuller transcript reaches memory without new machinery. Legacy array trackers load with an empty digest and converge after one re-export. Replaces two tracker tests that re-implemented the logic inline — and so passed with the bug present — with tests that drive the real plugin factory. All four new behavioural cases fail on the pre-fix adapter. --- opencode/RecallExtract.ts | 58 +++++++++++--- tests/opencode-integration.test.ts | 123 ++++++++++++++++++++++++----- 2 files changed, 150 insertions(+), 31 deletions(-) diff --git a/opencode/RecallExtract.ts b/opencode/RecallExtract.ts index 4ffdcef..2ebfab0 100644 --- a/opencode/RecallExtract.ts +++ b/opencode/RecallExtract.ts @@ -1,8 +1,16 @@ // opencode/RecallExtract.ts // Recall session extraction plugin for OpenCode // -// Hooks into session.idle to export completed sessions as markdown, -// dropping them into a directory that RecallBatchExtract.ts monitors. +// Hooks into session.idle to export the session as markdown, dropping it into +// a directory that RecallBatchExtract.ts monitors. +// +// MEASURED RUNTIME CONTRACT (OpenCode 1.18.5 — scripts/e2e-opencode-runtime.ts): +// - session.idle fires ONCE PER ASSISTANT TURN, not once per session. A +// three-turn session emits three events. The drop is therefore rewritten as +// the conversation grows; RecallBatchExtract re-extracts on file growth. +// - the payload carries properties.sessionID +// - OpenCode loads this plugin from $XDG_CONFIG_HOME/opencode/plugins/, the +// path lib/install-lib.sh installs to // // VERIFIED APIs USED: // - ctx.$ (Bun shell tagged template) — confirmed in OpenCode plugin docs @@ -11,6 +19,7 @@ import type { Plugin } from "@opencode-ai/plugin" import { existsSync, readFileSync, writeFileSync, mkdirSync } from "fs" +import { createHash } from "crypto" import { join } from "path" import { homedir } from "os" @@ -18,22 +27,45 @@ const RECALL_HOME = process.env.RECALL_HOME || join(homedir(), ".agents", "Recal const DROP_DIR = join(RECALL_HOME, "MEMORY", "opencode-sessions") const TRACKER_PATH = join(DROP_DIR, ".extracted.json") -/** Load persistent dedup tracker from disk */ -function loadTracker(): Set { +/** + * Load the persistent dedup tracker from disk. + * + * Maps session ID -> digest of the markdown last written for that session. + * `session.idle` fires once per assistant turn (measured against OpenCode + * 1.18.5: three turns produced three events), so the tracker must NOT record + * "this session is finished" — it records "this is the content we already + * dropped", which lets a later turn overwrite an earlier, shorter transcript. + * + * The legacy on-disk format was a bare array of session IDs, which carried no + * content digest. Those entries load with an empty digest so the next idle + * re-exports once and converges onto the current format. + */ +function loadTracker(): Map { try { if (existsSync(TRACKER_PATH)) { const data = JSON.parse(readFileSync(TRACKER_PATH, "utf-8")) - return new Set(Array.isArray(data) ? data : []) + if (Array.isArray(data)) { + return new Map(data.filter((id): id is string => typeof id === "string").map(id => [id, ""])) + } + if (data && typeof data === "object") { + return new Map(Object.entries(data as Record) + .map(([id, digest]) => [id, typeof digest === "string" ? digest : ""])) + } } } catch (error) { console.error(`[recall] OpenCode tracker unreadable; starting fresh: ${error instanceof Error ? error.message : String(error)}`) } - return new Set() + return new Map() } /** Save dedup tracker to disk */ -function saveTracker(tracker: Set): void { - writeFileSync(TRACKER_PATH, JSON.stringify([...tracker]) + "\n") +function saveTracker(tracker: Map): void { + writeFileSync(TRACKER_PATH, JSON.stringify(Object.fromEntries(tracker)) + "\n") +} + +/** Digest used to tell "same transcript again" from "the session grew". */ +function digest(markdown: string): string { + return createHash("sha256").update(markdown).digest("hex") } type Shell = (strings: TemplateStringsArray, ...values: unknown[]) => Promise @@ -137,14 +169,20 @@ export const RecallExtract: Plugin = async ({ $ }) => { return { event: async ({ event }: { event: unknown }) => { const sessionId = sessionIdFromEvent(event) - if (!sessionId || tracker.has(sessionId) || inFlight.has(sessionId)) return + // Only overlapping exports of the SAME session are suppressed. A session + // that already produced a drop must stay eligible: idle fires per turn, + // and skipping later idles would freeze the drop on the first turn and + // silently discard the rest of the conversation. + if (!sessionId || inFlight.has(sessionId)) return inFlight.add(sessionId) try { const markdown = await exportSession($ as unknown as Shell, sessionId) + const signature = digest(markdown) + if (tracker.get(sessionId) === signature) return const outFile = join(DROP_DIR, `${sessionId}.md`) writeFileSync(outFile, markdown) - tracker.add(sessionId) + tracker.set(sessionId, signature) saveTracker(tracker) } catch (error) { console.error(`[recall] OpenCode session export failed for ${sessionId}; will retry on a later idle event: ${error instanceof Error ? error.message : String(error)}`) diff --git a/tests/opencode-integration.test.ts b/tests/opencode-integration.test.ts index 6dc1631..8329397 100644 --- a/tests/opencode-integration.test.ts +++ b/tests/opencode-integration.test.ts @@ -272,46 +272,127 @@ Here's a basic example with multiple routes and a shared layout component that p describe('extraction tracker (dedup)', () => { let tempDir: string; + let dropDir: string; + let trackerPath: string; + let cacheBuster = 0; + + // `session.idle` fires once per assistant turn (measured against OpenCode + // 1.18.5), so these tests drive the real plugin factory rather than + // re-implementing tracker logic inline: a suppressed second idle is exactly + // the data-loss bug this suite has to catch. + async function loadPlugin(exports: Record, onExport?: (id: string) => void) { + process.env.RECALL_HOME = tempDir; + const mod = await import(`../opencode/RecallExtract.ts?tracker=${++cacheBuster}`); + return mod.RecallExtract({ + $: async (_s: TemplateStringsArray, ...values: unknown[]) => { + const id = String(values[0]); + onExport?.(id); + const body = exports[id]; + if (body === undefined) throw new Error(`no export configured for ${id}`); + return body; + }, + }); + } + + function exportJson(id: string, turns: string[]): string { + return JSON.stringify({ + info: { id, title: `Session ${id}` }, + messages: turns.map(text => ({ info: { role: 'user' }, parts: [{ type: 'text', text }] })), + }); + } + + const idle = (sessionID: string) => ({ event: { type: 'session.idle', properties: { sessionID } } }); beforeEach(() => { tempDir = mkdtempSync(join(tmpdir(), 'recall-tracker-')); + dropDir = join(tempDir, 'MEMORY', 'opencode-sessions'); + trackerPath = join(dropDir, '.extracted.json'); }); afterEach(() => { + delete process.env.RECALL_HOME; if (tempDir && existsSync(tempDir)) { rmSync(tempDir, { recursive: true, force: true }); } }); - test('tracker persists to JSON file', () => { - const trackerPath = join(tempDir, '.extracted.json'); - const tracker = new Set(['session-1', 'session-2']); + test('a later idle event re-exports a grown session instead of freezing turn 1', async () => { + const exports = { 'ses-grow': exportJson('ses-grow', ['TURN_ONE_MARKER']) }; + const plugin = await loadPlugin(exports); + + await plugin.event(idle('ses-grow')); + const afterFirst = readFileSync(join(dropDir, 'ses-grow.md'), 'utf-8'); + expect(afterFirst).toContain('TURN_ONE_MARKER'); + expect(afterFirst).not.toContain('TURN_TWO_MARKER'); + + // second assistant turn — the session now exports two messages + exports['ses-grow'] = exportJson('ses-grow', ['TURN_ONE_MARKER', 'TURN_TWO_MARKER']); + await plugin.event(idle('ses-grow')); + + const afterSecond = readFileSync(join(dropDir, 'ses-grow.md'), 'utf-8'); + expect(afterSecond).toContain('TURN_ONE_MARKER'); + expect(afterSecond).toContain('TURN_TWO_MARKER'); + }); + + test('an unchanged session is not rewritten on a repeat idle event', async () => { + const exports = { 'ses-same': exportJson('ses-same', ['ONLY_MARKER']) }; + let exportCount = 0; + const plugin = await loadPlugin(exports, () => { exportCount++; }); + + await plugin.event(idle('ses-same')); + const firstWrite = readFileSync(join(dropDir, 'ses-same.md'), 'utf-8'); + const firstTracker = readFileSync(trackerPath, 'utf-8'); + + await plugin.event(idle('ses-same')); + + expect(exportCount).toBe(2); // still asks OpenCode — that is how it learns nothing changed + expect(readFileSync(join(dropDir, 'ses-same.md'), 'utf-8')).toBe(firstWrite); + expect(readFileSync(trackerPath, 'utf-8')).toBe(firstTracker); + }); + + test('tracker persists a content digest per session, not a bare id list', async () => { + const plugin = await loadPlugin({ 'ses-shape': exportJson('ses-shape', ['SHAPE_MARKER']) }); + await plugin.event(idle('ses-shape')); - // Save - writeFileSync(trackerPath, JSON.stringify([...tracker])); + const saved = JSON.parse(readFileSync(trackerPath, 'utf-8')); + expect(Array.isArray(saved)).toBe(false); + expect(typeof saved['ses-shape']).toBe('string'); + expect(saved['ses-shape'].length).toBeGreaterThan(0); + }); + + test('a legacy array tracker still re-exports the session once and migrates', async () => { + mkdirSync(dropDir, { recursive: true }); + writeFileSync(trackerPath, JSON.stringify(['ses-legacy'])); - // Load - const loaded = JSON.parse(readFileSync(trackerPath, 'utf-8')); - const restoredSet = new Set(Array.isArray(loaded) ? loaded : []); + const plugin = await loadPlugin({ 'ses-legacy': exportJson('ses-legacy', ['LEGACY_MARKER']) }); + await plugin.event(idle('ses-legacy')); - expect(restoredSet.has('session-1')).toBe(true); - expect(restoredSet.has('session-2')).toBe(true); - expect(restoredSet.has('session-3')).toBe(false); + expect(readFileSync(join(dropDir, 'ses-legacy.md'), 'utf-8')).toContain('LEGACY_MARKER'); + const saved = JSON.parse(readFileSync(trackerPath, 'utf-8')); + expect(Array.isArray(saved)).toBe(false); + expect(typeof saved['ses-legacy']).toBe('string'); }); - test('corrupt tracker file falls back to empty set', () => { - const trackerPath = join(tempDir, '.extracted.json'); + test('corrupt tracker file falls back to empty state and still exports', async () => { + mkdirSync(dropDir, { recursive: true }); writeFileSync(trackerPath, 'not valid json!!!'); - let tracker: Set; - try { - const data = JSON.parse(readFileSync(trackerPath, 'utf-8')); - tracker = new Set(Array.isArray(data) ? data : []); - } catch { - tracker = new Set(); - } + const plugin = await loadPlugin({ 'ses-corrupt': exportJson('ses-corrupt', ['CORRUPT_MARKER']) }); + await plugin.event(idle('ses-corrupt')); + + expect(readFileSync(join(dropDir, 'ses-corrupt.md'), 'utf-8')).toContain('CORRUPT_MARKER'); + }); + + test('a failed export writes no drop and stays retryable on the next idle', async () => { + const exports: Record = {}; + const plugin = await loadPlugin(exports); + + await plugin.event(idle('ses-retry')); // no export configured -> throws + expect(existsSync(join(dropDir, 'ses-retry.md'))).toBe(false); - expect(tracker.size).toBe(0); + exports['ses-retry'] = exportJson('ses-retry', ['RETRY_MARKER']); + await plugin.event(idle('ses-retry')); + expect(readFileSync(join(dropDir, 'ses-retry.md'), 'utf-8')).toContain('RETRY_MARKER'); }); }); From ea9e42850dec863630a5a6aa4a66bc96993122ba Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:25:05 -0400 Subject: [PATCH 03/17] fix(opencode): stop OpenCode logging a plugin load error on every launch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenCode invokes EVERY export of a top-level plugins/*.ts as a plugin factory. opencode/RecallExtract.ts exported three test helpers beside its factory, so on each launch OpenCode called exportSession() with its plugin context as the shell, the tagged-template call hit a non-function, and the server logged 'failed to load plugin ... Object is not a function'. Observed against a real OpenCode 1.18.5 server; reproduced with the plugin installed by the real installer, and confirmed by exporting only the factory, which loads clean. The existing e2e could not catch it: it imports the adapter directly and supplies its own $, fabricating the very shape OpenCode breaks on. Helpers move to opencode/lib/session-export.ts. OpenCode globs only top-level plugins/*.ts, so a nested module is importable without being mistaken for a plugin — verified empirically before choosing this layout. Install and uninstall carry the nested helper, and uninstall only removes plugins/lib when Recall emptied it. A new test asserts each plugin entry exports exactly one function, so a stray helper export cannot silently reintroduce this. --- lib/install-lib.sh | 14 ++++ opencode/RecallExtract.ts | 97 ++------------------------ opencode/lib/session-export.ts | 107 +++++++++++++++++++++++++++++ tests/opencode-integration.test.ts | 29 +++++++- uninstall.sh | 22 ++++++ 5 files changed, 175 insertions(+), 94 deletions(-) create mode 100644 opencode/lib/session-export.ts diff --git a/lib/install-lib.sh b/lib/install-lib.sh index b44867e..1f95d5b 100644 --- a/lib/install-lib.sh +++ b/lib/install-lib.sh @@ -2294,6 +2294,20 @@ recall_install_opencode_plugins() { log_success "Installed $plugin plugin" fi done + + # Shared helpers the plugins import at runtime. They MUST land in a + # subdirectory: OpenCode globs plugins/*.ts and calls every export of each + # match as a plugin factory, so a helper module sitting beside the plugins + # would make OpenCode log a plugin-load error on every launch. + mkdir -p "$plugin_dir/lib" + local helper + for helper in session-export.ts; do + if [[ -f "$src_dir/lib/$helper" ]]; then + recall_copy_canonical "$src_dir/lib/$helper" "$RECALL_OPENCODE_PLUGINS_DIR/lib/$helper" + recall_link "$plugin_dir/lib/$helper" "$RECALL_OPENCODE_PLUGINS_DIR/lib/$helper" + log_success "Installed OpenCode plugin helper: $helper" + fi + done } recall_install_opencode_agent() { diff --git a/opencode/RecallExtract.ts b/opencode/RecallExtract.ts index 2ebfab0..b735cf0 100644 --- a/opencode/RecallExtract.ts +++ b/opencode/RecallExtract.ts @@ -11,6 +11,9 @@ // - the payload carries properties.sessionID // - OpenCode loads this plugin from $XDG_CONFIG_HOME/opencode/plugins/, the // path lib/install-lib.sh installs to +// - OpenCode calls EVERY export of a top-level plugin module as a plugin +// factory, so this file must export nothing but RecallExtract. Shared pure +// helpers live in ./lib/session-export.ts, which OpenCode does not glob. // // VERIFIED APIs USED: // - ctx.$ (Bun shell tagged template) — confirmed in OpenCode plugin docs @@ -22,6 +25,7 @@ import { existsSync, readFileSync, writeFileSync, mkdirSync } from "fs" import { createHash } from "crypto" import { join } from "path" import { homedir } from "os" +import { exportSession, sessionIdFromEvent, type Shell } from "./lib/session-export" const RECALL_HOME = process.env.RECALL_HOME || join(homedir(), ".agents", "Recall") const DROP_DIR = join(RECALL_HOME, "MEMORY", "opencode-sessions") @@ -68,99 +72,6 @@ function digest(markdown: string): string { return createHash("sha256").update(markdown).digest("hex") } -type Shell = (strings: TemplateStringsArray, ...values: unknown[]) => Promise - -type RecordLike = Record - -function asRecord(value: unknown): RecordLike | null { - return value !== null && typeof value === "object" && !Array.isArray(value) - ? value as RecordLike - : null -} - -/** Extract the current OpenCode event payload's session ID. */ -export function sessionIdFromEvent(event: unknown): string | null { - const record = asRecord(event) - if (!record) return null - if (record.type !== undefined && record.type !== "session.idle") return null - - const properties = asRecord(record.properties) - const candidates = [ - properties?.sessionID, - properties?.sessionId, - properties?.session_id, - record.sessionID, - record.sessionId, - record.session_id, - ] - return candidates.find((candidate): candidate is string => typeof candidate === "string" && candidate.length > 0) ?? null -} - -function partText(part: RecordLike): string[] { - const text: string[] = [] - if (typeof part.text === "string" && part.text.trim()) text.push(part.text) - - const state = asRecord(part.state) - if (typeof state?.title === "string" && state.title.trim()) text.push(`[tool: ${state.title}]`) - if (typeof state?.output === "string" && state.output.trim()) text.push(state.output) - - if (typeof part.description === "string" && part.description.trim()) text.push(part.description) - if (typeof part.prompt === "string" && part.prompt.trim()) text.push(part.prompt) - if (typeof part.filename === "string" && part.filename.trim()) text.push(`[file: ${part.filename}]`) - return text -} - -/** Convert current `opencode export ` JSON into the markdown drop format. */ -export function renderSessionExport(raw: string, fallbackSessionId: string): string { - let exported: RecordLike - try { - exported = JSON.parse(raw) as RecordLike - } catch { - if (raw.trimStart().startsWith("#")) return raw.trimEnd() + "\n" - throw new Error("OpenCode export was neither JSON nor markdown") - } - - const messages = exported.messages - if (!Array.isArray(messages)) throw new Error("OpenCode export JSON has no messages array") - - const info = asRecord(exported.info) - const sessionId = typeof info?.id === "string" ? info.id : fallbackSessionId - const title = typeof info?.title === "string" && info.title.trim() ? info.title : sessionId - const lines = [`# OpenCode Session: ${title}`, "", `Session ID: ${sessionId}`, ""] - - for (const message of messages) { - const messageRecord = asRecord(message) - const messageInfo = asRecord(messageRecord?.info) - const role = typeof messageInfo?.role === "string" ? messageInfo.role : "unknown" - const parts = Array.isArray(messageRecord?.parts) ? messageRecord.parts : [] - const content = parts.flatMap(part => { - const record = asRecord(part) - return record ? partText(record) : [] - }) - if (content.length === 0) continue - lines.push(`## ${role}`, "", content.join("\n\n"), "") - } - - if (lines.length === 4) throw new Error("OpenCode export JSON contained no readable message content") - return lines.join("\n").trimEnd() + "\n" -} - -async function shellOutputText(result: unknown): Promise { - if (typeof result === "string") return result - const record = asRecord(result) - if (record && typeof record.text === "function") { - return await (record.text as () => Promise)() - } - if (record && typeof record.stdout === "string") return record.stdout - return String(result ?? "") -} - -/** Run the supported OpenCode export command and normalize its full JSON output. */ -export async function exportSession(shell: Shell, sessionId: string): Promise { - const result = await shell`opencode export ${sessionId}` - return renderSessionExport(await shellOutputText(result), sessionId) -} - export const RecallExtract: Plugin = async ({ $ }) => { mkdirSync(DROP_DIR, { recursive: true }) const tracker = loadTracker() diff --git a/opencode/lib/session-export.ts b/opencode/lib/session-export.ts new file mode 100644 index 0000000..a2765f5 --- /dev/null +++ b/opencode/lib/session-export.ts @@ -0,0 +1,107 @@ +// opencode/lib/session-export.ts +// Pure helpers for turning an OpenCode session export into Recall's markdown +// drop format. +// +// WHY THIS FILE EXISTS: OpenCode invokes EVERY export of a top-level plugin +// module as a plugin factory. When these helpers lived in +// `opencode/RecallExtract.ts` alongside the factory, OpenCode called +// `exportSession(pluginContext)` on startup, the tagged-template call hit a +// context object instead of a shell function, and every launch logged +// `ERROR failed to load plugin ... "Object is not a function"`. +// +// OpenCode only globs `plugins/*.ts`, so a module nested under `plugins/lib/` +// is importable without being mistaken for a plugin. Keep it that way: the +// plugin entry points must export nothing but their factory. + +export type Shell = (strings: TemplateStringsArray, ...values: unknown[]) => Promise + +type RecordLike = Record + +function asRecord(value: unknown): RecordLike | null { + return value !== null && typeof value === "object" && !Array.isArray(value) + ? value as RecordLike + : null +} + +/** Extract the current OpenCode event payload's session ID. */ +export function sessionIdFromEvent(event: unknown): string | null { + const record = asRecord(event) + if (!record) return null + if (record.type !== undefined && record.type !== "session.idle") return null + + const properties = asRecord(record.properties) + const candidates = [ + properties?.sessionID, + properties?.sessionId, + properties?.session_id, + record.sessionID, + record.sessionId, + record.session_id, + ] + return candidates.find((candidate): candidate is string => typeof candidate === "string" && candidate.length > 0) ?? null +} + +function partText(part: RecordLike): string[] { + const text: string[] = [] + if (typeof part.text === "string" && part.text.trim()) text.push(part.text) + + const state = asRecord(part.state) + if (typeof state?.title === "string" && state.title.trim()) text.push(`[tool: ${state.title}]`) + if (typeof state?.output === "string" && state.output.trim()) text.push(state.output) + + if (typeof part.description === "string" && part.description.trim()) text.push(part.description) + if (typeof part.prompt === "string" && part.prompt.trim()) text.push(part.prompt) + if (typeof part.filename === "string" && part.filename.trim()) text.push(`[file: ${part.filename}]`) + return text +} + +/** Convert current `opencode export ` JSON into the markdown drop format. */ +export function renderSessionExport(raw: string, fallbackSessionId: string): string { + let exported: RecordLike + try { + exported = JSON.parse(raw) as RecordLike + } catch { + if (raw.trimStart().startsWith("#")) return raw.trimEnd() + "\n" + throw new Error("OpenCode export was neither JSON nor markdown") + } + + const messages = exported.messages + if (!Array.isArray(messages)) throw new Error("OpenCode export JSON has no messages array") + + const info = asRecord(exported.info) + const sessionId = typeof info?.id === "string" ? info.id : fallbackSessionId + const title = typeof info?.title === "string" && info.title.trim() ? info.title : sessionId + const lines = [`# OpenCode Session: ${title}`, "", `Session ID: ${sessionId}`, ""] + + for (const message of messages) { + const messageRecord = asRecord(message) + const messageInfo = asRecord(messageRecord?.info) + const role = typeof messageInfo?.role === "string" ? messageInfo.role : "unknown" + const parts = Array.isArray(messageRecord?.parts) ? messageRecord.parts : [] + const content = parts.flatMap(part => { + const record = asRecord(part) + return record ? partText(record) : [] + }) + if (content.length === 0) continue + lines.push(`## ${role}`, "", content.join("\n\n"), "") + } + + if (lines.length === 4) throw new Error("OpenCode export JSON contained no readable message content") + return lines.join("\n").trimEnd() + "\n" +} + +async function shellOutputText(result: unknown): Promise { + if (typeof result === "string") return result + const record = asRecord(result) + if (record && typeof record.text === "function") { + return await (record.text as () => Promise)() + } + if (record && typeof record.stdout === "string") return record.stdout + return String(result ?? "") +} + +/** Run the supported OpenCode export command and normalize its full JSON output. */ +export async function exportSession(shell: Shell, sessionId: string): Promise { + const result = await shell`opencode export ${sessionId}` + return renderSessionExport(await shellOutputText(result), sessionId) +} diff --git a/tests/opencode-integration.test.ts b/tests/opencode-integration.test.ts index 8329397..e08eac7 100644 --- a/tests/opencode-integration.test.ts +++ b/tests/opencode-integration.test.ts @@ -9,10 +9,37 @@ import { CREATE_TABLES } from '../src/db/schema'; // v2→v3 migration SQL (inlined — formerly exported from schema.ts, now in migrations.ts) const MIGRATE_V2_TO_V3 = "ALTER TABLE sessions ADD COLUMN source TEXT DEFAULT 'claude-code'"; import { linearizeSession } from '../pi/RecallExtract'; -import { exportSession, renderSessionExport, sessionIdFromEvent } from '../opencode/RecallExtract'; +// Helpers live in opencode/lib/ because OpenCode calls every export of a +// top-level plugin module as a plugin factory — see that file's header. +import { exportSession, renderSessionExport, sessionIdFromEvent } from '../opencode/lib/session-export'; // ─── OpenCode Runtime Contract Tests ─── +describe('OpenCode plugin module contract', () => { + // OpenCode globs plugins/*.ts and invokes EVERY export of each match as a + // plugin factory. A stray helper export made OpenCode log + // `failed to load plugin ... "Object is not a function"` on every launch, + // because exportSession() was called with the plugin context as its shell. + // Keep plugin entry points to exactly one exported factory. + const pluginEntries = ['RecallExtract', 'RecallPreCompact']; + + for (const entry of pluginEntries) { + test(`${entry}.ts exports only its plugin factory`, async () => { + const mod = await import(`../opencode/${entry}.ts`); + const exported = Object.keys(mod).filter(key => key !== 'default'); + + expect(exported).toHaveLength(1); + expect(typeof mod[exported[0]]).toBe('function'); + }); + } + + test('shared helpers are nested under opencode/lib so OpenCode does not glob them', () => { + expect(existsSync(join(import.meta.dir, '..', 'opencode', 'lib', 'session-export.ts'))).toBe(true); + expect(readdirSync(join(import.meta.dir, '..', 'opencode')).filter(f => f.endsWith('.ts')).sort()) + .toEqual(['RecallExtract.ts', 'RecallPreCompact.ts']); + }); +}); + describe('OpenCode runtime contract', () => { test('reads the current event hook sessionID and ignores other events', () => { expect(sessionIdFromEvent({ type: 'session.idle', properties: { sessionID: 'current-1' } })).toBe('current-1'); diff --git a/uninstall.sh b/uninstall.sh index 61e283c..24a7dd9 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -464,6 +464,28 @@ remove_opencode() { log_success "Removed OpenCode plugin: $p" fi done + + # Shared plugin helpers live under plugins/lib (see recall_install_opencode_plugins). + local helper + for helper in session-export.ts; do + local hf="$plugin_dir/lib/$helper" + if [[ -L "$hf" ]]; then + if [[ "$DRY_RUN" == "true" ]]; then + echo " [dry-run] would unlink Recall-managed symlink: $hf" + else + recall_unlink_if_managed "$hf" + fi + log_success "Removed OpenCode plugin helper symlink: $helper" + elif [[ -f "$hf" ]]; then + run rm -f "$hf" + log_success "Removed OpenCode plugin helper: $helper" + fi + done + # Only remove the helper directory when Recall emptied it — a user file there + # is not ours to delete. + if [[ -d "$plugin_dir/lib" ]] && [[ -z "$(ls -A "$plugin_dir/lib" 2>/dev/null)" ]]; then + run rmdir "$plugin_dir/lib" + fi if [[ -f "$agent_dir/recall-memory.md" ]]; then run rm -f "$agent_dir/recall-memory.md" log_success "Removed OpenCode agent: recall-memory.md" From 933e6102e92633dd86468829e70a8c914a9b4065 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:25:20 -0400 Subject: [PATCH 04/17] test(opencode): verify the runtime contract against a live OpenCode server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing e2e imports the adapter and synthesizes the session.idle payload, so it proves Recall's handler accepts that shape — not that OpenCode emits it, and not that OpenCode ever loads the plugin. This harness closes both gaps: - Discovery: installs via the real recall_install_opencode_platform, then observes OpenCode load the plugin from that path and asserts zero plugin load errors. - Frequency: counts session.idle from OpenCode's own /event stream across three turns of ONE session in ONE long-lived server process, so the number cannot be an artifact of process teardown. Measured 1.00 per assistant turn, and the script fails loudly if that ever changes. - Completeness: asserts the drop carries every turn, which is the end-to-end regression for the discarded-turns fix. The model is a local OpenAI-compatible stub — the subject under test is OpenCode's session lifecycle, so a hosted model would only add flakiness. A PATH shim supplies `opencode` because the plugin shells out to it, exactly as in a real install. The pinned version now lives once in scripts/lib/opencode-runtime.ts; the existing e2e resolves the CLI through it instead of repeating the pin. --- package.json | 3 +- scripts/e2e-opencode-runtime.ts | 287 ++++++++++++++++++++++++++++++++ scripts/e2e-opencode.ts | 14 +- scripts/lib/opencode-runtime.ts | 176 ++++++++++++++++++++ 4 files changed, 470 insertions(+), 10 deletions(-) create mode 100644 scripts/e2e-opencode-runtime.ts create mode 100644 scripts/lib/opencode-runtime.ts diff --git a/package.json b/package.json index 6901456..6a770bc 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,8 @@ "check:version": "bun run scripts/check-version.ts", "prepublishOnly": "bun run build", "build:claude-plugin": "bun run scripts/build-claude-plugin.ts", - "test:e2e:claude-plugin": "bun run build && bun run scripts/e2e-claude-plugin.ts" + "test:e2e:claude-plugin": "bun run build && bun run scripts/e2e-claude-plugin.ts", + "test:e2e:opencode:runtime": "bun run build && bun run scripts/e2e-opencode-runtime.ts" }, "dependencies": { "@modelcontextprotocol/sdk": "^1.25.3", diff --git a/scripts/e2e-opencode-runtime.ts b/scripts/e2e-opencode-runtime.ts new file mode 100644 index 0000000..7c4ee47 --- /dev/null +++ b/scripts/e2e-opencode-runtime.ts @@ -0,0 +1,287 @@ +#!/usr/bin/env bun + +/** + * OpenCode runtime contract verification. + * + * `scripts/e2e-opencode.ts` covers the extraction pipeline (export -> drop -> + * batch extract -> search) but reaches the adapter by importing it directly and + * synthesizing the event payload. That proves Recall's handler ACCEPTS a + * `session.idle` shape; it cannot prove OpenCode emits it, and it cannot prove + * OpenCode ever loads the plugin. + * + * This script closes both gaps against a live server: + * + * 1. Discovery — the plugin is installed by the real installer, and OpenCode + * is observed loading it from that path. + * 2. Frequency — `session.idle` is counted from OpenCode's own event stream + * across several turns of ONE session inside ONE long-lived process, so + * the number cannot be an artifact of process teardown. + * 3. Completeness — the drop reflects the WHOLE conversation, not just the + * turn that happened to trigger the first idle. + * + * The model is a local OpenAI-compatible stub: the subject under test is + * OpenCode's session lifecycle, so a hosted model would only add flakiness. + * + * Everything runs in disposable HOME / XDG / RECALL_HOME / database roots and + * the production database is asserted unchanged. + */ + +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'fs'; +import { homedir, tmpdir } from 'os'; +import { spawn } from 'child_process'; +import { join } from 'path'; +import { assertMetadataUnchanged, assertSafeTestDb, metadata, stringEnv } from './lib/e2e-isolation'; +import { + OPENCODE_PINNED_VERSION, + collectEvents, + runOpenCode, + startStubProvider, + stubProviderConfig, + writeOpenCodeShim, +} from './lib/opencode-runtime'; + +const repoRoot = join(import.meta.dir, '..'); +const productionDb = join(homedir(), '.agents', 'Recall', 'recall.db'); +const tempRoot = mkdtempSync(join(tmpdir(), 'recall-opencode-runtime-')); +const testHome = join(tempRoot, 'home'); +const testRecallHome = join(tempRoot, 'recall-home'); +const testDb = join(tempRoot, 'recall.db'); +const xdgConfig = join(tempRoot, 'xdg-config'); +const xdgData = join(tempRoot, 'xdg-data'); +const xdgState = join(tempRoot, 'xdg-state'); +const xdgCache = join(tempRoot, 'xdg-cache'); +const project = join(tempRoot, 'project'); +const testBin = join(tempRoot, 'bin'); +const opencodeConfigDir = join(xdgConfig, 'opencode'); + +const STUB_PORT = 47611; +const SERVER_PORT = 47612; +const TURN_MARKERS = ['RUNTIMETURNALPHA', 'RUNTIMETURNBRAVO', 'RUNTIMETURNCHARLIE']; +/** Generous: an idle can trail the POST response slightly. */ +const IDLE_SETTLE_MS = 6_000; + +function assert(condition: unknown, message: string): asserts condition { + if (!condition) throw new Error(message); +} + +let env: Record; +let server: ReturnType | undefined; +let stub: ReturnType | undefined; +let stream: ReturnType | undefined; + +async function waitForServer(base: string, log: () => string): Promise { + for (let attempt = 0; attempt < 120; attempt++) { + try { + if ((await fetch(`${base}/global/health`)).ok) return; + } catch { + // not listening yet + } + await Bun.sleep(500); + } + throw new Error(`OpenCode server never became healthy\n${log()}`); +} + +async function main(): Promise { + const productionBefore = metadata(productionDb); + assertSafeTestDb(testDb, productionDb); + + for (const path of [testHome, testRecallHome, xdgConfig, xdgData, xdgState, xdgCache, project, opencodeConfigDir, testBin]) { + mkdirSync(path, { recursive: true }); + } + // The plugin shells out to `opencode export`, so the CLI must be on PATH. + writeOpenCodeShim(testBin); + + env = stringEnv({ + ...process.env, + HOME: testHome, + RECALL_HOME: testRecallHome, + RECALL_DIR: testRecallHome, + RECALL_REPO_DIR: repoRoot, + RECALL_DB_PATH: testDb, + RECALL_SKIP_LEGACY_DATA_MIGRATIONS: '1', + OPENCODE_CONFIG_DIR: opencodeConfigDir, + XDG_CONFIG_HOME: xdgConfig, + XDG_DATA_HOME: xdgData, + XDG_STATE_HOME: xdgState, + XDG_CACHE_HOME: xdgCache, + OPENCODE_DISABLE_AUTOUPDATE: '1', + OPENCODE_DISABLE_PRUNE: '1', + npm_config_cache: join(tempRoot, 'npm-cache'), + PATH: `${testBin}:${process.env.PATH ?? ''}`, + }); + + console.log(`isolation.recall_home=${testRecallHome}`); + console.log(`isolation.opencode_config=${opencodeConfigDir}`); + console.log(`isolation.test_db=${testDb}`); + console.log(`isolation.production_db=${productionDb}`); + console.log(`isolation.production_before=${JSON.stringify(productionBefore)}`); + console.log('isolation.production_db_opened=false'); + + const version = runOpenCode(['--version'], env, repoRoot).trim(); + console.log(`opencode.version=${version}`); + assert( + version.split('.').slice(0, 2).join('.') === OPENCODE_PINNED_VERSION.split('.').slice(0, 2).join('.'), + `OpenCode ${version} is not contract-compatible with the pinned ${OPENCODE_PINNED_VERSION}; re-verify the runtime contract and bump OPENCODE_PINNED_VERSION`, + ); + + // ── 1. Discovery: install through the REAL installer, then let OpenCode find it + runOpenCode(['--version'], env, repoRoot); // warm any first-run scaffolding + const install = Bun.spawnSync({ + cmd: ['bash', '-c', 'source "$1"; recall_install_opencode_platform', '_', join(repoRoot, 'lib', 'install-lib.sh')], + cwd: repoRoot, + env, + }); + assert( + install.exitCode === 0, + `recall_install_opencode_platform failed (${install.exitCode})\n${install.stdout.toString()}\n${install.stderr.toString()}`, + ); + + const installedPlugin = join(opencodeConfigDir, 'plugins', 'RecallExtract.ts'); + assert(existsSync(installedPlugin), `installer did not place the extraction plugin at ${installedPlugin}`); + console.log(`opencode.installed_plugin=${installedPlugin}`); + + // The helpers must be nested: OpenCode calls every export of a top-level + // plugins/*.ts as a plugin factory. + const installedHelper = join(opencodeConfigDir, 'plugins', 'lib', 'session-export.ts'); + assert(existsSync(installedHelper), `installer did not place the plugin helper at ${installedHelper}`); + console.log(`opencode.installed_helper=${installedHelper}`); + + // Point OpenCode's model at the local stub, preserving the installer's MCP block. + stub = startStubProvider(STUB_PORT); + const configPath = join(opencodeConfigDir, 'opencode.json'); + const installedConfig = JSON.parse(readFileSync(configPath, 'utf-8')) as Record; + writeFileSync(configPath, JSON.stringify({ ...installedConfig, ...stubProviderConfig(stub.baseURL) }, null, 2)); + + const base = `http://127.0.0.1:${SERVER_PORT}`; + let serverLog = ''; + // --print-logs is required for plugin load failures to reach stderr, which is + // how this script proves the plugins loaded cleanly rather than merely ran. + const serveArgs = ['serve', '--port', String(SERVER_PORT), '--hostname', '127.0.0.1', '--print-logs', '--log-level', 'DEBUG']; + server = spawn( + process.env.OPENCODE_BIN ?? 'bunx', + process.env.OPENCODE_BIN + ? serveArgs + : ['--yes', '--package', `opencode-ai@${OPENCODE_PINNED_VERSION}`, 'opencode', ...serveArgs], + { cwd: project, env, stdio: ['ignore', 'pipe', 'pipe'] }, + ); + server.stdout?.on('data', chunk => { serverLog += chunk; }); + server.stderr?.on('data', chunk => { serverLog += chunk; }); + await waitForServer(base, () => serverLog); + + // ── 2. Frequency: count real session.idle events across turns of ONE session + stream = collectEvents(base); + await Bun.sleep(1_000); + + const session = await (await fetch(`${base}/session`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ title: 'Recall OpenCode runtime contract' }), + })).json() as { id: string }; + const sessionId = session.id; + console.log(`opencode.session=${sessionId}`); + + // `serve` loads plugins lazily when it bootstraps an instance, so discovery + // can only be asserted once a session exists. + // + // The plugin factory creates the drop directory. Its existence under a + // RECALL_HOME only this server knows about is proof OpenCode loaded the + // plugin from the path the installer wrote — nothing else touches it. + const dropDir = join(testRecallHome, 'MEMORY', 'opencode-sessions'); + for (let attempt = 0; attempt < 60 && !existsSync(dropDir); attempt++) await Bun.sleep(250); + assert( + existsSync(dropDir), + `OpenCode never loaded Recall's plugin: ${dropDir} was not created by the plugin factory\n${serverLog}`, + ); + console.log('opencode.plugin_loaded_by_host=true'); + + // A plugin whose factory ran can still have logged a load error — OpenCode + // reports one per export it could not invoke. Recall's plugins must load + // completely clean. + const loadErrors = serverLog.split('\n').filter(line => line.includes('failed to load plugin')); + assert( + loadErrors.length === 0, + `OpenCode reported ${loadErrors.length} plugin load error(s):\n${loadErrors.join('\n')}`, + ); + console.log('opencode.plugin_load_errors=0'); + + const idleCount = () => stream!.events.filter(e => e.type === 'session.idle' && e.sessionID === sessionId).length; + const perTurn: { turn: number; idleDelta: number }[] = []; + + for (const [index, marker] of TURN_MARKERS.entries()) { + const before = idleCount(); + const response = await fetch(`${base}/session/${sessionId}/message`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ + model: { providerID: 'recallstub', modelID: 'stub-model' }, + parts: [{ type: 'text', text: `Record this verification token verbatim: ${marker}` }], + }), + }); + if (!response.ok) throw new Error(`turn ${index + 1} was rejected (${response.status}): ${await response.text()}`); + await response.json(); + await Bun.sleep(IDLE_SETTLE_MS); + perTurn.push({ turn: index + 1, idleDelta: idleCount() - before }); + } + + const totalIdle = idleCount(); + const perTurnRate = totalIdle / TURN_MARKERS.length; + console.log(`opencode.session_idle_total=${totalIdle}`); + console.log(`opencode.session_idle_turns=${TURN_MARKERS.length}`); + console.log(`opencode.session_idle_per_turn=${perTurnRate.toFixed(2)}`); + console.log(`opencode.session_idle_deltas=${JSON.stringify(perTurn)}`); + + // The measured contract: one idle per assistant turn. If this ever becomes + // per-session (or debounced), the adapter's re-export strategy and the + // frequency recorded in docs/OPENCODE_INTEGRATION.md both need revisiting — + // so fail loudly rather than silently recording a different number. + assert( + totalIdle === TURN_MARKERS.length, + `session.idle fired ${totalIdle} times across ${TURN_MARKERS.length} turns (expected one per turn). ` + + `The runtime contract changed; re-verify docs/OPENCODE_INTEGRATION.md and opencode/RecallExtract.ts. ` + + `deltas=${JSON.stringify(perTurn)}`, + ); + for (const { turn, idleDelta } of perTurn) { + assert(idleDelta === 1, `turn ${turn} produced ${idleDelta} session.idle events, expected exactly 1`); + } + + // ── 3. Completeness: the drop must carry every turn, not only the first + const drop = join(dropDir, `${sessionId}.md`); + assert(existsSync(drop), `no markdown drop was written for ${sessionId}\n${serverLog}`); + const dropBody = readFileSync(drop, 'utf-8'); + const missing = TURN_MARKERS.filter(marker => !dropBody.includes(marker)); + assert( + missing.length === 0, + `the drop lost ${missing.join(', ')} — a later session.idle failed to re-export the grown session. ` + + `drop bytes=${dropBody.length}`, + ); + console.log(`opencode.drop_contains_all_turns=true (${TURN_MARKERS.length} turns, ${dropBody.length} bytes)`); + + const tracker = join(dropDir, '.extracted.json'); + assert(existsSync(tracker), 'the adapter did not persist its dedup tracker'); + const trackerBody = JSON.parse(readFileSync(tracker, 'utf-8')) as unknown; + assert( + !Array.isArray(trackerBody) && typeof (trackerBody as Record)[sessionId] === 'string', + `tracker must record a content digest per session so a grown session re-exports; got ${JSON.stringify(trackerBody)}`, + ); + console.log('opencode.tracker_records_content_digest=true'); + + // The export command itself must return the whole conversation. + const exported = runOpenCode(['export', sessionId], env, project); + const exportMissing = TURN_MARKERS.filter(marker => !exported.includes(marker)); + assert(exportMissing.length === 0, `opencode export omitted ${exportMissing.join(', ')}`); + console.log('opencode.export_contains_all_turns=true'); + + assertMetadataUnchanged(productionDb, productionBefore); + console.log('isolation.production_db_opened=false'); + console.log(`opencode.runtime_contract=verified against ${version}`); +} + +try { + await main(); +} finally { + stream?.stop(); + server?.kill('SIGKILL'); + stub?.stop(); + if (process.env.RECALL_E2E_KEEP === '1') console.error(`e2e.temp_root=${tempRoot}`); + else rmSync(tempRoot, { recursive: true, force: true }); +} diff --git a/scripts/e2e-opencode.ts b/scripts/e2e-opencode.ts index 0cbd7af..bfbc074 100644 --- a/scripts/e2e-opencode.ts +++ b/scripts/e2e-opencode.ts @@ -24,6 +24,7 @@ import { dirname, join } from 'path'; import { fileURLToPath } from 'url'; import { Database } from 'bun:sqlite'; import { assertMetadataUnchanged, assertSafeTestDb, metadata, stringEnv } from './lib/e2e-isolation'; +import { OPENCODE_PINNED_VERSION, openCodeCommand } from './lib/opencode-runtime'; const repoRoot = join(dirname(fileURLToPath(import.meta.url)), '..'); const productionDb = join(homedir(), '.agents', 'Recall', 'recall.db'); @@ -62,13 +63,8 @@ function run(command: string, args: string[], env: Record, timeo return output; } -function openCodeArgs(args: string[]): { command: string; args: string[] } { - if (process.env.OPENCODE_BIN) return { command: process.env.OPENCODE_BIN, args }; - return { command: 'bunx', args: ['--yes', '--package', 'opencode-ai@1.18.4', 'opencode', ...args] }; -} - function runOpenCode(args: string[], env: Record): string { - const command = openCodeArgs(args); + const command = openCodeCommand(args); const result = spawnSync(command.command, command.args, { cwd: repoRoot, env, @@ -83,7 +79,7 @@ function runOpenCode(args: string[], env: Record): string { } function runOpenCodeCombined(args: string[], env: Record): string { - const command = openCodeArgs(args); + const command = openCodeCommand(args); return run(command.command, command.args, env, 180_000); } @@ -146,7 +142,7 @@ function writeOpenCodeImportFixture(id: string, uniqueMarker: string): void { projectID: 'proj_recall_phase4', directory: repoRoot, title: `Recall Phase 4 ${id}`, - version: '1.18.4', + version: OPENCODE_PINNED_VERSION, time: { created: now, updated: now }, }, messages: [baseMessage(id, longText, now)], @@ -226,7 +222,7 @@ async function main(): Promise { console.log('isolation.production_db_opened=false'); const version = runOpenCode(['--version'], env).trim(); - assert(version === '1.18.4', `unexpected OpenCode version: ${version}`); + assert(version === OPENCODE_PINNED_VERSION, `unexpected OpenCode version: ${version} (pinned ${OPENCODE_PINNED_VERSION})`); const paths = runOpenCodeCombined(['debug', 'paths'], env); for (const expected of [xdgConfig, xdgData, xdgState, xdgCache]) { assert(paths.includes(expected), `OpenCode path was not isolated under ${expected}\n${paths}`); diff --git a/scripts/lib/opencode-runtime.ts b/scripts/lib/opencode-runtime.ts new file mode 100644 index 0000000..9447481 --- /dev/null +++ b/scripts/lib/opencode-runtime.ts @@ -0,0 +1,176 @@ +/** + * Shared OpenCode runtime plumbing for the isolated e2e scripts. + * + * The pinned version lives here only — `scripts/e2e-opencode.ts` and + * `scripts/e2e-opencode-runtime.ts` both resolve the CLI through this module so + * a version bump is a one-line change rather than a hunt across scripts. + */ +import { spawnSync } from 'child_process'; +import { mkdirSync, writeFileSync } from 'fs'; +import { join } from 'path'; + +/** + * The OpenCode release the adapter's runtime contract is verified against. + * + * `docs/OPENCODE_INTEGRATION.md` records the measured `session.idle` frequency + * next to this version; bump both together, and re-run + * `bun run test:e2e:opencode:runtime` so the recorded number stays honest. + */ +export const OPENCODE_PINNED_VERSION = '1.18.5'; + +/** Resolve the OpenCode CLI: an explicit binary, else the pinned npm package. */ +export function openCodeCommand(args: string[]): { command: string; args: string[] } { + if (process.env.OPENCODE_BIN) return { command: process.env.OPENCODE_BIN, args }; + return { + command: 'bunx', + args: ['--yes', '--package', `opencode-ai@${OPENCODE_PINNED_VERSION}`, 'opencode', ...args], + }; +} + +/** Run the OpenCode CLI and return stdout, throwing with full output on failure. */ +export function runOpenCode(args: string[], env: Record, cwd: string, timeout = 180_000): string { + const resolved = openCodeCommand(args); + const result = spawnSync(resolved.command, resolved.args, { + cwd, env, encoding: 'utf-8', timeout, maxBuffer: 20 * 1024 * 1024, + }); + if (result.error || result.status !== 0) { + throw new Error( + `${resolved.command} ${resolved.args.join(' ')} failed (${result.status})\n${result.stdout ?? ''}\n${result.stderr ?? ''}`, + ); + } + return result.stdout ?? ''; +} + +/** + * Put `opencode` on PATH for anything the runtime spawns. + * + * The extraction plugin shells out to `opencode export `, so the CLI has to + * be resolvable by name — exactly as it is in a real install. Without this the + * plugin's export fails with `command not found` and the drop is never written. + */ +export function writeOpenCodeShim(binDir: string): void { + mkdirSync(binDir, { recursive: true }); + const resolved = openCodeCommand([]); + const target = join(binDir, 'opencode'); + writeFileSync( + target, + `#!/bin/sh\nexec ${JSON.stringify(resolved.command)} ${resolved.args.map(a => JSON.stringify(a)).join(' ')} "$@"\n`, + { mode: 0o755 }, + ); +} + +/** + * A local OpenAI-compatible chat-completions stub. + * + * Keeps the runtime e2e hermetic: what is under test is OpenCode's own session + * lifecycle and event emission, not a model's output, so the reply only has to + * be well-formed and deterministic. Echoing the prompt lets a test assert that + * a specific turn reached the transcript. + */ +export function startStubProvider(port: number): { stop: () => void; baseURL: string } { + const server = Bun.serve({ + port, + async fetch(req) { + const url = new URL(req.url); + if (url.pathname.endsWith('/models')) { + return Response.json({ object: 'list', data: [{ id: 'stub-model', object: 'model' }] }); + } + if (!url.pathname.endsWith('/chat/completions')) return new Response('not found', { status: 404 }); + + const body = await req.json() as { messages?: unknown[]; model?: string; stream?: boolean }; + const lastUser = [...(body.messages ?? [])].reverse() + .find((m): m is { role: string; content: unknown } => + typeof m === 'object' && m !== null && (m as { role?: string }).role === 'user'); + const raw = typeof lastUser?.content === 'string' + ? lastUser.content + : (Array.isArray(lastUser?.content) ? lastUser.content : []) + .map(part => (typeof part === 'object' && part !== null ? String((part as { text?: string }).text ?? '') : '')) + .join(' '); + const reply = `STUB_REPLY(${raw.trim().slice(0, 120)})`; + const id = 'chatcmpl-recall-stub'; + const model = body.model ?? 'stub-model'; + const created = Math.floor(Date.now() / 1000); + const usage = { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 }; + + if (!body.stream) { + return Response.json({ + id, object: 'chat.completion', created, model, usage, + choices: [{ index: 0, message: { role: 'assistant', content: reply }, finish_reason: 'stop' }], + }); + } + + const frames = [ + { id, object: 'chat.completion.chunk', created, model, choices: [{ index: 0, delta: { role: 'assistant', content: reply }, finish_reason: null }] }, + { id, object: 'chat.completion.chunk', created, model, choices: [{ index: 0, delta: {}, finish_reason: 'stop' }], usage }, + ]; + const stream = new ReadableStream({ + start(controller) { + const encoder = new TextEncoder(); + for (const frame of frames) controller.enqueue(encoder.encode(`data: ${JSON.stringify(frame)}\n\n`)); + controller.enqueue(encoder.encode('data: [DONE]\n\n')); + controller.close(); + }, + }); + return new Response(stream, { headers: { 'content-type': 'text/event-stream', 'cache-control': 'no-cache' } }); + }, + }); + return { stop: () => server.stop(true), baseURL: `http://127.0.0.1:${port}/v1` }; +} + +/** The OpenCode config block that points every turn at the local stub provider. */ +export function stubProviderConfig(baseURL: string): Record { + return { + provider: { + recallstub: { + npm: '@ai-sdk/openai-compatible', + name: 'Recall Stub Provider', + options: { baseURL, apiKey: 'recall-stub-key' }, + models: { 'stub-model': { name: 'Recall Stub Model' } }, + }, + }, + model: 'recallstub/stub-model', + }; +} + +/** Collect OpenCode's own server-sent events until `stop()` is called. */ +export function collectEvents(serverBase: string): { + events: { at: number; type: string; sessionID?: string }[]; + stop: () => void; +} { + const events: { at: number; type: string; sessionID?: string }[] = []; + const controller = new AbortController(); + + void (async () => { + const res = await fetch(`${serverBase}/event`, { + signal: controller.signal, + headers: { accept: 'text/event-stream' }, + }); + const reader = res.body!.getReader(); + const decoder = new TextDecoder(); + let buffer = ''; + while (true) { + const { done, value } = await reader.read(); + if (done) break; + buffer += decoder.decode(value, { stream: true }); + const chunks = buffer.split('\n\n'); + buffer = chunks.pop() ?? ''; + for (const chunk of chunks) { + for (const line of chunk.split('\n')) { + if (!line.startsWith('data:')) continue; + try { + const parsed = JSON.parse(line.slice(5).trim()) as { type?: string; properties?: { sessionID?: string } }; + if (typeof parsed.type === 'string') { + events.push({ at: Date.now(), type: parsed.type, sessionID: parsed.properties?.sessionID }); + } + } catch { + // a partial or non-JSON frame is not evidence of anything — skip it + } + } + } + } + })().catch(() => { + // the stream is aborted at teardown; that rejection is expected + }); + + return { events, stop: () => controller.abort() }; +} From dc3988039193e7808dc0213d111211030626bf75 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:30:53 -0400 Subject: [PATCH 05/17] fix(hooks): stop losing extraction records when two hosts write at once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Concurrent Claude and OpenCode extraction against the shared WAL database lost records to 'database is locked' even though openDb sets busy_timeout=5000 — the isolated OpenCode e2e failed its own concurrency assertion this way. Cause: the batch writers probe for duplicates (skipDuplicates) before inserting, and SQLite fails a read-to-write upgrade inside a DEFERRED transaction with SQLITE_BUSY *immediately*, never consulting busy_timeout. Measured on this machine while a peer held the write lock: the deferred read-then-write path failed in 1ms, while taking the write lock up front waited the full timeout. So the timeout the comment relies on was never reachable on the retry-safe path extraction actually uses. Taking the transactions IMMEDIATE makes the peer wait honoured. Scoped to the four batch writers: the other hook transactions (consolidate-core, RecallPreCompact) already begin with a write, so busy_timeout applies to them and they are left alone. The new tests hold the write lock from a separate process — the writers are synchronous, so an in-process timer could never release it — and assert the write waits and lands. They fail with 'database is locked' on the deferred version. --- hooks/lib/sqlite-writers.ts | 28 +++- .../hooks/sqlite-writers-concurrency.test.ts | 138 ++++++++++++++++++ 2 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 tests/hooks/sqlite-writers-concurrency.test.ts diff --git a/hooks/lib/sqlite-writers.ts b/hooks/lib/sqlite-writers.ts index d1d4dc0..ac5dda3 100644 --- a/hooks/lib/sqlite-writers.ts +++ b/hooks/lib/sqlite-writers.ts @@ -183,7 +183,12 @@ export function writeDecisionsBatch( } return n; }); - return insertMany(items); + // IMMEDIATE, not the default DEFERRED: these batches read (the + // skipDuplicates probe) before they insert, and SQLite fails a + // read-to-write upgrade with SQLITE_BUSY *instantly*, ignoring + // busy_timeout. Taking the write lock up front is what makes the peer wait + // honoured — measured 1ms hard failure deferred vs ~500ms of waiting here. + return insertMany.immediate(items); } finally { db.close(); } @@ -260,7 +265,12 @@ export function writeLearningsBatch( } return n; }); - return insertMany(items); + // IMMEDIATE, not the default DEFERRED: these batches read (the + // skipDuplicates probe) before they insert, and SQLite fails a + // read-to-write upgrade with SQLITE_BUSY *instantly*, ignoring + // busy_timeout. Taking the write lock up front is what makes the peer wait + // honoured — measured 1ms hard failure deferred vs ~500ms of waiting here. + return insertMany.immediate(items); } finally { db.close(); } @@ -312,7 +322,12 @@ export function writeBreadcrumbsBatch( } return n; }); - return insertMany(items); + // IMMEDIATE, not the default DEFERRED: these batches read (the + // skipDuplicates probe) before they insert, and SQLite fails a + // read-to-write upgrade with SQLITE_BUSY *instantly*, ignoring + // busy_timeout. Taking the write lock up front is what makes the peer wait + // honoured — measured 1ms hard failure deferred vs ~500ms of waiting here. + return insertMany.immediate(items); } finally { db.close(); } @@ -416,7 +431,12 @@ export function writeExtractionErrors(dbPath: string, items: ExtractionErrorInpu } return n; }); - return insertMany(items); + // IMMEDIATE, not the default DEFERRED: these batches read (the + // skipDuplicates probe) before they insert, and SQLite fails a + // read-to-write upgrade with SQLITE_BUSY *instantly*, ignoring + // busy_timeout. Taking the write lock up front is what makes the peer wait + // honoured — measured 1ms hard failure deferred vs ~500ms of waiting here. + return insertMany.immediate(items); } finally { db.close(); } diff --git a/tests/hooks/sqlite-writers-concurrency.test.ts b/tests/hooks/sqlite-writers-concurrency.test.ts new file mode 100644 index 0000000..afed166 --- /dev/null +++ b/tests/hooks/sqlite-writers-concurrency.test.ts @@ -0,0 +1,138 @@ +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { mkdtempSync, rmSync, writeFileSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { Database } from 'bun:sqlite'; +import { CREATE_TABLES } from '../../src/db/schema'; +import { writeBreadcrumbsBatch, writeDecisionsBatch, writeLearningsBatch } from '../../hooks/lib/sqlite-writers'; + +/** + * Concurrent Claude + OpenCode extraction against one shared WAL database. + * + * The batch writers probe for duplicates (skipDuplicates) before inserting. + * Inside a DEFERRED transaction that read-then-write sequence makes SQLite + * fail the upgrade with SQLITE_BUSY *immediately* — `busy_timeout` is not + * consulted at all — so a peer's short transaction silently cost a record + * instead of being waited out. Measured: ~1ms hard failure when deferred, + * versus honouring the full timeout when the write lock is taken up front. + * + * These tests hold the write lock from a separate PROCESS (the writers are + * synchronous, so an in-process timer could never release it) and assert the + * write waits and lands. + */ +describe('shared-database extraction writers (concurrent hosts)', () => { + let tempDir: string; + let dbPath: string; + + /** Hold the write lock in a peer process, releasing it after `holdMs`. */ + async function peerHoldingWriteLock(holdMs: number): Promise<{ done: Promise }> { + const script = join(tempDir, 'peer.ts'); + writeFileSync(script, ` +import { Database } from 'bun:sqlite'; +const db = new Database(${JSON.stringify(dbPath)}); +db.exec('PRAGMA journal_mode = WAL'); +db.exec('PRAGMA busy_timeout = 5000'); +db.exec('BEGIN IMMEDIATE'); +db.prepare("INSERT INTO breadcrumbs (session_id, project, content, importance) VALUES ('peer','peer','peer holds the write lock',5)").run(); +console.log('LOCKED'); +await Bun.sleep(${holdMs}); +db.exec('COMMIT'); +db.close(); +`); + const child = Bun.spawn(['bun', 'run', script], { stdout: 'pipe', stderr: 'pipe' }); + + // Wait until the peer confirms it holds the lock. + const reader = child.stdout.getReader(); + const decoder = new TextDecoder(); + let seen = ''; + while (!seen.includes('LOCKED')) { + const { done, value } = await reader.read(); + if (done) throw new Error(`peer never took the lock: ${seen}${await new Response(child.stderr).text()}`); + seen += decoder.decode(value, { stream: true }); + } + return { done: child.exited.then(() => undefined) }; + } + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), 'recall-concurrency-')); + dbPath = join(tempDir, 'recall.db'); + const db = new Database(dbPath); + db.exec('PRAGMA journal_mode = WAL'); + db.run(CREATE_TABLES); + db.close(); + }); + + afterEach(() => { + if (tempDir) rmSync(tempDir, { recursive: true, force: true }); + }); + + test('a decision batch waits out a peer write lock instead of losing the record', async () => { + const peer = await peerHoldingWriteLock(600); + + const started = Date.now(); + const written = writeDecisionsBatch( + dbPath, + [{ sessionId: 'opencode-session', project: 'opencode', decision: 'Recall waits for a peer writer', importance: 6 }], + { skipDuplicates: true }, + ); + const waited = Date.now() - started; + await peer.done; + + expect(written).toBe(1); + // Proves the wait actually happened rather than the write racing in first. + expect(waited).toBeGreaterThan(200); + + const db = new Database(dbPath, { readonly: true }); + const rows = db.query('SELECT decision FROM decisions').all() as { decision: string }[]; + db.close(); + expect(rows.map(r => r.decision)).toContain('Recall waits for a peer writer'); + }); + + test('learning and breadcrumb batches also survive a peer write lock', async () => { + const peer = await peerHoldingWriteLock(500); + + const learnings = writeLearningsBatch( + dbPath, + [{ sessionId: 'claude-session', project: 'claude', problem: 'deferred upgrade returns SQLITE_BUSY instantly', solution: 'take the write lock up front', confidence: 'high', importance: 7 }], + { skipDuplicates: true }, + ); + const breadcrumbs = writeBreadcrumbsBatch( + dbPath, + [{ sessionId: 'opencode-session', project: 'opencode', content: 'concurrent hosts share one WAL database', importance: 5 }], + { skipDuplicates: true }, + ); + await peer.done; + + expect(learnings).toBe(1); + expect(breadcrumbs).toBe(1); + }); + + test('both hosts writing at once keep every record and their own source', async () => { + // No peer lock here — this is the plain interleaved case: two hosts writing + // the same tables must not drop or merge each other's rows. + const results = await Promise.all([ + Promise.resolve().then(() => writeDecisionsBatch( + dbPath, + [{ sessionId: 'claude-session', project: 'claude', decision: 'CLAUDE_SOURCED_DECISION', importance: 5 }], + { skipDuplicates: true }, + )), + Promise.resolve().then(() => writeDecisionsBatch( + dbPath, + [{ sessionId: 'opencode-session', project: 'opencode', decision: 'OPENCODE_SOURCED_DECISION', importance: 5 }], + { skipDuplicates: true }, + )), + ]); + + expect(results).toEqual([1, 1]); + + const db = new Database(dbPath, { readonly: true }); + const rows = db.query('SELECT project, decision FROM decisions ORDER BY decision').all() as { project: string; decision: string }[]; + db.close(); + + expect(rows).toHaveLength(2); + expect(rows.map(r => `${r.project}:${r.decision}`)).toEqual([ + 'claude:CLAUDE_SOURCED_DECISION', + 'opencode:OPENCODE_SOURCED_DECISION', + ]); + }); +}); From 7b54ec23ceaf2842f6d9b570a9d897bca1ef8a46 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:33:39 -0400 Subject: [PATCH 06/17] test(install): cover the documented restore/rollback path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ./install.sh restore is documented in docs/installation.md but recall_do_restore had no coverage, and neither did the collision backup that is the other half of rollback. Covers restoring the latest snapshot, restoring a specific timestamp, the pre-restore snapshot that makes a restore itself reversible, and non-zero exits for an unknown timestamp and for no backups at all. Also pins the automation contract: restore gates on a default-No confirm, and _confirm returns the default when stdin is not a TTY, so restore is a deliberate no-op in CI or scripts. That is asserted directly — it declines, mutates nothing, and writes no pre-restore snapshot — rather than left as a surprise. The restoring path is exercised by overriding _confirm in the sourced shell, so the safety default is tested rather than weakened. The collision tests prove a hand-edited user file at a Recall-managed path survives install as recoverable bytes, and that an identical file is replaced without a needless backup. --- tests/install/restore.test.ts | 204 ++++++++++++++++++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 tests/install/restore.test.ts diff --git a/tests/install/restore.test.ts b/tests/install/restore.test.ts new file mode 100644 index 0000000..c51376b --- /dev/null +++ b/tests/install/restore.test.ts @@ -0,0 +1,204 @@ +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, writeFileSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { spawnSync } from 'child_process'; + +const REPO = join(import.meta.dir, '..', '..'); + +/** + * Installer rollback: `./install.sh restore [TIMESTAMP]`, documented in + * docs/installation.md, had no coverage at all. + * + * `recall_do_restore` gates on `_confirm "Proceed with restore?" "N"`, and + * `_confirm` returns the default when stdin is not a TTY — so restore is a + * deliberate no-op in automation. To exercise the restoring path without + * changing that safety default, these tests override `_confirm` in the sourced + * shell, which is the ordinary seam for a bash function. The non-interactive + * default is asserted separately, because "declines and changes nothing" is + * itself the contract CI and scripts depend on. + */ +describe('installer restore (rollback)', () => { + let root: string; + let claudeDir: string; + let recallDir: string; + let backupBase: string; + + /** Run a bash snippet with install-lib sourced in a fully disposable HOME. */ + function sh(snippet: string, extraEnv: Record = {}) { + return spawnSync('bash', ['-c', `set -uo pipefail\nsource "$REPO/lib/install-lib.sh" >/dev/null 2>&1\n${snippet}`], { + encoding: 'utf-8', + env: { + ...process.env, + REPO, + HOME: root, + CLAUDE_DIR: claudeDir, + RECALL_DIR: recallDir, + BACKUP_BASE: backupBase, + RECALL_REPO_DIR: REPO, + NO_CONFIRM: 'true', + HAS_GUM: 'false', + ...extraEnv, + }, + }); + } + + /** Seed a backup snapshot as `recall_create_backup` would have written it. */ + function seedBackup(stamp: string, files: Record, markLatest = true) { + const dir = join(backupBase, stamp); + mkdirSync(dir, { recursive: true }); + for (const [name, body] of Object.entries(files)) writeFileSync(join(dir, name), body); + writeFileSync(join(dir, 'manifest.txt'), `seeded ${stamp}\n`); + if (markLatest) writeFileSync(join(backupBase, 'latest'), stamp); + return dir; + } + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'recall-restore-')); + claudeDir = join(root, '.claude'); + recallDir = join(root, '.agents', 'Recall'); + backupBase = join(recallDir, 'backups'); + mkdirSync(claudeDir, { recursive: true }); + mkdirSync(backupBase, { recursive: true }); + }); + + afterEach(() => { + if (root) rmSync(root, { recursive: true, force: true }); + }); + + test('restores the latest snapshot over the current files', () => { + writeFileSync(join(claudeDir, 'settings.json'), '{"current":true}'); + writeFileSync(join(claudeDir, '.mcp.json'), '{"mcpServers":{"drifted":{}}}'); + seedBackup('20260101120000', { + 'settings.json': '{"restored":true}', + '.mcp.json': '{"mcpServers":{"original":{}}}', + }); + + const result = sh('_confirm() { return 0; }\nrecall_do_restore ""'); + + expect(result.status).toBe(0); + expect(readFileSync(join(claudeDir, 'settings.json'), 'utf-8')).toBe('{"restored":true}'); + expect(readFileSync(join(claudeDir, '.mcp.json'), 'utf-8')).toBe('{"mcpServers":{"original":{}}}'); + }); + + test('snapshots the pre-restore state so a restore is itself reversible', () => { + writeFileSync(join(claudeDir, 'settings.json'), '{"about-to-be-replaced":true}'); + seedBackup('20260101120000', { 'settings.json': '{"restored":true}' }); + + const result = sh('_confirm() { return 0; }\nrecall_do_restore ""'); + expect(result.status).toBe(0); + + const preRestore = readdirSync(backupBase).filter(d => d.startsWith('pre_restore_')); + expect(preRestore).toHaveLength(1); + expect(readFileSync(join(backupBase, preRestore[0], 'settings.json'), 'utf-8')) + .toBe('{"about-to-be-replaced":true}'); + }); + + test('restores a specific timestamp rather than the latest', () => { + seedBackup('20260101120000', { 'settings.json': '{"older":true}' }, false); + seedBackup('20260202120000', { 'settings.json': '{"newer":true}' }); + + const result = sh('_confirm() { return 0; }\nrecall_do_restore "20260101120000"'); + + expect(result.status).toBe(0); + expect(readFileSync(join(claudeDir, 'settings.json'), 'utf-8')).toBe('{"older":true}'); + }); + + test('an unknown timestamp fails without touching current files', () => { + writeFileSync(join(claudeDir, 'settings.json'), '{"untouched":true}'); + seedBackup('20260101120000', { 'settings.json': '{"restored":true}' }); + + const result = sh('_confirm() { return 0; }\nrecall_do_restore "20990101000000"'); + + expect(result.status).not.toBe(0); + expect(readFileSync(join(claudeDir, 'settings.json'), 'utf-8')).toBe('{"untouched":true}'); + }); + + test('restore with no backups at all fails instead of reporting success', () => { + const result = sh('_confirm() { return 0; }\nrecall_do_restore ""'); + + expect(result.status).not.toBe(0); + expect(`${result.stdout}${result.stderr}`).toContain('No backups found'); + }); + + test('a non-interactive restore declines and changes nothing', () => { + writeFileSync(join(claudeDir, 'settings.json'), '{"current":true}'); + seedBackup('20260101120000', { 'settings.json': '{"restored":true}' }); + + // No _confirm override: this is exactly what CI or a script would hit. + const result = sh('recall_do_restore ""'); + + expect(result.status).toBe(0); + expect(`${result.stdout}${result.stderr}`).toContain('Restore cancelled'); + expect(readFileSync(join(claudeDir, 'settings.json'), 'utf-8')).toBe('{"current":true}'); + expect(readdirSync(backupBase).filter(d => d.startsWith('pre_restore_'))).toHaveLength(0); + }); +}); + +/** + * The other half of rollback: a user file sitting where Recall wants a symlink + * must survive install as recoverable bytes, not be overwritten in place. + */ +describe('installer collision backup', () => { + let root: string; + let recallDir: string; + let backupDir: string; + + beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'recall-collision-')); + recallDir = join(root, '.agents', 'Recall'); + backupDir = join(recallDir, 'backups', 'testrun'); + mkdirSync(recallDir, { recursive: true }); + }); + + afterEach(() => { + if (root) rmSync(root, { recursive: true, force: true }); + }); + + function sh(snippet: string) { + return spawnSync('bash', ['-c', `set -uo pipefail\nsource "$REPO/lib/install-lib.sh" >/dev/null 2>&1\n${snippet}`], { + encoding: 'utf-8', + env: { + ...process.env, + REPO, + HOME: root, + RECALL_DIR: recallDir, + BACKUP_DIR: backupDir, + RECALL_REPO_DIR: REPO, + NO_CONFIRM: 'true', + HAS_GUM: 'false', + }, + }); + } + + test('a drifted user file is preserved byte-for-byte before being symlinked', () => { + const canonical = join(recallDir, 'canonical.ts'); + writeFileSync(canonical, 'canonical body\n'); + const target = join(root, 'userland', 'canonical.ts'); + mkdirSync(join(root, 'userland'), { recursive: true }); + const userBody = 'the user hand-edited this and it must not vanish\n'; + writeFileSync(target, userBody); + + const result = sh(`recall_link "${target}" "${canonical}"`); + expect(result.status).toBe(0); + + // Target is now Recall's symlink... + expect(readFileSync(target, 'utf-8')).toBe('canonical body\n'); + // ...and the user's original bytes are recoverable from the backup tree. + const preserved = join(backupDir, 'collisions', 'userland', 'canonical.ts'); + expect(existsSync(preserved)).toBe(true); + expect(readFileSync(preserved, 'utf-8')).toBe(userBody); + }); + + test('a file identical to the canonical is replaced without a needless backup', () => { + const canonical = join(recallDir, 'canonical.ts'); + writeFileSync(canonical, 'same body\n'); + const target = join(root, 'userland', 'same.ts'); + mkdirSync(join(root, 'userland'), { recursive: true }); + writeFileSync(target, 'same body\n'); + + const result = sh(`recall_link "${target}" "${canonical}"`); + expect(result.status).toBe(0); + expect(existsSync(join(backupDir, 'collisions', 'userland', 'same.ts'))).toBe(false); + }); +}); From 3da57aca617dd13af6d7cb586b6d46992db13d76 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:35:36 -0400 Subject: [PATCH 07/17] ci: run the suite inside a git worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AGENTS.md mandates workers run in an isolated worktree, so a worktree is the default environment for automated work. Ten lifecycle/config tests once failed there while passing in a normal checkout (#243), which teaches agents to read a red suite as normal — the exact signal loss a suite exists to prevent. This leg creates a real worktree, asserts it starts without node_modules (#165's condition, so the workaround stays visible rather than accidental), installs there, and runs lint plus the full suite. Verified locally in a fresh worktree: 1298 pass, 0 fail. Closes the last open acceptance item on #243; the other two were already met — recall_configure_opencode_mcp and recall_configure_pi_mcp both exit 1 and leave a malformed config byte-identical. --- .github/workflows/ci.yml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 76ea0de..5587371 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,6 +43,36 @@ jobs: - name: OpenCode integration e2e run: bun run test:e2e:opencode + worktree: + name: Worktree baseline + runs-on: macos-latest + steps: + - name: Check out repository + uses: actions/checkout@v7 + + - name: Set up Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version-file: package.json + + # AGENTS.md mandates that workers run in an isolated git worktree, so a + # worktree is the DEFAULT environment for automated work — not an edge + # case. Ten lifecycle/config tests once failed there while passing in a + # normal checkout (#243), which trains agents to read a red suite as + # normal. This leg makes worktree-hostility fail loudly instead. + # + # A fresh worktree has no node_modules (#165), so dependencies are + # installed inside it rather than inherited from the primary checkout. + - name: Run the suite inside a git worktree + run: | + set -euo pipefail + git worktree add ../recall-worktree HEAD + cd ../recall-worktree + test ! -d node_modules + bun install --frozen-lockfile + bun run lint + bun test + ubuntu-smoke: name: Ubuntu compatibility smoke runs-on: ubuntu-latest From 0224157464cb5db9865f68bdfeedb6d40d00b9a7 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:37:50 -0400 Subject: [PATCH 08/17] docs(opencode): reconcile the OpenCode claims with what is proven MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The design doc asserted OpenCode 1.18.4 'emits session.idle through the event hook' and cited the pipeline e2e as pinning it. That e2e imports the adapter and supplies the payload itself, so it proved Recall ACCEPTS that shape — not that OpenCode sends it, and not that OpenCode loads the plugin at all. Phase 4 Evidence now says which of the two scripts proves what, and names compaction context injection as still unverified at runtime. Records the measured session.idle frequency (once per assistant turn, OpenCode 1.18.5) in place of the hedge about adding debouncing 'if future evidence shows repeated idle events' — the evidence exists now, and it points the other way. Also documents the single-export plugin contract and the PATH requirement. README moves OpenCode from Alpha/In progress to Beta with the scope stated: capture is runtime-verified, compaction injection is not. Fixes stale details in the same doc (7 tools -> 9, the tracker's digest semantics, missing files). --- CHANGELOG.md | 37 +++++++++++ FOR_OPENCODE.md | 5 ++ README.md | 2 +- docs/OPENCODE_INTEGRATION.md | 118 ++++++++++++++++++++++++++++------- 4 files changed, 138 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8551d97..e976dce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,43 @@ note in the 0.9.0 entry. OpenCode 1.18.4, verifies the current `session.idle` event and JSON export contract, exercises export retry, concurrent WAL writers, installer/update idempotence, JSONC-preserving uninstall, and searchable Recall retrieval. +- **OpenCode live-server verification** (`bun run test:e2e:opencode:runtime`) — + drives a real `opencode serve`, proving what the pipeline e2e structurally + cannot: that OpenCode loads Recall's plugin from the path the installer writes, + that it loads with zero plugin errors, and that OpenCode itself emits + `session.idle`. **Measured: `session.idle` fires once per assistant turn** + (three turns produced three events, OpenCode 1.18.5), counted from OpenCode's + own event stream in one long-lived process. The frequency is asserted, so a + runtime change fails the suite instead of silently invalidating the design. +- **Installer rollback coverage** — the documented `./install.sh restore` path + had no tests. Restoring the latest snapshot, restoring a specific timestamp, + the pre-restore snapshot, and non-zero exits for unknown/absent backups are now + covered, along with the collision backup that preserves a user's hand-edited + file before Recall symlinks over it. Restore's default-No confirm means it is a + deliberate no-op in non-interactive contexts; that is now asserted rather than + surprising. +- **CI runs the suite inside a git worktree** — the environment `AGENTS.md` + mandates for workers, so worktree-hostility fails loudly instead of training + agents to accept a red baseline (#243). + +### Fixed + +- **OpenCode multi-turn sessions no longer lose every turn after the first.** + Because `session.idle` fires per turn, the adapter's permanent dedup tracker + let the first idle win and discarded the rest: a real three-turn session left a + 154-byte drop holding only turn 1. The tracker now records a content digest per + session, so a grown session overwrites its earlier, shorter drop. +- **OpenCode no longer logs a plugin load error on every launch.** OpenCode calls + every export of a top-level `plugins/*.ts` as a plugin factory, so the + extraction plugin's exported test helpers were invoked with the plugin context + and failed with `Object is not a function`. Helpers moved to + `opencode/lib/session-export.ts`, which OpenCode does not glob. +- **Concurrent Claude and OpenCode extraction no longer loses records.** The batch + writers probe for duplicates before inserting, and SQLite fails that + read-to-write upgrade inside a DEFERRED transaction with `SQLITE_BUSY` + immediately — never consulting the `busy_timeout` the code relied on. Measured + 1ms hard failure versus the full timeout honoured once the write lock is taken + up front; the transactions are now `IMMEDIATE`. ### Changed diff --git a/FOR_OPENCODE.md b/FOR_OPENCODE.md index d9e9a96..7e73e75 100644 --- a/FOR_OPENCODE.md +++ b/FOR_OPENCODE.md @@ -145,6 +145,11 @@ A Recall plugin (`RecallExtract.ts`) runs inside OpenCode: 3. Drops the file into `$RECALL_HOME/MEMORY/opencode-sessions/` 4. A batch extraction job quality-gates these files into the shared SQLite database +`session.idle` fires once per assistant turn (measured against OpenCode 1.18.5), +so the drop is rewritten as the conversation grows and always reflects the whole +session rather than only its first turn. An unchanged session costs no write — +the plugin compares a digest of what it last wrote. + The plugin records a session only after the export and drop write succeed, so failed exports remain retryable. OpenCode must be available on `PATH`, and the plugin runtime requires Bun. Verify both with `opencode --version` and `bun --version`. diff --git a/README.md b/README.md index 93b1b81..03240c2 100644 --- a/README.md +++ b/README.md @@ -321,7 +321,7 @@ Recall is built around two integration surfaces: **MCP** (memory search and add, | ------------------------------------------------------------- | :-: | :--------------------------------------------------------: | ------------------------------------- | | [**Claude Code**](https://claude.com/claude-code) | ✅ | ✅ Stop · SessionStart · PreCompact | **Stable** — reference implementation; native plugin ships skills + MCP | | [**Pi**](https://pi.dev/) | ✅ | ⚠ Beta — native package with memory-injection + shutdown-capture extensions | Package + separate MCP adapter/config | -| [**OpenCode**](https://opencode.ai/) | ✅ | ⚠ Alpha — `recall-extract` plugin | In progress | +| [**OpenCode**](https://opencode.ai/) | ✅ | ⚠ Beta — `recall-extract` plugin; `session.idle` capture verified against a live OpenCode 1.18.5 server | Capture runtime-verified; compaction injection not yet | | [**Codex CLI**](https://github.com/openai/codex) | ✅ | — native plugin, explicit dump only | MCP + skills available | | [**Gemini CLI**](https://github.com/google-gemini/gemini-cli) | — | — | Coming soon | diff --git a/docs/OPENCODE_INTEGRATION.md b/docs/OPENCODE_INTEGRATION.md index f19bb69..48b544b 100644 --- a/docs/OPENCODE_INTEGRATION.md +++ b/docs/OPENCODE_INTEGRATION.md @@ -1,8 +1,11 @@ # Recall + OpenCode Integration Architecture > Design document for extending Recall to work with OpenCode alongside Claude Code. -> **v3 — Phase 4 verified against OpenCode 1.18.4.** The current event and -> export contract is pinned by `scripts/e2e-opencode.ts`. +> **v4 — Phase 4 verified against a live OpenCode 1.18.5 server.** The event +> contract, the measured `session.idle` frequency, and plugin discovery are +> pinned by `scripts/e2e-opencode-runtime.ts`; the extraction pipeline is pinned +> by `scripts/e2e-opencode.ts`. See [Phase 4 Evidence](#phase-4-evidence) for +> which script proves what. ## Executive Summary @@ -23,7 +26,7 @@ The entire integration is **three thin adapter layers**: │ ┌─────────────┐ ┌──────────────────┐ ┌────────────────────┐ │ │ │ recall.db │ │ MCP Server │ │ CLI (recall) │ │ │ │ (SQLite) │←─│ (recall-memory) │ │ search/add/stats │ │ -│ │ FTS5 + Vec │ │ 7 tools, stdio │ │ import/export │ │ +│ │ FTS5 + Vec │ │ 9 tools, stdio │ │ import/export │ │ │ └──────┬───────┘ └────────┬─────────┘ └────────────────────┘ │ │ │ │ │ │ │ ┌─────────────┴──────────────┐ │ @@ -74,13 +77,19 @@ atlas-recall/ ├── opencode/ # NEW — OpenCode adapter │ ├── RecallExtract.ts # Plugin: session extraction via CLI export │ ├── RecallPreCompact.ts # Plugin: context injection during compaction +│ ├── lib/session-export.ts # Shared helpers — nested so OpenCode does not +│ │ # glob them as plugin factories │ └── recall-memory.md # Agent: memory-aware agent definition ├── docs/ │ ├── FOR_CLAUDE.md # EXISTING — guide for Claude Code agents │ ├── FOR_OPENCODE.md # NEW — guide for OpenCode agents │ └── OPENCODE_INTEGRATION.md # THIS FILE +├── scripts/ +│ ├── e2e-opencode.ts # Pipeline e2e (export → drop → extract → search) +│ ├── e2e-opencode-runtime.ts # Live-server e2e (discovery, idle frequency) +│ └── lib/opencode-runtime.ts # Shared CLI pin, stub provider, event stream ├── install.sh # MODIFIED — detect + register for both platforms -└── package.json # MODIFIED — adds isolated OpenCode e2e script +└── package.json # MODIFIED — adds the isolated OpenCode e2e scripts ``` ## Component Details @@ -125,16 +134,45 @@ Implementation details and regression coverage live in **Why this approach:** - `opencode export` is the **verified current CLI command** — not an assumed API - `ctx.$` Bun shell is **confirmed in plugin docs** with examples -- Persistent dedup via JSON file at `~/.agents/Recall/MEMORY/opencode-sessions/.extracted.json` — survives plugin restarts +- Persistent dedup via JSON file at `~/.agents/Recall/MEMORY/opencode-sessions/.extracted.json` — survives plugin restarts. It maps session ID to a digest of the markdown last written, so a grown session re-exports and an unchanged one does not - The current payload is `event.properties.sessionID`; defensive fallbacks keep older property spellings harmless - Drop directory pattern: `RecallBatchExtract.ts` already runs every 30 minutes and can scan this directory - No new CLI commands needed — `RecallBatchExtract.ts` reads markdown files the same way it reads JSONL -**Runtime observation:** The isolated Phase 4 e2e invokes the current -`session.idle` event contract and verifies the full export path. The adapter -deduplicates by session ID and prevents overlapping exports; add time-based -debouncing only if future runtime evidence shows repeated idle events for one -session. +**Measured runtime contract (OpenCode 1.18.5).** `session.idle` fires **once per +assistant turn**, not once per session. Measured by +`scripts/e2e-opencode-runtime.ts` against a live server: three turns of one +session produced exactly three `session.idle` events (delta 1 per turn), counted +from OpenCode's own `GET /event` stream inside a single long-lived process so +the number cannot be an artifact of process teardown. + +That number drives the adapter's design, so it is asserted, not just recorded — +the runtime e2e fails if the frequency ever changes. + +Two consequences: + +- **Debouncing would be the wrong remedy.** Earlier drafts of this document + hedged toward adding it. Because idle marks the end of each turn rather than + the end of the session, the adapter must RE-export on later idles; suppressing + them freezes the drop on turn 1 and silently discards the rest of the + conversation. That was a real defect: a three-turn session left a 154-byte drop + holding only the first turn. +- **The tracker records a content digest**, not "this session is finished". A + grown session overwrites its earlier, shorter drop; an unchanged one costs no + write. `RecallBatchExtract` already re-extracts a drop that grows, so the + fuller transcript reaches memory without new machinery. + +**Plugin module contract.** OpenCode calls **every export** of a top-level +`plugins/*.ts` as a plugin factory. A plugin entry point must therefore export +nothing but its factory — shared helpers live in `opencode/lib/session-export.ts`, +which OpenCode does not glob. When the helpers sat beside the factory, OpenCode +called `exportSession()` with its plugin context and logged +`failed to load plugin ... "Object is not a function"` on every launch. + +**PATH requirement.** The plugin shells out to `opencode export `, so the +`opencode` CLI must be resolvable by name from the environment OpenCode runs in. +If it is not, the export fails and the drop is never written; the failure is +reported on stderr and retried on the next idle. ### 3. Compaction Context Injection (`opencode/RecallPreCompact.ts`) @@ -339,7 +377,7 @@ OpenCode prefixes MCP tools with the server name + underscore: ### Phase 2: Extraction Plugin - `RecallExtract.ts` plugin using the `opencode export` JSON CLI via `$` shell -- Persistent dedup tracker (`.extracted.json`) +- Persistent dedup tracker (`.extracted.json`, session ID → content digest) - Defensive `session.idle` event property access - Drop directory at `~/.agents/Recall/MEMORY/opencode-sessions/` - `RecallBatchExtract.ts` updated to scan drop directory @@ -351,9 +389,12 @@ OpenCode prefixes MCP tools with the server name + underscore: ### Phase 4: Testing + Polish — complete - Isolated e2e: OpenCode session → JSON export → markdown drop → RecallBatchExtract → search -- Concurrent Claude/OpenCode-style writers against one disposable WAL database -- Installer/update idempotence plus JSONC-preserving OpenCode uninstall rollback -- Runtime verification of the current `session.idle` payload, JSON export, and Bun availability +- Concurrent Claude and OpenCode writers against one disposable WAL database +- Installer/update idempotence, JSONC-preserving OpenCode uninstall, and the + documented `install.sh restore` rollback plus collision-backup preservation +- Live-server verification: plugin discovery from the installed path, zero + plugin load errors, the `session.idle` payload, and the measured one-idle-per + -turn frequency ## Red Team Findings (v1 → v2 Changes) @@ -370,15 +411,46 @@ OpenCode prefixes MCP tools with the server name + underscore: ## Phase 4 Evidence -1. OpenCode 1.18.4 emits `session.idle` through the `event` hook with - `properties.sessionID`; the e2e verifies the plugin factory exposes the real - `event` boundary, rejects the obsolete `session.idle` key, invokes that exact - payload, and verifies the resulting markdown drop. -2. `opencode export ` is the supported JSON export command. Recall - owns JSON-to-markdown normalization because the CLI does not expose the old - `-f markdown -o` session-export interface. -3. The e2e provisions Bun/OpenCode in disposable HOME and XDG config/data/state/ - cache roots, and fails if production DB metadata changes. +Two scripts cover this, and they prove different things. Read the distinction +before citing either as evidence. + +`scripts/e2e-opencode.ts` — **pipeline.** Imports the adapter directly and +supplies its own `$` and event payload. It proves Recall's handler ACCEPTS +`{type:'session.idle', properties:{sessionID}}` and that export -> drop -> +`RecallBatchExtract` -> search works, plus export-failure retry, concurrent WAL +writers, and installer/uninstall JSONC rollback. It does NOT prove OpenCode +emits that payload, and it cannot detect a plugin OpenCode never loads. + +`scripts/e2e-opencode-runtime.ts` — **runtime.** Drives a live `opencode serve` +and closes exactly those gaps: + +1. **Discovery.** Installs via the real `recall_install_opencode_platform`, then + observes OpenCode load the plugin from that path (the factory's drop + directory appears under a `RECALL_HOME` only this server knows) and asserts + zero `failed to load plugin` errors. +2. **Emission and frequency.** Counts `session.idle` from OpenCode's own + `GET /event` stream across three turns of one session in one long-lived + process: 3 events, 1.00 per assistant turn, payload `properties.sessionID`. +3. **Completeness.** Asserts the drop carries every turn, and that + `opencode export ` returns the whole conversation. + +`opencode export ` is the supported JSON export command; Recall owns +JSON-to-markdown normalization because the CLI does not expose the old +`-f markdown -o` session-export interface. + +Both scripts run in disposable HOME / XDG config, data, state, cache / +`RECALL_HOME` / database roots and fail if production DB metadata changes. The +runtime script serves the model from a local OpenAI-compatible stub: the subject +under test is OpenCode's session lifecycle, so a hosted model would only add +flakiness. The pinned version lives once, in +`scripts/lib/opencode-runtime.ts`. + +### Not yet verified at runtime + +`opencode/RecallPreCompact.ts` (compaction context injection) is covered only by +unit tests of its `(input, output)` contract. No test drives a real OpenCode +compaction, so its runtime behaviour is unproven — the README roadmap says so +rather than implying otherwise. ## Non-Goals From 6b018f86a022c69fcdfed37874addb0dabee605a Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:45:16 -0400 Subject: [PATCH 09/17] docs(dox): record the OpenCode and shared-database contracts opencode/ became a durable boundary with rules a future session cannot infer from the code, so it gets a child AGENTS.md: plugin entry points export exactly one factory, helpers nest under lib/ and must be installed and uninstalled with the plugins, and session.idle is per-turn so dedup is by content digest rather than 'session seen'. hooks/ gains the transaction rule the concurrency fix established: a write transaction that reads first must be IMMEDIATE, because SQLite fails the upgrade with SQLITE_BUSY instantly and never consults busy_timeout. tests/ records why OpenCode has two e2e scripts and which claims belong in each, so a host-behaviour claim does not get added to the script that structurally cannot support it. Root index gains opencode/, and scripts/ is named in the owned-at-root list it was missing from. --- AGENTS.md | 3 ++- hooks/AGENTS.md | 1 + opencode/AGENTS.md | 58 ++++++++++++++++++++++++++++++++++++++++++++++ tests/AGENTS.md | 1 + 4 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 opencode/AGENTS.md diff --git a/AGENTS.md b/AGENTS.md index 5403ce8..99cb3ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -176,8 +176,9 @@ Child AGENTS.md files own domain-specific local rules. Read the applicable one b - [`docs/AGENTS.md`](docs/AGENTS.md) — user-facing published docs, ADRs, agent skill docs (never plans/specs) - [`agent-skills/AGENTS.md`](agent-skills/AGENTS.md) — `recall-*` Agent Skill definitions - [`plugins/AGENTS.md`](plugins/AGENTS.md) — per-host native plugin manifests, MCP registration, and generated skill payloads +- [`opencode/AGENTS.md`](opencode/AGENTS.md) — OpenCode adapter plugins, their shared helpers, and the runtime contract they must satisfy -Owned at root (no child doc): lifecycle scripts (`install.sh`, `update.sh`, `uninstall.sh`) + their shared `lib/install-lib.sh` and `lib/jsonc-mcp.ts`; platform guides (`FOR_CLAUDE.md`, `FOR_OPENCODE.md`, `FOR_PI.md`); `CONTEXT.md`; and `assets/` (banner + VHS demo tapes/gifs). +Owned at root (no child doc): lifecycle scripts (`install.sh`, `update.sh`, `uninstall.sh`) + their shared `lib/install-lib.sh` and `lib/jsonc-mcp.ts`; `scripts/` (dev/CI helpers + the per-host isolated e2e harnesses); platform guides (`FOR_CLAUDE.md`, `FOR_OPENCODE.md`, `FOR_PI.md`); `CONTEXT.md`; and `assets/` (banner + VHS demo tapes/gifs). ## Maintaining this file diff --git a/hooks/AGENTS.md b/hooks/AGENTS.md index a0aae3c..a61ac06 100644 --- a/hooks/AGENTS.md +++ b/hooks/AGENTS.md @@ -25,6 +25,7 @@ Installed as per-file symlinks into `~/.claude/hooks/` from `~/.agents/Recall/sh - Generic hook helpers depend on `lib/events.ts`, `lib/extraction-provider.ts`, and the native-provider registry in `lib/hosts/`; native payloads, path encoding, commands, auth, and recursion guards stay in a host adapter. - Documented DRY exception: small utilities (e.g. bun-path resolution) are intentionally duplicated inside `RecallExtract.ts` / `RecallBatchExtract.ts` so they never reach into `src/`. Do not "DRY this up." - DB-path resolution is centralized in `lib/db-path.ts` — the CLI and every hook agree through it. +- **A write transaction that reads first must be IMMEDIATE.** SQLite fails a read-to-write upgrade inside a DEFERRED transaction with `SQLITE_BUSY` *instantly*, never consulting `busy_timeout` — so with two hosts extracting into the shared WAL database, the duplicate probe in `lib/sqlite-writers.ts` turned a peer's short transaction into a lost record. Those batches use `insertMany.immediate(...)`. A transaction whose first statement is a write is fine as-is (`consolidate-core.ts`, `RecallPreCompact.ts`). Regression: `tests/hooks/sqlite-writers-concurrency.test.ts`. - Extraction dual-write is REPLAYED (archive crash or partial SQLite failure both leave the conversation retryable), so `dualWriteToSqlite` passes `skipDuplicates` to the plain-INSERT writers in `lib/sqlite-writers.ts`: a row already present for the same session, keyed on (session_id, content), is skipped. Content-scoped, never session-scoped: in-session windows write many different rows under one session_id. The flag is opt-in: the correction writer must keep recording repeated identical corrections. - Shebang `#!/usr/bin/env bun`. No build step — editing the canonical file updates the live symlink. - Hook registration (`Stop`, `SessionStart`, `PreCompact`, `PostToolUse`, `UserPromptSubmit`) lives in `~/.claude/settings.json`; the installer wires it. diff --git a/opencode/AGENTS.md b/opencode/AGENTS.md new file mode 100644 index 0000000..191ba66 --- /dev/null +++ b/opencode/AGENTS.md @@ -0,0 +1,58 @@ +# opencode — OpenCode host integration + +> Child DOX. Root `AGENTS.md` carries repo-wide rules; this file owns local detail for `opencode/`. + +## Purpose + +The OpenCode adapter: the plugins OpenCode loads, their shared helpers, and the +memory-aware agent definition. + +## Ownership + +- `RecallExtract.ts` — plugin: exports the session on `session.idle` via + `opencode export `, writing a markdown drop that `RecallBatchExtract.ts` consumes +- `RecallPreCompact.ts` — plugin: pushes Recall context into compaction output +- `lib/session-export.ts` — pure helpers shared by the plugins and the tests +- `recall-memory.md` — agent definition + +Installed as per-file symlinks into `$OPENCODE_CONFIG_DIR/plugins/` (and +`plugins/lib/`) from `~/.agents/Recall/opencode/plugins/` by +`recall_install_opencode_plugins` in `lib/install-lib.sh`. + +## Local Contracts + +- **A plugin entry point exports exactly one thing: its factory.** OpenCode + globs top-level `plugins/*.ts` and calls EVERY export of each match as a + plugin factory. A stray helper export makes OpenCode log + `failed to load plugin ... "Object is not a function"` on every launch. + `tests/opencode-integration.test.ts` enforces this. +- **Shared helpers live in `lib/`.** OpenCode does not glob subdirectories, so a + nested module is importable without being mistaken for a plugin. Anything + added here must also be installed (see `recall_install_opencode_plugins`) and + removed on uninstall, or the plugin's import breaks at runtime. +- **`session.idle` fires once per assistant turn**, not once per session + (measured against OpenCode 1.18.5). The adapter must therefore RE-export on + later idles; suppressing them freezes the drop on turn 1 and silently discards + the rest of the conversation. Dedup is by content digest, never "session seen". +- The plugin shells out, so the `opencode` CLI must be resolvable by name on + `PATH` in whatever environment OpenCode runs in. +- Plugins are self-contained like `hooks/` — never import from `src/`. + +## Work Guidance + +- Changing the runtime contract (event shape, export command, discovery path) + means re-running `bun run test:e2e:opencode:runtime`, which verifies it against + a live server, and updating the measured facts in + `docs/OPENCODE_INTEGRATION.md`. +- The pinned OpenCode version lives once, in `scripts/lib/opencode-runtime.ts`. + +## Verification + +- `bun test tests/opencode-integration.test.ts` — unit + installer coverage +- `bun run test:e2e:opencode` — pipeline (export → drop → extract → search) +- `bun run test:e2e:opencode:runtime` — live server (discovery, idle frequency, + multi-turn completeness) + +## Child DOX Index + +No child docs — `lib/` shares these contracts. diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 6b342df..e5cb57f 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -24,6 +24,7 @@ Automated coverage for the CLI, MCP server, hooks, data layer, libraries, instal - `scripts/e2e-codex-plugin.ts` owns the isolated current-CLI verification for the native Codex marketplace, plugin install, and all nine MCP tools. - `scripts/e2e-claude-plugin.ts` does the same for Claude, and additionally seeds a legacy lifecycle install to prove the migration removes the duplicate skill symlinks and MCP registration idempotently. It must isolate the Claude home too (`HOME` + `CLAUDE_DIR`), not just the database, and assert both were left unchanged. - `scripts/e2e-pi-integration.ts` owns the isolated current-CLI verification for Pi's separate package, MCP adapter/config, lifecycle capture, and all nine skills/tools. +- OpenCode is split across two scripts on purpose. `scripts/e2e-opencode.ts` owns the pipeline (export → drop → `RecallBatchExtract` → search, plus retry, concurrent writers, installer/uninstall JSONC); it imports the adapter and supplies its own `$` and event payload, so it can never prove what the host actually does. `scripts/e2e-opencode-runtime.ts` owns the live-server contract — plugin discovery from the installed path, zero plugin load errors, OpenCode's own `session.idle` emission and frequency, and multi-turn drop completeness. Put host-behaviour claims in the runtime script; a claim the pipeline script cannot support does not belong in it. ## Verification From fb541de08bb1d1b80237e0aef7a83c7bc276e4eb Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:45:45 -0400 Subject: [PATCH 10/17] docs(handoff): record the Phase 4 outcome, three defects, and remaining gaps --- ...25-opencode-phase4-runtime-verification.md | 64 ++++++++++++++----- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md index d0c2e43..f578185 100644 --- a/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md +++ b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md @@ -1,6 +1,6 @@ # Recall OpenCode Phase 4 — Runtime Verification Handoff -Status: In progress +Status: Complete — branch `fm/recall-opencode-phase4-o1` ready for PR Phase: OpenCode integration — Phase 4 acceptance verification (real runtime) @@ -57,20 +57,54 @@ the measurement exposed, #243 characterisation, and reconciling Out of scope (unchanged from #248): installation reconciliation, the semantic #240/#241/#226 wave, release/version bump, and #236/#237/#238/#174. -## Plan - -1. Handoff (this file). -2. Fix the tracker so later idles re-export; keep retry-on-failure and - `.extracted.json`. -3. Unit regression at the plugin boundary: repeat idle with new content re-exports. -4. Committed real-runtime harness: plugin discovery + measured idle frequency + - multi-turn drop completeness. -5. Concurrent **Claude + OpenCode** writers (the existing e2e used two OpenCode - writers). -6. Installer backup/restore round-trip (existing coverage is surgical removal, not - restore). -7. Docs: correct the overclaims, record the measured number, reconcile README. -8. #243: verify precisely, close or characterise. +## Phase 4 acceptance — outcome + +| Item | Outcome | +|---|---| +| E2E session → export → drop → extract → search | Passes. `bun run test:e2e:opencode` green against OpenCode 1.18.5. | +| Concurrent Claude + OpenCode against one store | **Was broken.** Found, fixed, regression-tested (see below). | +| Installer rollback and restore | `install.sh restore` had zero coverage; now covered, including the collision backup and the non-interactive no-op. | +| `session.idle` frequency, measured | **1.00 per assistant turn** (3 turns → 3 events, OpenCode 1.18.5). Asserted, not just recorded. | + +## Defects found and fixed + +1. **Multi-turn data loss** (`opencode/RecallExtract.ts`). Permanent tracker + + per-turn idle meant only turn 1 was ever dropped. Reproduced against a real + three-turn session: 154-byte drop, 2 of 3 markers lost. +2. **Plugin load error on every launch.** OpenCode calls every export of a + top-level `plugins/*.ts` as a plugin factory; the exported test helpers made + it log `failed to load plugin ... "Object is not a function"`. Helpers moved + to `opencode/lib/session-export.ts`. +3. **Concurrent extraction lost records** (`hooks/lib/sqlite-writers.ts`). The + duplicate probe made these DEFERRED transactions read-then-write, and SQLite + fails that upgrade with `SQLITE_BUSY` instantly, never consulting + `busy_timeout`. Measured 1ms hard failure vs the full timeout honoured under + `IMMEDIATE`. + +Defects 1 and 2 were invisible to the existing e2e by construction: it imports +the adapter and supplies its own `$` and event payload. + +## #243 — closed + +All three acceptance items met: + +1. Fresh worktree matches a normal checkout — verified in a genuinely fresh + worktree: 1298 pass, 0 fail. Requires `bun install` inside it, which is #165's + gap, so #165 stays open and the CI job asserts the empty-`node_modules` + starting condition rather than hiding it. +2. `recall_configure_opencode_mcp` and `recall_configure_pi_mcp` both exit 1 on a + malformed config and leave it byte-identical. +3. CI now runs lint plus the full suite inside a git worktree. + +## Known gaps, deliberately not closed + +- `opencode/RecallPreCompact.ts` compaction injection is unit-tested only; no + test drives a real OpenCode compaction. Stated in the README row and in + `docs/OPENCODE_INTEGRATION.md`. +- One full-suite run failed once and was not reproduced in five subsequent runs; + the output was truncated so the test was not identified. Not diagnosed. +- Out of scope and untouched: installation reconciliation, the semantic + #240/#241/#226 wave, release/version bump, #236/#237/#238/#174, #165. ## Holds From 2342e513cc21890d2846ce1ca02f1a142549136b Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:50:20 -0400 Subject: [PATCH 11/17] ci: gate the OpenCode runtime contract, and cover uninstall of plugins/lib MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runtime e2e was not wired into CI, so nothing on a PR actually enforced the measured once-per-turn session.idle frequency that CHANGELOG and the design doc both claim is asserted. It runs on the macOS leg now, making that claim true. Verified the gate is not vacuous on the path CI takes: with OPENCODE_BIN unset (so the run goes through bunx, adding a process layer between the server and the captured log) a deliberately reintroduced second plugin export still produced 'OpenCode reported 1 plugin load error(s)' and failed the run. The pipeline e2e now also proves uninstall removes Recall's plugin and its nested helper while leaving a user-owned file in plugins/lib untouched — the directory Recall creates is only removed when Recall emptied it. --- .github/workflows/ci.yml | 8 ++++++++ scripts/e2e-opencode.ts | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5587371..0e91406 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,6 +43,14 @@ jobs: - name: OpenCode integration e2e run: bun run test:e2e:opencode + # The pipeline e2e above supplies its own `$` and event payload, so it + # cannot prove OpenCode loads the plugin or emits session.idle. This leg + # drives a live server and asserts the measured once-per-turn frequency, + # so a runtime change fails CI instead of silently invalidating the + # contract recorded in docs/OPENCODE_INTEGRATION.md. + - name: OpenCode runtime contract e2e + run: bun run test:e2e:opencode:runtime + worktree: name: Worktree baseline runs-on: macos-latest diff --git a/scripts/e2e-opencode.ts b/scripts/e2e-opencode.ts index bfbc074..a87ca3e 100644 --- a/scripts/e2e-opencode.ts +++ b/scripts/e2e-opencode.ts @@ -299,6 +299,15 @@ async function main(): Promise { assert(search.includes('Found ') && !search.includes('No results found.'), `Recall search could not find ${query}\n${search}\nLoA evidence: ${loaEvidence()}`); } + // The shared plugin helper lives in a subdirectory Recall created, so + // uninstall has to remove its own file without taking a user's file — or the + // directory — with it. + const pluginsDir = join(opencodeConfigDir, 'plugins'); + const installedHelper = join(pluginsDir, 'lib', 'session-export.ts'); + assert(existsSync(installedHelper), `installer did not place the plugin helper at ${installedHelper}`); + const userHelper = join(pluginsDir, 'lib', 'user-owned.ts'); + writeFileSync(userHelper, 'export const UserOwned = 1\n'); + const uninstall = spawnSync('bash', [join(repoRoot, 'uninstall.sh'), '--no-confirm', '--skip-pi', '--skip-omp'], { cwd: repoRoot, env: { ...env, CLAUDE_DIR: join(testHome, '.claude'), BACKUP_BASE: join(tempRoot, 'backups'), RECALL_SKIP_BUN_UNLINK: 'true' }, @@ -310,6 +319,11 @@ async function main(): Promise { assert(afterUninstall.includes('// User-owned OpenCode settings remain intact.'), 'uninstall removed user JSONC comments'); assert(afterUninstall.includes('other-server') && !afterUninstall.includes('recall-memory'), 'uninstall did not surgically remove Recall MCP'); + assert(!existsSync(join(pluginsDir, 'RecallExtract.ts')), 'uninstall left the OpenCode plugin behind'); + assert(!existsSync(installedHelper), 'uninstall left the OpenCode plugin helper behind'); + assert(existsSync(userHelper), 'uninstall removed a user-owned file from plugins/lib'); + console.log('opencode.uninstall_preserves_user_plugin_lib=verified'); + assertMetadataUnchanged(productionDb, productionBefore); console.log(`opencode.version=${version}`); console.log(`opencode.search_markers=${[marker, `${marker}_RETRY`, ...concurrentMarkers].join(',')}`); From fb166a8849b0e6c1bedba13d55365e802acf813a Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 11:50:32 -0400 Subject: [PATCH 12/17] docs(handoff): narrow the unreproduced flake to outside this branch --- .../2026-07-25-opencode-phase4-runtime-verification.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md index f578185..4b5842e 100644 --- a/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md +++ b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md @@ -101,8 +101,11 @@ All three acceptance items met: - `opencode/RecallPreCompact.ts` compaction injection is unit-tested only; no test drives a real OpenCode compaction. Stated in the README row and in `docs/OPENCODE_INTEGRATION.md`. -- One full-suite run failed once and was not reproduced in five subsequent runs; - the output was truncated so the test was not identified. Not diagnosed. +- One full-suite run reported a single failure once. It did not reproduce in + five subsequent full-suite runs, and the output had been truncated so the test + was never named. It is **not in the files this branch touches**: 15 targeted + runs of `sqlite-writers-concurrency`, `restore`, and `opencode-integration` + were clean. Pre-existing and undiagnosed; worth a separate issue if it recurs. - Out of scope and untouched: installation reconciliation, the semantic #240/#241/#226 wave, release/version bump, #236/#237/#238/#174, #165. From 6f59fdd9decd2de62c0184a35e64ae5318add5c9 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:29:32 -0400 Subject: [PATCH 13/17] no-mistakes(review): atomic drops, real concurrency test, ephemeral e2e ports --- ...25-opencode-phase4-runtime-verification.md | 11 ++- CHANGELOG.md | 2 +- docs/OPENCODE_INTEGRATION.md | 8 +- hooks/AGENTS.md | 2 +- hooks/lib/sqlite-writers.ts | 37 ++++---- lib/install-lib.sh | 9 +- opencode/AGENTS.md | 6 +- opencode/RecallExtract.ts | 21 ++++- scripts/e2e-opencode-runtime.ts | 36 +++++-- scripts/lib/opencode-runtime.ts | 22 ++++- tests/AGENTS.md | 2 + .../hooks/sqlite-writers-concurrency.test.ts | 94 ++++++++++++++----- uninstall.sh | 4 +- 13 files changed, 181 insertions(+), 73 deletions(-) diff --git a/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md index 4b5842e..2088dec 100644 --- a/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md +++ b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md @@ -103,9 +103,14 @@ All three acceptance items met: `docs/OPENCODE_INTEGRATION.md`. - One full-suite run reported a single failure once. It did not reproduce in five subsequent full-suite runs, and the output had been truncated so the test - was never named. It is **not in the files this branch touches**: 15 targeted - runs of `sqlite-writers-concurrency`, `restore`, and `opencode-integration` - were clean. Pre-existing and undiagnosed; worth a separate issue if it recurs. + was never named. Targeted reruns of `sqlite-writers-concurrency`, `restore`, + and `opencode-integration` were clean, but targeted runs cannot clear a + timing-sensitive failure on a loaded runner, so they are **not** evidence that + the flake lives outside this branch — the branch's own files remain suspects. + What was removed instead is the one assertion that could have produced it: the + concurrency test no longer asserts a wall-clock floor, only a happens-before + against a stamp the peer prints while it still holds the lock. Undiagnosed; + worth a separate issue if it recurs. - Out of scope and untouched: installation reconciliation, the semantic #240/#241/#226 wave, release/version bump, #236/#237/#238/#174, #165. diff --git a/CHANGELOG.md b/CHANGELOG.md index e976dce..35987e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ note in the 0.9.0 entry. ### Added - **OpenCode Phase 4 validation** — an isolated end-to-end harness provisions - OpenCode 1.18.4, verifies the current `session.idle` event and JSON export + OpenCode 1.18.5, verifies the current `session.idle` event and JSON export contract, exercises export retry, concurrent WAL writers, installer/update idempotence, JSONC-preserving uninstall, and searchable Recall retrieval. - **OpenCode live-server verification** (`bun run test:e2e:opencode:runtime`) — diff --git a/docs/OPENCODE_INTEGRATION.md b/docs/OPENCODE_INTEGRATION.md index 48b544b..640f597 100644 --- a/docs/OPENCODE_INTEGRATION.md +++ b/docs/OPENCODE_INTEGRATION.md @@ -160,7 +160,13 @@ Two consequences: - **The tracker records a content digest**, not "this session is finished". A grown session overwrites its earlier, shorter drop; an unchanged one costs no write. `RecallBatchExtract` already re-extracts a drop that grows, so the - fuller transcript reaches memory without new machinery. + fuller transcript reaches memory without new machinery — but only once it + grows past `GROWTH_THRESHOLD = 0.5` (`hooks/RecallBatchExtract.ts`): a drop + extracted at size S is not re-extracted until it exceeds 1.5 × S, so the tail + of a long session can sit in the drop file without reaching the database. The + drop itself does hold every turn; it is the drop-to-SQLite step that is + threshold-gated. Tracked in + [#250](https://github.com/edheltzel/Recall/issues/250). **Plugin module contract.** OpenCode calls **every export** of a top-level `plugins/*.ts` as a plugin factory. A plugin entry point must therefore export diff --git a/hooks/AGENTS.md b/hooks/AGENTS.md index a61ac06..2800fb9 100644 --- a/hooks/AGENTS.md +++ b/hooks/AGENTS.md @@ -25,7 +25,7 @@ Installed as per-file symlinks into `~/.claude/hooks/` from `~/.agents/Recall/sh - Generic hook helpers depend on `lib/events.ts`, `lib/extraction-provider.ts`, and the native-provider registry in `lib/hosts/`; native payloads, path encoding, commands, auth, and recursion guards stay in a host adapter. - Documented DRY exception: small utilities (e.g. bun-path resolution) are intentionally duplicated inside `RecallExtract.ts` / `RecallBatchExtract.ts` so they never reach into `src/`. Do not "DRY this up." - DB-path resolution is centralized in `lib/db-path.ts` — the CLI and every hook agree through it. -- **A write transaction that reads first must be IMMEDIATE.** SQLite fails a read-to-write upgrade inside a DEFERRED transaction with `SQLITE_BUSY` *instantly*, never consulting `busy_timeout` — so with two hosts extracting into the shared WAL database, the duplicate probe in `lib/sqlite-writers.ts` turned a peer's short transaction into a lost record. Those batches use `insertMany.immediate(...)`. A transaction whose first statement is a write is fine as-is (`consolidate-core.ts`, `RecallPreCompact.ts`). Regression: `tests/hooks/sqlite-writers-concurrency.test.ts`. +- **An EXPLICIT transaction that reads before it writes must be IMMEDIATE.** SQLite fails a read-to-write upgrade inside a DEFERRED transaction with `SQLITE_BUSY` *instantly*, never consulting `busy_timeout` — so with two hosts extracting into the shared WAL database, the duplicate probe in `lib/sqlite-writers.ts` turned a peer's short transaction into a lost record. Those batches use `insertMany.immediate(...)`. Two shapes need nothing: a transaction whose first statement is a write (`consolidate-core.ts`, `RecallPreCompact.ts`), and a read-then-write pair with NO explicit transaction around it — `writeLoaEntryFromExtraction`'s probe autocommits and releases its read lock before the INSERT takes the write lock with `busy_timeout` honoured. The rule buys lock-wait behavior, NOT atomicity: two concurrent replays of the same extraction can still both pass that LoA probe and insert. Regression: `tests/hooks/sqlite-writers-concurrency.test.ts`. - Extraction dual-write is REPLAYED (archive crash or partial SQLite failure both leave the conversation retryable), so `dualWriteToSqlite` passes `skipDuplicates` to the plain-INSERT writers in `lib/sqlite-writers.ts`: a row already present for the same session, keyed on (session_id, content), is skipped. Content-scoped, never session-scoped: in-session windows write many different rows under one session_id. The flag is opt-in: the correction writer must keep recording repeated identical corrections. - Shebang `#!/usr/bin/env bun`. No build step — editing the canonical file updates the live symlink. - Hook registration (`Stop`, `SessionStart`, `PreCompact`, `PostToolUse`, `UserPromptSubmit`) lives in `~/.claude/settings.json`; the installer wires it. diff --git a/hooks/lib/sqlite-writers.ts b/hooks/lib/sqlite-writers.ts index ac5dda3..53cf5ab 100644 --- a/hooks/lib/sqlite-writers.ts +++ b/hooks/lib/sqlite-writers.ts @@ -39,6 +39,15 @@ export interface WriteOptions { skipDuplicates?: boolean; } +// TRANSACTION MODE: every explicit transaction in this file commits through +// `.immediate(...)`, never the default DEFERRED. A DEFERRED transaction that +// reads before it writes fails the read-to-write upgrade with SQLITE_BUSY +// *instantly*, never consulting `busy_timeout` — so with two hosts extracting +// into the shared WAL database, the `skipDuplicates` probe turned a peer's +// short transaction into a lost record. Taking the write lock up front is what +// makes the peer wait honoured: measured 1ms hard failure deferred, versus the +// full timeout being honoured immediate. +// Regression: tests/hooks/sqlite-writers-concurrency.test.ts. function openDb(dbPath: string): Database { const db = new Database(dbPath); db.exec('PRAGMA journal_mode = WAL'); @@ -183,11 +192,8 @@ export function writeDecisionsBatch( } return n; }); - // IMMEDIATE, not the default DEFERRED: these batches read (the - // skipDuplicates probe) before they insert, and SQLite fails a - // read-to-write upgrade with SQLITE_BUSY *instantly*, ignoring - // busy_timeout. Taking the write lock up front is what makes the peer wait - // honoured — measured 1ms hard failure deferred vs ~500ms of waiting here. + // Reads (the skipDuplicates probe) before it inserts — see the transaction + // mode note above openDb. return insertMany.immediate(items); } finally { db.close(); @@ -265,11 +271,8 @@ export function writeLearningsBatch( } return n; }); - // IMMEDIATE, not the default DEFERRED: these batches read (the - // skipDuplicates probe) before they insert, and SQLite fails a - // read-to-write upgrade with SQLITE_BUSY *instantly*, ignoring - // busy_timeout. Taking the write lock up front is what makes the peer wait - // honoured — measured 1ms hard failure deferred vs ~500ms of waiting here. + // Reads (the skipDuplicates probe) before it inserts — see the transaction + // mode note above openDb. return insertMany.immediate(items); } finally { db.close(); @@ -322,11 +325,8 @@ export function writeBreadcrumbsBatch( } return n; }); - // IMMEDIATE, not the default DEFERRED: these batches read (the - // skipDuplicates probe) before they insert, and SQLite fails a - // read-to-write upgrade with SQLITE_BUSY *instantly*, ignoring - // busy_timeout. Taking the write lock up front is what makes the peer wait - // honoured — measured 1ms hard failure deferred vs ~500ms of waiting here. + // Reads (the skipDuplicates probe) before it inserts — see the transaction + // mode note above openDb. return insertMany.immediate(items); } finally { db.close(); @@ -431,11 +431,8 @@ export function writeExtractionErrors(dbPath: string, items: ExtractionErrorInpu } return n; }); - // IMMEDIATE, not the default DEFERRED: these batches read (the - // skipDuplicates probe) before they insert, and SQLite fails a - // read-to-write upgrade with SQLITE_BUSY *instantly*, ignoring - // busy_timeout. Taking the write lock up front is what makes the peer wait - // honoured — measured 1ms hard failure deferred vs ~500ms of waiting here. + // No probe here — this upsert opens with a write, so busy_timeout already + // applied. Immediate anyway, per the transaction mode note above openDb. return insertMany.immediate(items); } finally { db.close(); diff --git a/lib/install-lib.sh b/lib/install-lib.sh index 1f95d5b..7cf6640 100644 --- a/lib/install-lib.sh +++ b/lib/install-lib.sh @@ -46,6 +46,11 @@ fi : "${RECALL_CLAUDE_COMMANDS_DIR:=$RECALL_CLAUDE_ROOT/commands/Recall}" : "${RECALL_OPENCODE_ROOT:=$RECALL_DIR/opencode}" : "${RECALL_OPENCODE_PLUGINS_DIR:=$RECALL_OPENCODE_ROOT/plugins}" +# Single source of truth for what Recall owns under $OPENCODE_CONFIG_DIR/plugins. +# uninstall.sh sources this file, so both the install and the uninstall loops +# read the same names — adding a plugin or helper here is the only edit needed. +RECALL_OPENCODE_PLUGINS=(RecallExtract.ts RecallPreCompact.ts) +RECALL_OPENCODE_PLUGIN_HELPERS=(session-export.ts) : "${RECALL_PI_ROOT:=$RECALL_DIR/pi}" : "${RECALL_MEMORY_DIR:=$RECALL_DIR/MEMORY}" @@ -2287,7 +2292,7 @@ recall_install_opencode_plugins() { mkdir -p "$plugin_dir" local plugin - for plugin in RecallExtract.ts RecallPreCompact.ts; do + for plugin in "${RECALL_OPENCODE_PLUGINS[@]}"; do if [[ -f "$src_dir/$plugin" ]]; then recall_copy_canonical "$src_dir/$plugin" "$RECALL_OPENCODE_PLUGINS_DIR/$plugin" recall_link "$plugin_dir/$plugin" "$RECALL_OPENCODE_PLUGINS_DIR/$plugin" @@ -2301,7 +2306,7 @@ recall_install_opencode_plugins() { # would make OpenCode log a plugin-load error on every launch. mkdir -p "$plugin_dir/lib" local helper - for helper in session-export.ts; do + for helper in "${RECALL_OPENCODE_PLUGIN_HELPERS[@]}"; do if [[ -f "$src_dir/lib/$helper" ]]; then recall_copy_canonical "$src_dir/lib/$helper" "$RECALL_OPENCODE_PLUGINS_DIR/lib/$helper" recall_link "$plugin_dir/lib/$helper" "$RECALL_OPENCODE_PLUGINS_DIR/lib/$helper" diff --git a/opencode/AGENTS.md b/opencode/AGENTS.md index 191ba66..2cbf4d3 100644 --- a/opencode/AGENTS.md +++ b/opencode/AGENTS.md @@ -28,8 +28,10 @@ Installed as per-file symlinks into `$OPENCODE_CONFIG_DIR/plugins/` (and `tests/opencode-integration.test.ts` enforces this. - **Shared helpers live in `lib/`.** OpenCode does not glob subdirectories, so a nested module is importable without being mistaken for a plugin. Anything - added here must also be installed (see `recall_install_opencode_plugins`) and - removed on uninstall, or the plugin's import breaks at runtime. + added here must be named in `RECALL_OPENCODE_PLUGIN_HELPERS` (plugins go in + `RECALL_OPENCODE_PLUGINS`) in `lib/install-lib.sh`, or the plugin's import + breaks at runtime. Those two arrays are the single source of truth: install + and uninstall both loop over them, so one edit covers both. - **`session.idle` fires once per assistant turn**, not once per session (measured against OpenCode 1.18.5). The adapter must therefore RE-export on later idles; suppressing them freezes the drop on turn 1 and silently discards diff --git a/opencode/RecallExtract.ts b/opencode/RecallExtract.ts index b735cf0..0d5a60f 100644 --- a/opencode/RecallExtract.ts +++ b/opencode/RecallExtract.ts @@ -21,7 +21,7 @@ // - opencode export — current JSON export command; Recall normalizes it import type { Plugin } from "@opencode-ai/plugin" -import { existsSync, readFileSync, writeFileSync, mkdirSync } from "fs" +import { existsSync, readFileSync, writeFileSync, renameSync, mkdirSync } from "fs" import { createHash } from "crypto" import { join } from "path" import { homedir } from "os" @@ -62,9 +62,24 @@ function loadTracker(): Map { return new Map() } +/** + * Write via a temp file in the same directory and rename onto the target. + * + * `RecallBatchExtract` reads this directory from a SEPARATE process on a cron, + * and idle fires every turn, so a truncate-then-write would expose a torn read + * for the whole life of every session. The temp suffix is deliberately not + * `.md`: `findMarkdownSessions` only picks up `*.md`, so a partial file is + * never a candidate, and a leftover one after a crash is inert. + */ +function writeAtomic(path: string, contents: string): void { + const temp = `${path}.tmp` + writeFileSync(temp, contents) + renameSync(temp, path) +} + /** Save dedup tracker to disk */ function saveTracker(tracker: Map): void { - writeFileSync(TRACKER_PATH, JSON.stringify(Object.fromEntries(tracker)) + "\n") + writeAtomic(TRACKER_PATH, JSON.stringify(Object.fromEntries(tracker)) + "\n") } /** Digest used to tell "same transcript again" from "the session grew". */ @@ -92,7 +107,7 @@ export const RecallExtract: Plugin = async ({ $ }) => { const signature = digest(markdown) if (tracker.get(sessionId) === signature) return const outFile = join(DROP_DIR, `${sessionId}.md`) - writeFileSync(outFile, markdown) + writeAtomic(outFile, markdown) tracker.set(sessionId, signature) saveTracker(tracker) } catch (error) { diff --git a/scripts/e2e-opencode-runtime.ts b/scripts/e2e-opencode-runtime.ts index 7c4ee47..c0ab586 100644 --- a/scripts/e2e-opencode-runtime.ts +++ b/scripts/e2e-opencode-runtime.ts @@ -34,6 +34,7 @@ import { assertMetadataUnchanged, assertSafeTestDb, metadata, stringEnv } from ' import { OPENCODE_PINNED_VERSION, collectEvents, + reserveEphemeralPort, runOpenCode, startStubProvider, stubProviderConfig, @@ -54,8 +55,6 @@ const project = join(tempRoot, 'project'); const testBin = join(tempRoot, 'bin'); const opencodeConfigDir = join(xdgConfig, 'opencode'); -const STUB_PORT = 47611; -const SERVER_PORT = 47612; const TURN_MARKERS = ['RUNTIMETURNALPHA', 'RUNTIMETURNBRAVO', 'RUNTIMETURNCHARLIE']; /** Generous: an idle can trail the POST response slightly. */ const IDLE_SETTLE_MS = 6_000; @@ -147,22 +146,27 @@ async function main(): Promise { console.log(`opencode.installed_helper=${installedHelper}`); // Point OpenCode's model at the local stub, preserving the installer's MCP block. - stub = startStubProvider(STUB_PORT); + stub = startStubProvider(); + console.log(`stub.port=${stub.port}`); const configPath = join(opencodeConfigDir, 'opencode.json'); const installedConfig = JSON.parse(readFileSync(configPath, 'utf-8')) as Record; writeFileSync(configPath, JSON.stringify({ ...installedConfig, ...stubProviderConfig(stub.baseURL) }, null, 2)); - const base = `http://127.0.0.1:${SERVER_PORT}`; + const serverPort = reserveEphemeralPort(); + console.log(`opencode.serve_port=${serverPort}`); + const base = `http://127.0.0.1:${serverPort}`; let serverLog = ''; // --print-logs is required for plugin load failures to reach stderr, which is // how this script proves the plugins loaded cleanly rather than merely ran. - const serveArgs = ['serve', '--port', String(SERVER_PORT), '--hostname', '127.0.0.1', '--print-logs', '--log-level', 'DEBUG']; + const serveArgs = ['serve', '--port', String(serverPort), '--hostname', '127.0.0.1', '--print-logs', '--log-level', 'DEBUG']; + // Detached: `bunx` is only a wrapper, so the server it launches is reachable + // for teardown only as a process group. server = spawn( process.env.OPENCODE_BIN ?? 'bunx', process.env.OPENCODE_BIN ? serveArgs : ['--yes', '--package', `opencode-ai@${OPENCODE_PINNED_VERSION}`, 'opencode', ...serveArgs], - { cwd: project, env, stdio: ['ignore', 'pipe', 'pipe'] }, + { cwd: project, env, stdio: ['ignore', 'pipe', 'pipe'], detached: true }, ); server.stdout?.on('data', chunk => { serverLog += chunk; }); server.stderr?.on('data', chunk => { serverLog += chunk; }); @@ -276,12 +280,24 @@ async function main(): Promise { console.log(`opencode.runtime_contract=verified against ${version}`); } -try { - await main(); -} finally { +function teardown(): void { stream?.stop(); - server?.kill('SIGKILL'); + if (server?.pid !== undefined) { + // Negative pid targets the process group; ESRCH means it already exited. + try { process.kill(-server.pid, 'SIGKILL'); } catch { /* already gone */ } + } stub?.stop(); if (process.env.RECALL_E2E_KEEP === '1') console.error(`e2e.temp_root=${tempRoot}`); else rmSync(tempRoot, { recursive: true, force: true }); } + +// A detached child has no controlling terminal, so Ctrl-C reaches only this +// script — the group has to be reaped explicitly on signal, not just on return. +process.on('SIGINT', () => { teardown(); process.exit(130); }); +process.on('SIGTERM', () => { teardown(); process.exit(143); }); + +try { + await main(); +} finally { + teardown(); +} diff --git a/scripts/lib/opencode-runtime.ts b/scripts/lib/opencode-runtime.ts index 9447481..d304553 100644 --- a/scripts/lib/opencode-runtime.ts +++ b/scripts/lib/opencode-runtime.ts @@ -66,10 +66,12 @@ export function writeOpenCodeShim(binDir: string): void { * lifecycle and event emission, not a model's output, so the reply only has to * be well-formed and deterministic. Echoing the prompt lets a test assert that * a specific turn reached the transcript. + * + * Binds an ephemeral port and reports the one the OS assigned. */ -export function startStubProvider(port: number): { stop: () => void; baseURL: string } { +export function startStubProvider(): { stop: () => void; baseURL: string; port: number } { const server = Bun.serve({ - port, + port: 0, async fetch(req) { const url = new URL(req.url); if (url.pathname.endsWith('/models')) { @@ -114,7 +116,21 @@ export function startStubProvider(port: number): { stop: () => void; baseURL: st return new Response(stream, { headers: { 'content-type': 'text/event-stream', 'cache-control': 'no-cache' } }); }, }); - return { stop: () => server.stop(true), baseURL: `http://127.0.0.1:${port}/v1` }; + return { stop: () => server.stop(true), baseURL: `http://127.0.0.1:${server.port}/v1`, port: server.port }; +} + +/** + * Reserve a port the OS says is free, for a process that cannot report back. + * + * `opencode serve --port 0` gives no parseable way to learn what it picked, so + * the harness has to pick for it. Binding and immediately releasing leaves a + * small race, which still beats a hardcoded port that fails opaquely. + */ +export function reserveEphemeralPort(): number { + const probe = Bun.serve({ port: 0, fetch: () => new Response('probe') }); + const { port } = probe; + probe.stop(true); + return port; } /** The OpenCode config block that points every turn at the local stub provider. */ diff --git a/tests/AGENTS.md b/tests/AGENTS.md index e5cb57f..60c8a01 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -17,6 +17,8 @@ Automated coverage for the CLI, MCP server, hooks, data layer, libraries, instal - Reuse `fixtures/` rather than inlining large fixtures; use `helpers/setup.ts` for harness setup. - `install/` tests exercise `install.sh` / `update.sh` / `uninstall.sh` — keep them current when those scripts change. - Any end-to-end test that can open SQLite must set its own disposable `RECALL_DB_PATH` and `RECALL_HOME`, state those paths before writes, and prove the production database was not changed. +- **A test that claims to exercise concurrency must contend across PROCESSES.** The SQLite writers are synchronous, so `Promise.all` over two of them just runs them back to back and passes on code that loses records. Model contention on `peerHoldingWriteLock` in `hooks/sqlite-writers-concurrency.test.ts`, and prove the new test fails against the unfixed code before trusting it. +- Assert ordering, not elapsed wall-clock. A `toBeGreaterThan()` floor is a flake on a loaded runner; have the peer stamp a timestamp while it still holds the resource and assert a happens-before against that. ## Work Guidance diff --git a/tests/hooks/sqlite-writers-concurrency.test.ts b/tests/hooks/sqlite-writers-concurrency.test.ts index afed166..57139e7 100644 --- a/tests/hooks/sqlite-writers-concurrency.test.ts +++ b/tests/hooks/sqlite-writers-concurrency.test.ts @@ -16,16 +16,27 @@ import { writeBreadcrumbsBatch, writeDecisionsBatch, writeLearningsBatch } from * instead of being waited out. Measured: ~1ms hard failure when deferred, * versus honouring the full timeout when the write lock is taken up front. * - * These tests hold the write lock from a separate PROCESS (the writers are - * synchronous, so an in-process timer could never release it) and assert the - * write waits and lands. + * Every contending writer here lives in a separate PROCESS. The writers are + * synchronous, so two in-process calls can never overlap — `Promise.all` over + * them just runs them back to back and would pass on the broken code. */ +const WRITERS_MODULE = join(import.meta.dir, '..', '..', 'hooks', 'lib', 'sqlite-writers.ts'); + describe('shared-database extraction writers (concurrent hosts)', () => { let tempDir: string; let dbPath: string; - /** Hold the write lock in a peer process, releasing it after `holdMs`. */ - async function peerHoldingWriteLock(holdMs: number): Promise<{ done: Promise }> { + /** + * Hold the write lock in a peer process, releasing it after `holdMs`. + * + * The peer stamps `RELEASING ` on the line BEFORE its COMMIT, so a + * caller can assert a happens-before ("my write finished after the lock was + * still held") without a wall-clock floor that a descheduled runner breaks. + */ + async function peerHoldingWriteLock(holdMs: number): Promise<{ + done: Promise; + releasingAt: Promise; + }> { const script = join(tempDir, 'peer.ts'); writeFileSync(script, ` import { Database } from 'bun:sqlite'; @@ -36,6 +47,7 @@ db.exec('BEGIN IMMEDIATE'); db.prepare("INSERT INTO breadcrumbs (session_id, project, content, importance) VALUES ('peer','peer','peer holds the write lock',5)").run(); console.log('LOCKED'); await Bun.sleep(${holdMs}); +console.log('RELEASING ' + Date.now()); db.exec('COMMIT'); db.close(); `); @@ -50,7 +62,22 @@ db.close(); if (done) throw new Error(`peer never took the lock: ${seen}${await new Response(child.stderr).text()}`); seen += decoder.decode(value, { stream: true }); } - return { done: child.exited.then(() => undefined) }; + + const drained = (async () => { + while (true) { + const { done, value } = await reader.read(); + if (done) return; + seen += decoder.decode(value, { stream: true }); + } + })(); + + return { + done: child.exited.then(() => undefined), + releasingAt: drained.then(() => { + const stamp = seen.match(/RELEASING (\d+)/); + return stamp ? Number(stamp[1]) : null; + }), + }; } beforeEach(() => { @@ -69,18 +96,21 @@ db.close(); test('a decision batch waits out a peer write lock instead of losing the record', async () => { const peer = await peerHoldingWriteLock(600); - const started = Date.now(); const written = writeDecisionsBatch( dbPath, [{ sessionId: 'opencode-session', project: 'opencode', decision: 'Recall waits for a peer writer', importance: 6 }], { skipDuplicates: true }, ); - const waited = Date.now() - started; + const finished = Date.now(); await peer.done; expect(written).toBe(1); - // Proves the wait actually happened rather than the write racing in first. - expect(waited).toBeGreaterThan(200); + // Proves the wait actually happened rather than the write racing in first: + // the lock was still held when the peer stamped, so a write that completed + // after that stamp can only have waited for it. + const releasingAt = await peer.releasingAt; + expect(releasingAt).not.toBeNull(); + expect(finished).toBeGreaterThanOrEqual(releasingAt!); const db = new Database(dbPath, { readonly: true }); const rows = db.query('SELECT decision FROM decisions').all() as { decision: string }[]; @@ -108,22 +138,36 @@ db.close(); }); test('both hosts writing at once keep every record and their own source', async () => { - // No peer lock here — this is the plain interleaved case: two hosts writing - // the same tables must not drop or merge each other's rows. - const results = await Promise.all([ - Promise.resolve().then(() => writeDecisionsBatch( - dbPath, - [{ sessionId: 'claude-session', project: 'claude', decision: 'CLAUDE_SOURCED_DECISION', importance: 5 }], - { skipDuplicates: true }, - )), - Promise.resolve().then(() => writeDecisionsBatch( - dbPath, - [{ sessionId: 'opencode-session', project: 'opencode', decision: 'OPENCODE_SOURCED_DECISION', importance: 5 }], - { skipDuplicates: true }, - )), - ]); + // Both hosts run the REAL writer, and both are in flight while a third + // process still holds the write lock — so neither can win by finishing + // before the other starts. + const holder = await peerHoldingWriteLock(600); + + const claudeScript = join(tempDir, 'claude-host.ts'); + writeFileSync(claudeScript, ` +import { writeDecisionsBatch } from ${JSON.stringify(WRITERS_MODULE)}; +const written = writeDecisionsBatch( + ${JSON.stringify(dbPath)}, + [{ sessionId: 'claude-session', project: 'claude', decision: 'CLAUDE_SOURCED_DECISION', importance: 5 }], + { skipDuplicates: true }, +); +console.log('WROTE ' + written); +`); + const claudeHost = Bun.spawn(['bun', 'run', claudeScript], { stdout: 'pipe', stderr: 'pipe' }); + + const openCodeWritten = writeDecisionsBatch( + dbPath, + [{ sessionId: 'opencode-session', project: 'opencode', decision: 'OPENCODE_SOURCED_DECISION', importance: 5 }], + { skipDuplicates: true }, + ); + + const claudeOutput = await new Response(claudeHost.stdout).text(); + const claudeErrors = await new Response(claudeHost.stderr).text(); + await claudeHost.exited; + await holder.done; - expect(results).toEqual([1, 1]); + expect(openCodeWritten).toBe(1); + expect(`${claudeOutput}${claudeErrors}`).toContain('WROTE 1'); const db = new Database(dbPath, { readonly: true }); const rows = db.query('SELECT project, decision FROM decisions ORDER BY decision').all() as { project: string; decision: string }[]; diff --git a/uninstall.sh b/uninstall.sh index 24a7dd9..d265de7 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -450,7 +450,7 @@ remove_opencode() { local guide="$OPENCODE_CONFIG_DIR/Recall_GUIDE.md" local p - for p in RecallExtract.ts RecallPreCompact.ts; do + for p in "${RECALL_OPENCODE_PLUGINS[@]}"; do local pf="$plugin_dir/$p" if [[ -L "$pf" ]]; then if [[ "$DRY_RUN" == "true" ]]; then @@ -467,7 +467,7 @@ remove_opencode() { # Shared plugin helpers live under plugins/lib (see recall_install_opencode_plugins). local helper - for helper in session-export.ts; do + for helper in "${RECALL_OPENCODE_PLUGIN_HELPERS[@]}"; do local hf="$plugin_dir/lib/$helper" if [[ -L "$hf" ]]; then if [[ "$DRY_RUN" == "true" ]]; then From d5a53d5b843d352f00e77281e9519d3f6e65c729 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:34:43 -0400 Subject: [PATCH 14/17] no-mistakes(test): stub host pi in uninstall tests; align README OpenCode maturity --- ...25-opencode-phase4-runtime-verification.md | 37 ++++++++++++++----- README.md | 2 +- tests/AGENTS.md | 1 + tests/install/uninstall.test.ts | 22 ++++++++++- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md index 2088dec..df23873 100644 --- a/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md +++ b/.agents/atlas/handoffs/2026-07-25-opencode-phase4-runtime-verification.md @@ -101,16 +101,33 @@ All three acceptance items met: - `opencode/RecallPreCompact.ts` compaction injection is unit-tested only; no test drives a real OpenCode compaction. Stated in the README row and in `docs/OPENCODE_INTEGRATION.md`. -- One full-suite run reported a single failure once. It did not reproduce in - five subsequent full-suite runs, and the output had been truncated so the test - was never named. Targeted reruns of `sqlite-writers-concurrency`, `restore`, - and `opencode-integration` were clean, but targeted runs cannot clear a - timing-sensitive failure on a loaded runner, so they are **not** evidence that - the flake lives outside this branch — the branch's own files remain suspects. - What was removed instead is the one assertion that could have produced it: the - concurrency test no longer asserts a wall-clock floor, only a happens-before - against a stamp the peer prints while it still holds the lock. Undiagnosed; - worth a separate issue if it recurs. +- The full-suite failure that went unidentified is diagnosed. It was never one + flake but two, both of them tests inheriting host state rather than anything + hanging: `bun test --timeout 30000` is 1298 pass / 0 fail, and every affected + test passes in isolation. + - **Fixed here.** The three Pi-path tests in `tests/install/uninstall.test.ts` + let `remove_pi` reach the host's real `pi`, and `pi remove` costs ~10s on a + machine that actually has Pi installed - while on CI, where `pi` is absent, + it never runs at all. That is the entire 1s -> 12s spread, and the exact + three tests that failed the last pre-fix run. They now shadow `pi` with a + no-op stub on `PATH`, the way the file already stubs `bun unlink`; nothing + asserted on that call, so no coverage is lost. Now a stable 0.7-1.0s each. + - **Left to [#251](https://github.com/edheltzel/Recall/issues/251).** Both of + its named files still trip the 5s **default** on a cold network, and they + need different fixes: the `packFileList()` `beforeAll` in + `tests/install/npm-pack.test.ts` runs `npm pack --dry-run` with no timeout + argument, while the sibling `beforeAll` 18 lines above already carries an + explicit `180_000` for exactly this reason; `update.sh > --check prints + current + latest` in `tests/install/update.test.ts` is a plain `test` that + reaches the GitHub release API. Deliberately not taken here. A full suite is + 1298 pass / 0 fail whenever these two happen not to trip, so a single green + run is not evidence #251 is closed. + Neither is this branch. An interleaved A/B of `uninstall.test.ts` against the + base commit's `uninstall.sh` + `lib/install-lib.sh` reproduced the outlier on + base, and 15 targeted runs of `sqlite-writers-concurrency`, `restore`, and + `opencode-integration` were clean. The concurrency test was hardened anyway - + it no longer asserts a wall-clock floor, only a happens-before against a stamp + the peer prints while it still holds the lock. - Out of scope and untouched: installation reconciliation, the semantic #240/#241/#226 wave, release/version bump, #236/#237/#238/#174, #165. diff --git a/README.md b/README.md index 03240c2..6bc94fe 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ All coding agents forget when a session ends. Recall doesn't — it extracts, in Built on the [Model Context Protocol](https://modelcontextprotocol.io). One SQLite file. No phone-home. No vendor lock-in. -> Stable on [Claude Code](https://claude.com/claude-code). Beta on [Pi](https://pi.dev/) and Alpha for [OpenCode](https://opencode.ai/) (MCP works; lifecycle extensions are early). [Codex CLI](https://github.com/openai/codex) uses a native plugin for MCP and skills; lifecycle auto-capture is not yet supported. [Gemini CLI](https://github.com/google-gemini/gemini-cli) remains on the roadmap. See [Roadmap](#roadmap). +> Stable on [Claude Code](https://claude.com/claude-code). Beta on [Pi](https://pi.dev/) and Beta for [OpenCode](https://opencode.ai/) (capture is verified against a live server; compaction injection is not). [Codex CLI](https://github.com/openai/codex) uses a native plugin for MCP and skills; lifecycle auto-capture is not yet supported. [Gemini CLI](https://github.com/google-gemini/gemini-cli) remains on the roadmap. See [Roadmap](#roadmap). --- diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 60c8a01..711f2ca 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -19,6 +19,7 @@ Automated coverage for the CLI, MCP server, hooks, data layer, libraries, instal - Any end-to-end test that can open SQLite must set its own disposable `RECALL_DB_PATH` and `RECALL_HOME`, state those paths before writes, and prove the production database was not changed. - **A test that claims to exercise concurrency must contend across PROCESSES.** The SQLite writers are synchronous, so `Promise.all` over two of them just runs them back to back and passes on code that loses records. Model contention on `peerHoldingWriteLock` in `hooks/sqlite-writers-concurrency.test.ts`, and prove the new test fails against the unfixed code before trusting it. - Assert ordering, not elapsed wall-clock. A `toBeGreaterThan()` floor is a flake on a loaded runner; have the peer stamp a timestamp while it still holds the resource and assert a happens-before against that. +- A test that shells out to a lifecycle script must shadow every host CLI that script may invoke (`pi`, `bun unlink`, `npm`) with a stub on `PATH`. Otherwise the test inherits whatever the developer has installed — `pi remove` costs ~10s on a machine with Pi, and nothing at all on CI, so the same test is fast, slow, or over bun's 5s default depending on the host. See `stubPiOnPath` in `install/uninstall.test.ts`. Give any hook that must run a genuinely slow command (`bun run build`, `npm pack`) an explicit timeout argument instead of the 5s default. ## Work Guidance diff --git a/tests/install/uninstall.test.ts b/tests/install/uninstall.test.ts index 80e3b85..20c67b6 100644 --- a/tests/install/uninstall.test.ts +++ b/tests/install/uninstall.test.ts @@ -16,12 +16,30 @@ import { writeFileSync, } from 'fs'; import { tmpdir } from 'os'; -import { join } from 'path'; +import { dirname, join } from 'path'; import { legacyClaudeMemorySection, legacyPiMemorySection } from '../fixtures/legacy-memory-sections'; const REPO = process.cwd(); const UNINSTALL = join(REPO, 'uninstall.sh'); +/** + * Shadow the host's `pi` with a no-op and return the directory to prepend to + * PATH. + * + * `remove_pi` runs `pi remove ... --no-approve` whenever `pi` is on PATH, so on + * a developer machine that actually has Pi installed these tests inherit a real + * ~10s package operation and blow past bun's default per-test timeout, while + * CI, where `pi` is absent, never runs it at all. Nothing here asserts on that + * call (the Pi config edits are done by the script's own `bun -e` block), so + * stubbing it keeps the tests hermetic without losing coverage. Same reasoning + * as the `bun unlink` stub used further down. + */ +function stubPiOnPath(binDir: string): string { + mkdirSync(binDir, { recursive: true }); + writeFileSync(join(binDir, 'pi'), '#!/bin/sh\nexit 0\n', { mode: 0o755 }); + return binDir; +} + interface RunResult { stdout: string; stderr: string; @@ -75,6 +93,7 @@ function runUninstallIncludingPi( BACKUP_BASE: backupBase, PI_CONFIG_DIR: piConfigDir, HOME: claudeDir, + PATH: `${stubPiOnPath(join(dirname(claudeDir), 'stub-bin'))}:${process.env.PATH ?? ''}`, RECALL_SKIP_BUN_UNLINK: 'true', }, }, @@ -583,6 +602,7 @@ Preserve this. `#!/bin/sh\nif [ "$1" = unlink ]; then touch "$RECALL_TEST_UNLINK"; exit 0; fi\nexec ${process.execPath} "$@"\n`, { mode: 0o755 }, ); + stubPiOnPath(fakeBin); const result = runUninstallAll(claudeDir, backupBase, opencodeConfigDir, piConfigDir, unlinkMarker, fakeBin); From 7a6db4dab25918dc4d5c1e0dfb2bf9f7735b5fc1 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 13:57:34 -0400 Subject: [PATCH 15/17] no-mistakes(document): document OpenCode plugin helper install and uninstall surface --- docs/architecture.md | 1 + docs/installation.md | 2 +- docs/troubleshooting.md | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 21f16de..bcfc3c9 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -23,6 +23,7 @@ installed MCP adapter/configuration under `~/.pi/agent/`. │ └── Recall_GUIDE.md # Guide for Claude Code ├── opencode/ │ ├── plugins/ # OpenCode plugin canonicals +│ │ └── lib/ # Plugin helpers (.ts) │ └── Recall_GUIDE.md # Guide for OpenCode ├── pi/ │ └── Recall_GUIDE.md # Canonical guide linked into Pi home diff --git a/docs/installation.md b/docs/installation.md index 003c257..41137f0 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -276,7 +276,7 @@ cd /path/to/Recall - `~/.claude/hooks/lib/{extraction-*,pid-utils}.ts` — only Recall-owned files, never the whole `hooks/lib/` directory - The `## MEMORY` section in `~/.claude/CLAUDE.md` only if Recall generated it (current ownership marker or a normalized exact match of the complete legacy-generated body); unmarked customized/externally owned sections and the rest of `CLAUDE.md` are preserved; a marked section remains Recall-owned even if its body was edited - `~/.claude/MEMORY/extract_prompt.md` — only if unmodified from source; user-edited versions are preserved -- OpenCode MCP entry + plugins + agent + guide (unless `--skip-opencode`). An `opencode.json` that Recall cannot parse is reported and left untouched; the plugins, agent, and guide are still removed and the rest of the uninstall continues +- OpenCode MCP entry + plugins + the shared plugin helpers Recall installs under `plugins/lib/` + agent + guide (unless `--skip-opencode`). `plugins/lib/` itself is removed only when Recall emptied it, so your own files there survive. An `opencode.json` that Recall cannot parse is reported and left untouched; the plugins, agent, and guide are still removed and the rest of the uninstall continues - Recall's native Pi package registration, owned Pi MCP entry, guide link, and Recall-generated `AGENTS.md` MEMORY section (current marker or normalized exact legacy Pi body); legacy Recall extension/skill links are removed, while unrelated Pi packages and `pi-mcp-adapter` remain (unless `--skip-pi`) - `bun unlink` (removes `recall` and `recall-mcp` from your PATH) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index bf4e0b7..2926514 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -123,7 +123,7 @@ Extraction hooks fire on the `Stop` event. If the hook isn't registered in `sett 1. Verify the host and runtime: `opencode --version` and `bun --version`. 2. Verify the host export contract: `opencode export ` should print JSON. -3. Check the installed plugin and config: `ls ~/.config/opencode/plugins/RecallExtract.ts` and inspect `opencode.json` for `mcp.recall-memory`. +3. Check the installed plugin, its helper, and the config: `ls ~/.config/opencode/plugins/RecallExtract.ts ~/.config/opencode/plugins/lib/session-export.ts` and inspect `opencode.json` for `mcp.recall-memory`. The plugin imports the helper, so an install that predates it fails to load and captures nothing. Re-run `./install.sh` (or `./update.sh`) to place both. 4. Check the drop directory: `ls ~/.agents/Recall/MEMORY/opencode-sessions/`. 5. Run the batch extractor manually with `bun run ~/.agents/Recall/shared/hooks/RecallBatchExtract.ts --dry-run`. From c4a2da31fc5b0e9625c84909694056a29d046573 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 16:04:05 -0400 Subject: [PATCH 16/17] no-mistakes(document): point OpenCode tool docs at mcp-tools owner --- FOR_OPENCODE.md | 29 ++++++++++++++++++++++++++++- docs/OPENCODE_INTEGRATION.md | 16 +++++----------- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/FOR_OPENCODE.md b/FOR_OPENCODE.md index 7e73e75..9afa31b 100644 --- a/FOR_OPENCODE.md +++ b/FOR_OPENCODE.md @@ -79,6 +79,33 @@ Get database statistics (record counts, database size). Show a full Library of Alexandria entry with its extracted wisdom. +### recall-memory_memory_dump + +Persist a session explicitly, mid-conversation. Automatic extraction already captures +OpenCode sessions (see [How Extraction Works](#how-extraction-works)), so reach for this +only when the user asks for an immediate capture. + +Always pass the visible `messages` and `source: "opencode"`. With `messages` omitted, +Recall falls back to native transcript discovery, which checks Claude Code first and so +dumps the wrong host's session on a machine that has both installed. + +``` +recall-memory_memory_dump({ + title: "Auth middleware refactor", + source: "opencode", + messages: [{ role: "user", content: "..." }, { role: "assistant", content: "..." }] +}) +``` + +### recall-memory_decision_update + +Mark a stored decision superseded (a newer decision replaced it) or reverted (it was +wrong and was rolled back), by ID. + +``` +recall-memory_decision_update({ id: 42, action: "supersede" }) +``` + ## The CLI You can also use the `recall` CLI directly via Bash tool: @@ -133,7 +160,7 @@ OpenCode first. 2. **Record decisions** — When architectural decisions are made, use `recall-memory_memory_add` to record them 3. **Context for agents** — Before spawning subagents via `@agent`, call `recall-memory_context_for_agent` 4. **Onboarding check** — At session start, if the L0 identity tier is empty (no `~/.claude/MEMORY/identity.md` or the file is missing), suggest `recall onboard` once. Do not nag on subsequent turns. -5. **Never store secrets** — `recall-memory_memory_add` persists content verbatim into `recall.db` (and automatic extraction captures session text), and stored records can resurface in future sessions' L0/L1 context. Redact API keys, tokens, passwords, and credential-bearing snippets before recording (e.g. `[REDACTED:api-key]`). When dumping a session that touched credentials, say so and confirm with the user first. +5. **Never store secrets** — `recall-memory_memory_add` and `recall-memory_memory_dump` persist content verbatim into `recall.db` (and automatic extraction captures session text), and stored records can resurface in future sessions' L0/L1 context. Redact API keys, tokens, passwords, and credential-bearing snippets before recording (e.g. `[REDACTED:api-key]`). When dumping a session that touched credentials, say so and confirm with the user first. 6. **Record corrections** — When the user corrects you ("no, actually…", "that's wrong, use X"), record it immediately: `recall-memory_memory_add({ type: "learning", content: "", confidence: "high", importance: 7 })`. Corrections are the highest-signal and most perishable memory; do not wait for session end. ## How Extraction Works diff --git a/docs/OPENCODE_INTEGRATION.md b/docs/OPENCODE_INTEGRATION.md index 640f597..23d3724 100644 --- a/docs/OPENCODE_INTEGRATION.md +++ b/docs/OPENCODE_INTEGRATION.md @@ -359,17 +359,11 @@ All paths are **resolved to absolute** at install time. No tilde in stored confi ## Tool Name Mapping -OpenCode prefixes MCP tools with the server name + underscore: - -| Claude Code | OpenCode | Same Tool | -|-------------|----------|-----------| -| `memory_search` | `recall-memory_memory_search` | Yes — includes `table` hard filters and `bias_type` soft boosts | -| `memory_hybrid_search` | `recall-memory_memory_hybrid_search` | Yes | -| `memory_recall` | `recall-memory_memory_recall` | Yes | -| `memory_add` | `recall-memory_memory_add` | Yes | -| `context_for_agent` | `recall-memory_context_for_agent` | Yes | -| `memory_stats` | `recall-memory_memory_stats` | Yes | -| `loa_show` | `recall-memory_loa_show` | Yes | +OpenCode connects to the same MCP server as every other host and prefixes each tool with +the server name plus an underscore: `memory_search` becomes `recall-memory_memory_search`. + +Apply that one rule to any tool in [`mcp-tools.md`](mcp-tools.md), which documents all +nine with their parameters and return shapes. ## Implementation Phases From 34198996949bdbf2951857ff18d261eb5e0b9c53 Mon Sep 17 00:00:00 2001 From: Ed Heltzel <402910+edheltzel@users.noreply.github.com> Date: Sat, 25 Jul 2026 16:21:42 -0400 Subject: [PATCH 17/17] no-mistakes(document): fix OpenCode agent dump rule, point tools at owner --- opencode/recall-memory.md | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/opencode/recall-memory.md b/opencode/recall-memory.md index 7544d96..ff5f9fd 100644 --- a/opencode/recall-memory.md +++ b/opencode/recall-memory.md @@ -24,7 +24,8 @@ You have access to persistent memory via the recall-memory MCP server. - `recall-memory_memory_recall` for recent entries across all categories 5. **Session capture** — When the user says `/dump`, capture the session: - - `recall-memory_memory_dump` with a descriptive title + - `recall-memory_memory_dump` with a descriptive `title`, `source: "opencode"`, and the visible `messages` + - Always pass `messages`. Omit them and Recall falls back to native transcript discovery, which checks Claude Code first and captures the wrong host's session on a machine that has both - Messages become immediately searchable from any new session 6. **Never store secrets** — `recall-memory_memory_add` and `recall-memory_memory_dump` persist content verbatim: @@ -38,16 +39,12 @@ You have access to persistent memory via the recall-memory MCP server. ## Available Tools -| Tool | Purpose | -|------|---------| -| `recall-memory_memory_search` | FTS5 keyword search; supports `table` hard filters and `bias_type` soft boosts | -| `recall-memory_memory_hybrid_search` | Semantic + keyword search with RRF fusion | -| `recall-memory_memory_recall` | Recent context (LoA, decisions, breadcrumbs) | -| `recall-memory_memory_add` | Record decisions, learnings, or breadcrumbs | -| `recall-memory_context_for_agent` | Prepare context before spawning subagents | -| `recall-memory_memory_stats` | Database statistics | -| `recall-memory_memory_dump` | Dump current session to SQLite (mid-session) | -| `recall-memory_loa_show` | View full Library of Alexandria entry | +OpenCode prefixes every MCP tool with the server name plus an underscore: `memory_search` +becomes `recall-memory_memory_search`. + +Apply that one rule to any tool in +[`docs/mcp-tools.md`](https://github.com/edheltzel/Recall/blob/main/docs/mcp-tools.md), +which documents all nine with their parameters and return shapes. ## Codebase Scouting