diff --git a/scripts/hooks/preflight_reminder.py b/scripts/hooks/preflight_reminder.py index 530c9e7c..7ea081b2 100644 --- a/scripts/hooks/preflight_reminder.py +++ b/scripts/hooks/preflight_reminder.py @@ -2,7 +2,26 @@ When the user prompt indicates code-implementation intent, inject a system-reminder elevating bicameral.preflight above the agent's default -tool-selection priority. +tool-selection priority — but only as a write-op gate, not a discovery +gate. + +Reconciles with #146: that issue's failure mode was the agent doing +file inspection and then NEVER calling preflight at all. The original +fix (#155) over-corrected by telling the agent to call preflight +"before any file-inspection tool". That short-circuited the caller-LLM +discovery the rest of the contract depends on — preflight needs +``file_paths`` populated to do region-anchored retrieval, and the agent +can't extract file paths if we forbid it from looking at the codebase +first. + +Updated contract: + - Read / Grep / Glob FIRST — caller LLM resolves "the reorder feature" + to concrete file paths. + - bicameral.preflight(topic, file_paths) — fed by step 1's discovery, + so the server gets the deterministic binds_to lookup it needs. + - Write ops (Edit / Write / NotebookEdit / mutating Bash) — preflight + must precede the first one. This is the line that #146 was + actually defending; the asserter (assert_flow_2) gates exactly this. Per Claude Code 2.x hook contract: read JSON from stdin, write JSON to stdout shaped as ``{"hookSpecificOutput": {"hookEventName": @@ -24,12 +43,29 @@ REMINDER_TEXT = ( "\n" - "This prompt indicates code-implementation intent. Before invoking " - "any file-inspection tool (Read, Grep, Bash, Glob), call " - "`bicameral.preflight` to surface prior decisions, drifted regions, " - "and open questions for the affected area. The skill produces zero " - "output when nothing relevant is found, so the cost of firing is " - "bounded. Auto-fire is the contract; do not skip.\n" + "This prompt indicates code-implementation intent. Auto-fire is the " + "contract: `bicameral.preflight` MUST run BEFORE any write op (Edit, " + "Write, NotebookEdit, or Bash that mutates state — git commit, file " + "writes, mv/rm/etc.).\n" + "\n" + "Read-only discovery FIRST, then preflight, then writes.\n" + "\n" + "Recommended sequence:\n" + " 1. Use Read / Grep / Glob to map the user's request to concrete " + "file paths. The user often names a feature ('the reorder feature') " + "rather than a file (`reorder.ts`); resolve that mapping yourself " + "before calling preflight.\n" + " 2. Call `bicameral.preflight(topic, file_paths)` with BOTH a " + "natural-language topic AND the concrete file paths discovered in " + "step 1. `file_paths=[]` defeats region-anchored retrieval — the " + "server uses these to look up bound decisions deterministically; " + "topic alone falls back to fuzzy text similarity.\n" + " 3. Read the surfaced decisions / drifted regions / open questions, " + "then proceed with the implementation.\n" + "\n" + "The skill produces zero output when nothing relevant is found, so " + "the cost of firing is bounded. Skipping preflight is the contract " + "violation, not running discovery first.\n" "" ) diff --git a/skills/bicameral-preflight/SKILL.md b/skills/bicameral-preflight/SKILL.md index 2930574c..ffad3af4 100644 --- a/skills/bicameral-preflight/SKILL.md +++ b/skills/bicameral-preflight/SKILL.md @@ -139,10 +139,18 @@ case proceed directly to step 2. ### 2. Call `bicameral.preflight` for region-anchored and HITL state +**Discover first, then preflight.** Before this call, use Read / Grep / Glob to +resolve the user's request to concrete file paths. The user often names a +*feature* ("the reorder feature", "the rate limiter") rather than a *file*; the +caller LLM is responsible for that mapping — the server does deterministic +retrieval, not semantic guessing. A topic-only call falls back to fuzzy text +similarity over decision descriptions; passing `file_paths` engages the +high-precision `binds_to` graph lookup. + ``` bicameral.preflight( topic="", - file_paths=["", ...], # include if you've scoped the files + file_paths=["", ...], # discovered in step 1 ) ``` @@ -160,8 +168,12 @@ those into your in-scope set. The response also carries an optional `sync_metrics` field — skip rendering it. If `response.product_stage` is non-null, surface it verbatim to the user as a brief note (shown once per device only). -**Omit `file_paths`** if you haven't scoped the files yet (early "how should I -approach X?" queries). The handler still runs sync and HITL checks. +**`file_paths` may be omitted only** for genuinely abstract queries with no +file referent yet (e.g. *"how should I approach building a retry helper?"* — +no existing files to point at). For implementation prompts that name or imply +a feature backed by existing code, populate `file_paths` from your discovery. +The handler still runs sync and HITL checks either way; passing `file_paths` +just unlocks the precision channel. ### 2.5 Resolve pending compliance checks if present diff --git a/tests/test_preflight_hook.py b/tests/test_preflight_hook.py index 1c17600e..fe76eb01 100644 --- a/tests/test_preflight_hook.py +++ b/tests/test_preflight_hook.py @@ -96,3 +96,27 @@ def test_handles_natural_contradiction_prompt(): inner = _hook_output(json.loads(out)) assert "additionalContext" in inner assert "bicameral.preflight" in inner["additionalContext"] + + +def test_reminder_gates_writes_not_discovery(): + """The reminder must allow Read/Grep/Glob discovery before preflight, + and gate preflight against WRITE ops only. An earlier shape ("call + preflight before any file-inspection tool") short-circuited the + caller-LLM discovery the rest of the contract depends on (the agent + needs to map "the X feature" → concrete file paths via Read/Grep/Glob + before calling preflight). Lock the new posture in so future edits + don't quietly regress it. + """ + payload = {"prompt": "refactor the reorder feature to a text-editor flow"} + rc, out, _ = _run_hook(json.dumps(payload)) + assert rc == 0 + ctx = _hook_output(json.loads(out))["additionalContext"] + # Affirmative: discovery comes first, write op is the gate. + assert "Read-only discovery FIRST" in ctx + assert "BEFORE any write op" in ctx + assert "Edit, Write" in ctx + # The reminder should explicitly tell the agent to populate file_paths. + assert "file_paths" in ctx + # Negative: must NOT forbid file-inspection tools (the old shape). + assert "before any file-inspection tool" not in ctx + assert "Before invoking any file-inspection tool" not in ctx