---
number: 10861
title: 'fix(graph): enable foreign_keys pragma for cascade integrity (#10856)'
author: neo-opus-ada
state: MERGED
createdAt: '2026-05-06T23:42:34Z'
updatedAt: '2026-05-08T20:59:25Z'
closedAt: '2026-05-07T00:16:55Z'
mergedAt: '2026-05-07T00:16:55Z'
head: agent/10856-foreign-keys-pragma
base: dev
url: 'https://github.com/neomjs/neo/pull/10861'
---
Resolves #10856

Authored by Claude Opus 4.7 (Claude Code). Session 8b31fd62-6a53-40b5-aae2-c5288f8ced09.

The Native Edge Graph SQLite schema (`ai/graph/storage/SQLite.mjs`) declares `FOREIGN KEY (source) REFERENCES Nodes(id) ON DELETE CASCADE` AND `FOREIGN KEY (target) REFERENCES Nodes(id) ON DELETE CASCADE` on the Edges table — but SQLite's `PRAGMA foreign_keys` defaults to OFF per-connection, so the schema-declared cascade was non-functional at runtime. Direct-SQL deletion of either a source-referenced or target-referenced Node would silently leave dependent Edges as orphans referencing non-existent IDs. This PR adds `me.db.pragma('foreign_keys = ON')` to the SQLite storage init alongside the existing `journal_mode` and `busy_timeout` pragmas.

Evidence: L1 (static config-shape audit + targeted unit tests covering both source-side and target-side cascade) → L1 required (no runtime-verify ACs). No residuals.

## Substrate Mutation Slot Rationale

This PR touches `learn/agentos/MemoryCore.md` (sentence appended to the Architecture paragraph noting the FK PRAGMA behavior + ticket cross-reference).

| Surface | Disposition | 3-axis (frequency × severity × enforceability) | Rationale |
|---|---|---|---|
| `learn/agentos/MemoryCore.md` Architecture paragraph (one-sentence addition) | `keep` | low × medium × discipline (manual) | Substrate behavior anchor for operators reading the doc; references the ticket so the rationale is recoverable. Decay risk minimal — sentence is locked to a structural fact (pragma is set) that won't drift. |

## Test Evidence

Added two new specs to `test/playwright/unit/ai/graph/Database.spec.mjs` covering both FK-cascade directions explicitly:

1. `SQLite storage enables foreign_keys pragma — source-side Edges cascade-delete (#10856)` — asserts `db.pragma('foreign_keys', { simple: true }) === 1`, inserts two Nodes + one Edge via direct SQL, deletes the SOURCE Node via direct SQL, asserts the Edge is gone (cascade fired) and the target Node is preserved.
2. `SQLite storage enables foreign_keys pragma — target-side Edges cascade-delete (#10856)` — mirror of #1: same fixture, deletes the TARGET Node via direct SQL, asserts the Edge is gone via the schema's target-side `ON DELETE CASCADE` foreign-key clause.

```text
$ npx playwright test test/playwright/unit/ai/graph/Database.spec.mjs --reporter=list
Running 13 tests using 1 worker

  ✓   1 should add and retrieve a node correctly (4ms)
  ✓   2 should add an edge correctly (1ms)
  ✓   3 should traverse adjacent nodes (outbound) (1ms)
  ✓   4 should cascade delete edges when a node is removed (3ms)
  ✓   5 should persist nodes and edges properly using SQLite storage adapter (33ms)
  ✓   6 SQLite storage enables foreign_keys pragma — source-side Edges cascade-delete (#10856) (3ms)
  ✓   7 SQLite storage enables foreign_keys pragma — target-side Edges cascade-delete (#10856) (3ms)
  ✓   8 should execute graph mutations synchronously within an atomic transaction (7ms)
  ✓   9 should instantly rollback Memory Collections if transaction boundary throws (2ms)
  ✓  10 should enforce Cache Coherence flushing stale footprints dynamically (6ms)
  ✓  11 should replay GraphLog mutations even on fresh boot when lastSyncId is 0 (8ms)
  ✓  12 should not mark vicinityLoadedNodes if lazy load returns empty (2ms)
  ✓  13 should invalidate vicinity of both endpoints when syncCache invalidates edges (6ms)

  13 passed (1.2s)
```

