feat(grooming): session-counted shared groomer coordinator (#171)

[cdeust]

Closes #171

Summary

Replaces the raced per-session consolidate/groomer spawn with a session-counted, per-store coordinator. First session ensures the cycle runs; each session registers on SessionStart and deregisters on SessionEnd; the last exit stops it. Two concurrent sessions now produce exactly one grooming cycle per period. This issue changes who starts/stops the cycle, not what it does — the cycle is still consolidate_background (decay/compress/CLS/memify/wiki).

Discovery — current mechanism (as required)

• Where the cycle is spawned: mcp_server/hooks/session_start.py::_maybe_background_consolidate() runs on every SessionStart. It reads a global stamp ~/.claude/methodology/.last_consolidate; if older than CORTEX_CONSOLIDATE_TTL_HOURS (default 6h) it subprocess.Popens a detached mcp_server.hooks.consolidate_background worker (start_new_session=True).
• The duplication bug: the only race guard was writing "<iso> (in-flight)" to the stamp. But consolidate_background.read_stamp() does datetime.fromisoformat(raw), which cannot parse the " (in-flight)" suffix → returns None → the next concurrent session treats the store as never-consolidated → spawns a duplicate cycle against the same store. Confirmed by the pre-existing test_read_stamp_handles_inflight_marker (documents read_stamp returns None on the in-flight marker). This is exactly the concurrent-duplication arch: shared coordination daemon for grooming — one groomer across N sessions #171 describes.
• Where grooming state lives: mcp_server/core/grooming_health.py (is_stale, days_since, sourced GROOMING_STALENESS_THRESHOLD_DAYS=6.0) reads judgment-level ages; the DB age lookup is PgStatsMixin.get_grooming_ages. The mechanical cycle stamp is the .last_consolidate file above.
• The macOS half / scheduled groomer: scripts/com.cortex.scheduled-groomer.plist + scripts/groomer.py are the separate weekly launchd judgment-level groomer (wiki/distillation), already guarded by session_registry.has_active_session_window(). This PR generalizes the session-counting idea to the consolidate cycle without touching that path.
• Existing per-store/session locks reused as prior art: mcp_server/infrastructure/session_registry.py already does crash-safe, pid-liveness session counting (purge_dead_entries, has_active_session_window, _pid_alive); mcp_server/infrastructure/pipeline_install_lock.py is the cross-platform flock/msvcrt non-blocking lock pattern. The coordinator mirrors both rather than porting CBM's general service daemon (Cortex's need is narrower: one groomer per store).

Coordination design

New module mcp_server/infrastructure/groomer_coordinator.py (policy) + groomer_coordinator_io.py (fs/lock mechanism — SRP split to stay under the file-size limit). State lives under ~/.cache/cortex/groomer-coordinator/<store_key>/:

• Registry format: one file per session sessions/<claude_pid>.json = {v, pid, registered_at}, written atomically (tmp + os.replace). store_key = sha256(store-identity)[:16] (SQLite DB path on SQLite, DATABASE_URL on PG) so same-store windows share a coordinator and different stores never collide. Backend type is irrelevant to counting — works on both.
• Liveness / crash safety: live_session_count() sweeps and unlinks any sessions/<pid>.json whose pid is dead (os.kill(pid,0)), reclaiming a kill -9'd session's leaked registration before counting. A PermissionError counts as alive (never a false-dead reclaim).
• Single-instance: groomer.pid holds the running cycle's pid, validated by liveness on read (is_groomer_running) — a crashed cycle's stale pid names a dead process and is reclaimed, never a bare flag that latches forever.
• Exactly-one-per-period gate (ensure_cycle): under a non-blocking per-store flock (groomer.lock): if a cycle is running → SKIPPED_RUNNING; if the period stamp is fresh → SKIPPED_FRESH; else write the stamp (valid ISO, under the lock) BEFORE spawning, then spawn. The stamp is the barrier: a concurrent session that later takes the lock reads it fresh and skips. Lock serialises simultaneous decisions; stamp serialises sequential ones. spawn_fn is called at most once, only on STARTED.
• Last exit stops it (stop_if_last): deregister; if no live session remains, clear groomer.pid and invoke the injected stop_fn.
• Degrade honestly: any coordinator exception in the hook falls back to the legacy per-session stamp spawn with a logged NOTICE — never a silent skip.

Wiring: session_start.py registers (os.getpid()) + ensure_cycle; session_lifecycle.py (SessionEnd) stop_if_last(os.getpid()).

Evidence per done-criterion

tests_py/infrastructure/test_groomer_coordinator.py — 10 passed:

1. Two concurrent sessions → exactly one cycle per period:
   • test_two_sessions_one_cycle_per_period — two coordinator handles on one store, one STARTED, the other SKIPPED_*, spawn count == 1.
   • test_two_processes_one_cycle — two real OS processes race ensure_cycle; the flock+stamp barrier admits exactly one spawn (marker-file count == 1, exactly one STARTED).
   • test_cycle_count_over_window_is_one_per_period — 24 hourly opens × 3 sessions each → the runs.log STARTED counter reports 4 (24h / 6h), never 72.
