Skip to content

Commit e7f9a51

Browse files
committed
feat: implement ADR-128 wiring, ADR-129 hook, ADR-130 learning extraction
ADR-129: Add posttool-rename-sweep.py hook that detects git mv commands and greps for stale references to old filenames across .py/.json/.md/.yaml files. Prevents the class of bug that broke 83 tests in ADR-118. ADR-130: Extend pause-work with Phase 3 (EXTRACT LEARNINGS) that queries learning.db for session insights, filters for ADR-worthy decisions using a decision-vs-tip heuristic, and drafts skeleton ADRs with Proposed status. ADR-128: Wire joy-check instruction mode into skill-creation-pipeline (Phase 4 Step 3) and agent-upgrade (Phase 4 Step 2.5) so new/modified skills and agents are validated for positive framing automatically.
1 parent 92c2033 commit e7f9a51

File tree

5 files changed

+490
-6
lines changed

5 files changed

+490
-6
lines changed

hooks/posttool-rename-sweep.py

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
#!/usr/bin/env python3
2+
# hook-version: 1.0.0
3+
"""
4+
PostToolUse Hook: Post-Rename Reference Sweep
5+
6+
After a `git mv` command, scans .py/.json/.md/.yaml files for stale
7+
references to the old filename stem and prints a warning listing them.
8+
9+
Design Principles:
10+
- SILENT by default (only speaks when stale references are found)
11+
- Non-blocking (informational only — always exits 0)
12+
- Fast exit path for non-matching commands (<50ms requirement)
13+
14+
ADR: adr/129-post-rename-reference-sweep.md
15+
"""
16+
17+
import json
18+
import os
19+
import re
20+
import subprocess
21+
import sys
22+
from pathlib import Path
23+
24+
sys.path.insert(0, str(Path(__file__).parent / "lib"))
25+
from stdin_timeout import read_stdin
26+
27+
28+
def extract_git_mv_paths(command: str):
29+
"""Extract (old_path, new_path) from a git mv command string.
30+
31+
Handles:
32+
- Simple: git mv old new
33+
- With flags: git mv -f old new, git mv --force old new
34+
- Quoted: git mv "old path" "new path"
35+
- Chained: git mv old new && git add .
36+
37+
Returns None if the pattern is not found.
38+
"""
39+
# Skip optional flags (-f, --force, -k, -n, -v, etc.) before positional args
40+
# Then match either quoted or unquoted path arguments
41+
# Unquoted paths stop at whitespace, semicolons, pipes, and ampersands
42+
pattern = r"git\s+mv\s+(?:-\S+\s+)*(?:(['\"])(.+?)\1|([^\s;&|]+))\s+(?:(['\"])(.+?)\4|([^\s;&|]+))"
43+
match = re.search(pattern, command)
44+
if not match:
45+
return None
46+
# Groups: 1=old_quote, 2=old_quoted_path, 3=old_unquoted_path,
47+
# 4=new_quote, 5=new_quoted_path, 6=new_unquoted_path
48+
old_path = match.group(2) or match.group(3)
49+
new_path = match.group(5) or match.group(6)
50+
return old_path, new_path
51+
52+
53+
def grep_for_stem(stem: str, search_root: str) -> list:
54+
"""Run grep for stem across .py/.json/.md/.yaml files.
55+
56+
Returns a list of "file:line: text" strings (may be empty).
57+
Excludes .git/ and __pycache__/.
58+
"""
59+
try:
60+
result = subprocess.run(
61+
[
62+
"grep",
63+
"-rnF",
64+
"--include=*.py",
65+
"--include=*.json",
66+
"--include=*.md",
67+
"--include=*.yaml",
68+
"--include=*.yml",
69+
"--exclude-dir=.git",
70+
"--exclude-dir=__pycache__",
71+
stem,
72+
search_root,
73+
],
74+
capture_output=True,
75+
text=True,
76+
timeout=3,
77+
)
78+
if result.returncode not in (0, 1):
79+
# returncode 1 means no matches (normal), >1 is an error
80+
return []
81+
lines = [l for l in result.stdout.splitlines() if l.strip()]
82+
return lines
83+
except subprocess.TimeoutExpired:
84+
return []
85+
except Exception:
86+
return []
87+
88+
89+
def main():
90+
try:
91+
raw = read_stdin(timeout=5)
92+
if not raw:
93+
return
94+
95+
try:
96+
data = json.loads(raw)
97+
except json.JSONDecodeError:
98+
return
99+
100+
tool_name = data.get("tool_name", "")
101+
if tool_name != "Bash":
102+
return
103+
104+
# Skip failed git mv commands — no rename happened
105+
tool_result = data.get("tool_result", {})
106+
if tool_result.get("is_error", False):
107+
return
108+
109+
tool_input = data.get("tool_input", {})
110+
command = tool_input.get("command", "")
111+
112+
if "git mv" not in command:
113+
return
114+
115+
paths = extract_git_mv_paths(command)
116+
if not paths:
117+
return
118+
119+
old_path, new_path = paths
120+
121+
# Extract stem: strip directory and extension
122+
stem = Path(old_path).stem
123+
if not stem or len(stem) < 3:
124+
return
125+
126+
# Search from the repo root if possible, else cwd
127+
search_root = os.getcwd()
128+
129+
matches = grep_for_stem(stem, search_root)
130+
if not matches:
131+
return
132+
133+
# Filter out matches in the renamed file itself (new_path)
134+
new_path_abs = str(Path(new_path).resolve())
135+
filtered = []
136+
for line in matches:
137+
match_file = line.split(":")[0]
138+
try:
139+
if Path(match_file).resolve() == Path(new_path_abs):
140+
continue
141+
except Exception:
142+
pass
143+
filtered.append(line)
144+
145+
if not filtered:
146+
return
147+
148+
max_shown = 20
149+
print(f'[rename-sweep] Stale references to "{stem}" found after git mv:')
150+
for line in filtered[:max_shown]:
151+
# Make path relative to search_root for readability
152+
try:
153+
rel = os.path.relpath(line.split(":")[0], search_root)
154+
rest = ":".join(line.split(":")[1:])
155+
print(f" {rel}:{rest}")
156+
except Exception:
157+
print(f" {line}")
158+
if len(filtered) > max_shown:
159+
print(f" ... and {len(filtered) - max_shown} more")
160+
print("Consider updating these references before committing.")
161+
162+
except Exception as e:
163+
if os.environ.get("CLAUDE_HOOKS_DEBUG"):
164+
import traceback
165+
166+
print(f"[rename-sweep] HOOK-ERROR: {type(e).__name__}: {e}", file=sys.stderr)
167+
traceback.print_exc(file=sys.stderr)
168+
finally:
169+
sys.exit(0)
170+
171+
172+
if __name__ == "__main__":
173+
main()
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
#!/usr/bin/env python3
2+
"""
3+
Tests for the posttool-rename-sweep hook.
4+
5+
Run with: python3 -m pytest hooks/tests/test_posttool_rename_sweep.py -v
6+
"""
7+
8+
import importlib.util
9+
import json
10+
import subprocess
11+
import sys
12+
from pathlib import Path
13+
14+
# ---------------------------------------------------------------------------
15+
# Import the module under test
16+
# ---------------------------------------------------------------------------
17+
18+
HOOK_PATH = Path(__file__).parent.parent / "posttool-rename-sweep.py"
19+
20+
spec = importlib.util.spec_from_file_location("posttool_rename_sweep", HOOK_PATH)
21+
mod = importlib.util.module_from_spec(spec)
22+
spec.loader.exec_module(mod)
23+
24+
25+
# ---------------------------------------------------------------------------
26+
# Helpers
27+
# ---------------------------------------------------------------------------
28+
29+
30+
def run_hook(event: dict) -> tuple[str, str, int]:
31+
"""Run the hook with given event and return (stdout, stderr, exit_code)."""
32+
result = subprocess.run(
33+
[sys.executable, str(HOOK_PATH)],
34+
input=json.dumps(event),
35+
capture_output=True,
36+
text=True,
37+
timeout=10,
38+
)
39+
return result.stdout, result.stderr, result.returncode
40+
41+
42+
# ---------------------------------------------------------------------------
43+
# extract_git_mv_paths tests
44+
# ---------------------------------------------------------------------------
45+
46+
47+
class TestExtractGitMvPaths:
48+
"""Test regex parsing of git mv commands."""
49+
50+
def test_simple(self):
51+
assert mod.extract_git_mv_paths("git mv old.py new.py") == ("old.py", "new.py")
52+
53+
def test_with_directories(self):
54+
assert mod.extract_git_mv_paths("git mv hooks/old.py hooks/new.py") == (
55+
"hooks/old.py",
56+
"hooks/new.py",
57+
)
58+
59+
def test_chained_with_and(self):
60+
assert mod.extract_git_mv_paths("git mv old.py new.py && git add .") == (
61+
"old.py",
62+
"new.py",
63+
)
64+
65+
def test_chained_with_semicolon(self):
66+
assert mod.extract_git_mv_paths("git mv old.py new.py; echo done") == (
67+
"old.py",
68+
"new.py",
69+
)
70+
71+
def test_chained_with_pipe(self):
72+
assert mod.extract_git_mv_paths("git mv old.py new.py | cat") == (
73+
"old.py",
74+
"new.py",
75+
)
76+
77+
def test_flag_f(self):
78+
assert mod.extract_git_mv_paths("git mv -f old.py new.py") == (
79+
"old.py",
80+
"new.py",
81+
)
82+
83+
def test_flag_force(self):
84+
assert mod.extract_git_mv_paths("git mv --force old.py new.py") == (
85+
"old.py",
86+
"new.py",
87+
)
88+
89+
def test_double_quoted_paths(self):
90+
assert mod.extract_git_mv_paths('git mv "old file.py" "new file.py"') == (
91+
"old file.py",
92+
"new file.py",
93+
)
94+
95+
def test_single_quoted_paths(self):
96+
assert mod.extract_git_mv_paths("git mv 'old file.py' 'new file.py'") == (
97+
"old file.py",
98+
"new file.py",
99+
)
100+
101+
def test_no_git_mv(self):
102+
assert mod.extract_git_mv_paths("echo hello") is None
103+
104+
def test_incomplete_git_mv(self):
105+
assert mod.extract_git_mv_paths("git mv") is None
106+
107+
def test_git_mv_with_only_one_arg(self):
108+
# git mv with only source, no dest — should not match
109+
assert mod.extract_git_mv_paths("git mv old.py") is None
110+
111+
112+
# ---------------------------------------------------------------------------
113+
# Tool name and error filtering
114+
# ---------------------------------------------------------------------------
115+
116+
117+
class TestToolFiltering:
118+
"""Test that the hook correctly filters by tool name and error state."""
119+
120+
def test_non_bash_tool_silent(self):
121+
"""Non-Bash tools produce no output."""
122+
event = {
123+
"tool_name": "Edit",
124+
"tool_input": {"command": "git mv old.py new.py"},
125+
"tool_result": {},
126+
}
127+
stdout, stderr, rc = run_hook(event)
128+
assert rc == 0
129+
assert stdout == ""
130+
131+
def test_failed_git_mv_silent(self):
132+
"""Failed git mv commands (is_error=True) produce no output."""
133+
event = {
134+
"tool_name": "Bash",
135+
"tool_input": {"command": "git mv nonexistent.py dest.py"},
136+
"tool_result": {"is_error": True},
137+
}
138+
stdout, stderr, rc = run_hook(event)
139+
assert rc == 0
140+
assert stdout == ""
141+
142+
def test_no_git_mv_in_command_silent(self):
143+
"""Bash commands without git mv produce no output."""
144+
event = {
145+
"tool_name": "Bash",
146+
"tool_input": {"command": "ls -la"},
147+
"tool_result": {},
148+
}
149+
stdout, stderr, rc = run_hook(event)
150+
assert rc == 0
151+
assert stdout == ""
152+
153+
def test_short_stem_silent(self):
154+
"""Stems under 3 characters are skipped to avoid noisy results."""
155+
event = {
156+
"tool_name": "Bash",
157+
"tool_input": {"command": "git mv a.py b.py"},
158+
"tool_result": {},
159+
}
160+
stdout, stderr, rc = run_hook(event)
161+
assert rc == 0
162+
assert stdout == ""
163+
164+
165+
# ---------------------------------------------------------------------------
166+
# Always exits 0
167+
# ---------------------------------------------------------------------------
168+
169+
170+
class TestExitCode:
171+
"""Hook must always exit 0 (non-blocking)."""
172+
173+
def test_empty_stdin(self):
174+
result = subprocess.run(
175+
[sys.executable, str(HOOK_PATH)],
176+
input="",
177+
capture_output=True,
178+
text=True,
179+
timeout=10,
180+
)
181+
assert result.returncode == 0
182+
183+
def test_invalid_json(self):
184+
result = subprocess.run(
185+
[sys.executable, str(HOOK_PATH)],
186+
input="not json",
187+
capture_output=True,
188+
text=True,
189+
timeout=10,
190+
)
191+
assert result.returncode == 0
192+
193+
def test_valid_bash_no_git_mv(self):
194+
event = {
195+
"tool_name": "Bash",
196+
"tool_input": {"command": "echo hello"},
197+
"tool_result": {},
198+
}
199+
stdout, stderr, rc = run_hook(event)
200+
assert rc == 0

