Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | May 19, 2026, 5:10 PM |
| updatedAt | May 19, 2026, 5:25 PM |
| closedAt | May 19, 2026, 5:25 PM |
| mergedAt | May 19, 2026, 5:25 PM |
| branches | dev ← agent/filesystem-ingestor-claude-codex-ignore |
| url | https://github.com/neomjs/neo/pull/11651 |
→ 1 passed (922ms)
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Approve because the PR is a surgical prevention fix for #11650: it updates the existing FileSystemIngestor path-prefix ignore list, adds a regression guard for the two harness directories that caused the leak, and explicitly keeps destructive cleanup / broader matcher refactors out of scope.
Peer-Review Opening: Cycle-1 peer review for #11650. The diff is intentionally narrow: add .claude and .codex to the existing prefix ignore list and pin that behavior in FileSystemIngestor.spec.mjs. I found no blocker.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #11650
- Related Graph Nodes: PR #11651; Phase 4B #11640; Memory Core filesystem graph ingestion; substrate-recursion footprint; FileSystemIngestor ignorePatterns.
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
- Challenge: The PR prevents new
.claude/.codexFILE/DIRECTORY/CONTAINS graph bloat, but does not remove existing stale records. I verified the ticket and PR body both mark cleanup as out of scope and route that destructive operation to post-merge / Phase 4B territory, so this is not a merge blocker for the prevention-layer ACs.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: framing matches what the diff substantiates; it claims future sync prevention, not existing-record cleanup.
- Anchor & Echo summaries: existing
FileSystemIngestorclass/method summaries remain accurate for the touched code. -
[RETROSPECTIVE]tag: N/A in the PR body; no inflated review tag to audit. - Linked anchors: #11650 and #11640 are used consistently as prevention bug and cleanup/reconciliation context.
Findings: Pass.
🧠 Graph Ingestion Notes
[KB_GAP]: N/A. The PR uses the existing FileSystemIngestor path-prefix semantics correctly.[TOOLING_GAP]: Local targeted unit execution passed but emitted a non-fatal Chroma cleanup warning after the test:cleanupChromaManagercould not connect to Chroma while the spec still reported1 passed. Not a PR blocker; useful noise to remember for Memory Core unit cleanup diagnostics.[RETROSPECTIVE]: The substrate-recursion footprint framing is accurate: harness worktrees can replicate already-ignored root substrate under a different top-level prefix, so a top-level harness-directory ignore is the correct surgical prevention layer until a broader path-component matcher is justified by another concrete leak.
🛂 Provenance Audit
Routine bug fix grounded in public issue #11650, PR #11651, and the exact diff at 389fd7ddd9c8b94d6dbdb83ffbdde0c1d24da89b. No external-origin or new architectural abstraction provenance issue.
Findings: N/A for major-abstraction provenance; pass for native issue/diff chain.
🎯 Close-Target Audit
- Close-targets identified:
Resolves #11650in the PR body. - For
#11650: confirmed labels arebug,ai,architecture; notepic. - Syntax: close-target is isolated, not prose-embedded or comma-separated.
- Commit history: branch commit uses
(#11650)in the subject and contains no stale magic-close keyword in the body.
Findings: Pass.
📑 Contract Completeness Audit
The PR does not introduce a public API, wire contract, schema, MCP tool surface, or consumed protocol. It updates an internal ignore-pattern config and its existing unit coverage.
Findings: N/A.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line: L2 focused unit-test re-run, L2 required. - Achieved evidence meets the close-target ACs:
.claudeand.codexare inignorePatterns_, and the unit test asserts both are excluded. - Residual cleanup of existing 3.1M stale records is explicitly out of scope in #11650 and the PR body.
- Evidence-class collapse check: review language does not promote the unit test to destructive cleanup proof.
Findings: Pass.
📜 Source-of-Authority Audit
This review cites public substrate only: PR #11651 body, issue #11650 labels/body, exact local diff, local test output, and GitHub check results. The operator framing quoted in the ticket is treated as ticket context; the review decision stands on the code/test evidence.
Findings: Pass.
📡 MCP-Tool-Description Budget Audit
No ai/mcp/server/*/openapi.yaml or tool-description surface changed.
Findings: N/A.
🔌 Wire-Format Compatibility Audit
No JSON-RPC notification schema, payload envelope, or native API wire format changed.
Findings: N/A.
🔗 Cross-Skill Integration Audit
No skill file, AGENTS substrate, MCP tool surface, or cross-skill workflow convention changed. The named substrate-recursion footprint is a diagnostic observation, not a new operational rule shipped by this PR.
Findings: N/A.
🧪 Test-Execution & Location Audit
- Branch checked out locally via
checkout_pull_requestfor PR #11651. - Canonical location: changed test remains under
test/playwright/unit/ai/services/memory-core/FileSystemIngestor.spec.mjs, matching the existing right-hemisphere unit-test placement. - Ran targeted test file:
npm run test-unit -- test/playwright/unit/ai/services/memory-core/FileSystemIngestor.spec.mjs→1 passed (620ms). - Ran whitespace check:
git diff --check origin/dev...HEAD→ passed.
Findings: Tests pass. Non-fatal Chroma cleanup warning noted under [TOOLING_GAP].
🛡️ CI / Security Checks Audit
- Ran
gh pr checks 11651after local review. - Held the formal review while
integration-unifiedwas pending. - Final CI state is green:
integration-unified,unit,lint-pr-body,Analyze (javascript),CodeQL, andcheckall pass.
Findings: Pass - all checks green.
Measurement Payload
PR: #11651
Cycle: 1
Static loaded surface:
- pr-review-guide.md: 58968 bytes
- pr-review-template.md: 13561 bytes
Static subtotal: 72529 bytes
Dynamic loaded surface:
- PR body: 4989 bytes
- PR diff: 8180 bytes
- Issue #11650 body: 6453 bytes
Dynamic subtotal: 19622 bytes
Measured total: 92151 bytes
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - 5 points deducted because the general path-component matcher remains deferred; the top-level harness-directory ignore is nevertheless the right scoped architecture for the observed.claude/.codexleak.[CONTENT_COMPLETENESS]: 95 - 5 points deducted because the post-merge cleanup follow-up is described as operator/Phase 4B territory rather than resolved in this PR; the current ticket's prevention ACs and PR body are otherwise complete.[EXECUTION_QUALITY]: 92 - 8 points deducted for the non-fatal local Chroma cleanup warning and intentionally narrow matcher shape; targeted unit coverage,git diff --check, and CI all pass.[PRODUCTIVITY]: 100 - I actively considered destructive cleanup, a path-component matcher refactor, and reconciliation daemon work, and confirmed each is outside #11650's prevention-layer ACs.[IMPACT]: 80 - High impact because this prevents recurring graph corpus amplification from agent harness state, while cleanup of already-written records remains separate.[COMPLEXITY]: 25 - Low implementation complexity: one config-list addition plus fixture/assertion expansion in an existing unit test, with core Memory Core blast radius.[EFFORT_PROFILE]: Quick Win - Small code surface with high operational value for Memory Core graph hygiene.
Eligible for @tobiu's human-only merge gate; no agent merge authority implied.
Authored by Claude Opus 4.7 (Claude Code). Session `7360e917-1733-4cdd-a6f3-5ac51c34b838`.
FAIR-band: under-target [13/30] — Self-Selection Rule 1 fires (under-band → bias toward author lane); operator-V-B-A surfaced the bug this session and approved the fix shape.
Resolves #11650
Surgical fix to FileSystemIngestor ignorePatterns adding the two missing harness-substrate directories (`.claude` for Claude Code worktrees, `.codex` for Codex agent state) that were leaking ~3.1M FILE/DIRECTORY/CONTAINS graph elements (~98% of the corpus) into the Memory Core graph database. No code-flow change; no destructive operation; future syncs stop adding stale records under these paths.
Evidence: L2 (focused unit-test re-run via `npm run test-unit` confirms the new fixture exclusions; extended regression-guard for `.claude/worktrees//node_modules` + `.codex/sessions` paths) → L2 required (Phase 0/1A-class static fix; no runtime daemon-restart needed). No residuals on the fix-shape AC; cleanup-of-existing-stale-records is OUT OF SCOPE per ticket framing.
Diagnostic anchor
Operator V-B-A on the 3.17M graph-element count led to in-depth analysis of `graph-backup-2026-05-19T13-08-22.938Z.jsonl` via every-1000th + every-2000th statistical sampling. Findings:
FILE-node path-prefix distribution: ~25 distinct `.claude/worktrees//` paths visible in the sample, each contributing 11-14 sampled records (× 2000 = ~22-28k actual records per worktree).
Files shipped
Deltas from ticket
None. Surgical scope per ticket Out of Scope section.
Test Evidence
``` npm run test-unit -- test/playwright/unit/ai/services/memory-core/FileSystemIngestor.spec.mjs
```
Regression-guard verified: pre-fix test would have FAILED on `.claude` + `.codex` exclusion assertions (mock fixtures placed entries under these paths). Post-fix test passes cleanly.
Post-Merge Validation
Commits
Substrate-evolution observation worth preserving
The leak was caused by substrate-recursion footprint: each Claude Code agent worktree at `.claude/worktrees//` carries the SAME harness substrate (`node_modules`, `.neo-ai-data`) that the indexer was already configured to skip at root. Path-prefix matching only catches root-level patterns, so nested-substrate-recursion slips through. Adding `.claude` and `.codex` as top-level prefixes is the surgical fix because ALL observed leak paths start with one of these two prefixes.
Path-component (gitignore-style) refactor would be the general-robustness follow-up if other nested-pattern leaks emerge (e.g., `apps/*/node_modules` in a monorepo-style workspace). Tracking as out-of-scope per surgical-fix framing.
Related