fix(hooks): anchor plan-gate to project root + exempt plan file

hooks/pretool-plan-gate.py

@@ -3,19 +3,25 @@
 """
 PreToolUse:Write,Edit Hook: Plan Gate
 
-Blocks implementation when task_plan.md doesn't exist in the project root.
+Blocks implementation when task_plan.md doesn't exist at the project root.
 Forces agents to create a plan before writing implementation code.
 
 This is a HARD GATE — exits 0 with JSON permissionDecision:deny to block the Write/Edit tool.
 
 Detection logic:
 - Tool is Write or Edit
-- Target path is in agents/, skills/
-- task_plan.md does not exist in the project root
+- Target path is under agents/ or skills/ — gated regardless of file extension,
+  because SKILL.md and agent .md files are behavioral specs (implementation, not docs)
+- task_plan.md does not exist at the project root
+
+Project root resolution (Defect 1 fix): the plan is anchored to a stable
+project root, not the session pwd (which is often a deep subdirectory).
+Order: CLAUDE_PROJECT_DIR env → nearest ancestor of cwd containing .git → cwd.
 
 Allow-through conditions:
-- Target file is NOT in agents/, skills/
-- task_plan.md exists in the project root
+- Target file is NOT under agents/ or skills/
+- The target file is itself named task_plan.md (never gate the plan file)
+- task_plan.md exists at the project root
 - PLAN_GATE_BYPASS=1 env var (for use by the plans skill itself)
 """
 
@@ -31,8 +37,8 @@
 
 _BYPASS_ENV = "PLAN_GATE_BYPASS"
 