pipelines/agent-upgrade/SKILL.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,20 @@ Proceed with implementation? (or specify which items to include/exclude)
235235
**For peer inconsistencies**:
236236
- Align to the majority pattern observed across peers. If peers themselves are inconsistent, align to the most recent or highest-scoring peer.
237237

238-
**Step 3**: Do NOT change any of the following without explicit user direction because domain logic changes require deliberate user decision, not opportunistic bundling:
238+
**Step 3**: Run positive framing validation on the modified agent or skill.
239+
240+
After applying all approved improvements, invoke `joy-check --mode instruction` on the
241+
target file. This ensures that new or modified content uses positive framing per ADR-127.
242+
Positive framing produces instructions agents act on rather than rules they work around.
243+
244+
If joy-check flags lines in the NEW content (content that was just added/modified):
245+
- Fix those lines as part of this same implementation pass
246+
- Do not flag pre-existing content that was not part of the upgrade scope
247+
248+
If joy-check flags lines in EXISTING content (not modified by this upgrade):
249+
- Note them in the upgrade report but do not fix them (out of scope for this upgrade)
250+
251+
**Step 4**: Do NOT change any of the following without explicit user direction because domain logic changes require deliberate user decision, not opportunistic bundling:
239252
- Routing triggers (`triggers:` frontmatter field)
240253
- Domain coverage statements
241254
- Core methodology or phase structure (for skills)

0 commit comments

Comments
 (0)