Skip to content

Commit 492cbba

Browse files
committed
fix(v2.0): address /code-review findings (7-angle high-effort, 40/42 survived → ~12 distinct)
CRITICAL: - pre-commit gate was INVERTED: quality_score.py exits 2 on auto-fail (compile/syntax FAILURE) and 1 on score<80; the hook treated rc>=2 as a non-blocking "infra error", so an uncompilable deck sailed through while a 79/100 blocked. Now ANY nonzero rc blocks. HIGH: - git-guardrails bypassed by `git -C <dir>`/`git -c k=v` global options (patterns required git+subcommand adjacent) → added a global-options prefix; also catches `git add -- .` / `git add :/`. Verified: all bypass forms now DENY, safe forms (force-with-lease, commit, -C status) ALLOW. MEDIUM: - pre-compact + log-reminder classified plan status by WHOLE-FILE substring (APPROVED checked before DRAFT) → a DRAFT plan mentioning "APPROVED"/"COMPLETED" (a checklist item or the status legend) was mislabelled. Now parse the Status FIELD via regex. - pre-compact DRAFT-block default reverted ON→OFF (opt-in): default-ON blocked the harness's AUTOMATIC compaction, which can strand a user at the context ceiling (post-compact-restore re-injects the plan anyway). - statusline ctx-% path now mirrors context-monitor's CLAUDE_PROJECT_DIR-unset fallback (sessions/default/) exactly, instead of hashing the git toplevel. - nightly-repro: date-only last_verified_on now treated as END-of-day, so a same-day file edit isn't flagged a false STALE drift. Deferred (cleanup/altitude, no incorrect output): duplicated get_session_dir/passport helpers, redundant statusline subprocess spawns, the documented working-tree-on-partial-stage trade-off.
1 parent 615d779 commit 492cbba

6 files changed

Lines changed: 64 additions & 37 deletions

File tree

.claude/hooks/git-guardrails.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,22 +35,29 @@
3535
import sys
3636
from pathlib import Path
3737