2. Crash reclaim: test_crash_dead_pid_reclaimed, test_crash_next_session_still_grooms, test_crash_does_not_leave_groomer_latched — a dead-pid registration/pid-file is swept by the next session's liveness sweep and grooming continues.
3. Last exit stops: test_last_exit_stops_groomer (stop fires only when the final live session deregisters, proven with a real live sleeper subprocess as the second session), test_stop_clears_single_instance_marker.

24h zero-duplication observation

Per the issue, the "zero duplicated consolidate runs over 24h" is post-merge operational evidence — it accrues after deploy (like #166's week-of-cycles). The mechanism to observe it ships here: the per-store runs.log (NDJSON, one line per STARTED/SKIPPED_*) and GroomerCoordinator.count_cycles_since(ts), which returns the STARTED count in a window (expected ≤ 1 per 6h period). grep '"outcome": "started"' ~/.cache/cortex/groomer-coordinator/<key>/runs.log gives the duplicate-run count directly.

Gates

• ruff check . — clean (tree-wide). ruff format --check . — clean (tree-wide, 996 files).
• pytest tests_py/hooks (superset incl. coordinator + session_registry + consolidate_background) — 136 passed. tests_py/infrastructure/test_groomer_coordinator.py — 10 passed. tests_py/scripts/test_groomer.py — 8 passed. tests_py/hooks/test_consolidate_background.py — 5 passed (legacy stamp path preserved as the degrade fallback). PG-absent reds identical-on-main are CI's to verify with PG.

Completion Ledger

#	Requirement	Status
1	Discover + describe current mechanism, grooming state, existing locks	Done (Discovery section)
2	Session-counted coordinator: first ensures, register/deregister, last stops	Done (ensure_cycle/register/deregister/stop_if_last)
3	Exactly one cycle per period across N sessions	Done (flock + stamp barrier; 2-process test)
4	Crash-safe registration (pid-liveness reclaim)	Done (live_session_count sweep)
5	Single-instance per store (pid file + liveness, not bare flag)	Done (is_groomer_running)
6	Works on both backends (store-type irrelevant)	Done (store_key per store identity)
7	Degrade honestly with logged NOTICE, never silent skip	Done (_legacy_background_consolidate fallback)
8	Test: two concurrent sessions → exactly one cycle	Done
9	Test: crash reclaim continues grooming	Done
10	Test: last exit stops groomer	Done
11	24h observation mechanism (duplicate-run counter/log)	Done (runs.log + count_cycles_since); observation accrues post-deploy
12	Gates: ruff check + ruff format --check (tree-wide) + pytest	Done (see Gates)

🤖 Generated with Claude Code

[cdeust]

Type Check gate — fixed + attribution

Blocking regression fixed. The reportOptionalMemberAccess: 1 was mine, at groomer_coordinator.py:85 (identity.encode(...) where identity inferred str | None). Root cause was deeper than a type slip: resolve_store_key imported a non-existent symbol MemoryConfig, so the ImportError was silently caught by except Exception: return "default" and every store collapsed onto one "default" key — the per-store isolation never worked. Fixed by using the real get_memory_settings() API, str()-coercing the pydantic-Unknown attributes, and get(k) or dflt so identity is a definite str. No # type: ignore. Two regression tests now assert the key is a real 16-hex hash, not "default".

Two more type issues in my files, both fixed properly:

• reportArgumentType — _log_run's rec was inferred dict[str, str]; annotated dict[str, object].
• reportAttributeAccessIssue ×4 (msvcrt.locking/LK_*) — switched the io module's Windows branch from the imported IS_WINDOWS alias to a direct if sys.platform == "win32":, so pyright statically prunes the unreachable branch (real platform narrowing).

Attribution (same type-check env, main vs this branch)

Rule	main	branch	delta
reportArgumentType	117	117	+0
reportAssignmentType	27	27	+0
reportAttributeAccessIssue	387	387	+0
reportCallIssue	42	42	+0
reportGeneralTypeIssues	3	3	+0
reportMissingImports	16	16	+0
reportOperatorIssue	1	1	+0
reportReturnType	54	54	+0
reportOptionalMemberAccess	0	0	+0 (was +1 before this fix)
TOTAL	647	647	+0

Per-file, my four files: groomer_coordinator.py 0→0, groomer_coordinator_io.py 0→0, session_lifecycle.py 0→0, session_start.py 3→3 (pre-existing on main at L115/L976/L1104 — psycopg row factory, TupleRow.get, store.count_memories — none on lines I touched).

My branch contributes zero new errors of any rule. The +79 vs the committed 568 baseline is entirely pre-existing on main = baseline drift from merges landed after the baseline was last set (#181/#183/#185/#186). That drift is not mine to absorb or to rebaseline unilaterally — flagging it here; a separate rebaseline commit on main citing those merges is the likely fix.

(Local absolute totals read 647 vs CI's 643 — a 4-error gap from optional stub packages my local .venv lacks vs the full CI type-check env; it does not affect the within-env main-vs-branch delta, which is 0.)

Gate tails

• ruff check . → All checks passed!
• ruff format --check . → 996 files already formatted
• pytest tests_py/infrastructure/test_groomer_coordinator.py → 12 passed
• pyright ratchet (--blocking reportOptionalMemberAccess reportOptionalSubscript) → PASS: no blocking rule regressed (exit 0)