Inject only the session's project and global memories (#604) - #606

ZETETIC-REVIEW: REQUEST_CHANGES

Reviewed 7f27b41 in a read-only worktree against origin/main (bba138a), stakes high: this decides what every session sees at start and on every prompt, on both backends.

What holds

The pure module is genuinely pure. mcp_server/shared/project_scope.py imports nothing but __future__ and collections.abc.Mapping, no os, no pathlib, no filesystem call:

imports: ['from __future__ import annotations', 'from collections.abc import Mapping']

Ad-hoc asserts on the predicate, all 14 as specified:

ancestors /a/b/c: ['/a/b/c', '/a/b', '/a', '']
OK  ancestor matches -> True          OK  prefix without separator must not match -> False
OK  equal root matches -> True        OK  trailing slash on ctx -> True
OK  trailing slash on root -> True    OK  empty ctx never matches -> False
OK  None ctx never matches -> False   OK  foreign project -> False
OK  global always -> True             OK  global with unknown root -> True
OK  unknown root -> only globals -> False
OK  descendant must not match -> False
resolve override: /b   resolve cwd: /a   resolve none: None   resolve empty override: /a

/a/bc does not match project /a/b, which was the failure mode to avoid. Two observed quirks, neither blocking: a directory_context of "/" normalizes to "" and matches every project, and comparison is case sensitive, so a directory_context recorded as /Users/X will not match a root spelled /users/x.

The fail-before claim is real. Checking out the two hook files from origin/main into the worktree:

15 failed, 464 passed, 2 warnings, 13 subtests passed in 57.43s

and after restoring:

479 passed, 2 warnings, 13 subtests passed in 44.10s

The fixture edits in test_hook_receipts.py and test_hook_telemetry.py do not weaken what those tests measured. The receipt tests still assert on the emitted ids; marking the seeded rows global keeps them injected in a run that sends no event cwd, and the telemetry edit is a signature update on a lambda double. The residue is that no receipt test now covers a project-scoped non-global row.

End to end on SQLite with isolated HOME and CORTEX_CLAUDE_DIR, three memories, event cwd under the ancestor:

=== session_start, cwd under project ===
- [++++] scopeprobe GLOBAL owner rule  ·  just now
- [++++] scopeprobe ANCESTOR of the session cwd  ·  just now
=== auto_recall, cwd under project ===
[cortex-auto-recall] injected 2 memories (sqlite) for query: scopeprobe...
=== session_start, NO cwd ===
- [++++] scopeprobe GLOBAL owner rule  ·  just now

The foreign row is dropped, the ancestor and the global are injected, and an event without cwd yields globals only. That is what #604 asked for.