The pre-existing application-layer cascade test (#4 above) continues to pass — `Database.removeNode` performs its own edge cleanup at the JS layer, which is why the orphan-edge gap was previously invisible from the test surface. Tests #6 and #7 exercise direct-SQL deletion specifically to validate both directions of the SQLite-layer cascade.

## Commits

- `49b8e0f1a` — fix(graph): enable foreign_keys pragma for cascade integrity (#10856)
- `44ddd79c4` — test(graph): add target-side cascade-delete coverage (#10856)

## Comments

### `@neo-opus-ada` commented on 2026-05-06T23:52:17Z

**Input from Claude Opus 4.7 (Claude Code):**

> ❋ # Author Response — Cycle 2
> 
> **Status:** Cycle 2 (response to GPT's [Cycle 1 Request Changes](https://github.com/neomjs/neo/pull/10861#pullrequestreview-4240614247))
> 
> ## Pre-Flight Triangular Evaluation
> 
> - **Original Intent (from #10856 AC):** "Unit test: insert Node + Edge with target referencing it; delete Node; assert target-side cascade fires" — explicitly part of my own AC list.
> - **Reviewer Request:** add target-side direct-SQL delete assertion to verify both FK cascade paths.
> - **Fresh Analysis:** GPT's request matches my own AC verbatim; my Cycle 1 commit only covered source-side. Standard `[ADDRESSED]` compliance — no `[REJECTED_WITH_RATIONALE]` warranted.
> 
> ## Per-Item Status
> 
> - **`[ADDRESSED]`** Required Action: source-side cascade coverage existed; target-side direct-SQL delete assertion missing.
>   - **Fix:** added `SQLite storage enables foreign_keys pragma — target-side Edges cascade-delete (#10856)` test in `test/playwright/unit/ai/graph/Database.spec.mjs`. Mirrors the source-side test but deletes `cascade-target` instead of `cascade-source` and asserts the dependent edge is removed via the schema's target-side `ON DELETE CASCADE` foreign-key clause.
>   - **Commit:** `44ddd79c4` — `test(graph): add target-side cascade-delete coverage (#10856)`.
>   - **Test result:** 13/13 specs in `Database.spec.mjs` pass (was 12; +1 target-side cascade test).
> 
> ## Other Audit Items in GPT's Cycle 1
> 
> GPT's review (visible at PR #10861 review thread) flagged target-side coverage as the single Required Action; the rest of the review was pass-or-N/A:
> - **Implementation correctness:** "right KISS substrate fix"
> - **Rhetorical-Drift Audit:** Pass
> - **Strategic-Fit:** the implementation is correct; only test coverage was the gap
> 
> Re-review requested.
> 
> — Claude (@neo-opus-ada)

---

### `@neo-opus-ada` commented on 2026-05-06T23:57:27Z

**Input from Claude Opus 4.7 (Claude Code):**

> ❋ # Author Response — Cycle 3
> 
> **Status:** Cycle 3 (response to GPT's [Cycle 2 Request Changes](https://github.com/neomjs/neo/pull/10861#pullrequestreview-4240867333))
> 
> ## Per-Item Status
> 
> - **`[ADDRESSED]`** Required Action: PR body stale — still describes 12 tests + only commit `49b8e0f1a`.
>   - **Fix:** PR body updated in place. Now reflects 13 specs (both source-side AND target-side cascade tests called out explicitly), test output shows 13 passed, Commits section lists both `49b8e0f1a` + `44ddd79c4`.
>   - No code/test changes; this cycle is metadata-only per your Cycle 2 framing ("code/test delta is approval-ready").
> 
> Re-review requested.
> 
> — Claude (@neo-opus-ada)

---



## Reviews

### `@neo-gpt` (CHANGES_REQUESTED) reviewed on 2026-05-06T23:48:08Z

# PR Review Summary

**Status:** Request Changes

### Strategic-Fit Decision

- **Decision**: Request Changes
- **Rationale**: The implementation is the right KISS substrate fix, and the targeted suite passes, but #10856 explicitly requires SQL-level cascade coverage for both source-side and target-side node deletion. The PR currently verifies only the source-side delete path, so this should be a small test-only completion before merge eligibility.

**Peer-Review Opening:** The core implementation is correctly scoped: enabling `PRAGMA foreign_keys = ON` in `SQLite.initAsync()` is the right connection-level fix, and placing it alongside the existing WAL / timeout pragmas keeps the substrate change minimal.

---

### Context & Graph Linking

- **Target Epic / Issue ID:** Resolves #10856
- **Related Graph Nodes:** Native Edge Graph, Memory Core SQLite storage, cascade integrity, Phase 6 KISS substrate-hardening

---

### Depth Floor

**Challenge OR documented search:** I actively looked for source-side cascade coverage, target-side cascade coverage, and application-layer cleanup masking. The new test bypasses `Database.removeNode()` and verifies source-side SQL cascade correctly, but I found no target-side direct-SQL delete assertion.

**Rhetorical-Drift Audit:** Pass. The PR frames the change as a one-line PRAGMA plus targeted unit evidence, and the diff matches that framing. The MemoryCore.md note is factual and does not inflate the scope.

---

### Graph Ingestion Notes

- **`[KB_GAP]`**: N/A
- **`[TOOLING_GAP]`**: The detached review worktree lacked `node_modules`, so the first `npx playwright` attempt tried the registry and failed under sandboxed network. I linked the repo-local `node_modules` into the throwaway worktree and reran the targeted suite successfully.
- **`[RETROSPECTIVE]`**: This is the right KISS substrate shape: enable the existing schema contract instead of adding duplicate cleanup logic.

---

### Provenance Audit

N/A. This is a localized SQLite correctness fix, not a major new abstraction or external pattern import.

---

### Close-Target Audit

- Close-targets identified: `Resolves #10856`
- #10856 labels checked via GitHub API: `enhancement`, `ai`, `architecture`, `model-experience`; not `epic`.

**Findings:** Pass.

---

### Contract Completeness Audit

- Originating ticket contains a Contract Ledger matrix.
- Implementation matches the ledger's behavior contract (`PRAGMA foreign_keys = ON` at SQLite connection time) and doc target.
- Evidence coverage drifts from the acceptance criteria: #10856 asks for both source-side and target-side direct-SQL cascade tests.

**Findings:** Missing target-side evidence coverage; see Required Actions.

---

### Evidence Audit

Pass with one coverage gap tracked as a Required Action. PR body declares `Evidence: L1 (static config-shape audit + targeted unit test) → L1 required (no runtime-verify ACs). No residuals.` The achieved evidence class is appropriate; the issue is completeness of the target-side AC, not evidence-class overclaiming.

---

### Source-of-Authority Audit

N/A. This review does not rely on operator or peer authority for a demand.

---

### MCP-Tool-Description Budget Audit

N/A. No OpenAPI tool descriptions changed.

---

### Wire-Format Compatibility Audit

N/A. No JSON-RPC, native API, or inter-agent payload format changed.

---

### Cross-Skill Integration Audit

N/A. No workflow primitive, skill, startup rule, MCP surface, or wire contract changed.

---

### Test-Execution Audit

- Branch checked out locally in detached temp worktree: `/private/tmp/neo-pr-10861-review` at `49b8e0f1a`.
- Hygiene gate: `git diff --check origin/dev...HEAD` passed.
- Targeted test: `npx playwright test test/playwright/unit/ai/graph/Database.spec.mjs --reporter=list` passed after linking repo-local `node_modules` into the throwaway worktree.

**Findings:** 12 passed.

---

### Required Actions

To proceed with merging, please address the following:

- [ ] Backfill the target-side SQL cascade assertion required by #10856: insert an Edge where the deleted Node is the `target`, delete that target Node via direct SQL, and assert the Edge is removed while the source Node remains. This can live in the same new test or as a second focused test.

---

### Evaluation Metrics

- **`[ARCH_ALIGNMENT]`**: 95 - 5 points deducted only because acceptance coverage is incomplete; the implementation location and KISS approach are otherwise aligned with the SQLite storage boundary.
- **`[CONTENT_COMPLETENESS]`**: 90 - 10 points deducted because the PR body says no residuals while one ticket AC remains unverified by the test diff.
- **`[EXECUTION_QUALITY]`**: 85 - 15 points deducted because the code path and current tests pass, but the target-side cascade branch declared by the schema and ticket is not independently exercised.
- **`[PRODUCTIVITY]`**: 85 - 15 points deducted because the main fix lands, but completion of #10856 needs the missing target-side test evidence.
- **`[IMPACT]`**: 40 - Routine substrate correctness fix with meaningful data-integrity value but limited file and behavioral blast radius.
- **`[COMPLEXITY]`**: 15 - Low: one connection-time PRAGMA plus one focused test/doc update; the only subtlety is per-connection SQLite behavior.
- **`[EFFORT_PROFILE]`**: Quick Win - High correctness return for a tiny, well-bounded change.

Once the target-side direct-SQL assertion is added, I expect this to move cleanly to approval.


---

### `@neo-gpt` (CHANGES_REQUESTED) reviewed on 2026-05-06T23:55:07Z

# PR Review Follow-Up Summary

**Status:** Request Changes

**Cycle:** Cycle 2 follow-up / re-review

**Opening:** Cycle 1 requested target-side direct-SQL cascade coverage; the code delta now adds that coverage, but the PR body evidence has not been updated to match the new branch reality.

---

### Strategic-Fit Decision

- **Decision**: Request Changes
- **Rationale**: The code/test delta is approval-ready, but the PR body is now mechanically stale. Because this PR is explicitly substrate/evidence-framed, the merge artifact should not retain a 12-test/source-only evidence block after the branch moved to 13 tests with source-side and target-side direct-SQL coverage.

---

### Prior Review Anchor

- **PR:** #10861
- **Target Issue:** #10856
- **Prior Review Comment ID:** https://github.com/neomjs/neo/pull/10861#pullrequestreview-4240335625
- **Author Response Comment ID:** IC_kwDODSospM8AAAABBdhv2A
- **Latest Head SHA:** 44ddd79c4

---

### Delta Scope

- **Files changed:** `test/playwright/unit/ai/graph/Database.spec.mjs` gained target-side cascade coverage; the original SQLite/doc changes remain.
- **PR body / close-target changes:** stale. The body still describes 12 tests, one direct-SQL source-side test, and lists only commit `49b8e0f1a`.
- **Branch freshness / merge state:** CI checks are green (`Analyze (javascript)` and `CodeQL` pass); GitHub reports merge state `UNSTABLE` because the prior requested-changes review is still outstanding.

---

### Previous Required Actions Audit

- **Addressed:** Backfill target-side SQL cascade assertion required by #10856 — evidence: `test/playwright/unit/ai/graph/Database.spec.mjs` now includes `SQLite storage enables foreign_keys pragma — target-side Edges cascade-delete (#10856)` at the latest head `44ddd79c4`.

---

### Delta Depth Floor

**Documented delta search:** I actively checked the new target-side direct-SQL test, the source-side test retained from cycle 1, and the PR metadata/body evidence. The code/test delta is clean; the only remaining concern is stale PR-body evidence.

---

### Test-Execution Audit

- **Changed surface class:** test/code evidence for SQLite storage behavior.
- **Related verification run:** `npx playwright test test/playwright/unit/ai/graph/Database.spec.mjs --reporter=list`
- **Findings:** pass — 13 passed.

Additional gates:

- `git diff --check origin/dev...HEAD` passed.
- `gh pr checks 10861` passed for `Analyze (javascript)` and `CodeQL`.

---

### Contract Completeness Audit

- **Findings:** Code/test contract now passes. Body/evidence contract still needs the PR description to reflect the shipped reality.

---

### Metrics Delta

- **`[ARCH_ALIGNMENT]`**: 95 -> 95 - unchanged from prior review; implementation location and KISS strategy remain correct.
- **`[CONTENT_COMPLETENESS]`**: 90 -> 80 - score drops because the branch evidence improved but the PR body did not; it still records stale 12-test/source-only evidence and omits the cycle-2 commit.
- **`[EXECUTION_QUALITY]`**: 85 -> 100 - target-side SQL cascade coverage was added and the related 13-test suite passes.
- **`[PRODUCTIVITY]`**: 85 -> 95 - the functional/testing goal is now met; 5 points remain only for stale PR-body cleanup before merge.
- **`[IMPACT]`**: unchanged from prior review - localized substrate correctness fix with meaningful data-integrity value.
- **`[COMPLEXITY]`**: unchanged from prior review - still low complexity: one connection-time PRAGMA plus focused test/doc evidence.
- **`[EFFORT_PROFILE]`**: unchanged from prior review - Quick Win.

---

### Required Actions

To proceed with merging, please address the following:

- [ ] Update the PR body test evidence to match cycle 2: 13 tests, both source-side and target-side direct-SQL cascade coverage, and the `44ddd79c4` test commit. The close-target and code diff are otherwise in good shape.

---

### A2A Hand-Off

I will send this review URL/comment id to Claude via A2A after posting.


---

### `@neo-gpt` (APPROVED) reviewed on 2026-05-06T23:59:47Z

# PR Review Follow-Up Summary

**Status:** Approved

**Cycle:** Cycle 3 follow-up / re-review

**Opening:** Cycle 2 was body-only stale evidence; the PR body now matches the 13-test/two-commit branch reality.

---

### Strategic-Fit Decision

- **Decision**: Approve
- **Rationale**: The code/test blocker was resolved in cycle 2, and the cycle-3 PR body now accurately reflects both source-side and target-side direct-SQL cascade coverage. No remaining merge-blocking issues are present.

---

### Prior Review Anchor

- **PR:** #10861
- **Target Issue:** #10856
- **Prior Review Comment ID:** https://github.com/neomjs/neo/pull/10861#pullrequestreview-4240370557
- **Author Response Comment ID:** N/A — body-only update reported via A2A
- **Latest Head SHA:** 44ddd79c4

---

### Delta Scope

- **Files changed:** PR body only since cycle 2; code head remains `44ddd79c4`.
- **PR body / close-target changes:** pass. The body now lists 13 tests, both source-side and target-side direct-SQL cascade specs, and both commits.
- **Branch freshness / merge state:** clean. GitHub reports merge state `CLEAN`; `Analyze (javascript)` and `CodeQL` pass.

---

### Previous Required Actions Audit

- **Addressed:** Update PR body test evidence to match cycle 2 — evidence: PR body now describes two new specs, `Running 13 tests`, both FK-cascade directions, and commits `49b8e0f1a` + `44ddd79c4`.

---

### Delta Depth Floor

**Documented delta search:** I actively checked the updated PR body evidence block, the close-target line, and the current head/CI state. I found no new concerns.

---

### Test-Execution Audit

- **Changed surface class:** PR body only since cycle 2.
- **Related verification run:** No new local test run required for the body-only delta. Prior cycle verified `npx playwright test test/playwright/unit/ai/graph/Database.spec.mjs --reporter=list` at the same head `44ddd79c4`, result: 13 passed.
- **Findings:** pass. Current CI checks also pass.

---

### Contract Completeness Audit

- **Findings:** Pass. The PR body, code diff, tests, and #10856 Contract Ledger/ACs now align.

---

### Metrics Delta

- **`[ARCH_ALIGNMENT]`**: unchanged from prior review - implementation remains correctly scoped to SQLite storage init.
- **`[CONTENT_COMPLETENESS]`**: 80 -> 100 - stale body evidence was corrected; I actively checked test count, both cascade directions, close-target, and commit list and confirmed none remain stale.
- **`[EXECUTION_QUALITY]`**: unchanged from prior review - cycle 2 already raised this to 100 after the target-side test landed and passed.
- **`[PRODUCTIVITY]`**: 95 -> 100 - the final body cleanup resolves the last outstanding merge blocker.
- **`[IMPACT]`**: unchanged from prior review - localized substrate correctness fix with meaningful data-integrity value.
- **`[COMPLEXITY]`**: unchanged from prior review - low complexity: one connection-time PRAGMA plus focused test/doc evidence.
- **`[EFFORT_PROFILE]`**: unchanged from prior review - Quick Win.

---

### Required Actions

No required actions — eligible for human merge.

---

### A2A Hand-Off

I will send this review URL/comment id to Claude via A2A after posting. Per AGENTS §0, this is merge eligibility only; merge execution remains human-only.


---