-# Paths that ARE implementation code — only these get gated.
-# Everything else (docs, config, CI, plans, tests, scripts, hooks) passes through.
+# Paths that ARE gated — all files under these get gated regardless of extension,
+# because SKILL.md and agent .md files are behavioral specs (implementation, not docs).
 _GATED_PREFIXES = (
     "/agents/",
     "/skills/",
@@ -45,6 +51,21 @@ def _is_gated(file_path: str) -> bool:
     return any(prefix in normalised for prefix in _GATED_PREFIXES)
 
 
+def _find_project_root(event_cwd: str) -> Path:
+    """Resolve a stable project root to anchor task_plan.md.
+
+    Order: CLAUDE_PROJECT_DIR env → nearest ancestor of cwd containing .git → cwd.
+    """
+    env = os.environ.get("CLAUDE_PROJECT_DIR")
+    if env:
+        return Path(env).resolve()
+    cur = Path(event_cwd or ".").resolve()
+    for p in [cur, *cur.parents]:
+        if (p / ".git").exists():
+            return p
+    return cur
+
+
 def main() -> None:
     debug = os.environ.get("CLAUDE_HOOKS_DEBUG")
 
@@ -68,16 +89,22 @@ def main() -> None:
     if not file_path:
         sys.exit(0)
 
-    # Only gate implementation code (agents/, skills/, pipelines/).
-    # Everything else (docs, config, CI, plans, tests, scripts, hooks) passes through.
+    # Never gate the plan file itself — it is the unblocker (Defect 2).
+    if Path(file_path).name == "task_plan.md":
+        if debug:
+            print("[plan-gate] Target is task_plan.md — allowing through", file=sys.stderr)
+        sys.exit(0)
+
+    # Gate all files under agents/ or skills/ regardless of extension, because
+    # SKILL.md and agent .md files are behavioral specs. Everything else allows.
     if not _is_gated(file_path):
         if debug:
             print(f"[plan-gate] Not a gated path, allowing: {file_path}", file=sys.stderr)
         sys.exit(0)
 
-    # Resolve project root: prefer event["cwd"], then CLAUDE_PROJECT_DIR, then cwd.
-    cwd_str = event.get("cwd") or os.environ.get("CLAUDE_PROJECT_DIR", ".")
-    base_dir = Path(cwd_str).resolve()
+    # Resolve a stable project root to anchor the plan (Defect 1):
+    # CLAUDE_PROJECT_DIR → nearest ancestor with .git → cwd.
+    base_dir = _find_project_root(event.get("cwd", "."))
 
     plan_path = base_dir / "task_plan.md"
     if plan_path.is_file():

tests/test_plan_gate_hook.py

@@ -0,0 +1,201 @@
+#!/usr/bin/env python3
+"""Tests for pretool-plan-gate.py — the plan-gate hook.
+
+TDD: each test was written BEFORE its fix and verified to FAIL on the
+unfixed code, then PASS after the fix. Covers three defects:
+- Defect 1: project-root resolution (walk up to .git), so a repo-root
+  task_plan.md satisfies the gate regardless of session pwd depth.
+- Defect 2: the plan file itself (task_plan.md) is never gated.
+- Defect 3: docstring/comments accurately state all agents//skills/ files
+  (including .md) are gated.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import subprocess
+import sys
+from pathlib import Path
+
+# Resolve paths
+REPO_ROOT = Path(__file__).resolve().parent.parent
+HOOKS_DIR = REPO_ROOT / "hooks"
+HOOK = HOOKS_DIR / "pretool-plan-gate.py"
+
+
+def _run(event: dict, env_extra: dict | None = None):
+    """Invoke the hook with a synthetic event on stdin; return CompletedProcess."""
+    env = os.environ.copy()
+    # Ensure a clean slate — no inherited bypass / project-dir override.
+    env.pop("PLAN_GATE_BYPASS", None)
+    env.pop("CLAUDE_PROJECT_DIR", None)
+    if env_extra:
+        env.update(env_extra)
+    return subprocess.run(
+        [sys.executable, str(HOOK)],
+        input=json.dumps(event),
+        capture_output=True,
+        text=True,
+        env=env,
+        timeout=10,
+    )
+
+
+def _is_deny(result) -> bool:
+    """A deny prints JSON with permissionDecision:deny to stdout."""
+    stdout = result.stdout.strip()
+    if not stdout:
+        return False
+    try:
+        parsed = json.loads(stdout)
+    except json.JSONDecodeError:
+        return False
+    hso = parsed.get("hookSpecificOutput", {})
+    return hso.get("permissionDecision") == "deny"
+
+
+def _make_git_repo(tmp_path: Path) -> Path:
+    """Create a directory that looks like a git root (has .git)."""
+    (tmp_path / ".git").mkdir()
+    return tmp_path
+
+
+# ---------------------------------------------------------------------------
+# Acceptance 3: gated path, no plan at git root -> deny
+# ---------------------------------------------------------------------------
+
+
+def test_gated_path_no_plan_denies(tmp_path):
+    root = _make_git_repo(tmp_path)
+    skill_file = root / "skills" / "x" / "SKILL.md"
+    skill_file.parent.mkdir(parents=True)
+    event = {
+        "hook_event_name": "PreToolUse",
+        "tool_name": "Write",
+        "tool_input": {"file_path": str(skill_file)},
+        "cwd": str(root),
+    }
+    result = _run(event)
+    assert result.returncode == 0, f"Hook exited non-zero: {result.stderr}"
+    assert _is_deny(result), f"Expected deny, got stdout={result.stdout!r}"
+
+
+# ---------------------------------------------------------------------------
+# Acceptance 1 / Defect 1: plan at git root, deep cwd -> allow
+# ---------------------------------------------------------------------------
+
+
+def test_plan_at_git_root_deep_cwd_allows(tmp_path):
+    root = _make_git_repo(tmp_path)
+    (root / "task_plan.md").write_text("plan\n")
+    deep = root / "skills" / "engineering" / "opensearch-api-client" / "scripts"
+    deep.mkdir(parents=True)
+    skill_file = root / "skills" / "engineering" / "opensearch-api-client" / "SKILL.md"
+    event = {
+        "hook_event_name": "PreToolUse",
+        "tool_name": "Edit",
+        "tool_input": {"file_path": str(skill_file)},
+        "cwd": str(deep),
+    }
+    result = _run(event)
+    assert result.returncode == 0, f"Hook exited non-zero: {result.stderr}"
+    assert not _is_deny(result), f"Expected allow (plan at git root, deep cwd), got deny. stdout={result.stdout!r}"
+
+
+# ---------------------------------------------------------------------------
+# Acceptance 2 / Defect 2: writing task_plan.md is never gated
+# ---------------------------------------------------------------------------
+
+
+def test_writing_plan_file_in_gated_subtree_allows(tmp_path):
+    root = _make_git_repo(tmp_path)
+    # No task_plan.md at root yet — the point is to create one under skills/.
+    target = root / "skills" / "x" / "task_plan.md"
+    target.parent.mkdir(parents=True)
+    event = {
+        "hook_event_name": "PreToolUse",
+        "tool_name": "Write",
+        "tool_input": {"file_path": str(target)},
+        "cwd": str(root),
+    }
+    result = _run(event)
+    assert result.returncode == 0, f"Hook exited non-zero: {result.stderr}"
+    assert not _is_deny(result), f"task_plan.md must never be gated. stdout={result.stdout!r}"
+
+
+# ---------------------------------------------------------------------------
+# Acceptance 4: non-gated path -> allow
+# ---------------------------------------------------------------------------
+
+
+def test_non_gated_path_allows(tmp_path):
+    root = _make_git_repo(tmp_path)
+    doc = root / "docs" / "x.md"
+    doc.parent.mkdir(parents=True)
+    event = {
+        "hook_event_name": "PreToolUse",
+        "tool_name": "Write",
+        "tool_input": {"file_path": str(doc)},
+        "cwd": str(root),
+    }
+    result = _run(event)
+    assert result.returncode == 0, f"Hook exited non-zero: {result.stderr}"
+    assert not _is_deny(result), f"Non-gated path must allow. stdout={result.stdout!r}"
+
+
+# ---------------------------------------------------------------------------
+# Acceptance 5: PLAN_GATE_BYPASS=1 -> allow
+# ---------------------------------------------------------------------------
+
+
+def test_bypass_env_allows(tmp_path):
+    root = _make_git_repo(tmp_path)
+    skill_file = root / "skills" / "x" / "SKILL.md"
+    skill_file.parent.mkdir(parents=True)
+    event = {
+        "hook_event_name": "PreToolUse",
+        "tool_name": "Write",
+        "tool_input": {"file_path": str(skill_file)},
+        "cwd": str(root),
+    }
+    result = _run(event, env_extra={"PLAN_GATE_BYPASS": "1"})
+    assert result.returncode == 0, f"Hook exited non-zero: {result.stderr}"
+    assert not _is_deny(result), f"PLAN_GATE_BYPASS=1 must allow. stdout={result.stdout!r}"
+
+
+# ---------------------------------------------------------------------------
+# Defect 1 (env override): CLAUDE_PROJECT_DIR takes precedence
+# ---------------------------------------------------------------------------
+
+
+def test_claude_project_dir_env_used_for_root(tmp_path):
+    root = tmp_path / "proj"  # no .git — force use of the env var
+    (root / "skills" / "x").mkdir(parents=True)
+    (root / "task_plan.md").write_text("plan\n")
+    skill_file = root / "skills" / "x" / "SKILL.md"
+    event = {
+        "hook_event_name": "PreToolUse",
+        "tool_name": "Write",
+        "tool_input": {"file_path": str(skill_file)},
+        "cwd": str(root / "skills" / "x"),
+    }
+    result = _run(event, env_extra={"CLAUDE_PROJECT_DIR": str(root)})
+    assert result.returncode == 0, f"Hook exited non-zero: {result.stderr}"
+    assert not _is_deny(result), f"CLAUDE_PROJECT_DIR should anchor root. stdout={result.stdout!r}"
+
+
+# ---------------------------------------------------------------------------
+# Defect 3: docstring/comments accurately describe gating scope
+# ---------------------------------------------------------------------------
+
+
+def test_docstring_states_all_files_gated():
+    source = HOOK.read_text()
+    lowered = source.lower()
+    # Misleading "passes through" wording for docs/config/CI/tests must be gone.
+    assert "passes through" not in lowered, "Misleading 'passes through' wording still present"
+    # Must state gating is regardless of extension / behavioral spec rationale.
+    assert "regardless of extension" in lowered or "behavioral spec" in lowered, (
+        "Docstring/comments do not state that all agents//skills/ files are gated"
+    )