fix: keep team decisions scoped to their project

[cdeust]

Agent decisions currently set is_global, so they bypass the project predicate introduced in #606. This change stores is_team_decision separately, scopes both team-decision readers by project, preserves ingestion scope through database reopening, and stops initialization from promoting decisions to global scope. Explicit global writes and the content detector retain their behavior.

Refs #611. ADR-1083 is proposed for owner review. Task B is outside this PR.

Validation

• PostgreSQL affected suite: 182 passed. SQLite: 175 passed, 7 PostgreSQL-only skips.
• Migration script suite: 24 passed, including real PostgreSQL rollback and idempotency tests.
• Production pg_dump -Fc fully restored into an isolated database. Reclassification changes 98 of 160 global candidates; second pass changes zero. All 47,490 rows and every field outside the three scope fields have identical before/after fingerprints.
• Ruff, pyright (zero diagnostics), project craftsmanship against origin/main, wiki mirrors, and doc claims pass.
• A review-discovered ingestion issue is fixed: four of eight origin/class reopen cases failed before persisting write_class; all eight pass on both backends afterward.

Owner decisions and rollout

Production has not been reclassified. All 47 rows without directory_context also have no domain, so domain mapping cannot resolve them. Their IDs and the operator procedure are in the validation report. In particular, 4353879 needs an explicitly approved mapping to japonais-2027; 4356519 also remains in the parent's Team Decisions query pending review.

Without that mapping, the real snapshot acceptance condition still fails: the parent auto-recall includes the Score audit decision. The full SessionStart launcher validation also timed out at 60 seconds; direct query tests passed but installed-plugin acceptance remains outstanding. This PR is a draft for review, not a release-ready claim that #611 is resolved.

Deploy the reviewed code before applying the data operation: an old installation's initialization backfill can reverse a data-only repair. Re-run the installed-plugin acceptance after release and update, as required by the owner plan.

Completion ledger

The validation report maps schema, write, read, migration, rollback, unresolved-data, and archive-check paths to their tests and snapshot evidence. The unresolved production acceptance is called out above and remains required before closing #611.

[cdeust]

ZETETIC-REVIEW: REQUEST_CHANGES

