Issue #13 / audit M-2: every replay batch since 2026-05-12T18:48:47.211Z
failed with SQLITE_CONSTRAINT_FOREIGNKEY (code 19). The synthetic
`memory-trigger-<int>` TurnID that `AppDelegateMemoryReplaySink`
generated had no matching row in the replay-DB's `turns` table, so the
FK on `events.turn_id REFERENCES turns(turn_id)` rejected every memory
emission. Because `flushPending` wrapped the entire batch in one
transaction, the poisoned event took every real event in the same
batch down on rollback. The log re-buffered the same batch, retried,
failed again, and at 3 consecutive failures escalated to CRITICAL.
By end of day batch_size was 31+ and consecutive_failures was 23+.
Real turn events (text_delta, tool_call_requested, turn_end) queued
behind the poison were silently lost.
Two-layer fix per audit recommendation:
LAYER 1 — Stop synthesizing fake TurnIDs.
- `AppDelegate` now owns a per-process `memorySentinelTurnID: TurnID`
generated at delegate construction.
- `installAgent` writes the matching row into the replay-DB `turns`
table immediately after `beginSession` (ended_at stays NULL for the
life of the session — sentinel turns never end).
- `AppDelegateMemoryReplaySink` now carries the sentinel and attributes
every memory event to it. The Int64 `triggerTurnId` from the Memory
module is no longer turned into a TurnID stub; it still rides in the
JSON payload for forensic queries.
LAYER 2 — Batch-splitting defense in `ReplayLog.flushPending`.
- When the batch transaction fails, retry each event in its OWN
transaction. Good rows persist; only the actually-bad rows are
dropped, with their turn_ids logged at error level.
- If EVERY row also fails individually, fall back to the pre-fix
behavior (re-buffer at head, count toward chronic escalation). This
preserves ME-05 durability for genuine I/O faults.
- If ANY row persisted, reset the consecutive-failure counter — the
log isn't stuck, just dropping a specific poisoned shape.
Tests:
- `test_L10_batchSplitOnFKFailurePreservesOtherEvents`: stage 3 good
events + 1 poison + 1 turn_end against a turn that has no real row,
endTurn flushes, assert the 4 good events persist and the poison
row is absent. Regression guard against ever taking 30 events down
for one bad turn_id again.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Issue #15 / audit M-4: `MemoryExtractionOrchestrator.process` hashed
`TurnID.rawValue` (a UUID string) with FNV-1a and stored the Int64
result in `facts.source_turn_id`. After the FK was dropped in `a5999e2`,
inserts succeeded — but the value joined nothing in the `turns` table.
Live DB pre-fix: `SELECT source_turn_id FROM facts;` → `7747877241524896942`
(an FNV hash). No row in `turns` had `id=7747877241524896942`. Anyone
trying to debug "which turn caused this fact?" via SQL got blank rows.
Audit option 1 (drop the hash) chosen over option 2 (thread the real
rowid through appendTurn → flushPair → ExtractionJob): smallest surgical
change. Option 2 remains available as a future enhancement.
Changes:
- Dropped `stableHash(_ s: String) -> Int64` from
`MemoryExtractionOrchestrator`.
- Orchestrator now passes `0` as a sentinel `sourceTurnId` meaning "no
correlated turn rowid." Fact-to-turn correlation is via `valid_from`
timestamp proximity until / unless the real rowid is threaded.
- Schema comment on `facts.source_turn_id` rewritten to acknowledge the
column is effectively unused, and to document the trade-off rather
than the stale FNV-hash hand-wave.
Tests: `testProcessUsesSentinelSourceTurnIdNotFNVHash` asserts the
orchestrator passes 0 — not a giant FNV hash — for a realistic
UUID-shaped TurnID. Regression guard against re-introducing the hash.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Issue #12 / audit M-1: `facts_vec` was declared in the schema but no
producer ever wrote into it. The hybrid search SQL's `vc` CTE returned
zero rows on every call, and RRF degenerated to FTS5-only ranking.
Semantic recall ("dog" → "Toby" via embedding similarity) silently
didn't work despite vec0 being loaded (v0.1.6) and the embedder client
(`OllamaEmbeddingClient`) being healthy.
Live evidence pre-fix: `SELECT COUNT(*) FROM facts` → 1, but
`SELECT COUNT(*) FROM facts_vec_rowids` → 0.
`MemoryStore.init` now accepts an optional `embedder: any
EmbeddingProviding?`. When non-nil, `insertNewFact` runs the embedder
against the subject/predicate/object triple AFTER the facts row is
committed and writes a matching `facts_vec` row. Embedder failures and
SQLite errors during the vec insert are logged but never roll back the
facts row, per audit recommendation: "log + leave the FTS row (degraded
but searchable) rather than rolling back the fact row."
`applyOp` is now `async throws` (was `throws`); all call sites already
used `try await` because `MemoryStore` is an actor.
Wiring change in `AppDelegate.installMemory`: the existing
`OllamaEmbeddingClient` (previously constructed only for `HybridSearch`)
is now constructed FIRST and shared between the writer side (MemoryStore
facts_vec inserts) and the reader side (HybridSearch). Same vector
space on both sides; if embedder init fails we degrade gracefully —
store still opens, search_memory just isn't registered.
Tests: `MemoryStoreVecWriteTests` covers three cases — embedder
injected (vec row present), no embedder (vec row absent, fact persists),
embedder throws (vec row absent, fact persists, no rollback).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Issue #16 / audit M-5: `MemoryQueries.hybridSearchSQL` binds the raw
user query directly to `facts_fts MATCH ?`. FTS5's MATCH grammar accepts
only terms, AND/OR/NOT/NEAR, column filters, and phrase quoting — raw
natural language with `?`, `'`, `:`, `-`, `*`, or other reserved
characters throws `fts5: syntax error` at the SQLite layer. Live probe:
sqlite3 jarvis.db "SELECT rowid FROM facts_fts
WHERE facts_fts MATCH 'what is my dog name?'"
→ Error: stepping, fts5: syntax error near "?"
Today's session showed zero `search_memory` calls (preamble gap, #14),
so the bug never surfaced live — but the moment that fix lands every
natural-language query with punctuation would have failed.
Adds `MemoryQueries.sanitizeFTS5Query` — strips non-alphanumeric chars,
splits on whitespace, double-quotes each token. Turns
`"what is my dog's name?"` into `"what" "is" "my" "dog" "s" "name"` —
a FTS5-safe phrase-OR query. Short-circuits to empty results when no
usable tokens remain (binding an empty MATCH is itself a syntax error).
`HybridSearch.searchFacts` calls the sanitizer before binding to MATCH.
Embedding still runs on the raw query so semantic recall isn't degraded
by the FTS-safe rewrite — the vec0 cohort sees natural language while
the FTS cohort sees the sanitized form.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Issue #14 / audit M-3: agent never called `search_memory` in production
because the preamble enumerated 7 introspection tools by name but omitted
both memory tools. Anthropic's tools array is canonical, but the model's
preference is shaped by the preamble's explicit "always prefer calling
those tools" rule — without a name in the preamble, search_memory fell
into the bin of "tools the agent technically has access to but won't
think to use." Today's session reproduced it: user asked "what is my
dog's name?" and the agent answered "I don't have anything in memory
about your dog" without calling the tool. Zero `searchFacts:` log lines
across all sessions.
Adds search_memory + forget_fact to the named-tool list with the cue from
SearchMemoryTool.toolDescription: "When the user references something
they may have told you before, or asks 'do you remember…', call
search_memory FIRST. Never claim to remember without searching."
Preamble length stays well under the 4096-char cache-eligibility
boundary; the existing `test_preambleDoesNotEnableCacheUnintentionally`
still passes. New regression guard `test_preambleNamesMemoryTools`
asserts both tools are named.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
james.maesandClaude Opus 4.7