fix(hooks): agent_briefing falls back on a degenerate roster, not just an absent directory

[cdeust]

Closes #400. Closes #401.

The defect

_load_specialist_agents() armed _FALLBACK_AGENTS only when
~/.claude/agents/ was absent. Under the plugin-only-dispatch
architecture that directory exists and holds dispatch.md alone, so the
discovered roster was {"dispatch"}, engineer was never a known
specialist, and the briefing hook silently never fired. CI never saw it:
there the directory is absent, so the fallback armed and the bug was
invisible.

The fix, at the root

The fallback predicate becomes "the roster is empty after removing
reserved meta-agent names" instead of "the directory is missing". An absent
directory and a degenerate one are the same failure mode for a briefing
consumer: no specialist to scope memories to. Nothing is special-cased at
the call site.

The agents root moves from Path.home()/".claude"/"agents" to
config.CLAUDE_DIR/"agents" — the CORTEX_CLAUDE_DIR seam (#219) every
other real-data path in this project already uses. That is what makes a
hermetic reproduction possible at all.

Evidence

The regression test spawns the real hook in a subprocess with
CORTEX_CLAUDE_DIR pointed at a throwaway tree containing only
agents/dispatch.md, and asserts both that the briefing fires and that a
receipt row is persisted. Against the pre-fix source it fails with the
reported symptom verbatim — skip: agent 'engineer' not a specialist —
and passes after. Five roster shapes are covered: absent, empty, unparsable,
dispatch-only, and dispatch-plus-a-real-specialist.

Full suite in deterministic order: 7243 passed, 142 skipped, 0 failed.
ruff check and ruff format --check green.

Two things worth a reviewer's eye

_NON_SPECIALIST_META_AGENTS carries no # source: citation, deliberately.
An earlier revision cited ~/.claude/agents/dispatch.md "in this repo";
that file ships in no repository, so the citation was unverifiable by
anyone else — the §8 failure mode in its own right. It is now recorded as
what it is, a convention of the consumer environment whose consequence is
pinned by tests. Filtering a reserved name is also logged now, because the
entire defect class here is silent inactivity.

agent_briefing.py was already over the local 300-line cap before this work
(452 lines) and the fix took it to 476. Rather than defer it, keyword
extraction, the PG connect/query pair and the stderr logger move to sibling
modules; the file is back to 297 lines, so #401 closes by construction.

Out of scope, filed not swallowed

A full-suite run under pytest-randomly surfaced one order-dependent
failure in tests_py/benchmarks/test_lib_init_no_psycopg.py, invisible in
deterministic order and absent when the directory runs alone. It is outside
this change's blast radius and is filed with its measurements as #402.

🤖 Generated with Claude Code

https://claude.ai/code/session_01263uv1QqR8TVzw2jXYUrXn

[cdeust]

ZETETIC-REVIEW: APPROVE

Scope of this review: only 725fe4dc (the first commit, 6c508450, was already reviewed and its blocking finding fixed — not re-judged here).

Move 0 — Ledger reconciliation and seen-defect check

• The commit message itself names both changes it makes (honest source note; log the dropped-name drop) and both are present in the diff — no unmapped path.
• One deferral is present: "One order-dependent failure surfaced under pytest-randomly, outside this change's blast radius and filed with its evidence as Order-dependent failure: test_benchmark_db_resolves_with_psycopg_present fails under random test ordering #402." Verified independently: issue Order-dependent failure: test_benchmark_db_resolves_with_psycopg_present fails under random test ordering #402 is open, carries measured evidence (4 comparative runs: random vs -p no:randomly vs isolated), and names the polluter as outside mcp_server/hooks/agent_briefing*.py. This is a §14.3-acceptable deferral (issue cited + blast radius independently confirmed external), not a bypass.
• Verdict short-circuit: N/A — both checks pass, proceeding.

Stakes calibration

Medium (hook code touched by 1 author in the current window, not auth/billing/crypto, no schema/API change). Full Moves 1–4 applied; 5–6 at the changed lines.

1. Is the split behavior-preserving?

Yes, verified independently, not just asserted:

• The three re-exported names (_extract_task_keywords, _connect, _fetch_agent_context) and the constants (_DATABASE_URL, _MAX_MEMORIES, _MIN_HEAT) are pulled into agent_briefing.py's own module namespace via ordinary from X import Y statements. This is what tests_py/hooks/test_agent_briefing.py's patch.object(hook, "_connect", ...) and hook._extract_task_keywords(...) calls rely on — plain Python import-binding, not __all__ (see NIT below). process_event() still resolves these names off its own module globals, which import rebinds correctly, so patching still intercepts the right call site.
• _load_specialist_agents() (the CLAUDE_DIR-reading roster loader) was not moved and its body is otherwise unchanged except doc comments — confirmed by diff. The CLAUDE_DIR import (from mcp_server.infrastructure.config import CLAUDE_DIR) is untouched, so the monkeypatch behavior tests rely on (monkeypatch.setattr(hook, "CLAUDE_DIR", tmp_path)) is unaffected by the split.
• Import graph among the four files is an acyclic DAG confirmed by reading each file's imports: agent_briefing_log.py (stdlib only) ← agent_briefing_query.py (imports only _log) ← agent_briefing.py; agent_briefing_keywords.py is a leaf (no local imports) ← agent_briefing.py. No cycle, matches the rationale documented in agent_briefing_log.py's own docstring.
• Ran ruff check + ruff format --check on the post-commit contents of all four files independently (via git show, since my local tree is on an unrelated branch): all pass.
• Reproduced the orchestrator's numbers is not repeated here (already measured); I additionally confirmed CI on PR fix(hooks): agent_briefing falls back on a degenerate roster, not just an absent directory #403 is green across all 19 required checks.

2. Are the four files real responsibility boundaries, or line-count gaming?

Real boundaries: agent_briefing_keywords.py = pure text processing, zero I/O (SRP: text-to-keywords); agent_briefing_query.py = the hook's only I/O (PG connect + two SELECT passes); agent_briefing_log.py = a one-function shared primitive that both the entry point and the query module need, sitting below both to avoid the cycle noted above; agent_briefing.py keeps event gating + the roster loader (the part that reads CLAUDE_DIR, deliberately not moved). This reads as "extract by dependency direction and I/O boundary," not "chop wherever the line count lands." No new abstraction, no interface introduced speculatively — this is behavior-preserving Extract Module (Fowler 2018).

3. Is the new comment honest and sufficient per §8?

Yes. It explicitly disclaims being a # source: citation, states plainly what the constant is (a convention of the consumer environment, not a fact this repo can pin), and names two concrete regression tests as the verifiable consequence. I confirmed both cited tests exist on the PR branch: test_agents_dir_with_only_dispatch_falls_back_to_builtin_set (tests_py/hooks/test_agent_briefing.py) and test_agent_briefing_falls_back_when_only_dispatch_agent_is_installed (tests_py/hooks/test_hook_receipts.py, the end-to-end reproduction). This is the correct fix for what the first commit's review blocked — the original comment named an unverifiable file location as though it were a sourced fact; this version names a design decision as a design decision and points to the tests that pin its behavior, which is exactly what §8 asks for when there is no external source to cite.

4. Layer rule, dead code, shims, language, caps

• Layer: hooks/ importing infrastructure/ (CLAUDE_DIR) and handlers/ (injection_receipts) is permitted per the project's own dependency table (docs/module-inventory.md: hooks/ may import infrastructure, core, shared). No violation.
• Dead code / shims: none. Every added symbol is either called (_log in the new if dropped: branch) or re-exported and consumed by tests/entry point. No commented-out code, no TODO-without-ticket.
• Language: consistent English throughout all four files and the commit message.
• Size caps (project-local 300/40, CLAUDE.md § Code Style): agent_briefing.py 297, agent_briefing_keywords.py 94, agent_briefing_query.py 124, agent_briefing_log.py 18 — all under 300. process_event() (~59 lines by line count) is over the 40-line advisory/local cap, but it is untouched by this commit (confirmed by diff — no hunk touches its body); pre-existing, out of this diff's blast radius, not something to relitigate here.

5. What tests can't exercise

• Import-order / module-cache interaction: scripts/test-agent-briefing.py (not part of the pytest suite, not referenced in any CI workflow) deletes only mcp_server.hooks.agent_briefing from sys.modules before re-import, not the three new submodules — harmless here because _connect()'s import psycopg is lazy inside the function body, so the psycopg stub-swap still takes effect at call time regardless of submodule caching. I ran this script directly; it has one failing assertion (feynman not recognized as a specialist) that is pre-existing — reproduced identically against the pre-725fe4dc tree (6c508450) and depends on this machine's local ~/.claude/agents contents, not on anything this commit touches. Not a regression, not blocking.
• CI/local divergence: none observed — CI (Python 3.10–3.13, Windows/SQLite, fuzz, type-check) is green on all 19 checks as of this review.

Non-blocking observations

• MINOR: the new if dropped: _log(...) branch (the second stated purpose of this commit — making the drop diagnosable) has no test asserting the log message fires or its content. test_agents_dir_with_dispatch_and_a_real_specialist_excludes_dispatch exercises the code path (dropped is non-empty) but only asserts on the returned roster, not on stderr/capsys. Low severity (it's a diagnostic log line, not decision logic), but the commit's own stated goal is diagnosability — worth a follow-up assertion.
• NIT: the docstrings/commit message say names are "re-exported via __all__". The actual re-export mechanism is the plain from module import name statement in agent_briefing.py (that's what tests patch against); __all__ only matters for from agent_briefing import *, which nothing in the codebase does. Harmless, but imprecise — could be reworded to "re-exported (see __all__)" or similar to avoid overstating what __all__ is doing.

Verdict

APPROVE. No blocking findings against 725fe4dc. Behavior-preservation of the module split is verified (not just asserted), the four-way decomposition reflects real SRP boundaries, and the replaced _NON_SPECIALIST_META_AGENTS comment corrects the exact defect the first commit's review flagged. Both non-blocking notes (missing log-message assertion; __all__ phrasing) are suggestions for a follow-up, not conditions for this merge.