From f6180e4d8408d969efc2a4b05cc43f15296ec201 Mon Sep 17 00:00:00 2001 From: Josh Kappler Date: Sat, 4 Jul 2026 01:51:28 -0700 Subject: [PATCH 1/3] fix(invalidate/store): wire reconcileWrite into the live write path; BEGIN IMMEDIATE for read-modify-write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two concurrency holes in durable writes, both invisible to a green suite because each needs a second writer. 1. The advertised no-silent-LWW guard never ran in production. STATUS §14 claims 'resolveConflicts + reconcileWrite optimistic concurrency (never silent LWW)', but reconcileWrite had zero live call sites. On the real path (learn -> classifyRelation), two parallel sessions' default remembers land at workspace scope with equal precedence and classify as refines: the newer fact supersedes the older. An edge is recorded but no dispute is surfaced. The parallel-conflict eval stayed green because its 'real Runtime concurrent-session stress' section called reconcileWrite directly instead of going through learn(). - src/invalidate/invalidator.ts: at equal precedence between two user-asserted facts, route the refines decision through reconcileWrite. base_seen_fact_id matching the existing fact -> clean supersede; no base seen -> conflicts (both disputed, CONFLICTS_WITH edge, conflict note at injection, resolve_conflict settles it). Deterministic parser facts are exempt: their evidence is commit-anchored, so re-extraction still refreshes values. - src/runtime.ts: learn() takes { baseSeenFactId }; RememberFactInput gains baseSeenFactId; new currentFactIdFor(subject, predicate, sessionId?) returns the active same-scope fact a remember would collide with. - Interactive writers do read-then-write themselves: the CLI remember command and the TUI 'n' prompt resolve currentFactIdFor at write time and pass it as the base, so a deliberate human update supersedes exactly as before (the core-memory-lifecycle eval's remember->supersede check stays green). Agents get no such auto-base: MCP remember accepts an optional base_seen_fact_id (still exactly 8 tools; I8 untouched), and without it a contradicting equal-precedence write returns status 'disputed' instead of silently winning. That asymmetry is the point: the human at the terminal sees the value they're replacing; a parallel agent session usually doesn't. - src/eval/suites/parallel-conflict.ts: section 5 now drives real learn() races (two Runtimes, one store) instead of calling reconcileWrite directly; runner is async. cli.ts awaits it; test/eval wrappers updated. - test/runtime/forget.test.ts: the sequential-update case now passes the base it saw, matching the CLI's behavior. 2. Overlapping promotion sweeps lose promotions permanently. tx() is BEGIN deferred; the sweep reads then writes in one deferred tx. Two processes on one WAL DB (SessionEnd hook vs the MCP daemon's checkpoint/promote) both snapshot, the first commits, the second's first write throws SQLITE_BUSY_SNAPSHOT - busy_timeout does NOT retry snapshot conflicts - and hooks swallow it (I9). The sweep is scoped to the ended session, so no later sweep revisits those candidates. - src/store/db.ts: txImmediate (both drivers expose .immediate on wrapped transactions). src/store/facts.repo.ts: transactionImmediate. - src/promote/probation.ts (sweep + reviewFactForWorkspace) and the invalidator apply path now run BEGIN IMMEDIATE, so concurrent read-modify-write serializes at BEGIN via busy_timeout and each sweep runs on a fresh snapshot. Tests: test/invalidate/concurrent-writes.test.ts (dispute without base seen, clean supersede with it, interactive currentFactIdFor flow, precedence + parser-refresh guards); test/store/tx-immediate.test.ts (two connections, one file DB: reproduces SQLITE_BUSY_SNAPSHOT under deferred, proves txImmediate commits while the concurrent writer fails fast with retryable SQLITE_BUSY, wiring checks for sweep + apply). Counters 292 -> 301. docs/STATUS.md §14 row + AGENT_USAGE.md remember params updated. Verified on Windows 11 (Node 22): npx tsc --noEmit clean; npx biome check src test clean; npx vitest run 289/301 with only the 12 pre-existing win32 failures (fixed separately in the Windows PR); npx tsx src/cli.ts eval conflict 62/62 PASS with silentOverwrites=0 across real learn() races. Full 'eval all' cannot run on Windows without the Windows PR's .cmd fix; 19/19 verified on a local integration merge of the two branches, including the core-memory-lifecycle remember->supersede check. --- AGENT_USAGE.md | 6 +- README.md | 2 +- docs/STATUS.md | 8 +- src/cli.ts | 5 +- src/eval/suites/parallel-conflict.ts | 259 ++++++---------------- src/invalidate/invalidator.ts | 39 +++- src/mcp/tools.ts | 14 +- src/promote/probation.ts | 12 +- src/runtime.ts | 63 ++++-- src/store/db.ts | 16 +- src/store/facts.repo.ts | 9 +- src/tui/app.ts | 10 +- test/eval/m3-suites.test.ts | 4 +- test/eval/parallel-conflict.test.ts | 31 +-- test/invalidate/concurrent-writes.test.ts | 144 ++++++++++++ test/runtime/forget.test.ts | 6 +- test/store/tx-immediate.test.ts | 121 ++++++++++ 17 files changed, 510 insertions(+), 239 deletions(-) create mode 100644 test/invalidate/concurrent-writes.test.ts create mode 100644 test/store/tx-immediate.test.ts diff --git a/AGENT_USAGE.md b/AGENT_USAGE.md index 189e1ff..26dc5e7 100644 --- a/AGENT_USAGE.md +++ b/AGENT_USAGE.md @@ -326,7 +326,11 @@ description: Local-first persistent memory for this project. Use it whenever - Treat any `[mem:*]` block already in your context as authoritative. ## Tools (MCP server: graphctx) -- remember(text, kind?, scope?, ttl?) +- remember(text, kind?, subject?, predicate?, session_id?, base_seen_fact_id?) + - pass `base_seen_fact_id` (from `recall`/`why`) when changing an existing + value: with it the old fact is superseded cleanly; without it a + contradicting write at equal precedence is marked disputed and surfaced + for `resolve_conflict` (never silent last-writer-wins) - recall(query, budget_tokens?) - inject_context(event, session_id?, user_prompt?) - checkpoint_session(session_id?) diff --git a/README.md b/README.md index 280762a..b3ce460 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,7 @@ npx tsx src/cli.ts bench Expected current state: -- `npx vitest run`: 292 tests pass. +- `npx vitest run`: 301 tests pass. - `npm run pack:smoke`: packed tarball installs, runs the demo, and serves MCP from a clean temp app. - `npx tsx src/cli.ts eval all`: 19 gate suites pass. diff --git a/docs/STATUS.md b/docs/STATUS.md index fa0566b..904649b 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -37,7 +37,7 @@ | retrieve/retriever | §13 | ✅ | vector ∪ BM25 + entity + scope; commit-anchored filter | | retrieve/vectors | §13 | ✅ | sqlite-vec hybrid; offline local embedder; BM25 fallback (I9) | | retrieve/signals + rank | §13 | ✅ | scope/entity scorers; fusion w/ confidence + recency (S5) | -| resolve/precedence + conflicts | §14 | ✅ | full precedence (prose < user profile, D14); resolveConflicts + reconcileWrite optimistic concurrency (never silent LWW) | +| resolve/precedence + conflicts | §14 | ✅ | full precedence (prose < user profile, D14); resolveConflicts + reconcileWrite optimistic concurrency wired into the live learn() path (never silent LWW: equal-precedence user contradictions dispute unless the writer passed base_seen_fact_id) | | inject/gate | §15 | ✅ | centroid drift + entity-change + event-class; selective PreToolUse | | inject/ledger | §15 | ✅ | DB-backed cross-process/cross-channel anti-repetition | | inject/budget | §15 | ✅ | utility ranking + redundancy penalty + must-include bonuses + caps | @@ -76,9 +76,9 @@ |---|---|---| | `hook ` p95 | < 150ms | 15.51ms ✅ | -_Last updated: Private-key scanning now covers DSA, encrypted PEM, and PGP private key block headers, MCP `resolve_conflict` now evaluates user, workspace, and current-session facts together, `graphctx resolve` now refuses non-open-loop facts instead of silently superseding durable memory, Storage migration 0006 adds a composite invalidation conflict lookup index, Session-scoped invalidation now keeps identical open loops isolated across sessions while deduping repeats within a session, Demo seed memory now uses Runtime.rememberFact, open-loop session memory now carries current git anchors, CLI `tui --tab` now fails fast on invalid tabs, CLI/MCP fact-kind validation uses a shared `FACT_KINDS` domain enum, TUI promotion uses Runtime/Probation gates instead of direct activation, Runtime/TUI/CLI/MCP explicit remember paths share safe git-anchored invalidating writes, Runtime/TUI/MCP forget paths share git-anchored temporal closeout, MCP forget stamps git closeout anchors, retrieval candidates are scope-filtered before semantic reranking, retrieval candidates receive final workspace/session scope filtering, superseded facts close temporal validity with `t_expired` and commit anchors, retrieval recency ranking uses the runtime clock, ranking/conflict tests use deterministic fact IDs and clocks, vector embedding cache uses the runtime clock, outcome telemetry uses an injectable clock, injection anti-repetition ledger uses the runtime clock, promotion verification timestamps use the runtime clock, promotion gates rescan secret-shaped fact identity, why renders invalidating commit closeout anchors, revert revalidation respects branch representation, revert revalidation skips bad historical anchors, atomic temporal lifecycle closeout anchors, TUI read model redacts secret fact text, conflict notes redact identity/object secrets, structured why redacts evidence chain metadata, structured why redacts fact identity and scope strings, structured why redacts promotion audit text, structured why redacts git anchors, structured why redacts evidence payloads and tags, why output shows expired/invalidated temporal closeout, send-edge safety scans fact tags for secrets, command cards avoid false HEAD verification claims, low-trust claims blocked from PreToolUse guardrail injection, candidate facts kept out of vector search until promotion, lifecycle-synced vector index entries for expired/reactivated facts, secret-redacted semantic rerank text, retriever-level send-unsafe fact suppression, tag-update secret sensitivity restamping, secret-redacted FTS/vector fact indexing, FTS path-query fail-soft handling, runtime-level injection context redaction, hook tool-result redaction before injection context, shared retrieval-context redaction for CLI/MCP/hooks, atomic invalidation effects, supported-by temporal evidence edges, tool-arg redaction before retrieval, hook prompt redaction before retrieval, transcript-tail redaction before retrieval, symlink-safe workspace config reads, symlink-safe local store paths, symlink-safe AGENTS.md boot capsule writes, symlink-safe generic adapter marker writes, symlink-safe OpenCode config writes, symlink-safe Cursor mcp writes, symlink-safe Claude settings writes, workspace-confined CI workflow extraction, workspace-confined agent prose extraction, workspace-confined Docker/Compose extraction, workspace-confined test config extraction, workspace-confined tooling config extraction, workspace-confined tsconfig extraction, workspace-confined editorconfig extraction, workspace-confined runtime version extraction, workspace-confined lockfile extraction, workspace-confined package script evidence, shared realpath workspace evidence checks, symlink-aware injection staleness checks, generated-marker symlink skip, procedure verifier secret scanning, extraction subject secret scanning, hook session-id redaction, MCP session-reference secret refusal, open-loop session metadata secret refusal, explicit memory metadata secret refusal, packageManager canonical facts, package-manager-aware script extraction, runtime-version deterministic extraction, multi-cookie session redaction, MCP error redaction, fail-closed retrieval for unvalidated commit-scoped facts, semantic CLI recall ranking, auth/cookie header secret scanning, scoped semantic retrieval expansion, workspace-confined injection staleness checks, package metadata extraction, high-trust test/Docker/tooling/tsconfig deterministic extraction, high-trust Python toolchain extraction, high-trust pyproject package-manager extraction, pyproject dependency-group Python command extraction, high-entropy redaction hardening, repo-id-isolated temporal validity, measured hook latency, fail-closed adapter install/uninstall, typed CLI error formatting, answer-bearing coding query expansion, Python package-manager retrieval expansion, redacted TUI selected-fact detail panels, TUI page/jump keyboard navigation, composed TUI empty states with operator status surfaces, SessionStart suppression of superseded user preferences, chunk-safe TUI key decoding with left/right and j/k navigation, bounded secret-redacted TUI prompt input, vector-safe commit-hygiene query expansion, searchable TUI memory lists, and tox/nox Python test harness extraction. 292 tests, 19 gate suites green, all I1-I9 hold._ +_Last updated: Durable-write concurrency hardened (reconcileWrite now guards the live learn() path so equal-precedence user contradictions dispute instead of silently superseding, remember accepts base_seen_fact_id for clean updates, and read-modify-write transactions run BEGIN IMMEDIATE so overlapping promotion sweeps serialize instead of losing promotions to swallowed SQLITE_BUSY_SNAPSHOT errors), Private-key scanning now covers DSA, encrypted PEM, and PGP private key block headers, MCP `resolve_conflict` now evaluates user, workspace, and current-session facts together, `graphctx resolve` now refuses non-open-loop facts instead of silently superseding durable memory, Storage migration 0006 adds a composite invalidation conflict lookup index, Session-scoped invalidation now keeps identical open loops isolated across sessions while deduping repeats within a session, Demo seed memory now uses Runtime.rememberFact, open-loop session memory now carries current git anchors, CLI `tui --tab` now fails fast on invalid tabs, CLI/MCP fact-kind validation uses a shared `FACT_KINDS` domain enum, TUI promotion uses Runtime/Probation gates instead of direct activation, Runtime/TUI/CLI/MCP explicit remember paths share safe git-anchored invalidating writes, Runtime/TUI/MCP forget paths share git-anchored temporal closeout, MCP forget stamps git closeout anchors, retrieval candidates are scope-filtered before semantic reranking, retrieval candidates receive final workspace/session scope filtering, superseded facts close temporal validity with `t_expired` and commit anchors, retrieval recency ranking uses the runtime clock, ranking/conflict tests use deterministic fact IDs and clocks, vector embedding cache uses the runtime clock, outcome telemetry uses an injectable clock, injection anti-repetition ledger uses the runtime clock, promotion verification timestamps use the runtime clock, promotion gates rescan secret-shaped fact identity, why renders invalidating commit closeout anchors, revert revalidation respects branch representation, revert revalidation skips bad historical anchors, atomic temporal lifecycle closeout anchors, TUI read model redacts secret fact text, conflict notes redact identity/object secrets, structured why redacts evidence chain metadata, structured why redacts fact identity and scope strings, structured why redacts promotion audit text, structured why redacts git anchors, structured why redacts evidence payloads and tags, why output shows expired/invalidated temporal closeout, send-edge safety scans fact tags for secrets, command cards avoid false HEAD verification claims, low-trust claims blocked from PreToolUse guardrail injection, candidate facts kept out of vector search until promotion, lifecycle-synced vector index entries for expired/reactivated facts, secret-redacted semantic rerank text, retriever-level send-unsafe fact suppression, tag-update secret sensitivity restamping, secret-redacted FTS/vector fact indexing, FTS path-query fail-soft handling, runtime-level injection context redaction, hook tool-result redaction before injection context, shared retrieval-context redaction for CLI/MCP/hooks, atomic invalidation effects, supported-by temporal evidence edges, tool-arg redaction before retrieval, hook prompt redaction before retrieval, transcript-tail redaction before retrieval, symlink-safe workspace config reads, symlink-safe local store paths, symlink-safe AGENTS.md boot capsule writes, symlink-safe generic adapter marker writes, symlink-safe OpenCode config writes, symlink-safe Cursor mcp writes, symlink-safe Claude settings writes, workspace-confined CI workflow extraction, workspace-confined agent prose extraction, workspace-confined Docker/Compose extraction, workspace-confined test config extraction, workspace-confined tooling config extraction, workspace-confined tsconfig extraction, workspace-confined editorconfig extraction, workspace-confined runtime version extraction, workspace-confined lockfile extraction, workspace-confined package script evidence, shared realpath workspace evidence checks, symlink-aware injection staleness checks, generated-marker symlink skip, procedure verifier secret scanning, extraction subject secret scanning, hook session-id redaction, MCP session-reference secret refusal, open-loop session metadata secret refusal, explicit memory metadata secret refusal, packageManager canonical facts, package-manager-aware script extraction, runtime-version deterministic extraction, multi-cookie session redaction, MCP error redaction, fail-closed retrieval for unvalidated commit-scoped facts, semantic CLI recall ranking, auth/cookie header secret scanning, scoped semantic retrieval expansion, workspace-confined injection staleness checks, package metadata extraction, high-trust test/Docker/tooling/tsconfig deterministic extraction, high-trust Python toolchain extraction, high-trust pyproject package-manager extraction, pyproject dependency-group Python command extraction, high-entropy redaction hardening, repo-id-isolated temporal validity, measured hook latency, fail-closed adapter install/uninstall, typed CLI error formatting, answer-bearing coding query expansion, Python package-manager retrieval expansion, redacted TUI selected-fact detail panels, TUI page/jump keyboard navigation, composed TUI empty states with operator status surfaces, SessionStart suppression of superseded user preferences, chunk-safe TUI key decoding with left/right and j/k navigation, bounded secret-redacted TUI prompt input, vector-safe commit-hygiene query expansion, searchable TUI memory lists, and tox/nox Python test harness extraction. 301 tests, 19 gate suites green, all I1-I9 hold._ -_Quality counters: Tests: 292. Gate suites: 19._ +_Quality counters: Tests: 301. Gate suites: 19._ _Runtime-clock note: direct CLI, eval, and benchmark retrievers pass `Runtime.clock`, so pull and push retrieval use the same recency clock seam._ @@ -119,4 +119,4 @@ now have explicit scanners instead of relying only on entropy fallback._ | Code quality | ✅ | new `eval quality` passes 6/6: full-repo Biome, strict TS config/scripts, shared command-surface helpers for CLI help/docs/README reachability, eval-suite runner/test coverage, final README docs-as-code, and generated migration packaging guard | _Loop note: composite metric = (failing_gates × 100) + (un-perfected aspects); within-aspect -measured gains are recorded in the `memory` graph. Tests: 292, gate suites: 19 (`eval all` includes run/memory/promote/drift/retrieval/gate/security/branch/temporal/conflict/procedure/mcp/storage/telemetry/provenance/resilience/benchmarks/cli-docs-demo/quality)._ +measured gains are recorded in the `memory` graph. Tests: 301, gate suites: 19 (`eval all` includes run/memory/promote/drift/retrieval/gate/security/branch/temporal/conflict/procedure/mcp/storage/telemetry/provenance/resilience/benchmarks/cli-docs-demo/quality)._ diff --git a/src/cli.ts b/src/cli.ts index 30291e2..f920bf5 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -207,6 +207,9 @@ program subject: opts.subject, predicate: opts.predicate, kind, + // Interactive read-then-write: the CLI resolves the current value at + // write time, so a deliberate update supersedes instead of disputing. + baseSeenFactId: rt.currentFactIdFor(opts.subject, opts.predicate), }); refreshAgentsCapsule(rt); process.stdout.write(`Remembered ${fact.fact_id}: ${renderCard(fact).markdown}\n`); @@ -621,7 +624,7 @@ program const { runParallelConflictEval, formatParallelConflictReport } = await import( "./eval/suites/parallel-conflict.js" ); - const r = runParallelConflictEval(); + const r = await runParallelConflictEval(); process.stdout.write(formatParallelConflictReport(r)); return r.pass; }; diff --git a/src/eval/suites/parallel-conflict.ts b/src/eval/suites/parallel-conflict.ts index 5dae4aa..dcfc630 100644 --- a/src/eval/suites/parallel-conflict.ts +++ b/src/eval/suites/parallel-conflict.ts @@ -28,7 +28,8 @@ import { Runtime } from "../../runtime.js"; // and the adversarial edges (high-vs-high, non-ancestor base, one-sided // branch, base_seen idempotency). // 5. Real Runtime concurrent stress — two durable writers over the same store -// race stale-base writes; outcomes preserve/audit both sides. +// race through the REAL learn() path: contradictions dispute unless the +// writer provably saw the value it replaces; outcomes audit both sides. // 6. Headline metric — silentWrongWinners MUST be 0: an ambiguous contradiction // resolving to `apply` is a silent LWW = catastrophic. @@ -606,59 +607,58 @@ function runReconcile(detail: string[]): { section: SectionResult; silentWrongWi } // ---- section 5: real Runtime concurrent-session stress ---------------------- +// +// Drives the REAL write path (Runtime.learn → classifyRelation, which routes +// equal-precedence user contradictions through reconcileWrite) — not +// reconcileWrite called directly — so the no-silent-LWW guarantee is proven +// where production actually writes. interface DurableWriterSpec { - sessionId: string; object: string; - branch: string; + branch?: string; trustTier?: Fact["trust_tier"]; assertedBy?: Fact["source"]["asserted_by"]; - baseGitHead?: string; - validFromCommit?: string; } interface ConcurrentCase { label: string; first: DurableWriterSpec; second: DurableWriterSpec; - expectSecond: ConcurrencyOutcome["kind"]; + // The second writer read the first writer's fact before writing (optimistic + // concurrency base, SPEC §14) — the only clean supersede at equal rank. + secondSawFirst?: boolean; + expectSecond: "disputed" | "partition" | "supersede"; } const CONCURRENT_CASES: ConcurrentCase[] = [ { - label: "same-branch stale writers dispute", - first: { sessionId: "race-a", object: "pnpm", branch: "main", trustTier: "low" }, - second: { sessionId: "race-b", object: "yarn", branch: "main", trustTier: "low" }, + label: "parallel default remembers (neither saw the other) dispute", + first: { object: "pnpm" }, + second: { object: "yarn" }, expectSecond: "disputed", }, { - label: "branch-disjoint stale writers partition", - first: { sessionId: "race-a", object: "pnpm", branch: "main", trustTier: "low" }, - second: { sessionId: "race-b", object: "yarn", branch: "feat", trustTier: "low" }, + label: "branch-disjoint writers partition", + first: { object: "pnpm", branch: "main" }, + second: { object: "yarn", branch: "feat" }, expectSecond: "partition", }, { - label: "structured stale writer invalidates lower-trust current", - first: { - sessionId: "race-a", - object: "npm", - branch: "main", - trustTier: "low", - validFromCommit: "c0", - }, - second: { - sessionId: "race-b", - object: "pnpm", - branch: "main", - trustTier: "high", - assertedBy: "deterministic_parser", - baseGitHead: "c1", - }, - expectSecond: "invalidate", + label: "an update that saw the current value supersedes cleanly", + first: { object: "pnpm" }, + second: { object: "yarn" }, + secondSawFirst: true, + expectSecond: "supersede", + }, + { + label: "structured evidence supersedes lower-trust prose", + first: { object: "npm", trustTier: "low", assertedBy: "deterministic_parser" }, + second: { object: "pnpm", trustTier: "high", assertedBy: "deterministic_parser" }, + expectSecond: "supersede", }, ]; -function runConcurrentStress(detail: string[]): ConcurrentStressResult { +async function runConcurrentStress(detail: string[]): Promise { let cases = 0; let passed = 0; let silentOverwrites = 0; @@ -666,65 +666,40 @@ function runConcurrentStress(detail: string[]): ConcurrentStressResult { for (const c of CONCURRENT_CASES) { cases += 1; - const result = runConcurrentCase(c); - outcomes[result.secondOutcome] = (outcomes[result.secondOutcome] ?? 0) + 1; + const result = await runConcurrentCase(c); + outcomes[result.outcome] = (outcomes[result.outcome] ?? 0) + 1; if (result.silentOverwrite) silentOverwrites += 1; const ok = - result.firstOutcome === "apply" && - result.secondOutcome === c.expectSecond && - !result.silentOverwrite && - result.loserAuditable; + result.outcome === c.expectSecond && !result.silentOverwrite && result.loserAuditable; if (ok) passed += 1; detail.push( - ` ${ok ? "✓" : "✗"} concurrent writers: ${c.label} → first=${result.firstOutcome} second=${result.secondOutcome} silentOverwrite=${result.silentOverwrite} loserAuditable=${result.loserAuditable}`, + ` ${ok ? "✓" : "✗"} concurrent writers: ${c.label} → outcome=${result.outcome} silentOverwrite=${result.silentOverwrite} loserAuditable=${result.loserAuditable}`, ); } detail.push( - ` ${silentOverwrites === 0 ? "✓" : "✗"} concurrent writers: ${silentOverwrites} silent overwrites across ${cases} real Runtime races`, + ` ${silentOverwrites === 0 ? "✓" : "✗"} concurrent writers: ${silentOverwrites} silent overwrites across ${cases} real learn() races`, ); return { cases, passed, silentOverwrites, outcomes }; } -function runConcurrentCase(c: ConcurrentCase): { - firstOutcome: ConcurrencyOutcome["kind"]; - secondOutcome: ConcurrencyOutcome["kind"]; +async function runConcurrentCase(c: ConcurrentCase): Promise<{ + outcome: string; silentOverwrite: boolean; loserAuditable: boolean; -} { +}> { const dir = mkdtempSync(join(tmpdir(), "graphctx-conflict-")); - const seed = new Runtime({ workspaceDir: dir, userId: "conflict-eval" }); const writerA = new Runtime({ workspaceDir: dir, userId: "conflict-eval" }); const writerB = new Runtime({ workspaceDir: dir, userId: "conflict-eval" }); const inspector = new Runtime({ workspaceDir: dir, userId: "conflict-eval" }); try { - const base = insertRuntimeFact(seed, { - sessionId: "race-base", - object: "base", - branch: "main", - trustTier: "low", - validFromCommit: "c0", - }); - const first = applyDurableWrite(writerA, { - spec: c.first, - baseSeenFactId: base.fact_id, - current: base, - }); - const currentForSecond = currentDurableFact(writerB) ?? first.fact; - const second = applyDurableWrite(writerB, { - spec: c.second, - baseSeenFactId: base.fact_id, - current: currentForSecond, - }); - const audit = auditConcurrentResult(inspector, currentForSecond, second.fact, second.outcome); - return { - firstOutcome: first.outcome.kind, - secondOutcome: second.outcome.kind, - silentOverwrite: audit.silentOverwrite, - loserAuditable: audit.loserAuditable, - }; + const first = await writerA.learn(durableWrite(writerA, c.first)); + const second = await writerB.learn( + durableWrite(writerB, c.second), + c.secondSawFirst ? { baseSeenFactId: first.fact_id } : {}, + ); + return auditConcurrentResult(inspector, first.fact_id, second.fact_id, c); } finally { - seed.close(); writerA.close(); writerB.close(); inspector.close(); @@ -732,139 +707,49 @@ function runConcurrentCase(c: ConcurrentCase): { } } -function applyDurableWrite( - rt: Runtime, - opts: { spec: DurableWriterSpec; baseSeenFactId: string; current: Fact | null }, -): { fact: Fact; outcome: ConcurrencyOutcome } { - const intentFact = runtimeIntentFact(rt, opts.spec); - const outcome = reconcileWrite( - { - base_seen_fact_id: opts.baseSeenFactId, - base_git_head: opts.spec.baseGitHead, - branch: opts.spec.branch, - fact: intentFact, - }, - opts.current, - isAncestor, - ); - const inserted = insertRuntimeFact( - rt, - opts.spec, - outcome.kind === "disputed" ? "disputed" : "active", - ); - applyDurableOutcome(rt, inserted, opts.current, outcome); - return { fact: rt.facts.get(inserted.fact_id) ?? inserted, outcome }; -} - -function insertRuntimeFact( - rt: Runtime, - spec: DurableWriterSpec, - status: Fact["status"] = "active", -): Fact { - return rt.facts.insert({ +function durableWrite(rt: Runtime, spec: DurableWriterSpec): Parameters[0] { + return { subject: "repo", predicate: "package_manager", object: spec.object, fact_kind: "constraint", temporal_kind: "static", - scope: rt.scope(spec.sessionId), - trust_tier: spec.trustTier ?? "low", - status, - promotion_state: "workspace_active", - source: { asserted_by: spec.assertedBy ?? "user", event_ids: [] }, - git: { branch: spec.branch, valid_from_commit: spec.validFromCommit }, - }); -} - -function runtimeIntentFact(rt: Runtime, spec: DurableWriterSpec): Fact { - return fact({ - subject: "repo", - predicate: "package_manager", - object: spec.object, - scope: rt.scope(spec.sessionId), + scope: rt.scope(), + trust_tier: spec.trustTier ?? "high", status: "active", promotion_state: "workspace_active", - trust_tier: spec.trustTier ?? "low", source: { asserted_by: spec.assertedBy ?? "user", event_ids: [] }, - git: { branch: spec.branch, valid_from_commit: spec.validFromCommit }, - }); + git: spec.branch ? { branch: spec.branch } : undefined, + }; } -function applyDurableOutcome( +function auditConcurrentResult( rt: Runtime, - inserted: Fact, - current: Fact | null, - outcome: ConcurrencyOutcome, -): void { - if (!current) return; - switch (outcome.kind) { - case "apply": { - if (objStr(inserted) !== objStr(current)) { - rt.edges.add(inserted.fact_id, "SUPERSEDES", current.fact_id, inserted.fact_id); - rt.edges.add(current.fact_id, "SUPERSEDED_BY", inserted.fact_id, inserted.fact_id); - rt.facts.update(current.fact_id, { - status: "superseded", - invalidated_by: inserted.fact_id, - }); - } - return; - } - case "partition": - return; - case "invalidate": { - rt.edges.add(inserted.fact_id, "INVALIDATES", current.fact_id, inserted.fact_id); - rt.facts.expire(current.fact_id, inserted.fact_id, "c1"); - return; - } - case "disputed": { - rt.edges.add(inserted.fact_id, "CONFLICTS_WITH", current.fact_id, inserted.fact_id); - rt.facts.update(current.fact_id, { - status: "disputed", - contradiction_count: current.contradiction_count + 1, - }); - return; - } - } -} + firstId: string, + secondId: string, + c: ConcurrentCase, +): { outcome: string; silentOverwrite: boolean; loserAuditable: boolean } { + const first = rt.facts.get(firstId); + const second = rt.facts.get(secondId); + const bothPersisted = first != null && second != null; + const edgeKinds = [...rt.edges.touching(firstId), ...rt.edges.touching(secondId)].map( + (e) => e.edge_kind, + ); -function currentDurableFact(rt: Runtime): Fact | null { - const active = rt.facts - .bySubjectPredicate("repo", "package_manager", { user_id: rt.userId }) - .filter((f) => f.status === "active" && f.promotion_state === "workspace_active"); - active.sort((a, b) => { - const byTime = a.time.t_recorded.localeCompare(b.time.t_recorded); - if (byTime !== 0) return byTime; - return a.fact_id.localeCompare(b.fact_id); - }); - return active.at(-1) ?? null; -} + let outcome = "none"; + if (first?.status === "disputed" && second?.status === "disputed") outcome = "disputed"; + else if (first?.status === "superseded" || first?.status === "expired") outcome = "supersede"; + else if (first?.status === "active" && second?.status === "active") outcome = "partition"; -function auditConcurrentResult( - rt: Runtime, - current: Fact, - incoming: Fact, - outcome: ConcurrencyOutcome, -): { silentOverwrite: boolean; loserAuditable: boolean } { - const currentAfter = rt.facts.get(current.fact_id); - const incomingAfter = rt.facts.get(incoming.fact_id); - const bothPersisted = currentAfter !== null && incomingAfter !== null; - const edgeCount = - rt.edges.touching(current.fact_id).length + rt.edges.touching(incoming.fact_id).length; - const loserAuditable = - bothPersisted && - (outcome.kind === "partition" || - currentAfter.status === "expired" || - currentAfter.status === "superseded" || - currentAfter.status === "disputed" || - incomingAfter.status === "disputed" || - edgeCount > 0); - const silentOverwrite = outcome.kind === "apply" || !bothPersisted || !loserAuditable; - return { silentOverwrite, loserAuditable }; + const loserAuditable = bothPersisted && (outcome === "partition" || edgeKinds.length > 0); + const silentOverwrite = + !bothPersisted || !loserAuditable || (outcome === "supersede" && c.expectSecond === "disputed"); + return { outcome, silentOverwrite, loserAuditable }; } // ---- top-level runner ------------------------------------------------------- -export function runParallelConflictEval(): ParallelConflictReport { +export async function runParallelConflictEval(): Promise { const detail: string[] = []; detail.push("[1] precedence ladder (cross-product of all 9 ranks)"); @@ -883,8 +768,8 @@ export function runParallelConflictEval(): ParallelConflictReport { const { section: reconcile, silentWrongWinners } = runReconcile(detail); detail.push(""); - detail.push("[5] real Runtime concurrent-session stress"); - const concurrent = runConcurrentStress(detail); + detail.push("[5] real Runtime concurrent-session stress (live learn() path)"); + const concurrent = await runConcurrentStress(detail); const determinismCases = 2; // stable total order + equal-rank tiebreak const determinismPassed = diff --git a/src/invalidate/invalidator.ts b/src/invalidate/invalidator.ts index 9bb39aa..f9e9be0 100644 --- a/src/invalidate/invalidator.ts +++ b/src/invalidate/invalidator.ts @@ -1,4 +1,6 @@ import type { Fact } from "../core/types.js"; +import { reconcileWrite } from "../resolve/conflicts.js"; +import { precedenceRank } from "../resolve/precedence.js"; import type { EdgesRepo } from "../store/edges.repo.js"; import type { EpisodesRepo } from "../store/episodes.repo.js"; import type { FactsRepo } from "../store/facts.repo.js"; @@ -38,7 +40,10 @@ export class Invalidator { this.llm = deps.llm ?? nullLlmAgent; } - async processIncomingFact(incoming: Fact): Promise { + async processIncomingFact( + incoming: Fact, + opts: { baseSeenFactId?: string } = {}, + ): Promise { const actions: InvalidationAction[] = []; const ctx: RelationContext = { workspaceDir: this.deps.workspaceDir, @@ -53,6 +58,32 @@ export class Invalidator { let verdict = classifyRelation(incoming, existing, ctx); + // `refines` supersedes silently, which is only safe when the writer + // outranks the existing fact or provably saw the value it replaces. Two + // user-asserted facts at the same precedence (the shape two parallel + // sessions' default remembers produce at workspace scope) go through + // reconcileWrite (SPEC §14): without a matching base_seen_fact_id the + // contradiction is disputed and surfaced, never a silent + // last-writer-wins. Deterministic parser facts are exempt — their + // evidence is anchored to the current commit, so a changed value IS the + // newer world state (re-extraction refresh). + if ( + verdict.relation === "refines" && + incoming.source.asserted_by === "user" && + existing.source.asserted_by === "user" && + precedenceRank(incoming, incoming.scope.session_id) === + precedenceRank(existing, incoming.scope.session_id) + ) { + const outcome = reconcileWrite( + { base_seen_fact_id: opts.baseSeenFactId, fact: incoming }, + existing, + () => false, // no ancestor proof on this path; unused for equal-trust pairs + ); + if (outcome.kind === "disputed") { + verdict = { relation: "conflicts", reason: outcome.reason, deterministic: true }; + } + } + // LLM fallback ONLY when deterministic rules abstain. if (!verdict.deterministic && verdict.relation === "unrelated") { const llmRel = await this.consultLlm(incoming, existing); @@ -100,7 +131,11 @@ export class Invalidator { } private apply(incoming: Fact, existing: Fact, relation: Relation): string | null { - return this.deps.facts.transaction(() => this.applyInTransaction(incoming, existing, relation)); + // BEGIN IMMEDIATE: apply is read-modify-write on facts+edges; see + // FactsRepo.transactionImmediate for why deferred is unsafe here. + return this.deps.facts.transactionImmediate(() => + this.applyInTransaction(incoming, existing, relation), + ); } private applyInTransaction(incoming: Fact, existing: Fact, relation: Relation): string | null { diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 4057182..3ae2ae5 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -25,6 +25,10 @@ const rememberInput = z.object({ subject: z.string().default("user"), predicate: z.string().default("note"), session_id: z.string().optional(), + // fact_id last seen for this subject/predicate (from recall/why). With it a + // contradicting value supersedes cleanly; without it, an equal-precedence + // contradiction is disputed and surfaced (SPEC §14, never silent LWW). + base_seen_fact_id: z.string().optional(), }); const recallInput = z.object({ @@ -84,7 +88,14 @@ export const MCP_TOOLS: McpTool[] = [ name: "remember", description: "Store a user-asserted fact/event/procedure in graphCTX memory.", inputSchema: s( - { text: nonEmptyStr, kind: factKindSchema, subject: str, predicate: str, session_id: str }, + { + text: nonEmptyStr, + kind: factKindSchema, + subject: str, + predicate: str, + session_id: str, + base_seen_fact_id: str, + }, ["text"], ), outputSchema: s({ fact_id: str, status: str }, ["fact_id", "status"]), @@ -103,6 +114,7 @@ export const MCP_TOOLS: McpTool[] = [ predicate: a.predicate, kind: a.kind, sessionId: a.session_id, + baseSeenFactId: a.base_seen_fact_id, tags: ["mcp_remember"], }); refreshAgentsCapsule(rt); diff --git a/src/promote/probation.ts b/src/promote/probation.ts index 926cc81..8dc7f2f 100644 --- a/src/promote/probation.ts +++ b/src/promote/probation.ts @@ -50,11 +50,19 @@ export class Probation { workspace_id?: string; session_id?: string; }): SweepResult { - return this.deps.facts.transaction(() => this.sweepSessionToWorkspaceInTransaction(scope)); + // BEGIN IMMEDIATE: two overlapping sweeps (SessionEnd hook vs the MCP + // daemon's checkpoint/promote) on one WAL DB must serialize at BEGIN. A + // deferred tx reads first, and its later writes die with + // SQLITE_BUSY_SNAPSHOT once the other sweep commits — the hook swallows + // that (I9) and the ended session's promotions are lost permanently, + // since no later sweep revisits that session's candidates. + return this.deps.facts.transactionImmediate(() => + this.sweepSessionToWorkspaceInTransaction(scope), + ); } reviewFactForWorkspace(factId: string): PromotionReview | null { - return this.deps.facts.transaction(() => { + return this.deps.facts.transactionImmediate(() => { const fact = this.deps.facts.get(factId); if (!fact) return null; if ( diff --git a/src/runtime.ts b/src/runtime.ts index 7d2edd7..d135b09 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -48,6 +48,11 @@ export interface RememberFactInput { kind?: FactKind; sessionId?: string; tags?: string[]; + // Optimistic-concurrency base (SPEC §14): the fact_id the writer last saw + // for this subject/predicate (from recall/why). With it, a contradicting + // value supersedes cleanly; without it, an equal-precedence contradiction + // is disputed and surfaced instead of silently superseding. + baseSeenFactId?: string; } // Central wiring: opens stores, git, repos, planner. Used by CLI + adapters + @@ -262,11 +267,14 @@ export class Runtime { // Insert a fact AND run invalidation against existing memory (M1). Returns the // inserted fact; invalidation effects (supersede/expire/dispute) are applied // as a side effect and never throw out to the caller (I9). - async learn(input: Parameters[0]): Promise> { + async learn( + input: Parameters[0], + opts: { baseSeenFactId?: string } = {}, + ): Promise> { const fact = this.facts.insert(input); try { const inv = await this.invalidator(); - await inv.processIncomingFact(fact); + await inv.processIncomingFact(fact, opts); } catch { // invalidation must never break a write (I9) } @@ -341,6 +349,26 @@ export class Runtime { }); } + // The active fact a new explicit remember for (subject,predicate) would + // collide with at the same scope level. Interactive writers (CLI, TUI) read + // this at write time and pass it as baseSeenFactId — the read-then-write + // reconcileWrite's optimistic concurrency expects, so a deliberate update + // supersedes cleanly. Agents calling MCP remember pass the fact_id they + // actually retrieved (recall/why) instead; without one, an + // equal-precedence contradiction is disputed, never silently superseded. + currentFactIdFor(subject = "repo", predicate = "note", sessionId?: string): string | undefined { + const current = this.facts + .bySubjectPredicate(subject, predicate, { user_id: this.userId }) + .filter((f) => f.status === "active") + .filter((f) => + sessionId + ? f.scope.session_id === sessionId + : !f.scope.session_id && f.scope.workspace_id === this.workspaceId, + ) + .sort((a, b) => a.time.t_recorded.localeCompare(b.time.t_recorded)); + return current.at(-1)?.fact_id; + } + // Store explicit user memory with the same lifecycle as CLI/MCP/TUI remember: // safe intake, current git anchoring, active trust, and invalidation. async rememberFact(input: RememberFactInput): Promise { @@ -354,20 +382,23 @@ export class Runtime { kind, session_id: input.sessionId, }); - return await this.learn({ - subject, - predicate, - object: input.text, - fact_kind: kind, - temporal_kind: kind === "task_state" || kind === "open_loop" ? "dynamic" : "static", - scope: this.scope(input.sessionId), - trust_tier: "high", - status: "active", - promotion_state: input.sessionId ? "session_only" : "workspace_active", - source: { asserted_by: "user", event_ids: [], raw_quote: `user said: ${input.text}` }, - git: await this.currentGitAnchor(), - tags: input.tags ?? ["user_explicit"], - }); + return await this.learn( + { + subject, + predicate, + object: input.text, + fact_kind: kind, + temporal_kind: kind === "task_state" || kind === "open_loop" ? "dynamic" : "static", + scope: this.scope(input.sessionId), + trust_tier: "high", + status: "active", + promotion_state: input.sessionId ? "session_only" : "workspace_active", + source: { asserted_by: "user", event_ids: [], raw_quote: `user said: ${input.text}` }, + git: await this.currentGitAnchor(), + tags: input.tags ?? ["user_explicit"], + }, + { baseSeenFactId: input.baseSeenFactId }, + ); } // Resolve an open loop so it stops resurfacing (M1 §7). diff --git a/src/store/db.ts b/src/store/db.ts index f743bf7..509f751 100644 --- a/src/store/db.ts +++ b/src/store/db.ts @@ -23,8 +23,22 @@ export function openDb(path: string): DB { } } -// Transaction helper. +// Transaction helper (BEGIN deferred). export function tx(db: DB, fn: () => T): T { const wrapped = db.transaction(fn); return wrapped() as T; } + +// Read-modify-write transaction helper. BEGIN IMMEDIATE takes the writer lock +// up front, so concurrent read-modify-write transactions serialize at BEGIN +// (waiting via busy_timeout) instead of snapshotting on their first read and +// dying mid-transaction with SQLITE_BUSY_SNAPSHOT when another writer commits +// first — busy_timeout does NOT retry snapshot conflicts. +export function txImmediate(db: DB, fn: () => T): T { + const wrapped = db.transaction(fn) as ((...args: unknown[]) => unknown) & { + immediate?: (...args: unknown[]) => unknown; + }; + // Both drivers (better-sqlite3 and bun:sqlite) expose .immediate on the + // wrapped transaction function. + return (wrapped.immediate ? wrapped.immediate() : wrapped()) as T; +} diff --git a/src/store/facts.repo.ts b/src/store/facts.repo.ts index d8fd5f6..de80c4c 100644 --- a/src/store/facts.repo.ts +++ b/src/store/facts.repo.ts @@ -10,7 +10,7 @@ import type { ScoredFact, } from "../core/types.js"; import { redactSecrets, sensitivityForText } from "../security/secrets.js"; -import { type DB, tx } from "./db.js"; +import { type DB, tx, txImmediate } from "./db.js"; interface FactRow { fact_id: string; @@ -170,6 +170,13 @@ export class FactsRepo { return tx(this.db, fn); } + // For read-modify-write transactions (sweep/apply): writer lock at BEGIN, + // so concurrent processes serialize instead of failing mid-transaction with + // SQLITE_BUSY_SNAPSHOT (see txImmediate). + transactionImmediate(fn: () => T): T { + return txImmediate(this.db, fn); + } + // I1: defaults to candidate / session_only unless explicitly overridden. insert(input: NewFact): Fact { const id = factId(); diff --git a/src/tui/app.ts b/src/tui/app.ts index 3b80766..72965cc 100644 --- a/src/tui/app.ts +++ b/src/tui/app.ts @@ -165,10 +165,12 @@ export class TuiApp { this.refresh("refused secret-bearing memory"); return; } - void this.rt.rememberFact({ text: v }).then((f) => { - this.refresh(`remembered ${f.fact_id.slice(-8)}`); - this.draw(); - }); + void this.rt + .rememberFact({ text: v, baseSeenFactId: this.rt.currentFactIdFor() }) + .then((f) => { + this.refresh(`remembered ${f.fact_id.slice(-8)}`); + this.draw(); + }); }); } else if (k.name === "o") { this.ask("New open loop", (v) => { diff --git a/test/eval/m3-suites.test.ts b/test/eval/m3-suites.test.ts index 33cf9b5..5bff58b 100644 --- a/test/eval/m3-suites.test.ts +++ b/test/eval/m3-suites.test.ts @@ -10,8 +10,8 @@ describe("M3 gate suites", () => { expect(r.pass).toBe(true); }); - it("parallel-conflict: no silent last-writer-wins", () => { - const r = runParallelConflictEval(); + it("parallel-conflict: no silent last-writer-wins", async () => { + const r = await runParallelConflictEval(); expect(r.silentWrongWinners).toBe(0); expect(r.pass).toBe(true); }); diff --git a/test/eval/parallel-conflict.test.ts b/test/eval/parallel-conflict.test.ts index d679c90..1ed1c03 100644 --- a/test/eval/parallel-conflict.test.ts +++ b/test/eval/parallel-conflict.test.ts @@ -4,50 +4,51 @@ import { runParallelConflictEval } from "../../src/eval/suites/parallel-conflict // Comprehensive conflict & precedence regression gate (SPEC §14, M3). Proves the // full precedence ladder, deterministic total ordering, resolveConflicts // semantics, and the reconcileWrite matrix all hold — and that NO genuine -// contradiction is ever silently resolved (no last-writer-wins). +// contradiction is ever silently resolved (no last-writer-wins), on the REAL +// learn() write path, not just against reconcileWrite called directly. describe("parallel-conflict & precedence (comprehensive)", () => { - it("passes the full gated verdict with zero silent wrong winners", () => { - const r = runParallelConflictEval(); + it("passes the full gated verdict with zero silent wrong winners", async () => { + const r = await runParallelConflictEval(); expect(r.cases).toBeGreaterThan(0); expect(r.passed).toBe(r.cases); expect(r.silentWrongWinners).toBe(0); expect(r.pass).toBe(true); }); - it("verifies every ordered precedence-rank pair (full cross-product)", () => { - const r = runParallelConflictEval(); + it("verifies every ordered precedence-rank pair (full cross-product)", async () => { + const r = await runParallelConflictEval(); // 9 ranks → C(9,2) = 36 ordered (higher, lower) pairs. expect(r.ladder.pairs).toBe(36); expect(r.ladder.passed).toBe(r.ladder.pairs); }); - it("byPrecedence is a stable total order with deterministic contentKey tiebreak", () => { - const r = runParallelConflictEval(); + it("byPrecedence is a stable total order with deterministic contentKey tiebreak", async () => { + const r = await runParallelConflictEval(); expect(r.determinism.stable).toBe(true); expect(r.determinism.tiebreakByContentKey).toBe(true); }); - it("resolveConflicts semantics and reconcileWrite matrix fully pass", () => { - const r = runParallelConflictEval(); + it("resolveConflicts semantics and reconcileWrite matrix fully pass", async () => { + const r = await runParallelConflictEval(); expect(r.resolve.passed).toBe(r.resolve.cases); expect(r.reconcile.passed).toBe(r.reconcile.cases); }); - it("runs real Runtime concurrent writers without silent overwrites", () => { - const r = runParallelConflictEval(); + it("runs real Runtime concurrent writers through learn() without silent overwrites", async () => { + const r = await runParallelConflictEval(); expect(r.concurrent.cases).toBeGreaterThan(0); expect(r.concurrent.passed).toBe(r.concurrent.cases); expect(r.concurrent.silentOverwrites).toBe(0); expect(r.concurrent.outcomes).toMatchObject({ disputed: 1, partition: 1, - invalidate: 1, + supersede: 2, }); }); - it("is deterministic across runs", () => { - const a = runParallelConflictEval(); - const b = runParallelConflictEval(); + it("is deterministic across runs", async () => { + const a = await runParallelConflictEval(); + const b = await runParallelConflictEval(); expect(b.cases).toBe(a.cases); expect(b.passed).toBe(a.passed); expect(b.silentWrongWinners).toBe(a.silentWrongWinners); diff --git a/test/invalidate/concurrent-writes.test.ts b/test/invalidate/concurrent-writes.test.ts new file mode 100644 index 0000000..48fbcf5 --- /dev/null +++ b/test/invalidate/concurrent-writes.test.ts @@ -0,0 +1,144 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { Runtime } from "../../src/runtime.js"; + +// STATUS §14 advertises "never silent LWW", and reconcileWrite (SPEC §14) +// implements it — but the live write path (learn → classifyRelation) resolved +// two parallel sessions' default remembers as `refines`: the newer fact +// silently superseded the older with no dispute surfaced. These tests pin the +// guard to the REAL path: a contradicting durable write at equal precedence +// only supersedes when the writer provably saw the value it replaces. + +describe("concurrent contradictory durable writes (live path)", () => { + let dir: string; + let rt: Runtime; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "gctx-race-")); + rt = new Runtime({ workspaceDir: dir, userId: "u" }); + }); + + afterEach(() => { + rt.close(); + rmSync(dir, { recursive: true, force: true }); + }); + + it("a contradicting remember that never saw the current value disputes instead of superseding", async () => { + const first = await rt.rememberFact({ + text: "use pnpm for installs", + subject: "repo", + predicate: "package_manager", + kind: "decision", + }); + const second = await rt.rememberFact({ + text: "use yarn for installs", + subject: "repo", + predicate: "package_manager", + kind: "decision", + }); + + const older = rt.facts.get(first.fact_id); + const newer = rt.facts.get(second.fact_id); + expect(older?.status).toBe("disputed"); + expect(newer?.status).toBe("disputed"); + const kinds = rt.edges.touching(first.fact_id).map((e) => e.edge_kind); + expect(kinds).toContain("CONFLICTS_WITH"); + }); + + it("an update that provably saw the current value supersedes cleanly", async () => { + const first = await rt.rememberFact({ + text: "use pnpm for installs", + subject: "repo", + predicate: "package_manager", + kind: "decision", + }); + const second = await rt.rememberFact({ + text: "use yarn for installs", + subject: "repo", + predicate: "package_manager", + kind: "decision", + baseSeenFactId: first.fact_id, + }); + + const older = rt.facts.get(first.fact_id); + expect(older?.status).toBe("superseded"); + expect(older?.time.invalidated_by).toBe(second.fact_id); + expect(rt.facts.get(second.fact_id)?.status).toBe("active"); + }); + + it("an interactive update via currentFactIdFor supersedes like the CLI does", async () => { + const first = await rt.rememberFact({ + text: "use pnpm for installs", + subject: "repo", + predicate: "package_manager", + kind: "decision", + }); + const second = await rt.rememberFact({ + text: "use yarn for installs", + subject: "repo", + predicate: "package_manager", + kind: "decision", + baseSeenFactId: rt.currentFactIdFor("repo", "package_manager"), + }); + + expect(rt.currentFactIdFor("repo", "package_manager")).toBe(second.fact_id); + expect(rt.facts.get(first.fact_id)?.status).toBe("superseded"); + expect(rt.facts.get(second.fact_id)?.status).toBe("active"); + }); + + it("higher-precedence trusted value still supersedes lower-trust prose without a dispute", async () => { + const prose = await rt.learn({ + subject: "repo", + predicate: "package_manager", + object: "npm", + fact_kind: "constraint", + temporal_kind: "static", + scope: { user_id: "u", workspace_id: rt.workspaceId }, + trust_tier: "low", + status: "active", + promotion_state: "workspace_active", + source: { asserted_by: "deterministic_parser", event_ids: [] }, + }); + const decision = await rt.rememberFact({ + text: "pnpm", + subject: "repo", + predicate: "package_manager", + kind: "decision", + }); + + expect(rt.facts.get(prose.fact_id)?.status).toBe("superseded"); + expect(rt.facts.get(decision.fact_id)?.status).toBe("active"); + }); + + it("deterministic re-extraction still refreshes parser facts without a dispute", async () => { + const v1 = await rt.learn({ + subject: "repo", + predicate: "test_command", + object: "npm test", + fact_kind: "procedural", + temporal_kind: "static", + scope: { user_id: "u", workspace_id: rt.workspaceId }, + trust_tier: "high", + status: "active", + promotion_state: "workspace_active", + source: { asserted_by: "deterministic_parser", event_ids: [] }, + }); + const v2 = await rt.learn({ + subject: "repo", + predicate: "test_command", + object: "pnpm test", + fact_kind: "procedural", + temporal_kind: "static", + scope: { user_id: "u", workspace_id: rt.workspaceId }, + trust_tier: "high", + status: "active", + promotion_state: "workspace_active", + source: { asserted_by: "deterministic_parser", event_ids: [] }, + }); + + expect(rt.facts.get(v1.fact_id)?.status).toBe("superseded"); + expect(rt.facts.get(v2.fact_id)?.status).toBe("active"); + }); +}); diff --git a/test/runtime/forget.test.ts b/test/runtime/forget.test.ts index d782bcc..add2f6e 100644 --- a/test/runtime/forget.test.ts +++ b/test/runtime/forget.test.ts @@ -83,7 +83,7 @@ describe("Runtime forget", () => { }); describe("Runtime remember", () => { - it("anchors explicit memory and invalidates older same-key facts", async () => { + it("anchors explicit memory and invalidates older same-key facts the writer saw", async () => { const dir = mkdtempSync(join(tmpdir(), "graphctx-runtime-remember-")); try { execFileSync("git", ["-c", "init.defaultBranch=main", "init", "-q", "."], { @@ -109,11 +109,15 @@ describe("Runtime remember", () => { predicate: "test_command", kind: "procedural", }); + // The updating writer saw the value it replaces (SPEC §14 optimistic + // concurrency) — without baseSeenFactId an equal-precedence + // contradiction is disputed instead of superseded. const second = await rt.rememberFact({ text: "test with pnpm test", subject: "repo", predicate: "test_command", kind: "procedural", + baseSeenFactId: first.fact_id, }); const older = rt.facts.get(first.fact_id); diff --git a/test/store/tx-immediate.test.ts b/test/store/tx-immediate.test.ts new file mode 100644 index 0000000..4964429 --- /dev/null +++ b/test/store/tx-immediate.test.ts @@ -0,0 +1,121 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { Runtime } from "../../src/runtime.js"; +import { type DB, openDb, tx, txImmediate } from "../../src/store/db.js"; + +// Two connections to one WAL database, interleaved in-process. A deferred +// read-modify-write transaction snapshots at its first read; if another +// connection commits a write before the deferred tx's first write, that write +// dies with SQLITE_BUSY_SNAPSHOT — and busy_timeout does NOT retry snapshot +// conflicts. That is the promotion-sweep shape (two SessionEnd hooks, or a +// hook racing the MCP daemon, on one workspace DB): the I9 swallow hides the +// error and the ended session's promotions are lost for good. + +const errCode = (e: unknown): string => (e as { code?: string }).code ?? ""; + +describe("read-modify-write transactions under write contention", () => { + let dir: string; + let a: DB; + let b: DB; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "gctx-txim-")); + const file = join(dir, "workspace.db"); + a = openDb(file); + b = openDb(file); + a.exec("CREATE TABLE IF NOT EXISTS scratch (id INTEGER PRIMARY KEY AUTOINCREMENT, v TEXT)"); + }); + + afterEach(() => { + a.close(); + b.close(); + rmSync(dir, { recursive: true, force: true }); + }); + + it("deferred read-modify-write dies mid-transaction on a snapshot conflict", () => { + let code = ""; + try { + tx(a, () => { + a.prepare("SELECT count(*) AS c FROM scratch").get(); // take the read snapshot + b.prepare("INSERT INTO scratch (v) VALUES ('other')").run(); // concurrent writer commits + a.prepare("INSERT INTO scratch (v) VALUES ('mine')").run(); // deferred upgrade fails + }); + } catch (e) { + code = errCode(e); + } + expect(code).toBe("SQLITE_BUSY_SNAPSHOT"); + }); + + it("txImmediate takes the write lock up front so the in-flight sweep cannot be poisoned", () => { + // With the write lock held from BEGIN, the concurrent writer is the one + // that fails — fast, with plain retryable SQLITE_BUSY at its own boundary — + // while the read-modify-write completes and commits. + b.exec("PRAGMA busy_timeout = 50"); + let concurrentCode = ""; + const result = txImmediate(a, () => { + a.prepare("SELECT count(*) AS c FROM scratch").get(); + try { + b.prepare("INSERT INTO scratch (v) VALUES ('other')").run(); + } catch (e) { + concurrentCode = errCode(e); + } + a.prepare("INSERT INTO scratch (v) VALUES ('mine')").run(); + return "committed"; + }); + expect(result).toBe("committed"); + expect(concurrentCode).toBe("SQLITE_BUSY"); + const rows = a.prepare("SELECT v FROM scratch ORDER BY id").all() as Array<{ v: string }>; + expect(rows.map((r) => r.v)).toEqual(["mine"]); + }); +}); + +describe("read-modify-write call sites run BEGIN IMMEDIATE", () => { + let dir: string; + let rt: Runtime; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "gctx-txim-rt-")); + rt = new Runtime({ workspaceDir: dir, userId: "u" }); + }); + + afterEach(() => { + rt.close(); + rmSync(dir, { recursive: true, force: true }); + }); + + it("the promotion sweep uses the immediate transaction", async () => { + let immediateCalls = 0; + const original = rt.facts.transactionImmediate.bind(rt.facts); + rt.facts.transactionImmediate = (fn) => { + immediateCalls += 1; + return original(fn); + }; + await rt.runPromotionSweep("s1"); + expect(immediateCalls).toBeGreaterThan(0); + }); + + it("the invalidator apply path uses the immediate transaction", async () => { + let immediateCalls = 0; + const original = rt.facts.transactionImmediate.bind(rt.facts); + rt.facts.transactionImmediate = (fn) => { + immediateCalls += 1; + return original(fn); + }; + const first = await rt.rememberFact({ + text: "use pnpm", + subject: "repo", + predicate: "package_manager", + kind: "decision", + }); + await rt.rememberFact({ + text: "use yarn", + subject: "repo", + predicate: "package_manager", + kind: "decision", + baseSeenFactId: first.fact_id, + }); + expect(immediateCalls).toBeGreaterThan(0); + }); +}); From 7790c88f6db7a443a29ac0fce10c0699e8c43e6d Mon Sep 17 00:00:00 2001 From: Josh Kappler Date: Sun, 5 Jul 2026 16:03:03 -0700 Subject: [PATCH 2/3] fix(mcp): resolve_conflict now reaches disputed facts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Equal-precedence contradictory remembers mark both facts disputed, but conflictCandidates only queried active-status facts — the conflict vanished from resolve_conflict the moment it was created and could never be settled through the advertised tool. Add a disputed(scope) repo view and fold disputed facts into the candidate set with the same user/workspace/session scope layering; regression-test the exact MCP handler path. Doc counters now track the live vitest list count (315): the guard in eval cli-docs-demo compares against the live total, and the drifted 297 on main is part of what is failing CI there. --- README.md | 2 +- docs/STATUS.md | 6 +-- src/mcp/tools.ts | 13 +++++ src/store/facts.repo.ts | 10 ++++ test/mcp/resolve-conflict.test.ts | 84 +++++++++++++++++++++++++++++++ 5 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 test/mcp/resolve-conflict.test.ts diff --git a/README.md b/README.md index 4af9349..4ca6d4e 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,7 @@ npx tsx src/cli.ts bench Expected current state: -- `npx vitest run`: 313 tests pass. +- `npx vitest run`: 315 tests pass. - `npm run pack:smoke`: packed tarball installs, runs the demo, and serves MCP from a clean temp app. - `npx tsx src/cli.ts eval all`: 19 gate suites pass. diff --git a/docs/STATUS.md b/docs/STATUS.md index e5e5c3f..01b32b4 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -76,9 +76,9 @@ |---|---|---| | `hook ` p95 | < 150ms | 15.51ms ✅ | -_Last updated: Durable-write concurrency hardened (reconcileWrite now guards the live learn() path so equal-precedence user contradictions dispute instead of silently superseding, remember accepts base_seen_fact_id for clean updates, and read-modify-write transactions run BEGIN IMMEDIATE so overlapping promotion sweeps serialize instead of losing promotions to swallowed SQLITE_BUSY_SNAPSHOT errors), Windows is now a first-class dev platform (homedir-derived user-global paths, named-pipe daemon sockets, shim-free dev-tool spawns in eval suites, forward-slash generated-marker facts, windows-latest CI job), Private-key scanning now covers DSA, encrypted PEM, and PGP private key block headers, MCP `resolve_conflict` now evaluates user, workspace, and current-session facts together, `graphctx resolve` now refuses non-open-loop facts instead of silently superseding durable memory, Storage migration 0006 adds a composite invalidation conflict lookup index, Session-scoped invalidation now keeps identical open loops isolated across sessions while deduping repeats within a session, Demo seed memory now uses Runtime.rememberFact, open-loop session memory now carries current git anchors, CLI `tui --tab` now fails fast on invalid tabs, CLI/MCP fact-kind validation uses a shared `FACT_KINDS` domain enum, TUI promotion uses Runtime/Probation gates instead of direct activation, Runtime/TUI/CLI/MCP explicit remember paths share safe git-anchored invalidating writes, Runtime/TUI/MCP forget paths share git-anchored temporal closeout, MCP forget stamps git closeout anchors, retrieval candidates are scope-filtered before semantic reranking, retrieval candidates receive final workspace/session scope filtering, superseded facts close temporal validity with `t_expired` and commit anchors, retrieval recency ranking uses the runtime clock, ranking/conflict tests use deterministic fact IDs and clocks, vector embedding cache uses the runtime clock, outcome telemetry uses an injectable clock, injection anti-repetition ledger uses the runtime clock, promotion verification timestamps use the runtime clock, promotion gates rescan secret-shaped fact identity, why renders invalidating commit closeout anchors, revert revalidation respects branch representation, revert revalidation skips bad historical anchors, atomic temporal lifecycle closeout anchors, TUI read model redacts secret fact text, conflict notes redact identity/object secrets, structured why redacts evidence chain metadata, structured why redacts fact identity and scope strings, structured why redacts promotion audit text, structured why redacts git anchors, structured why redacts evidence payloads and tags, why output shows expired/invalidated temporal closeout, send-edge safety scans fact tags for secrets, command cards avoid false HEAD verification claims, low-trust claims blocked from PreToolUse guardrail injection, candidate facts kept out of vector search until promotion, lifecycle-synced vector index entries for expired/reactivated facts, secret-redacted semantic rerank text, retriever-level send-unsafe fact suppression, tag-update secret sensitivity restamping, secret-redacted FTS/vector fact indexing, FTS path-query fail-soft handling, runtime-level injection context redaction, hook tool-result redaction before injection context, shared retrieval-context redaction for CLI/MCP/hooks, atomic invalidation effects, supported-by temporal evidence edges, tool-arg redaction before retrieval, hook prompt redaction before retrieval, transcript-tail redaction before retrieval, symlink-safe workspace config reads, symlink-safe local store paths, symlink-safe AGENTS.md boot capsule writes, symlink-safe generic adapter marker writes, symlink-safe OpenCode config writes, symlink-safe Cursor mcp writes, symlink-safe Claude settings writes, workspace-confined CI workflow extraction, workspace-confined agent prose extraction, workspace-confined Docker/Compose extraction, workspace-confined test config extraction, workspace-confined tooling config extraction, workspace-confined tsconfig extraction, workspace-confined editorconfig extraction, workspace-confined runtime version extraction, workspace-confined lockfile extraction, workspace-confined package script evidence, shared realpath workspace evidence checks, symlink-aware injection staleness checks, generated-marker symlink skip, procedure verifier secret scanning, extraction subject secret scanning, hook session-id redaction, MCP session-reference secret refusal, open-loop session metadata secret refusal, explicit memory metadata secret refusal, packageManager canonical facts, package-manager-aware script extraction, runtime-version deterministic extraction, multi-cookie session redaction, MCP error redaction, fail-closed retrieval for unvalidated commit-scoped facts, semantic CLI recall ranking, auth/cookie header secret scanning, scoped semantic retrieval expansion, workspace-confined injection staleness checks, package metadata extraction, high-trust test/Docker/tooling/tsconfig deterministic extraction, high-trust Python toolchain extraction, high-trust pyproject package-manager extraction, pyproject dependency-group Python command extraction, high-entropy redaction hardening, repo-id-isolated temporal validity, measured hook latency, fail-closed adapter install/uninstall, typed CLI error formatting, answer-bearing coding query expansion, Python package-manager retrieval expansion, redacted TUI selected-fact detail panels, TUI page/jump keyboard navigation, composed TUI empty states with operator status surfaces, SessionStart suppression of superseded user preferences, chunk-safe TUI key decoding with left/right and j/k navigation, bounded secret-redacted TUI prompt input, vector-safe commit-hygiene query expansion, searchable TUI memory lists, and tox/nox Python test harness extraction. 313 tests, 19 gate suites green, all I1-I9 hold._ +_Last updated: Durable-write concurrency hardened (reconcileWrite now guards the live learn() path so equal-precedence user contradictions dispute instead of silently superseding, remember accepts base_seen_fact_id for clean updates, and read-modify-write transactions run BEGIN IMMEDIATE so overlapping promotion sweeps serialize instead of losing promotions to swallowed SQLITE_BUSY_SNAPSHOT errors), Windows is now a first-class dev platform (homedir-derived user-global paths, named-pipe daemon sockets, shim-free dev-tool spawns in eval suites, forward-slash generated-marker facts, windows-latest CI job), Private-key scanning now covers DSA, encrypted PEM, and PGP private key block headers, MCP `resolve_conflict` now evaluates user, workspace, and current-session facts together and includes disputed facts so surfaced contradictions stay resolvable through the advertised tool, `graphctx resolve` now refuses non-open-loop facts instead of silently superseding durable memory, Storage migration 0006 adds a composite invalidation conflict lookup index, Session-scoped invalidation now keeps identical open loops isolated across sessions while deduping repeats within a session, Demo seed memory now uses Runtime.rememberFact, open-loop session memory now carries current git anchors, CLI `tui --tab` now fails fast on invalid tabs, CLI/MCP fact-kind validation uses a shared `FACT_KINDS` domain enum, TUI promotion uses Runtime/Probation gates instead of direct activation, Runtime/TUI/CLI/MCP explicit remember paths share safe git-anchored invalidating writes, Runtime/TUI/MCP forget paths share git-anchored temporal closeout, MCP forget stamps git closeout anchors, retrieval candidates are scope-filtered before semantic reranking, retrieval candidates receive final workspace/session scope filtering, superseded facts close temporal validity with `t_expired` and commit anchors, retrieval recency ranking uses the runtime clock, ranking/conflict tests use deterministic fact IDs and clocks, vector embedding cache uses the runtime clock, outcome telemetry uses an injectable clock, injection anti-repetition ledger uses the runtime clock, promotion verification timestamps use the runtime clock, promotion gates rescan secret-shaped fact identity, why renders invalidating commit closeout anchors, revert revalidation respects branch representation, revert revalidation skips bad historical anchors, atomic temporal lifecycle closeout anchors, TUI read model redacts secret fact text, conflict notes redact identity/object secrets, structured why redacts evidence chain metadata, structured why redacts fact identity and scope strings, structured why redacts promotion audit text, structured why redacts git anchors, structured why redacts evidence payloads and tags, why output shows expired/invalidated temporal closeout, send-edge safety scans fact tags for secrets, command cards avoid false HEAD verification claims, low-trust claims blocked from PreToolUse guardrail injection, candidate facts kept out of vector search until promotion, lifecycle-synced vector index entries for expired/reactivated facts, secret-redacted semantic rerank text, retriever-level send-unsafe fact suppression, tag-update secret sensitivity restamping, secret-redacted FTS/vector fact indexing, FTS path-query fail-soft handling, runtime-level injection context redaction, hook tool-result redaction before injection context, shared retrieval-context redaction for CLI/MCP/hooks, atomic invalidation effects, supported-by temporal evidence edges, tool-arg redaction before retrieval, hook prompt redaction before retrieval, transcript-tail redaction before retrieval, symlink-safe workspace config reads, symlink-safe local store paths, symlink-safe AGENTS.md boot capsule writes, symlink-safe generic adapter marker writes, symlink-safe OpenCode config writes, symlink-safe Cursor mcp writes, symlink-safe Claude settings writes, workspace-confined CI workflow extraction, workspace-confined agent prose extraction, workspace-confined Docker/Compose extraction, workspace-confined test config extraction, workspace-confined tooling config extraction, workspace-confined tsconfig extraction, workspace-confined editorconfig extraction, workspace-confined runtime version extraction, workspace-confined lockfile extraction, workspace-confined package script evidence, shared realpath workspace evidence checks, symlink-aware injection staleness checks, generated-marker symlink skip, procedure verifier secret scanning, extraction subject secret scanning, hook session-id redaction, MCP session-reference secret refusal, open-loop session metadata secret refusal, explicit memory metadata secret refusal, packageManager canonical facts, package-manager-aware script extraction, runtime-version deterministic extraction, multi-cookie session redaction, MCP error redaction, fail-closed retrieval for unvalidated commit-scoped facts, semantic CLI recall ranking, auth/cookie header secret scanning, scoped semantic retrieval expansion, workspace-confined injection staleness checks, package metadata extraction, high-trust test/Docker/tooling/tsconfig deterministic extraction, high-trust Python toolchain extraction, high-trust pyproject package-manager extraction, pyproject dependency-group Python command extraction, high-entropy redaction hardening, repo-id-isolated temporal validity, measured hook latency, fail-closed adapter install/uninstall, typed CLI error formatting, answer-bearing coding query expansion, Python package-manager retrieval expansion, redacted TUI selected-fact detail panels, TUI page/jump keyboard navigation, composed TUI empty states with operator status surfaces, SessionStart suppression of superseded user preferences, chunk-safe TUI key decoding with left/right and j/k navigation, bounded secret-redacted TUI prompt input, vector-safe commit-hygiene query expansion, searchable TUI memory lists, and tox/nox Python test harness extraction. 315 tests, 19 gate suites green, all I1-I9 hold._ -_Quality counters: Tests: 313. Gate suites: 19._ +_Quality counters: Tests: 315. Gate suites: 19._ _Runtime-clock note: direct CLI, eval, and benchmark retrievers pass `Runtime.clock`, so pull and push retrieval use the same recency clock seam._ @@ -119,4 +119,4 @@ now have explicit scanners instead of relying only on entropy fallback._ | Code quality | ✅ | new `eval quality` passes 6/6: full-repo Biome, strict TS config/scripts, shared command-surface helpers for CLI help/docs/README reachability, eval-suite runner/test coverage, final README docs-as-code, and generated migration packaging guard | _Loop note: composite metric = (failing_gates × 100) + (un-perfected aspects); within-aspect -measured gains are recorded in the `memory` graph. Tests: 313, gate suites: 19 (`eval all` includes run/memory/promote/drift/retrieval/gate/security/branch/temporal/conflict/procedure/mcp/storage/telemetry/provenance/resilience/benchmarks/cli-docs-demo/quality)._ +measured gains are recorded in the `memory` graph. Tests: 315, gate suites: 19 (`eval all` includes run/memory/promote/drift/retrieval/gate/security/branch/temporal/conflict/procedure/mcp/storage/telemetry/provenance/resilience/benchmarks/cli-docs-demo/quality)._ diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 3ae2ae5..6e0167c 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -245,6 +245,19 @@ function conflictCandidates(rt: Runtime, sessionId?: string): ScoredFact[] { .filter((fact) => !fact.scope.session_id), ); if (sessionId) add(rt.facts.activeAsOf(rt.scope(sessionId))); + // Disputed facts are exactly what this tool exists to settle: the write path + // marks equal-precedence contradictions disputed (SPEC §14), so the resolver + // must see them even though the active-status queries above filter them out. + // Same scope layering: user-global, current workspace, current session. + add( + rt.facts + .disputed({ user_id: rt.userId }) + .filter( + (fact) => + (!fact.scope.workspace_id || fact.scope.workspace_id === rt.workspaceId) && + (!fact.scope.session_id || fact.scope.session_id === sessionId), + ), + ); return [...byId.values()].map((fact) => ({ fact, score: 1 })); } diff --git a/src/store/facts.repo.ts b/src/store/facts.repo.ts index de80c4c..e4d4b9c 100644 --- a/src/store/facts.repo.ts +++ b/src/store/facts.repo.ts @@ -516,6 +516,16 @@ export class FactsRepo { return this.hydrateMany(rows); } + // Disputed facts in scope — the conflict-resolution surface. Active-status + // queries exclude these by design, so resolvers need a dedicated view. + disputed(scope: ScopeFilter): Fact[] { + const { clause, params } = scopeClause(scope); + const rows = this.db + .prepare(`SELECT * FROM facts WHERE status = 'disputed' ${clause}`) + .all(...params) as FactRow[]; + return this.hydrateMany(rows); + } + all(scope: ScopeFilter): Fact[] { const { clause, params } = scopeClause(scope); const rows = this.db diff --git a/test/mcp/resolve-conflict.test.ts b/test/mcp/resolve-conflict.test.ts new file mode 100644 index 0000000..ad1f231 --- /dev/null +++ b/test/mcp/resolve-conflict.test.ts @@ -0,0 +1,84 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { MCP_TOOLS } from "../../src/mcp/tools.js"; +import { Runtime } from "../../src/runtime.js"; + +// PR #9 review regression: equal-precedence contradictory remembers mark BOTH +// facts disputed, but conflictCandidates only queried active-status facts — +// the conflict vanished from resolve_conflict the moment it was created, so +// the surfaced dispute could never be settled through the advertised tool. + +const resolveTool = MCP_TOOLS.find((t) => t.name === "resolve_conflict"); +if (!resolveTool) throw new Error("resolve_conflict tool not registered"); + +interface ResolveOutput { + winners: string[]; + conflicts: Array<{ conflict_id: string; summary: string }>; +} + +describe("mcp resolve_conflict reaches disputed facts", () => { + let dir: string; + let rt: Runtime; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "gctx-resolve-")); + rt = new Runtime({ workspaceDir: dir, userId: "u" }); + }); + + afterEach(() => { + rt.close(); + rmSync(dir, { recursive: true, force: true }); + }); + + it("surfaces a disputed workspace pair as winner + conflict, not {[],[]}", async () => { + const first = await rt.rememberFact({ + text: "use pnpm for installs", + subject: "repo", + predicate: "package_manager", + kind: "decision", + }); + const second = await rt.rememberFact({ + text: "use yarn for installs", + subject: "repo", + predicate: "package_manager", + kind: "decision", + }); + expect(rt.facts.get(first.fact_id)?.status).toBe("disputed"); + expect(rt.facts.get(second.fact_id)?.status).toBe("disputed"); + + const out = (await resolveTool.handler(rt, {})) as ResolveOutput; + const pairWinners = out.winners.filter((id) => id === first.fact_id || id === second.fact_id); + expect(pairWinners).toHaveLength(1); + expect(out.conflicts).toHaveLength(1); + expect(out.conflicts[0]?.summary).toContain("package_manager"); + }); + + it("reaches session-scoped disputes only for the referenced session", async () => { + const first = await rt.rememberFact({ + text: "tabs", + subject: "repo", + predicate: "indent_style", + kind: "decision", + sessionId: "s1", + }); + const second = await rt.rememberFact({ + text: "spaces", + subject: "repo", + predicate: "indent_style", + kind: "decision", + sessionId: "s1", + }); + expect(rt.facts.get(first.fact_id)?.status).toBe("disputed"); + expect(rt.facts.get(second.fact_id)?.status).toBe("disputed"); + + const inSession = (await resolveTool.handler(rt, { session_id: "s1" })) as ResolveOutput; + expect(inSession.conflicts).toHaveLength(1); + expect(inSession.conflicts[0]?.summary).toContain("indent_style"); + + const outsideSession = (await resolveTool.handler(rt, {})) as ResolveOutput; + expect(outsideSession.winners).toHaveLength(0); + expect(outsideSession.conflicts).toHaveLength(0); + }); +}); From e4956383b66fd2a8324ae29ad74d5e0ff08a188b Mon Sep 17 00:00:00 2001 From: Josh Kappler Date: Sun, 5 Jul 2026 16:08:46 -0700 Subject: [PATCH 3/3] fix(test): collect the win32-only home-paths test on every platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit it.runIf excludes the test at collection time, so vitest list returns 303 on linux and 304 on windows for the same tree — no single docs test counter can satisfy the cli-docs-demo counter guard on both CI platforms (main is red on exactly this, tests 297/303 and 297/304). Runtime ctx.skip keeps collection platform-independent; the body still only runs on Windows, where the USERPROFILE redirect exists. --- test/adapters/home-paths.test.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/adapters/home-paths.test.ts b/test/adapters/home-paths.test.ts index 797964a..6f0bfc6 100644 --- a/test/adapters/home-paths.test.ts +++ b/test/adapters/home-paths.test.ts @@ -45,9 +45,13 @@ describe("user-global paths derive from os.homedir(), not $HOME", () => { expect(p).toBe(join(homedir(), ".codex", "skills", "graphctx", "SKILL.md")); }); - it.runIf(process.platform === "win32")( - "global claude hook install lands in the user profile when HOME is unset", - () => { + // Runtime skip (not runIf): a collection-time exclusion makes `vitest list` + // platform-dependent, so no single docs test counter can match the live + // count on both CI platforms. The USERPROFILE redirect only exists on + // Windows, so the body still must not run elsewhere. + it("global claude hook install lands in the user profile when HOME is unset", (ctx) => { + if (process.platform !== "win32") return ctx.skip(); + { const fakeProfile = mkdtempSync(join(tmpdir(), "gctx-profile-")); try { // biome-ignore lint/performance/noDelete: assigning undefined stores the string "undefined" in process.env @@ -66,6 +70,6 @@ describe("user-global paths derive from os.homedir(), not $HOME", () => { } finally { rmSync(fakeProfile, { recursive: true, force: true }); } - }, - ); + } + }); });