38+
# Optional git GLOBAL options that can sit between `git` and the subcommand —
39+
# `git -C <dir>`, `git -c k=v`, `--git-dir=`, `--work-tree=`, `--no-pager`,
40+
# `--paginate`. Without this prefix, `git -C /repo reset --hard` (or `git -c x=y
41+
# clean -fd`) silently bypasses every guardrail.
42+
_GO = (r"(?:-C\s+\S+\s+|-c\s+\S+\s+|--git-dir(?:=\S+\s+|\s+\S+\s+)|"
43+
r"--work-tree(?:=\S+\s+|\s+\S+\s+)|--no-pager\s+|--paginate\s+|-p\s+)*")
44+
3845
# (compiled pattern, human reason, safe alternative)
3946
GIT_DENY = [
40-
(re.compile(r"\bgit\s+reset\s+--hard\b"),
47+
(re.compile(r"\bgit\s+" + _GO + r"reset\s+--hard\b"),
4148
"git reset --hard discards uncommitted work irrecoverably.",
4249
"Use `git stash` (recoverable) or reset specific paths."),
43-
(re.compile(r"\bgit\s+clean\b.*(--force\b|(?<![\w-])-[a-z]*f)"),
50+
(re.compile(r"\bgit\s+" + _GO + r"clean\b.*(--force\b|(?<![\w-])-[a-z]*f)"),
4451
"git clean -f/--force deletes UNTRACKED files — including data not yet committed.",
4552
"Inspect with `git clean -n` first; delete specific paths by hand."),
46-
(re.compile(r"\bgit\s+push\b.*(--force(?![\w-])|(?<!-)\s-f\b)"),
53+
(re.compile(r"\bgit\s+" + _GO + r"push\b.*(--force(?![\w-])|(?<!-)\s-f\b)"),
4754
"git push --force clobbers remote history.",
4855
"Use `git push --force-with-lease` if you truly must rewrite a branch."),
49-
(re.compile(r"\bgit\s+add\s+(-A\b|--all\b|\.(?:\s|$))"),
50-
"Blanket staging (git add -A / . ) can stage data, secrets, or settings.local.json. "
56+
(re.compile(r"\bgit\s+" + _GO + r"add\s+(?:--\s+)?(-A\b|--all\b|\.(?:\s|$)|:/)"),
57+
"Blanket staging (git add -A / . / -- . / :/) can stage data, secrets, or settings.local.json. "
5158
"The /commit skill forbids it.",
5259
"Stage specific files: `git add path/to/file ...`."),
53-
(re.compile(r"\bgit\s+(checkout|restore)\s+(--\s+)?\.(?:\s|$)"),
60+
(re.compile(r"\bgit\s+" + _GO + r"(checkout|restore)\s+(--\s+)?\.(?:\s|$)"),
5461
"Mass discard of working-tree changes is irreversible.",
5562
"Discard specific files, or `git stash` to keep them recoverable."),
5663
]

.claude/hooks/log-reminder.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import os
3636
import sys
3737
import hashlib
38+
import re
3839
import subprocess
3940
from pathlib import Path
4041
from datetime import datetime
@@ -72,10 +73,15 @@ def active_plan(project_dir: str) -> str | None:
7273
text = p.read_text(encoding="utf-8", errors="replace")
7374
except Exception:
7475
continue
75-
up = text.upper()
76-
if "COMPLETED" in up and "STATUS" in up and "DRAFT" not in up and "APPROVED" not in up:
76+
# Parse the Status FIELD, not a whole-file substring — a DRAFT plan that
77+
# merely mentions "APPROVED"/"COMPLETED" must not be mis-labelled.
78+
m = re.search(r"^\s*\**\s*status\s*\**\s*:\s*\**\s*"
79+
r"(draft|approved|completed|implemented|in[ -]?progress)",
80+
text, re.IGNORECASE | re.MULTILINE)
81+
v = m.group(1).lower() if m else "in-progress"
82+
if v.startswith(("completed", "implemented")):
7783
continue
78-
status = "APPROVED" if "APPROVED" in up else ("DRAFT" if "DRAFT" in up else "in-progress")
84+
status = "APPROVED" if v.startswith("approved") else ("DRAFT" if v.startswith("draft") else "in-progress")
7985
return f"{p.name} ({status})"
8086
return None
8187

.claude/hooks/pre-compact.py

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
Fires before context compaction to:
66
1. Capture current state (active plan, current task, recent decisions)
77
so post-compact-restore.py can surface it afterwards.
8-
2. Block compaction once when an active plan is still DRAFT, to avoid
9-
losing mid-plan context before the user has approved it. ON by
10-
default (v2.0); opt out via CLAUDE_PRECOMPACT_BLOCK_ON_DRAFT=0.
11-
Blocks at most once per DRAFT plan, fail-open — a user can always
12-
re-run compaction to proceed.
8+
2. OPTIONALLY block compaction once when an active plan is still DRAFT,
9+
to avoid losing mid-plan context before approval. **Opt-in** via
10+
CLAUDE_PRECOMPACT_BLOCK_ON_DRAFT=1 (default OFF — the v2.0 default-ON
11+
flip was reverted: blocking the harness's *automatic* compaction can
12+
strand a user at the context ceiling, and post-compact-restore.py
13+
re-injects the plan afterward anyway). Blocks at most once per DRAFT
14+
plan, fail-open.
1315
1416
The blocking protocol follows modern Claude Code semantics:
1517
exit 0 + JSON {"decision": "block", "reason": "..."} on stdout.
@@ -61,16 +63,18 @@ def find_active_plan(project_dir: str) -> dict | None:
6163
for plan_file in plan_files[:3]: # Check last 3 plans
6264
content = plan_file.read_text()
6365

64-
# Skip completed plans
65-
if "COMPLETED" in content.upper():
66-
continue
67-
68-
# Extract status
69-
status = "in_progress"
70-
if "APPROVED" in content.upper():
71-
status = "approved"
72-
elif "DRAFT" in content.upper():
73-
status = "draft"
66+
# Parse the plan's Status FIELD (e.g. "**Status:** DRAFT"), not a
67+
# whole-file substring — a DRAFT plan whose body merely mentions
68+
# "APPROVED" or "COMPLETED" (a checklist item, or the legend
69+
# "Status (DRAFT/APPROVED/COMPLETED)") must not be mis-classified.
70+
m = re.search(r"^\s*\**\s*status\s*\**\s*:\s*\**\s*"
71+
r"(draft|approved|completed|implemented|in[ -]?progress)",
72+
content, re.IGNORECASE | re.MULTILINE)
73+
v = m.group(1).lower() if m else "in_progress"
74+
if v.startswith(("completed", "implemented")):
75+
continue # skip finished plans
76+
status = "approved" if v.startswith("approved") else (
77+
"draft" if v.startswith("draft") else "in_progress")
7478

7579
# Find current task (first unchecked item)
7680
current_task = None
@@ -151,7 +155,7 @@ def should_block_draft(plan_info: dict | None) -> tuple[bool, str]:
151155
which would make this guard fire again on every subsequent
152156
compaction of the same DRAFT plan.
153157
"""
154-
if os.environ.get("CLAUDE_PRECOMPACT_BLOCK_ON_DRAFT", "1") != "1":
158+
if os.environ.get("CLAUDE_PRECOMPACT_BLOCK_ON_DRAFT", "0") != "1":
155159
return False, ""
156160
if not plan_info or plan_info.get("status") != "draft":
157161
return False, ""

.claude/scripts/statusline.sh

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,14 @@ fi
6464
# Context % — best-effort, persisted by context-monitor.py under the
6565
# session dir keyed by md5(project_dir)[:8].
6666
ctx=""
67-
# Hash the SAME key context-monitor.py writes under: CLAUDE_PROJECT_DIR if set,
68-
# else the git toplevel (the project root) — NOT the raw cwd, which differs in
69-
# a subdirectory and would point at the wrong sessions folder.
70-
proj="${CLAUDE_PROJECT_DIR:-$(git -C "$cwd" rev-parse --show-toplevel 2>/dev/null || printf '%s' "$cwd")}"
71-
hash="$(printf '%s' "$proj" | python3 -c 'import sys,hashlib; print(hashlib.md5(sys.stdin.read().encode()).hexdigest()[:8])' 2>/dev/null)"
67+
# Mirror context-monitor.py's get_session_dir() EXACTLY: CLAUDE_PROJECT_DIR set →
68+
# hash it; unset/empty → the writer falls back to sessions/default/, so do the same
69+
# (hashing the git toplevel here would point at the wrong folder on that path).
70+
if [ -n "${CLAUDE_PROJECT_DIR:-}" ]; then
71+
hash="$(printf '%s' "$CLAUDE_PROJECT_DIR" | python3 -c 'import sys,hashlib; print(hashlib.md5(sys.stdin.read().encode()).hexdigest()[:8])' 2>/dev/null)"
72+
else
73+
hash="default"
74+
fi
7275
pct_file="$HOME/.claude/sessions/${hash}/context-pct.txt"
7376
[ -f "$pct_file" ] && ctx="ctx $(cat "$pct_file" 2>/dev/null)%"
7477
set -e

.githooks/pre-commit

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,12 @@ else
7272
[ -f "$f" ] || continue
7373
python3 scripts/quality_score.py "$f"
7474
rc=$?
75-
if [ "$rc" -eq 1 ]; then
76-
echo "✗ quality gate: '$f' scored below 80." >&2
75+
# quality_score.py: rc 2 = auto-fail (compile/syntax FAILURE), rc 1 =
76+
# score < 80 (or scorer error / file-not-found). BOTH must block — a
77+
# file that fails to compile is the worst case, not an "infra error".
78+
if [ "$rc" -ne 0 ]; then
79+
echo "✗ quality gate: '$f' failed (rc=$rc — 2=compile/syntax failure, 1=score<80 or scorer error)." >&2
7780
FAIL=1
78-
elif [ "$rc" -ge 2 ]; then
79-
echo "⚠ quality_score.py errored on '$f' (rc=$rc) — not blocking on infra error; review manually." >&2
8081
fi
8182
done <<< "$STAGED"
8283
else

scripts/nightly-repro-check.sh

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,15 @@ if not pdir.is_dir():
2626
print("no quality_reports/passports/ — nothing to check"); sys.exit(0)
2727
2828
def parse_iso(s):
29-
s = s.strip().strip('"').strip("'").replace("Z", "+00:00")
30-
try: return datetime.datetime.fromisoformat(s).timestamp()
31-
except Exception: return None
29+
s = s.strip().strip('"').strip("'")
30+
date_only = ("T" not in s) and (":" not in s) # e.g. "2026-06-01"
31+
try:
32+
dt = datetime.datetime.fromisoformat(s.replace("Z", "+00:00"))
33+
if date_only: # treat as END of that day so a
34+
dt = dt.replace(hour=23, minute=59, second=59) # same-day edit isn't STALE
35+
return dt.timestamp()
36+
except Exception:
37+
return None
3238
3339
stale = []
3440
for pf in sorted(pdir.glob("*.yaml")):

0 commit comments

Comments
 (0)