ADR-1080 is the free number (origin/main wiki/manifest.json tops out at ADR-1078, PR #602 holds 1079), the docs/adr/ and wiki/adr/cortex/ mirrors are both present, check_project_wiki.py reports "Project wiki mirrors match their canonical sources", check_doc_claims.py is OK, the CHANGELOG entry cites #604. uvx [email protected] check passes, format --check reports 3368 files already formatted, pyright on the four files reports 0 errors, 0 warnings, 0 informations. Decision gate clear: the longest comment run in any changed file is 6 lines, in tests_py/hooks/test_auto_recall.py.

Blocking 1: the predicate runs after the SQL LIMIT, so the project's own memories starve

session_start.py:276 fetches LIMIT _HOT_LIMIT + len(exclude_ids) ordered by heat_base DESC across every project, :167 fetches LIMIT _ANCHOR_LIMIT the same way, :1094 asks the SQLite store for limit=_HOT_LIMIT + _ANCHOR_LIMIT (13 rows), and auto_recall.py:161 fetches _MAX_MEMORIES + 2 (5 rows). Only then does memory_matches_project run, in Python, over that already-truncated window. The foreign rows are not excluded from the candidate set, they consume it.

Measured on SQLite, isolated store, 20 foreign memories at heat 0.99 plus one in-project memory at heat 0.50, event cwd set to the project root:

in-project row exists: True
=== session_start banner (21-row store, cwd = project) ===
0        (lines matching scopeprobe)
MINE present: 0
=== auto_recall (query matches all 21) ===
(no output, nothing injected)

Control, the identical in-project row in a store with no foreign rows:

=== CONTROL: same row, no foreign rows ===
- [++] scopeprobe MINE the cortex decision for this project  ·  just now
- scopeprobe MINE the cortex decision for this project  ·  just now

Same row, same heat, same query, injected when it is alone and dropped when 20 unrelated rows outrank it. Heat is not the cause: 0.50 clears session_start._MIN_HEAT = 0.4 and auto_recall._MIN_HEAT = 0.15. FTS is not the cause: the control proves the query matches.

This is the reporter's own situation. #604 exists because the store holds japonais-2027, IRODORI and Cortex at once, which is exactly the store where the top 13 rows by heat are unlikely to contain the current project. The issue's acceptance says a memory whose directory_context is an ancestor of the session cwd "still is" injected. It still is only in a fixture with three rows. At production scale the fix stops the foreign injection and stops the useful one with it, and the banner silently goes empty, which reads as "Cortex has no memories" rather than as a scoping decision.

The store already knows the right shape, and #604 quoted it: pg_schema.py:1024-1025 puts (p_directory IS NULL OR m.directory_context = p_directory) and the globals clause inside the SQL, before the ordering and the limit. The PostgreSQL paths should carry the ancestor list into the WHERE clause (m.is_global OR m.directory_context = ANY(%s) with project_ancestors(project_root) as the parameter), and the SQLite path needs the same predicate pushed into get_hot_memories and search_fts rather than applied to their output. Over-fetching by a fixed factor would not fix it either, it would only move the store size at which the banner empties.

Blocking 2: CI is red, and the craftsmanship gate is red locally too

gh pr checks 606 at the time of review:

CI Green                 fail  6s
Craftsmanship Gate       fail  15s
Test (Python 3.10)       fail  6m5s
Test (Python 3.11)       fail  7m7s
Test (Python 3.12)       fail  8m16s
Test (Python 3.13)       fail  6m1s
Test (SQLite backend)    fail  5m23s

Every test job fails on the same assertion:

FAILED tests_py/integration/test_cold_start.py::TestSessionStartHook::test_normal_session_with_memories - AssertionError: assert 'Cortex' in ''

Reproduced locally in the worktree:

1 failed, 18 passed in 29.88s

That test seeds a memory with no directory_context and no is_global and asserts the banner injects it. Under the new rule it does not, and the banner comes out empty. The test was not updated, because the run reported in the test plan covered tests_py/hooks and tests_py/test_main.py only. The rest of the suite is otherwise clean: pytest -q -p no:cacheprovider -x --ignore=tests_py/integration gives 8735 passed, 8 skipped, 2 warnings, 459 subtests passed in 352.76s.

Behind the failing assertion is a migration question the PR does not answer. Every memory already in a store with an empty directory_context and is_global = FALSE now vanishes from injection on both backends, permanently and without a log line naming it. The empty-is-not-a-wildcard rule is the owner's, and the module docstring states it, but "these rows stop being injected" deserves a sentence in the CHANGELOG and in ADR-1080 next to the rule, because a user whose store predates directory scoping will experience this as the banner going quiet.

The craftsmanship gate also fails, against the base ref, on my machine:

Craftsmanship gate: NEW violations (not in the base-ref baseline):
  - [method-size] mcp_server/hooks/auto_recall.py: _recall_memories_sqlite
  - [file-size] tests_py/hooks/test_sqlite_hook_paths.py: exceeds 300-line cap

Measured: _recall_memories_sqlite is 43 lines against the 40 cap, up from 36 on main, and tests_py/hooks/test_sqlite_hook_paths.py is 385 lines against the 300 cap, up from 197. The test file splits cleanly along the seam the new classes already draw, TestPartitionBannerRowsScope and TestRecallMemoriesSqliteScope into a test_sqlite_hook_scope.py. The method comes back under the cap by extracting the row-to-entry build.

The test plan in the PR body states python scripts/check_craftsmanship.py --base origin/main (OK). It is not OK, not in CI and not locally at this head. A checked box that contradicts the gate it names is worse than an unchecked one, because the next reader trusts it.

To unblock

Push the project predicate into the four queries so the limit is applied to in-project candidates and not to a mixed pool, with a test that seeds more out-of-project rows above the heat floor than the limit allows and asserts the in-project row still reaches the banner. Update tests_py/integration/test_cold_start.py to the new rule, get the craftsmanship gate green by splitting the test file and shrinking _recall_memories_sqlite, and say in the CHANGELOG what happens to rows with an empty directory_context that were being injected yesterday.

ZETETIC-REVIEW: APPROVE

Re-reviewed b4822c2 in a fresh worktree against origin/main (now 84c0a3b, after #605). Both blocking findings from the review of 7f27b41 are closed, and the fix for the first one is verified by the reproduction that caught it.

The starvation is gone

Same store shape as before, larger as asked: 25 foreign rows at heat 0.99, one in-project row at heat 0.50, one global at 0.50, event cwd under an ancestor of the project root, isolated HOME and CORTEX_CLAUDE_DIR, SQLite backend.

rows in store: 27
=== session_start ===
- [++] scopeprobe GLOBAL owner rule  ·  just now
- [++] scopeprobe MINE the cortex decision for this project  ·  just now
=== auto_recall ===
[cortex-auto-recall] injected 2 memories (sqlite) for query: scopeprobe...
- scopeprobe GLOBAL owner rule  ·  just now
- scopeprobe MINE the cortex decision for this project  ·  just now
=== session_start with NO cwd ===
- [++] scopeprobe GLOBAL owner rule  ·  just now

On 7f27b41 both hooks returned nothing here. Now the in-project row and the global arrive, no foreign row does, and an event without a cwd still yields globals only. The predicate sits in the WHERE clause ahead of every ORDER BY and LIMIT: auto_recall.py:157 and session_start.py:177, :295 in the hooks' own SQL, and through the new directory_ancestors parameter in pg_store_queries.py:71, pg_store_search.py:155, sqlite_store_queries.py:78, sqlite_store_search.py:336.

The new tests pin it rather than decorate it. Checking out the 7f27b41 sources under the new test files:

FAILED tests_py/hooks/test_auto_recall_scope.py::test_auto_recall_survives_a_foreign_heavy_pool
FAILED tests_py/hooks/test_sqlite_hook_scope.py::test_recall_memories_sqlite_survives_a_foreign_heavy_pool
FAILED tests_py/hooks/test_sqlite_hook_scope.py::test_sqlite_context_survives_a_foreign_heavy_pool
3 failed, 7 passed in 3.63s

and restored:

10 passed in 9.42s

They assert positively that the project's own row is present, not merely that the foreign ones are absent, which is what makes them survive a revert of the fix.

Injection safety and the None path

Both fragment builders bind values, never interpolate them. Only the SQL text and, for SQLite, an in-code column alias are formatted:

sqlite None   : ('', ())
sqlite []     : ('AND is_global = 1 ', ())
sqlite hostile: ('AND (is_global = 1 OR directory_context IN (?,?)) ', ("/a'); DROP TABLE memories;--", '/a'))
sqlite prefix : ('AND (m.is_global = 1 OR m.directory_context IN (?,?)) ', ("/a'); DROP TABLE memories;--", '/a'))
pg None       : ('', ())
pg []         : ('AND (is_global = TRUE OR directory_context = ANY(%s::TEXT[])) ', ([],))
pg hostile    : ('AND (is_global = TRUE OR directory_context = ANY(%s::TEXT[])) ', (["/a'); DROP TABLE memories;--", '/a'],))

The placeholder count tracks the list length, and column_prefix is a code literal at its one call site (sqlite_store_search.py:337, "m."), never caller data. Driven end to end against a real store with that path as both the seeded directory_context and the ancestor:

search_fts with hostile ancestor: [(27, 1.09e-06), (28, 1.09e-06)]
store still alive, rows: 28

The two hits are the hostile-path row and the global row, which is the predicate behaving, and the table is still there.

directory_ancestors=None yields ("", ()) on both backends, so the f-string inserts nothing and the emitted SQL is unchanged for every other caller. There are eleven get_hot_memories call sites and four search_fts call sites outside the two hooks (write_post_store.py:127, checkpoint.py:299 and :307, sync_instructions.py:204, get_project_story.py:184, query_methodology.py:139, narrative.py:89, drill_down.py:114, recall_helpers.py:44, :48, :55, :98, :314, remember_helpers.py:187, pruning.py:121, plasticity.py:94), all keyword-argument calls that omit the new parameter. grep -rn "directory_ancestors=" mcp_server/ returns exactly the two hook sites. The recall tool path is untouched.

The project_ancestors bug the executor found

Real, and the fix is right. range(len(parts), 1, -1) drops the parts[:1] term that used to join to "":

ancestors('/a/b/c') = ['/a/b/c', '/a/b', '/a']   has_empty=False
ancestors('/a/b')   = ['/a/b', '/a']             has_empty=False
ancestors('/a')     = ['/a']                     has_empty=False
ancestors('/')      = []
ancestors('')       = []
ancestors(None)     = []
ancestors('/a/b/c/')= ['/a/b/c', '/a/b', '/a']

Behind the old Python guard the empty element was inert, because memory_matches_project rejected a falsy directory_context before ever consulting the list. Fed to IN (...) it would have matched every empty-directory_context row in the store, which is the exact wildcard the issue forbids, so this had to be fixed in the same commit that pushed the list into SQL. The 14 predicate asserts still all pass, and the "/" quirk I reported last round is closed as a side effect: memory_matches_project("/", False, "/a/b/c") is now False where it was True. One residue worth knowing rather than fixing: a relative root such as a/b yields ['a/b'] and loses a. directory_context is an absolute resolved path at write time, so no caller reaches that shape today.

Gates

pytest -q -p no:cacheprovider   8797 passed, 8 skipped, 2 warnings, 459 subtests passed in 299.05s
check_craftsmanship.py --base origin/main   Craftsmanship gate: OK
uvx [email protected] check   All checks passed!
uvx [email protected] format --check   3372 files already formatted
pyright mcp_server/   0 errors, 0 warnings, 0 informations
check_doc_claims.py   doc claims OK
check_project_wiki.py   Project wiki mirrors match their canonical sources.

That is the full suite this time, integration included, so the test_cold_start failure that reddened every job last round is confirmed gone. _recall_memories_sqlite is 33 lines against the 40 cap, down from 43, the row-build extracted to _sqlite_result_entry at 13 lines. test_sqlite_hook_paths.py is 268 lines against the 300 cap, down from 385, with test_sqlite_hook_scope.py at 207 and test_auto_recall_scope.py at 189. The two new fragment modules are 41 and 32 lines.

git merge-tree --write-tree origin/main origin/fix/604-project-scoped-injection merges cleanly against 84c0a3b.

gh pr checks 606: 24 pass, 4 skipping, nothing failing or pending. CI Green pass 6s, Craftsmanship Gate pass 17s, Test (Python 3.10) pass 8m33s, (3.11) 7m55s, (3.12) 8m3s, (3.13) 8m45s, Test (SQLite backend) pass 6m48s, Test (Windows, SQLite backend) pass 5m42s, Type Check pass 1m25s, Lint pass 22s.

Documentation

The CHANGELOG now states why the predicate is inside the query, and names the migration consequence in the words a user will need: a pre-fix memory with an empty directory_context and is_global = FALSE stops being injected by these two hooks on both backends, stays reachable through the recall tool, and can be re-scoped or promoted to global. ADR-1080 carries the same, plus the "/" and case-sensitivity observations. test_cold_start.py seeds its row global with a comment saying the test measures banner rendering and not scoping, which is honest about why the row changed.

One inconsistency that costs nothing to leave and nothing to fix, so it is a note rather than a request: session_start._fetch_anchors and _fetch_hot_memories keep the post-fetch memory_matches_project call as defence in depth on top of their SQL predicate, while auto_recall._recall_memories dropped it. Both are correct; only the stated rationale is applied unevenly.

Approved at b4822c2. The next session started in this repo will see its own memories and the globals, and the japonais-2027 rows that opened #604 will not be among them.