fix: stop marking unpersisted extractions as complete, harden uninstall

AGENTS.md

@@ -28,7 +28,7 @@ Top-level directories, by purpose (one line each — not a file enumeration):
 - `agent-skills/` — canonical Agent Skills (SKILL.md, one per skill dir) installed to `~/.claude/skills` and `~/.omp/agent/skills`, discovered by Pi through the root package manifest, and generated into native host plugin payloads — the single `recall-*` command surface (the former `/Recall:*` slash commands, #228)
 - `plugins/` — native host plugin bundles, one per host: Codex in `plugins/recall/`, Claude Code in `plugins/recall-claude/`, each packaging MCP plus its own skill payload
 - `docs/` — user-facing published docs + ADRs (`docs/adr/`) + agent skill docs (`docs/agents/`)
-- `lib/` — shared bash for the install / update / uninstall lifecycle scripts
+- `lib/` — shared bash for the install / update / uninstall lifecycle scripts, plus the dependency-free `jsonc-mcp.ts` runtime helper they shell out to for JSONC config edits
 - `opencode/` — OpenCode host integration (plugins / hooks / guide)
 - `pi/` — Pi package extensions for native lifecycle capture and memory injection
 - `scripts/` — dev / CI helper scripts (version check, e2e)
@@ -177,7 +177,7 @@ Child AGENTS.md files own domain-specific local rules. Read the applicable one b
 - [`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
 
-Owned at root (no child doc): lifecycle scripts (`install.sh`, `update.sh`, `uninstall.sh`) + their shared `lib/install-lib.sh`; 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`; platform guides (`FOR_CLAUDE.md`, `FOR_OPENCODE.md`, `FOR_PI.md`); `CONTEXT.md`; and `assets/` (banner + VHS demo tapes/gifs).
 
 ## Maintaining this file
 

docs/OPENCODE_INTEGRATION.md

@@ -235,7 +235,7 @@ register_opencode_mcp() {
   local resolved_db_path
   resolved_db_path="$(eval echo "$RECALL_DB_PATH_DEFAULT")"  # Resolve ~ to absolute
 
-  # Use bun -e with jsonc-parser for a surgical JSONC-safe merge.
+  # Use the bundled dependency-free JSONC helper for a surgical merge.
   local mem_mcp_path
   mem_mcp_path="$(which recall-mcp 2>/dev/null || echo "$HOME/.bun/bin/recall-mcp")"
 
@@ -264,9 +264,11 @@ install_opencode_guide() {
 ```
 
 The live installer delegates to `lib/install-lib.sh`'s shared
-`_recall_jsonc_merge_mcp_entry` helper. It preserves comments, trailing commas,
-sibling MCP entries, and custom Recall entry fields; malformed or unwritable
-configs fail before writing. Uninstall uses the matching surgical removal helper.
+`_recall_jsonc_merge_mcp_entry` helper and the bundled dependency-free
+`lib/jsonc-mcp.ts` parser. It preserves comments, trailing commas, sibling MCP
+entries, and custom Recall entry fields; malformed or unwritable configs fail
+before writing. Uninstall uses the matching surgical removal helper, leaves a
+malformed OpenCode config unchanged, and continues the remaining integrations.
 
 ## Database Schema Change
 
@@ -364,13 +366,14 @@ OpenCode prefixes MCP tools with the server name + underscore:
 | Tilde in RECALL_DB_PATH | MEDIUM | Absolute path resolved at install |
 | `recall import --source` doesn't exist | LOW-MEDIUM | Eliminated — uses drop dir + RecallBatchExtract |
 | Bun not guaranteed | LOW-MEDIUM | Documented as requirement (same as Claude Code) |
-| JSONC parsing | LOW | `jsonc-parser` surgical merge/removal preserves comments and refuses malformed writes |
+| JSONC parsing | LOW | Bundled dependency-free parser performs surgical merge/removal, preserves comments, and refuses malformed writes |
 
 ## Phase 4 Evidence
 
 1. OpenCode 1.18.4 emits `session.idle` through the `event` hook with
-   `properties.sessionID`; the e2e invokes that exact payload and verifies the
-   resulting markdown drop.
+   `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 <session-id>` 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.

docs/architecture.md

@@ -212,7 +212,8 @@ The current Claude lifecycle adapter tries Claude Haiku first and falls back to
 - **File permissions** set to 0600 (owner read/write only)
 - **Chunked extraction** for sessions >120K characters with meta-extraction merging
 - **Quality gate** rejects extractions missing required sections
-- **Retry window** of 24 hours for failed extractions
+- **Persistence check** marks a quality-passing extraction failed rather than extracted when its SQLite dual-write fails (for example an unwritable or locked database), so a session is never recorded as complete when some of its records did not land
+- **Retry window** of 24 hours for failed extractions (quality-gate, extraction, and persistence failures alike)
 - **Parameterized queries** — no SQL injection vectors
 - **PRAGMA user_version** migration system for schema upgrades
 

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 (unless `--skip-opencode`)
+- 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
 - 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)
 

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.
+- 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.
 

hooks/RecallBatchExtract.ts

@@ -74,6 +74,10 @@ export interface TrackerEntry {
 
 export type Tracker = Record<string, TrackerEntry>;
 
+export function isExtractionFailureOutput(output: string): boolean {
+  return /quality gate failed|all extraction methods failed|\[FabricExtract\] (?:markdown )?extraction failed|sqlite write failed|persistence failed/i.test(output);
+}
+
 // Parse args (top-level so main() and re-entrant tests share)
 const args = process.argv.slice(2);
 const dryRun = args.includes('--dry-run');
@@ -327,7 +331,7 @@ function extractFile(convPath: string, cwd: string): boolean {
       throw result.error ?? new Error(`RecallExtract exited with status ${result.status}`);
     }
     // Check if quality gate failed
-    if (output.includes('QUALITY GATE FAILED') || output.includes('All extraction methods failed') || output.includes('Extraction failed')) {
+    if (isExtractionFailureOutput(output)) {
       log(`  QUALITY GATE FAILED or extraction failed for ${convPath}`);
       return false;
     }
@@ -336,7 +340,7 @@ function extractFile(convPath: string, cwd: string): boolean {
     // execSync throws on non-zero exit, but RecallExtract exits 0 even on failure
     // Check stderr/stdout for quality gate failure
     const output = err.stdout || err.stderr || err.message || '';
-    if (output.includes('QUALITY GATE FAILED') || output.includes('All extraction methods failed')) {
+    if (isExtractionFailureOutput(output)) {
       log(`  QUALITY GATE FAILED for ${convPath}`);
       return false;
     }
@@ -392,12 +396,17 @@ async function main() {
     const success = extractFile(candidate.path, cwd);
 
     if (success) {
-      extracted++;
       // RecallExtract's own markAsExtracted already updates the tracker.
       // Belt-and-suspenders: stamp it again here so dedup stays consistent
       // even if the child process exited before reaching its own writer.
-      recordSuccess(dbPath, candidate.path, candidate.size);
-      log(`  SUCCESS: Extracted and tracked`);
+      if (recordSuccess(dbPath, candidate.path, candidate.size)) {
+        extracted++;
+        log(`  SUCCESS: Extracted and tracked`);
+      } else {
+        failed++;
+        recordFailure(dbPath, candidate.path, candidate.size, 'RecallBatchExtract: tracker write failure');
+        log(`  FAILED: Extraction completed but tracking failed; will retry after 24h cooldown`);
+      }
     } else {
       failed++;
       recordFailure(dbPath, candidate.path, candidate.size, 'RecallBatchExtract: quality gate or runtime failure');

hooks/RecallExtract.ts

@@ -614,7 +614,14 @@ async function extractAndAppend(conversationPath: string, cwd: string): Promise<
       markAsFailed(convHash, `quality gate failed: ${core.quality?.reason}`);
       return;
     }
-    logExtract("QUALITY GATE PASSED: extraction contains required sections");
+    const persistenceFailed = core.outcome === 'persistence_failed';
+    if (persistenceFailed) {
+      const failures = JSON.stringify(core.dualWrite?.failures ?? {});
+      console.error(`[FabricExtract] SQLite write failed: ${failures}`);
+      logExtract(`FAILURE: SQLite write failed: ${failures}`);
+    } else {
+      logExtract("QUALITY GATE PASSED: extraction contains required sections");
+    }
 
     // Scrub the extracted text BEFORE it reaches any on-disk archive writer
     // (#132): the SQLite path already scrubs inside runExtractCore, but the
@@ -677,6 +684,11 @@ async function extractAndAppend(conversationPath: string, cwd: string): Promise<
     appendErrors(extracted, sessionLabel, timestamp);
 
     // 6. Mark as extracted (dedup)
+    if (persistenceFailed) {
+      const failures = JSON.stringify(core.dualWrite?.failures ?? {});
+      markAsFailed(convHash, `SQLite write failed: ${failures}`);
+      return;
+    }
     markAsExtracted(convHash);
 
     logExtract(`SUCCESS: All memory files updated for session=${sessionLabel}`);
@@ -768,7 +780,14 @@ async function extractAndAppendMarkdown(mdPath: string, cwd: string): Promise<vo
       markAsFailed(convHash, `quality gate failed (markdown): ${core.quality?.reason}`);
       return;
     }
-    logExtract("QUALITY GATE PASSED (markdown)");
+    const persistenceFailed = core.outcome === 'persistence_failed';
+    if (persistenceFailed) {
+      const failures = JSON.stringify(core.dualWrite?.failures ?? {});
+      console.error(`[FabricExtract] SQLite write failed (markdown): ${failures}`);
+      logExtract(`FAILURE (markdown): SQLite write failed: ${failures}`);
+    } else {
+      logExtract("QUALITY GATE PASSED (markdown)");
+    }
 
     // Scrub the extracted text BEFORE it reaches any on-disk archive writer
     // (#132) — same single markdown-write seam as the Stop path. Redacts
@@ -807,6 +826,11 @@ async function extractAndAppendMarkdown(mdPath: string, cwd: string): Promise<vo
     appendErrors(extracted, sessionLabel, timestamp);
 
     // 5. Mark as extracted
+    if (persistenceFailed) {
+      const failures = JSON.stringify(core.dualWrite?.failures ?? {});
+      markAsFailed(convHash, `SQLite write failed (markdown): ${failures}`);
+      return;
+    }
     markAsExtracted(convHash);
 
     logExtract(`SUCCESS (markdown): All memory files updated for ${sessionLabel} (${sessionId})`);

hooks/lib/extract-core.ts

@@ -34,7 +34,7 @@ import {
 } from './extraction-parsers';
 
 /** Why a core run did not produce a SQLite write. */
-export type ExtractOutcome = 'extracted' | 'extraction_failed' | 'quality_failed';
+export type ExtractOutcome = 'extracted' | 'extraction_failed' | 'quality_failed' | 'persistence_failed';
 
 /** Injected RecallExtract-resident behavior the lib core can't import directly. */
 export interface ExtractCoreDeps {
@@ -64,7 +64,7 @@ export interface ExtractCoreResult {
   redactions?: string[];
   /** #156 injection/exfil flag findings across persisted fields (surfaced only; persisted text is unchanged). */
   threats?: ThreatFinding[];
-  /** Present only when outcome === 'extracted'. */
+  /** Present when the quality-passed write attempt reached SQLite. */
   dualWrite?: DualWriteResult;
 }
 
@@ -73,10 +73,11 @@ export interface ExtractCoreResult {
  *
  * Returns early (no SQLite write) when the model produces nothing
  * ('extraction_failed') or the quality gate rejects the output
- * ('quality_failed'); the caller decides how to log / mark those. On success the
- * SQLite dual-write has already happened (with scrubbed text) and `dualWrite`
- * carries the per-table counts; `extracted`/`topics`/`summary` are the RAW values
- * for the caller's legacy markdown side-effects.
+ * ('quality_failed'). A failed SQLite write is reported as 'persistence_failed'
+ * with its failure details so callers cannot mark an empty extraction complete.
+ * On success the SQLite dual-write has already happened (with scrubbed text) and
+ * `dualWrite` carries the per-table counts; `extracted`/`topics`/`summary` are
+ * the RAW values for the caller's legacy markdown side-effects.
  */
 export async function runExtractCore(
   dbPath: string,
@@ -125,6 +126,19 @@ export async function runExtractCore(
     topics: scrubbedTopics.map((r) => r.text),
   });
 
+  if (Object.keys(dualWrite.failures).length > 0) {
+    return {
+      outcome: 'persistence_failed',
+      quality,
+      extracted,
+      topics,
+      summary,
+      redactions,
+      threats,
+      dualWrite,
+    };
+  }
+
   return {
     outcome: 'extracted',
     quality,

hooks/lib/extraction-parsers.ts

@@ -15,8 +15,12 @@ import {
   writeExtractionSession,
   writeLearningsBatch,
   writeLoaEntryFromExtraction,
+  type WriteOptions,
 } from './sqlite-writers';
 
+/** This seam is replayed on retry: every plain-INSERT writer skips rows it already wrote. */
+const RETRY_SAFE: WriteOptions = { skipDuplicates: true };
+
 export interface DualWriteContext {
   sessionId: string;
   sessionLabel: string;
@@ -139,6 +143,11 @@ export function parseErrorPatternItems(extracted: string): ExtractionErrorInput[
  * Dual-write extracted content into SQLite. Every section is wrapped in its
  * own try/catch — a failure on one writer must not block the others, and the
  * whole function must not throw.
+ *
+ * The plain-INSERT writers run with `skipDuplicates` because this seam is
+ * replayed: the Stop hook marks a conversation extracted only after its markdown
+ * archive writes, and a partial SQLite failure marks it failed + retryable. See
+ * the RETRY IDEMPOTENCY note in sqlite-writers.ts.
  */
 export function dualWriteToSqlite(
   dbPath: string,
@@ -155,7 +164,7 @@ export function dualWriteToSqlite(
   };
 
   if (!ensureDbWritable(dbPath)) {
-    result.failures._db = 'not writable';
+    result.failures._db = 'not writable or locked';
     return result;
   }
 
@@ -174,21 +183,30 @@ export function dualWriteToSqlite(
   }
 
   try {
-    result.decisions = writeDecisionsBatch(dbPath, parseDecisionItems(ctx.extracted, ctx));
+    result.decisions = writeDecisionsBatch(
+      dbPath,
+      parseDecisionItems(ctx.extracted, ctx),
+      RETRY_SAFE
+    );
   } catch (e: any) {
     result.failures.decisions = e?.message || String(e);
   }
 
   try {
-    result.learnings = writeLearningsBatch(dbPath, parseLearningItems(ctx.extracted, ctx));
+    result.learnings = writeLearningsBatch(
+      dbPath,
+      parseLearningItems(ctx.extracted, ctx),
+      RETRY_SAFE
+    );
   } catch (e: any) {
     result.failures.learnings = e?.message || String(e);
   }
 
   try {
     result.breadcrumbs = writeBreadcrumbsBatch(
       dbPath,
-      parseBreadcrumbItems(ctx.extracted, ctx)
+      parseBreadcrumbItems(ctx.extracted, ctx),
+      RETRY_SAFE
     );
   } catch (e: any) {
     result.failures.breadcrumbs = e?.message || String(e);
@@ -201,15 +219,19 @@ export function dualWriteToSqlite(
   }
 
   try {
-    const loaId = writeLoaEntryFromExtraction(dbPath, {
-      title: `${ctx.sessionLabel} — ${ctx.timestamp}`,
-      description: ctx.summary,
-      fabricExtract: ctx.extracted,
-      sessionId: ctx.sessionId,
-      project: ctx.project,
-      tags: ctx.topics.join(','),
-      messageCount: ctx.messageCount ?? null,
-    });
+    const loaId = writeLoaEntryFromExtraction(
+      dbPath,
+      {
+        title: `${ctx.sessionLabel} — ${ctx.timestamp}`,
+        description: ctx.summary,
+        fabricExtract: ctx.extracted,
+        sessionId: ctx.sessionId,
+        project: ctx.project,
+        tags: ctx.topics.join(','),
+        messageCount: ctx.messageCount ?? null,
+      },
+      RETRY_SAFE
+    );
     result.loa = loaId ? 1 : 0;
   } catch (e: any) {
     result.failures.loa = e?.message || String(e);

hooks/lib/insession.ts

@@ -380,7 +380,8 @@ export type ExtractionOutcome =
   | 'no_transcript'
   | 'empty_slice'
   | 'extraction_failed'
-  | 'quality_failed';
+  | 'quality_failed'
+  | 'persistence_failed';
 
 export interface ExtractionResult {
   ran: boolean;
@@ -463,10 +464,10 @@ export async function runInSessionExtraction(
       };
     }
 
-    // extraction_failed | quality_failed — consume the window but keep the cursor
+    // extraction_failed | quality_failed | persistence_failed — consume the window but keep the cursor
     // so the same slice (plus new content) is retried next time. No data loss.
     resetWindow(dbPath, params.sessionId);
-    return { ran: false, outcome: core.outcome };
+    return { ran: false, outcome: core.outcome, dualWrite: core.dualWrite };
   } finally {
     childReleaseLock(dbPath, params.convPath, deps.pid);
   }