Scope reviewed: full diff (git diff origin/main...HEAD, 32 files, ~1235 added lines), ADR-1083, the reclassification scripts and their tests, both hook read paths, both storage backends, schema migration ordering, and the validation doc. Ran the full team-scope-related suite locally against real PostgreSQL (CORTEX_MEMORY_STORE_BACKEND=postgresql, CORTEX_TEST_DATABASE_URL=postgresql:///cortex_611_test): 59 passed. CI on the PR is fully green (all jobs pass, including Craftsmanship Gate, Type Check, Fuzz, Docker Smoke).

Blocking

1. scripts/reclassify_team_scope.py:88-97 (classify()). The script cannot tell an explicit or historically-detector-confirmed global row apart from one that merely still carries is_global=TRUE for any other legacy reason, and it defaults to clearing is_global whenever today's detect_global() does not confirm the content, regardless of whether the row shows any sign of being a victim of the ADR-0200/Team-scoped decisions are stored as is_global, so they still reach every project after #604 #611 defect (agent_context set + decision content). Reproduced directly:
   classify(ScopeRow(1, "The parser uses XML.", [], "project-a", "/project-a", "", True, False), ScopeMappings({}, {}, frozenset()))
   -> ScopeChange(is_global=False, is_team_decision=False, reason="recorded_project")
   This row has an empty agent_context, so propagates_to_team()/is_team_decision() would never have set it global under the old rule either; whatever originally set is_global=TRUE on a row like this (an explicit remember with is_global=True, or an earlier positive detector run) is indistinguishable from noise to this script, and it gets silently cleared unless the operator has already enumerated its ID in keep_global_ids. tests_py/scripts/test_reclassify_team_scope.py has no case covering this: test_nondecision_and_existing_marker (line 76) only asserts on is_team_decision for the no-agent-context row, never checks is_global. The only test that shows an explicit global staying global (test_global_detector_and_explicit_override, line 49) covers detector-match and keep_global_ids, not this gap.
   This is the exact failure mode named in my task brief as REQUEST_CHANGES ("a way to lose an explicit global"). The mitigations that exist (dry-run-first default, PostgreSQL backup verification, owner review of the JSON report, is_team_decision, keep_global_ids) reduce but do not close this: they all depend on the operator already knowing, outside the database, which legacy rows were explicitly global, which the ADR's own text admits is not always available ("Legacy rows do not persist the reason for the global flag").
   Requested fix: narrow classify()'s default so a resolved-project row only loses is_global when it exhibits the actual defect signature (non-empty agent_context and decision content) or fails an owner-reviewed allowlist that defaults toward preserving scope, not toward clearing it; alternatively, require every currently-global row without the defect signature to land in unresolved/retained-for-review rather than being auto-cleared. Add a test asserting an explicit-global, non-decision, no-agent-context row is left untouched by a dry run.

Verified correct (not blocking)

2. Write path (mcp_server/core/team_scope.py, mcp_server/core/global_detector.py:262-276, mcp_server/handlers/remember.py:285-334, mcp_server/handlers/remember_helpers.py:696-745, mcp_server/core/memory_ingest.py:88-118, mcp_server/infrastructure/pg_store_write.py:29-109, mcp_server/infrastructure/sqlite_store.py:340-380). resolve_global_scope() no longer accepts team_decision; grepped every git grep -n "is_global" -- mcp_server writer: the only remaining writers are the two remember paths (explicit args.is_global / detector), anchor.py (an explicit, unrelated operation), and ingest_memory (which now reads memory.get("is_global", False) verbatim instead of deriving it from propagates_to_team). No remaining path sets is_global from a team-decision cue.

3. Read path (mcp_server/hooks/session_start.py:214-236, mcp_server/hooks/agent_briefing_query.py:32-113, mcp_server/hooks/auto_recall.py unchanged). _fetch_team_decisions dropped is_protected=TRUE AND is_global=TRUE in favor of is_team_decision=TRUE AND (is_global=TRUE OR directory_context = ANY(ancestors)) (session_start.py:228-231). agent_briefing_query.py's pass 2 applies the same predicate (line 103-105) and pass 1 gained one too (line 62). auto_recall.py needed no change: it already scopes every memory by project (from Inject only the session's project and global memories (#604) #606) and the ADR states ordinary recall treats team decisions as ordinary memories, which is correct since is_team_decision alone carries no visibility, only directory_context/is_global do. Verified the discriminating test is tests_py/hooks/test_hook_receipts.py::test_agent_briefing_emits_receipt_with_marker (architect-authored row in /another-project, asserted absent from stdout); the PG differential test in test_team_project_scope.py:63-85 exercises both readers directly against a real store.
   SQLite gap (non-blocking, report per instructions): mcp_server/hooks/session_start.py's SQLite path (_sqlite_context/_sqlite_banner_rows, unchanged by this PR) has no equivalent "Team Decisions" query at all; is_team_decision is written but never specifically read there. It is not a leak (memory_matches_project still gates every row by directory_context/is_global in _partition_banner_rows), just a feature asymmetry pre-dating this PR. The ADR's verification claim ("both backends must exclude...include") is trivially true for SQLite only because SQLite never surfaces a team-decision section to begin with.

4. Schema and upgrade (mcp_server/infrastructure/pg_schema.py:79-80,1390-1399,1946-1969). get_all_ddl() runs MIGRATIONS_DDL (the ADD COLUMN IF NOT EXISTS) and TEAM_DECISION_BACKFILL_PG before CURRENT_MEMORIES_VIEW_DDL's CREATE OR REPLACE VIEW, so the view is always recreated with the new column already present; readers that need is_team_decision (session_start.py, agent_briefing_query.py) select it from memories m directly, never through current_memories, so PostgreSQL's SELECT * view-snapshot gotcha does not bite either way. Schema application is content-hash gated (mcp_server/infrastructure/pg_store_ddl.py:143-158, compute_ddl_hash() over the full ordered DDL text vs the recorded schema_meta hash), not a version constant the PR forgot to bump, so an upgraded install re-applies the full idempotent DDL set automatically the next time a store is constructed. Backfill (team_scope_backfill.py) only ever sets is_team_decision=TRUE from is_protected=TRUE AND is_team_decision=FALSE, never touches is_global, and both backends have a same-run-changes-nothing-a-second-time test (tests_py/infrastructure/test_team_scope_backfill.py:87-89 and 108-109, plus a schema-upgrade case at line 111).

5. Reclassification script mechanics other than the classify() gap above (scripts/reclassify_team_scope_db.py). PostgreSQL: SET TRANSACTION ISOLATION LEVEL SERIALIZABLE plus FOR UPDATE OF m gated on require_marker (apply mode) at line 43,75; apply_changes commits explicitly and raises if any row's rowcount != 1, verified by both a SQLite unit test (test_partial_failure_rolls_back) and a real-PostgreSQL test (test_pg_later_update_failure_rolls_back). verify_backup (line 12-23) checks the PGDMP magic bytes, requires "TABLE DATA public memories" in pg_restore --list, and does a full pg_restore --file /dev/null decode; only gated on args.apply and args.database_url (reclassify_team_scope.py:129), so a --sqlite-path --apply run has no backup requirement at all (non-blocking finding, worth documenting since SQLite is a real deployment target too, not just a dev toy). Fetch is scoped to WHERE m.is_global = TRUE only, so it never touches rows that were never global. Idempotence for the part of the design that does work (team-flag propagation, directory resolution) is demonstrated end to end in tests_py/scripts/test_reclassify_team_scope.py::test_dry_run_apply_and_second_run_are_idempotent and the real-PostgreSQL test_reclassify_team_scope_pg.py::test_pg_apply_second_run_and_persisted_marker, plus the production-snapshot experiment in docs/validation/issue-611-team-scope.md (98 changes then 0 on a second pass against a full restore).

6. Tests (item 5 of my brief). tests_py/hooks/test_team_project_scope.py and test_hook_receipts.py cover: project-A decision absent from project B for both auto_recall and session_start subprocess hooks; explicit is_global=True visible in both projects; same-project cross-agent visibility for both PG raw-SQL readers directly (test_pg_team_readers_scope_both_agent_passes); idempotent reclassification (covered above). tests_py/core/test_memory_ingest_scope_persistence.py proves scope privileges survive a real schema reopen on both backends across an 4x2 origin/write_class matrix. Gap besides the classify() one above: no test for the SQLite --apply path's missing backup requirement.
   Frozen fixture: tests_py/fixtures/w3_4/remember.py.txt is not a pristine, hands-off oracle; it was already touched once before (for [bug] remember never propagates decisions to is_global, so Team Decisions stays empty (ADR-0200) #561, per _remember_bulk_fakes.py's _FROZEN_IMPORTS comment) and is now touched a second time to add the team_decision= kwarg verbatim from the live handler. insert_and_post_process defaults team_decision to False, so this edit was not required to keep the fixture executable; it was done for fidelity. The edit is a faithful, minimal, mechanical parameter-threading change, not a rewrite of decision logic, so the bulk/differential tests that use this fixture (batch encode ordering, gate short-circuiting, partial-failure handling) still prove what they claim to prove. But each manual sync erodes the "frozen" framing: recommend either renaming the file away from "frozen" or documenting an explicit update policy, since a reference body that gets hand-edited in lockstep with the code it is meant to check drift against cannot, by construction, ever show drift.

7. Public-repo hygiene (docs/validation/issue-611-team-scope.md). The absolute path /Users/cdeust/Developments/japonais-2027 (line 52) matches the accepted /Developments/ shape and names no content. The 47 memory IDs (lines 71-73) are opaque integers, not excerpts. Line 106 ("Do not claim the issue fixed in a release or publish the LinkedIn draft before this") and line 131 ("or message to Denis") are personal workflow/business-planning references naming a third party by first name, unrelated to the technical change; recommend removing both before this lands on a shared branch of a public repository. Not blocking merge of the code change, but should be scrubbed from this file.

8. CI: gh pr checks 613 -R cdeust/Cortex reports every job pass (Test Python 3.10/3.11/3.12/3.13, SQLite backend, Windows SQLite, Craftsmanship Gate, Type Check, Lint, CodeQL analyze, Fuzz PR batch, Docker Smoke, plugin-scanner, HOL plugin scan). Nothing skipping except expected conditional jobs (CodeQL/Docker builds, Tag a release, scheduled fuzz).

Move 0 (ledger reconciliation and seen-defect check)

The PR body and validation doc enumerate every remaining gap explicitly (unresolved production rows, the 60-second SessionStart launcher timeout, no production reclassification yet) and tie each to a named follow-up rather than dismissing it as "pre-existing" or "unrelated." No un-issued seen-defect rationalization found. This is an honest draft PR that says outright it is not claiming #611 closed.

What would unblock merge

Close finding 1: bound classify()'s default behavior to the actual defect signature, or invert the default so an unrecognized legacy-global row is left global and reported for owner review rather than cleared, and add a test for the no-agent-context / non-decision / currently-global case. The rest of the change (schema, write path, both hook readers, backfill, migration ordering) is correct and well tested.

[cdeust]

ZETETIC-REVIEW: APPROVE

Re-review of commit f48f17a on branch fix/611-team-scope, same worktree. Verified independently, not just read the diff.

Blocking finding from the first review: closed

scripts/reclassify_team_scope.py:88-109 (classify()). The rule is now: a currently-global row loses is_global only when it carries the actual defect signature, bool(row.agent_context) and is_decision_content(row.content). Any other global row is kept, tagged reason="global_origin_not_the_defect", and surfaced in make_report()["unexplained_global_ids"]; the owner can clear one explicitly through the new clear_global_ids mapping list, and load_mappings rejects an id present in both keep_global_ids and clear_global_ids.

I reproduced my original counterexample against the new code and it is now protected:
classify(ScopeRow(1, "The parser uses XML.", [], "project-a", "/project-a", "", True, False), ScopeMappings({}, {}, frozenset(), frozenset())) -> is_global stays True, reason="global_origin_not_the_defect".

I then tried to construct two more counterexamples, independently of the new test file:

1. A row that still loses an explicit global: a promoted-looking row (agent context present, decision-shaped content, is_global=True) is cleared by default, since it is indistinguishable from what ADR-0200 would have produced, exactly the intended narrowing. keep_global_ids still overrides it (kept = id in keep_global_ids or detected, checked independently of promoted), so an owner who knows a promoted-looking row was actually an explicit act can protect it by id. I could not make a genuinely non-defect-shaped row lose is_global.
2. A row that wrongly keeps a promoted team decision global: a row with no prior is_global=True and a promoted (agent-context + decision) profile classifies is_global=False, is_team_decision=True, matching row5 in my test run below; fetch_rows also only ever selects WHERE m.is_global = TRUE, so a never-global row is not even a candidate. I could not make classify() turn a plain team decision global.

The new regression test (tests_py/scripts/test_reclassify_team_scope.py::test_a_global_row_the_defect_could_not_have_produced_stays_global) encodes exactly my original repro plus the promoted/keep_global_ids interaction; test_the_owner_can_clear_a_global_the_script_would_keep and test_an_id_cannot_be_both_kept_and_cleared cover the new escape hatch and its guard rail. 109 tests pass locally (tests_py/hooks/test_team_project_scope.py, test_hook_receipts.py, test_agent_briefing.py; tests_py/scripts/test_reclassify_team_scope.py and _pg.py; tests_py/core/test_global_scope_resolution.py, test_memory_ingest_scope_persistence.py, test_memory_ingest_capture_origin.py; tests_py/handlers/test_remember_team_scope.py; tests_py/infrastructure/test_team_scope_backfill.py; tests_py/invariants/test_I2_canonical_writer.py), run against real PostgreSQL (CORTEX_MEMORY_STORE_BACKEND=postgresql, CORTEX_TEST_DATABASE_URL=postgresql:///cortex_611_test).

Minor, non-blocking observation on the new logic

classify() cannot clear a row the content detector currently confirms as cross-project, even through clear_global_ids: kept = id in keep_global_ids or detected short-circuits before cleared is checked, so a detector false positive has no override path in this script (the operator would have to fix the detector or edit the row directly). This matches the ADR wording precisely ("the content detector also preserves a row") so it is a deliberate, documented choice, and it biases toward the safer direction for an operation the PR's own procedure gates behind backup and dry-run review. Not blocking.

The reason field is overwritten to "global_origin_not_the_defect" when a row is kept-but-unexplained, discarding whatever directory-resolution reason (recorded_project / owner_memory_mapping / verified_domain_mapping) it held before. Cosmetic only, does not affect which rows get resolved or how, and does not appear in any assertion. Not blocking.

Hygiene items from the first review: closed

docs/validation/issue-611-team-scope.md no longer mentions the LinkedIn draft plan or a third party by name (grepped both terms, no matches). A new "Rerun after review" section documents the fresh figures on a second restore of the same archive: 93 changes, 0 on the second pass, 67 retained globals (46 unresolved, 6 named and kept as not produced by the defect), 47,041 rows unchanged before and after.

Documentation state not yet pushed (report, not blocking)

The ADR wording update that is supposed to accompany f48f17a (narrowing "otherwise evaluate the existing content detector and clear is_global..." to describe the new defect-signature-only rule) is present only as a staged, uncommitted change in this worktree; git show origin/fix/611-team-scope:docs/adr/ADR-1083-....md still carries the old wording that describes the behavior my first review flagged, not the behavior actually implemented by f48f17a. Recommend committing and pushing that wording change (docs/adr and its wiki mirror) before merge so the ADR matches the code the PR actually ships.

Two items intentionally left unchanged, my view

(a) SQLite --apply has no --backup requirement (reclassify_team_scope_db.py, verify_backup only gated on args.database_url). Non-blocking for this PR: the operator procedure this PR documents targets PostgreSQL production, SQLite's own backup (copying the file) is far lower-friction than pg_dump verification, and the correctness of classify() does not depend on which backend applies it. Fine to track as a follow-up issue rather than block here.

(b) SQLite session_start has no Team Decisions reader at all (is_team_decision is write-only on that path). Non-blocking: it is not a leak, since memory_matches_project already gates every SQLite banner row by directory_context/is_global regardless of the team flag; it is a feature gap, not a correctness gap, and #611's actual defect (cross-project leak) does not depend on it. Fine as a follow-up issue.

CI

gh pr checks 613 -R cdeust/Cortex, snapshot at review time: Lint, Changed paths, CodeQL, the three Analyze jobs, HOL plugin scan, plugin-scanner and the identity-contract check have passed. Build Package, Craftsmanship Gate, Type Check, Docker Smoke, Validate MCP host configurations, the two Fuzz PR-batch jobs, and all five Test jobs (Python 3.10-3.13, SQLite backend, Windows SQLite) are still pending on the fresh run triggered by f48f17a; none has failed so far. I ran the directly affected suite locally against real PostgreSQL instead of waiting on the full matrix (109 passed, see above).

Verdict

APPROVE on the code. The blocking finding from the first review is closed with a verified fix and a regression test that encodes my exact reproduction; the two hygiene items are closed; the two consciously-deferred asymmetries are reasonable follow-up-issue material, not blockers. Please push the staged ADR wording commit so the merged ADR text matches the shipped behavior, and confirm the still-pending CI jobs land green before merging.

[cdeust]

ZETETIC-REVIEW: APPROVE

Final confirmation on head commit 334e15d, branch fix/611-team-scope.

Verified independently, not from the coordinator's description alone:

• git show 334e15d --stat touches exactly two files: docs/adr/ADR-1083-separate-project-team-decisions-from-global-knowledge.md and wiki/adr/cortex/1083-separate-project-team-decisions-from-global-knowledge.md, 12 insertions and 8 deletions total across both, no code or test files.
• git show origin/fix/611-team-scope:docs/adr/ADR-1083-...md now reads "Operators can preserve IDs known to have been explicitly global through keep_global_ids, and the content detector also preserves a row. Otherwise is_global is cleared only for the rows ADR-0200 promoted: a decision written under an agent context, which receives the team marker. Any other global row is kept and reported, because an explicit act the script cannot see made it global; the owner can clear one by ID through clear_global_ids." This matches the narrowed rule implemented in f48f17a and reviewed previously, not the old broader wording. The wiki mirror is byte-identical to the doc for this section (diffed both, no output).
• gh pr checks 613 -R cdeust/Cortex on this head: 24 checks pass (all five Test jobs on Python 3.10 through 3.13, SQLite backend, Windows SQLite backend, Craftsmanship Gate, Type Check, Lint, Build Package, Docker Smoke, Validate MCP host configurations, both Fuzz PR-batch jobs, CI Green, Release dependency set, CodeQL, the three Analyze jobs, HOL plugin scan, plugin-scanner, the identity-contract check), 4 skipping (Docker image builds and scheduled fuzz, expected on a PR run), 0 failed.

This closes the last open item from my prior round (round 2, comment #613 (comment)): the ADR wording now matches the shipped behavior, and the full CI matrix that was pending at that time has since gone green with nothing red.

My verdict from round 2 stands: the blocking finding from round 1 (classify() in scripts/reclassify_team_scope.py losing an explicit global whenever the content detector did not confirm it) was fixed in f48f17a and independently verified with my own counterexamples, not just the added test. Nothing in 334e15d touches code, tests, or the validation doc, so that verification still holds at this head.

Verdict: APPROVE at 334e15d.