From 5d0d92b553c492b7a71fb8ac5eb741072c1f6a04 Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:59:05 +0900 Subject: [PATCH 01/14] fix(agents): unify provider hook adapters and guard notebook targets Move shared Codex, Devin, pi, and Hermes adapter logic into hooks/lib/provider_adapter.py. Make the Devin adapter executable and pass normalized payloads from every provider. Stop mutating Hermes input; log session-start compaction only for compact/resume. Cover NotebookEdit in the file guard and lint, and single-file Grep in the read guard. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../hooks/rules/related_test_extensions.json | 10 + agents/codex/hooks/hook_adapter.py | 147 +-------- agents/devin/hooks/hook_adapter.py | 241 ++------------ agents/hermes/hooks/hook_adapter.py | 246 ++------------ agents/hooks.json | 6 +- agents/hooks/audit_log.py | 4 +- agents/hooks/guard_file.py | 12 +- agents/hooks/lib/hook_input.py | 11 +- agents/hooks/lib/provider_adapter.py | 309 ++++++++++++++++++ agents/pi/hooks/hook_adapter.py | 241 ++------------ pyproject.toml | 1 + tests/agents/hooks/test_hook_boundary.py | 49 +-- tests/agents/hooks/test_tool_target_fields.py | 69 ++++ tests/agents/test_entry_points.py | 33 ++ tests/agents/test_hook_generation.py | 18 +- tests/agents/test_provider_adapters.py | 128 ++++++++ tests/agents/test_python_runtime.py | 81 +++-- 17 files changed, 762 insertions(+), 844 deletions(-) mode change 100644 => 100755 agents/devin/hooks/hook_adapter.py create mode 100644 agents/hooks/lib/provider_adapter.py create mode 100644 tests/agents/hooks/test_tool_target_fields.py create mode 100644 tests/agents/test_entry_points.py create mode 100644 tests/agents/test_provider_adapters.py diff --git a/.agents/hooks/rules/related_test_extensions.json b/.agents/hooks/rules/related_test_extensions.json index 05b2d9a0..09706332 100644 --- a/.agents/hooks/rules/related_test_extensions.json +++ b/.agents/hooks/rules/related_test_extensions.json @@ -4,6 +4,7 @@ "tests/agents/hooks/test_check_selection.py" ], "agents/hooks/rules/secret_path_policy.json": [ + "tests/agents/hooks/test_secret_path_policy.py", "tests/agents/codex/test_hook_adapter_cli.py", "tests/agents/hooks/test_verification_rule.py" ], @@ -331,5 +332,14 @@ "agents/pi/hooks/hook_adapter.py": [ "tests/agents/pi/test_pi_hook_adapter.py", "tests/agents/test_hook_generation.py" + ], + "agents/hooks/lib/provider_adapter.py": [ + "tests/agents/test_provider_adapters.py", + "tests/agents/hooks/test_hook_boundary.py", + "tests/agents/codex/test_hook_adapter_cli.py", + "tests/agents/devin/test_hook_adapter.py", + "tests/agents/hermes/test_hermes_hook_adapter.py", + "tests/agents/pi/test_pi_hook_adapter.py", + "tests/agents/test_python_runtime.py" ] } diff --git a/agents/codex/hooks/hook_adapter.py b/agents/codex/hooks/hook_adapter.py index 6de6b9af..e50cc4e9 100755 --- a/agents/codex/hooks/hook_adapter.py +++ b/agents/codex/hooks/hook_adapter.py @@ -1,152 +1,25 @@ #!/usr/bin/env -S python3 -IB """Translate Codex hook payloads at the boundary to shared hook implementations.""" -import json -import os from pathlib import Path import sys SCRIPT = Path(__file__).resolve() -COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name == ".codex" else "hooks") +# Deployed adapters live in ~/./hooks beside ~/.claude/hooks; the source tree keeps agents/hooks. +COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name.startswith(".") else "hooks") sys.path.insert(0, str(COMMON)) sys.path.insert(0, str(COMMON / "lib")) -import audit_log -import guard_command -import guard_git -import guard_file -import hook_input -import lint_format -import patch_input -import secret_path_policy -import shell_syntax +import provider_adapter -EXTENSIONS = lint_format.EXTENSIONS - - -def shared_directory() -> Path: - return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" - - -def translated(payload: dict, tool: str, values: dict) -> dict: - return { - "tool_name": tool, - "tool_input": values, - "session_id": payload.get("session_id") or "", - "cwd": payload.get("cwd") or "", - "provider": "codex", - } - - -def lint(payload: dict) -> int: - """Report shared quality diagnostics using Codex's PostToolUse JSON contract.""" - lint_format.emit(lint_format.diagnostics(payload)) - return 0 - - -def harness(payload: dict) -> int: - values = payload.get("tool_input") or {} - body = hook_input.patch_body(values, payload.get("tool_name")) - for name in patch_input.paths(body): - absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) - if status := guard_file.check( - "harness", translated(payload, "apply_patch", {"file_path": absolute}), shared_directory() - ): - return status - return 0 - - -def check_paths(mode: str, payload: dict) -> int: - policy = Path( - os.environ.get("AGENT_SECRET_PATH_POLICY", str(shared_directory() / "rules/secret_path_policy.json")) - ) - rules = secret_path_policy.load(policy) - values = payload.get("tool_input") or {} - command = values.get("command") or values.get("cmd") or "" - if mode == "command": - try: - commands = shell_syntax.parse(command) - except shell_syntax.UnsupportedSyntax as error: - raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error - candidates = [word for item in commands for word in (*item.arguments, *item.redirections)] - candidates += [word.partition("=")[2] for word in candidates if "=" in word] - else: - candidates = [ - values.get("file_path") or values.get("path") or "", - *patch_input.paths(hook_input.patch_body(values, payload.get("tool_name"))), - ] - for value in candidates: - if rule := secret_path_policy.blocked_rule(value, rules): - audit_log.blocked( - "Bash" if mode == "command" else "apply_patch", - command if mode == "command" else value, - f"{rule['reason']}: {value}", - "adapt_guard_secret_paths.sh", - payload.get("session_id") or "", - payload=payload, - targets=() if mode == "command" else (value,), - ) - raise ValueError( - f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" - ) - return 0 - - -def dispatch(kind: str, arguments: list[str], payload: dict) -> int: - values = payload.get("tool_input") or {} - if kind == "shell": - command = translated(payload, "Bash", {"command": values.get("command") or values.get("cmd") or ""}) - if arguments[0] in {"allowed", "forbidden"}: - return guard_command.check(arguments[0], command, shared_directory()) - if arguments[0] == "git": - return guard_git.check(command) - return guard_file.check("commit", command, shared_directory()) - if kind == "lint": - return lint(payload) - if kind == "harness": - return harness(payload) - if kind == "paths": - return check_paths(arguments[0], payload) - if arguments[0] == "prompt": - return guard_file.check("prompt", payload, shared_directory()) - return guard_file.check( - "write", - translated( - payload, - "Edit", - {"content": patch_input.added_text(hook_input.patch_body(values, payload.get("tool_name")))}, - ), - shared_directory(), - ) - - -def main(arguments: list[str]) -> int: - kind = arguments[0] if arguments else "" - rest = arguments[1:] - valid = ( - (kind in {"lint", "harness"} and not rest) - or (kind == "shell" and len(rest) == 1 and rest[0] in {"allowed", "forbidden", "git", "commit"}) - or ( - kind in {"paths", "content"} - and len(rest) == 1 - and rest[0] in ({"command", "patch"} if kind == "paths" else {"prompt", "apply-patch"}) - ) +ADAPTER = provider_adapter.Adapter( + provider_adapter.Profile( + name="codex", + content_modes=("prompt", "apply-patch"), + cwd_fallback=provider_adapter.no_cwd, ) - if not valid or any(value in {"-h", "--help"} for value in rest): - print( - "Usage: hook_adapter.py ", - file=sys.stderr, - ) - return 1 - try: - payload = json.load(sys.stdin) - if payload.get("cwd"): - os.chdir(payload["cwd"]) - return dispatch(kind, rest, payload) - except (OSError, ValueError, TypeError, AttributeError) as error: - print(f"BLOCKED: {error}", file=sys.stderr) - return 2 +) if __name__ == "__main__": - sys.exit(main(sys.argv[1:])) + sys.exit(ADAPTER.main(sys.argv[1:])) diff --git a/agents/devin/hooks/hook_adapter.py b/agents/devin/hooks/hook_adapter.py old mode 100644 new mode 100755 index 8a245a98..f0440b2c --- a/agents/devin/hooks/hook_adapter.py +++ b/agents/devin/hooks/hook_adapter.py @@ -1,224 +1,49 @@ #!/usr/bin/env -S python3 -IB """Translate Devin hook payloads at the boundary to shared hook implementations.""" -import json import os from pathlib import Path import sys SCRIPT = Path(__file__).resolve() +# Deployed adapters live in ~/./hooks beside ~/.claude/hooks; the source tree keeps agents/hooks. COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name.startswith(".") else "hooks") sys.path.insert(0, str(COMMON)) sys.path.insert(0, str(COMMON / "lib")) -import audit_log -import guard_command -import guard_git -import guard_file -import hook_input -import lint_format -import patch_input -import secret_path_policy -import shell_syntax -import verification_session - -EXTENSIONS = lint_format.EXTENSIONS - -CANONICAL_TOOLS = { - "exec": "Bash", - "read": "Read", - "write": "Write", - "edit": "Edit", - "multi_edit": "MultiEdit", - "notebook_edit": "NotebookEdit", - "run_subagent": "Task", - "webfetch": "WebFetch", -} - -NATIVE_EVENTS = { - "session-start": "SessionStart", - "pre-tool-use": "PreToolUse", - "stop": "Stop", - "session-end": "SessionEnd", -} - - -def shared_directory() -> Path: - return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" - - -def normalized(payload: dict) -> dict: - """Preserve the provider event while supplying the fields shared checks require.""" - result = dict(payload) - result.setdefault("cwd", os.environ.get("DEVIN_PROJECT_DIR") or "") - result["provider"] = "devin" - if canonical := CANONICAL_TOOLS.get(result.get("tool_name") or ""): - result["tool_name"] = canonical - return result - - -def devin_result(event: str, result: dict) -> dict: - """Convert the shared Claude-shaped decision into Devin's native output contract.""" - specific = result.get("hookSpecificOutput") - if isinstance(specific, dict) and specific.get("permissionDecision") == "deny": - reason = specific.get("permissionDecisionReason") or "Blocked by hook policy" - return {"decision": "block", "reason": reason} - if message := result.get("systemMessage"): - return { - "hookSpecificOutput": { - "hookEventName": NATIVE_EVENTS.get(event, event), - "additionalContext": message, - } - } - return result - - -def native(event: str, payload: dict) -> int: - normalized_payload = normalized(payload) - try: - result = verification_session.dispatch("devin", event, normalized_payload) - except (OSError, ValueError, TypeError, KeyError) as error: - result = verification_session.failure(event, normalized_payload, error) - print(json.dumps(devin_result(event, result), ensure_ascii=True, separators=(",", ":"))) - return 0 - - -def audit(kind: str, payload: dict) -> int: - context = normalized(payload) - entry = audit_log.record(kind, context, "devin") - cwd = context.get("cwd") or "." - audit_log.append_record(entry, cwd=Path(cwd)) - return 0 - - -def lint(payload: dict) -> int: - """Report shared quality diagnostics using Devin's PostToolUse JSON contract.""" - lint_format.emit(lint_format.diagnostics(normalized(payload))) - return 0 - - -def harness(payload: dict) -> int: - values = payload.get("tool_input") or {} - candidates = [values.get("file_path") or values.get("path") or ""] - candidates += patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))) - for name in candidates: - if not name: - continue - absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) - if status := guard_file.check( - "harness", - normalized(payload) | {"tool_name": "apply_patch", "tool_input": {"file_path": absolute}}, - shared_directory(), - ): - return status - return 0 - - -def check_paths(mode: str, payload: dict) -> int: - policy = Path( - os.environ.get("AGENT_SECRET_PATH_POLICY", str(shared_directory() / "rules/secret_path_policy.json")) - ) - rules = secret_path_policy.load(policy) - values = payload.get("tool_input") or {} - command = values.get("command") or values.get("cmd") or "" - if mode == "command": - try: - commands = shell_syntax.parse(command) - except shell_syntax.UnsupportedSyntax as error: - raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error - candidates = [word for item in commands for word in (*item.arguments, *item.redirections)] - candidates += [word.partition("=")[2] for word in candidates if "=" in word] - else: - candidates = [ - values.get("file_path") or values.get("path") or "", - *patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))), - ] - context = normalized(payload) - for value in candidates: - if rule := secret_path_policy.blocked_rule(value, rules): - audit_log.blocked( - "Bash" if mode == "command" else "apply_patch", - command if mode == "command" else value, - f"{rule['reason']}: {value}", - "adapt_guard_secret_paths.sh", - payload.get("session_id") or "", - payload=context, - targets=() if mode == "command" else (value,), - ) - raise ValueError( - f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" - ) - return 0 - - -def content(mode: str, payload: dict) -> int: - if mode == "apply-patch": - values = payload.get("tool_input") or {} - body = hook_input.patch_body(values, normalized(payload).get("tool_name") or "apply_patch") - return guard_file.check( - "write", - normalized(payload) | {"tool_name": "Edit", "tool_input": {"content": patch_input.added_text(body)}}, - shared_directory(), - ) - return guard_file.check(mode, normalized(payload), shared_directory()) - - -def dispatch(kind: str, arguments: list[str], payload: dict) -> int: - values = payload.get("tool_input") or {} - if kind == "native": - return native(arguments[0], payload) - if kind == "audit": - return audit(arguments[0], payload) - if kind == "shell": - command = normalized(payload) | { - "tool_name": "Bash", - "tool_input": {"command": values.get("command") or values.get("cmd") or ""}, - } - if arguments[0] in {"allowed", "forbidden"}: - return guard_command.check(arguments[0], command, shared_directory()) - if arguments[0] == "git": - return guard_git.check(command) - return guard_file.check("commit", command, shared_directory()) - if kind == "lint": - return lint(payload) - if kind == "harness": - return harness(payload) - if kind == "paths": - return check_paths(arguments[0], payload) - return content(arguments[0], payload) - - -def main(arguments: list[str]) -> int: - kind = arguments[0] if arguments else "" - rest = arguments[1:] - valid = ( - (kind == "native" and len(rest) == 1 and rest[0] in NATIVE_EVENTS) - or (kind == "audit" and len(rest) == 1 and rest[0] in {"tool", "compaction"}) - or (kind in {"lint", "harness"} and not rest) - or (kind == "shell" and len(rest) == 1 and rest[0] in {"allowed", "forbidden", "git", "commit"}) - or (kind == "paths" and len(rest) == 1 and rest[0] in {"command", "patch"}) - or (kind == "content" and len(rest) == 1 and rest[0] in {"prompt", "read", "write", "apply-patch"}) +import provider_adapter + + +def project_directory() -> str: + """Devin omits cwd from hook input and exports the project directory instead.""" + return os.environ.get("DEVIN_PROJECT_DIR") or "" + + +ADAPTER = provider_adapter.Adapter( + provider_adapter.Profile( + name="devin", + tools={ + "exec": "Bash", + "read": "Read", + "write": "Write", + "edit": "Edit", + "multi_edit": "MultiEdit", + "notebook_edit": "NotebookEdit", + "run_subagent": "Task", + "webfetch": "WebFetch", + }, + events={ + "session-start": "SessionStart", + "pre-tool-use": "PreToolUse", + "stop": "Stop", + "session-end": "SessionEnd", + }, + compaction_events={"SessionStart": "SessionStart", "PostCompaction": "PostCompaction"}, + formatter=provider_adapter.additional_context, + cwd_fallback=project_directory, ) - if not valid or any(value in {"-h", "--help"} for value in rest): - print( - "Usage: hook_adapter.py ", - file=sys.stderr, - ) - return 1 - try: - payload = json.load(sys.stdin) - if not isinstance(payload, dict): - payload = {} - cwd = payload.get("cwd") or os.environ.get("DEVIN_PROJECT_DIR") - if cwd: - os.chdir(cwd) - return dispatch(kind, rest, payload) - except (OSError, ValueError, TypeError, AttributeError) as error: - print(f"BLOCKED: {error}", file=sys.stderr) - return 2 +) if __name__ == "__main__": - sys.exit(main(sys.argv[1:])) + sys.exit(ADAPTER.main(sys.argv[1:])) diff --git a/agents/hermes/hooks/hook_adapter.py b/agents/hermes/hooks/hook_adapter.py index 030f67be..51bef562 100755 --- a/agents/hermes/hooks/hook_adapter.py +++ b/agents/hermes/hooks/hook_adapter.py @@ -1,232 +1,44 @@ #!/usr/bin/env -S python3 -IB """Translate Hermes hook payloads at the boundary to shared hook implementations.""" -import json -import os from pathlib import Path import sys SCRIPT = Path(__file__).resolve() +# Deployed adapters live in ~/./hooks beside ~/.claude/hooks; the source tree keeps agents/hooks. COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name.startswith(".") else "hooks") sys.path.insert(0, str(COMMON)) sys.path.insert(0, str(COMMON / "lib")) -import audit_log -import guard_command -import guard_git -import guard_file -import hook_dispatch -import hook_input -import lint_format -import patch_input -import secret_path_policy -import shell_syntax -import verification_session - -CANONICAL_TOOLS = { - "terminal": "Bash", - "exec_command": "Bash", - "shell": "Bash", - "read_file": "Read", - "write_file": "Write", - "edit_file": "Edit", - "patch": "Edit", - "apply_patch": "apply_patch", - "web_search": "WebSearch", - "fetch_url": "WebFetch", -} - -NATIVE_EVENTS = { - "session-start": "on_session_start", - "pre-tool-use": "pre_tool_call", - "stop": "pre_verify", - "session-end": "on_session_end", -} - - -def shared_directory() -> Path: - return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" - - -def manifest_path() -> Path: - return Path(os.environ.get("AGENT_HARNESS_HERMES_MANIFEST", str(Path.home() / ".hermes/hooks.json"))) - - -def normalized(payload: dict) -> dict: - """Preserve the provider event while supplying the fields shared checks require.""" - result = dict(payload) - values = result.get("tool_input") - if not isinstance(values, dict): - values = result.get("args") - result["tool_input"] = dict(values) if isinstance(values, dict) else {} - result["tool_input"].setdefault("file_path", result["tool_input"].get("path") or "") - result.setdefault("cwd", os.getcwd()) - result["provider"] = "hermes" - if canonical := CANONICAL_TOOLS.get(result.get("tool_name") or ""): - result["tool_name"] = canonical - return result - - -def hermes_result(event: str, result: dict) -> dict: - """Convert the shared Claude-shaped decision into Hermes's blocking contract.""" - specific = result.get("hookSpecificOutput") - if isinstance(specific, dict) and specific.get("permissionDecision") == "deny": - reason = specific.get("permissionDecisionReason") or "Blocked by hook policy" - return {"decision": "block", "reason": reason} - if message := result.get("systemMessage"): - return {"systemMessage": message} - return result - - -def native(event: str, payload: dict) -> int: - normalized_payload = normalized(payload) - try: - result = verification_session.dispatch("hermes", event, normalized_payload) - except (OSError, ValueError, TypeError, KeyError) as error: - result = verification_session.failure(event, normalized_payload, error) - print(json.dumps(hermes_result(event, result), ensure_ascii=True, separators=(",", ":"))) - return 0 - - -def audit(kind: str, payload: dict) -> int: - context = normalized(payload) - entry = audit_log.record(kind, context, "hermes") - cwd = context.get("cwd") or "." - audit_log.append_record(entry, cwd=Path(cwd)) - return 0 - - -def lint(payload: dict) -> int: - """Report shared quality diagnostics using the post-tool JSON contract.""" - lint_format.emit(lint_format.diagnostics(normalized(payload))) - return 0 - - -def harness(payload: dict) -> int: - values = normalized(payload).get("tool_input") or {} - candidates = [values.get("file_path") or values.get("path") or ""] - candidates += patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))) - for name in candidates: - if not name: - continue - absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) - if status := guard_file.check( - "harness", - normalized(payload) | {"tool_name": "apply_patch", "tool_input": {"file_path": absolute}}, - shared_directory(), - ): - return status - return 0 - - -def check_paths(mode: str, payload: dict) -> int: - policy = Path( - os.environ.get("AGENT_SECRET_PATH_POLICY", str(shared_directory() / "rules/secret_path_policy.json")) - ) - rules = secret_path_policy.load(policy) - values = normalized(payload).get("tool_input") or {} - command = values.get("command") or values.get("cmd") or "" - if mode == "command": - try: - commands = shell_syntax.parse(command) - except shell_syntax.UnsupportedSyntax as error: - raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error - candidates = [word for item in commands for word in (*item.arguments, *item.redirections)] - candidates += [word.partition("=")[2] for word in candidates if "=" in word] - else: - candidates = [ - values.get("file_path") or values.get("path") or "", - *patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))), - ] - context = normalized(payload) - for value in candidates: - if rule := secret_path_policy.blocked_rule(value, rules): - audit_log.blocked( - "Bash" if mode == "command" else "apply_patch", - command if mode == "command" else value, - f"{rule['reason']}: {value}", - "adapt_guard_secret_paths.sh", - payload.get("session_id") or "", - payload=context, - targets=() if mode == "command" else (value,), - ) - raise ValueError( - f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" - ) - return 0 - - -def content(mode: str, payload: dict) -> int: - if mode == "apply-patch": - values = normalized(payload).get("tool_input") or {} - body = hook_input.patch_body(values, normalized(payload).get("tool_name") or "apply_patch") - return guard_file.check( - "write", - normalized(payload) | {"tool_name": "Edit", "tool_input": {"content": patch_input.added_text(body)}}, - shared_directory(), - ) - return guard_file.check(mode, normalized(payload), shared_directory()) - - -def dispatch(kind: str, arguments: list[str], payload: dict) -> int: - values = normalized(payload).get("tool_input") or {} - if kind == "native": - return native(arguments[0], payload) - if kind == "audit": - return audit(arguments[0], payload) - if kind == "dispatch": - return hook_dispatch.run(manifest_path(), arguments[0], payload) - if kind == "shell": - command = normalized(payload) | { - "tool_name": "Bash", - "tool_input": {"command": values.get("command") or values.get("cmd") or ""}, - } - if arguments[0] in {"allowed", "forbidden"}: - return guard_command.check(arguments[0], command, shared_directory()) - if arguments[0] == "git": - return guard_git.check(command) - return guard_file.check("commit", command, shared_directory()) - if kind == "lint": - return lint(payload) - if kind == "harness": - return harness(payload) - if kind == "paths": - return check_paths(arguments[0], payload) - return content(arguments[0], payload) - - -def main(arguments: list[str]) -> int: - kind = arguments[0] if arguments else "" - rest = arguments[1:] - valid = ( - (kind == "native" and len(rest) == 1 and rest[0] in NATIVE_EVENTS) - or (kind == "audit" and len(rest) == 1 and rest[0] in {"tool", "compaction"}) - or (kind == "dispatch" and len(rest) == 1) - or (kind in {"lint", "harness"} and not rest) - or (kind == "shell" and len(rest) == 1 and rest[0] in {"allowed", "forbidden", "git", "commit"}) - or (kind == "paths" and len(rest) == 1 and rest[0] in {"command", "patch"}) - or (kind == "content" and len(rest) == 1 and rest[0] in {"prompt", "read", "write", "apply-patch"}) +import provider_adapter + +ADAPTER = provider_adapter.Adapter( + provider_adapter.Profile( + name="hermes", + tools={ + "terminal": "Bash", + "exec_command": "Bash", + "shell": "Bash", + "read_file": "Read", + "write_file": "Write", + "edit_file": "Edit", + "patch": "Edit", + "apply_patch": "apply_patch", + "web_search": "WebSearch", + "fetch_url": "WebFetch", + }, + events={ + "session-start": "on_session_start", + "pre-tool-use": "pre_tool_call", + "stop": "pre_verify", + "session-end": "on_session_end", + }, + compaction_events={"on_session_start": "SessionStart"}, + manifest_env="AGENT_HARNESS_HERMES_MANIFEST", + manifest_default=".hermes/hooks.json", ) - if not valid or any(value in {"-h", "--help"} for value in rest): - print( - "Usage: hook_adapter.py |shell allowed/forbidden/git/commit|lint|harness" - "|paths command/patch|content prompt/read/write/apply-patch>", - file=sys.stderr, - ) - return 1 - try: - payload = json.load(sys.stdin) - if not isinstance(payload, dict): - payload = {} - cwd = payload.get("cwd") - if isinstance(cwd, str) and cwd: - os.chdir(cwd) - return dispatch(kind, rest, payload) - except (OSError, ValueError, TypeError, AttributeError) as error: - print(f"BLOCKED: {error}", file=sys.stderr) - return 2 +) if __name__ == "__main__": - sys.exit(main(sys.argv[1:])) + sys.exit(ADAPTER.main(sys.argv[1:])) diff --git a/agents/hooks.json b/agents/hooks.json index 3163b6f6..cd958a84 100644 --- a/agents/hooks.json +++ b/agents/hooks.json @@ -48,7 +48,7 @@ ] }, { - "matcher": "Read", + "matcher": "Read|Grep", "hooks": [ { "command": "\"$HOME/.claude/hooks/audit_log.py\" tool claude", @@ -61,7 +61,7 @@ ] }, { - "matcher": "Write|Edit", + "matcher": "Write|Edit|NotebookEdit", "hooks": [ { "command": "\"$HOME/.claude/hooks/audit_log.py\" tool claude", @@ -80,7 +80,7 @@ ], "PostToolUse": [ { - "matcher": "Write|Edit", + "matcher": "Write|Edit|NotebookEdit", "hooks": [ { "command": "\"$HOME/.claude/hooks/lint_format.py\"", diff --git a/agents/hooks/audit_log.py b/agents/hooks/audit_log.py index 6992c5c8..9f1a5e9a 100755 --- a/agents/hooks/audit_log.py +++ b/agents/hooks/audit_log.py @@ -17,6 +17,7 @@ from session_store import file_lock, private_directory, worktree_id +import hook_input import patch_input @@ -89,8 +90,7 @@ def _targets(tool: str, values: dict, cwd: Path) -> list[str]: if isinstance(command, str): names = patch_input.paths(command) elif tool in FILE_TOOLS: - file_path = values.get("file_path") or values.get("path") - names = (file_path,) if isinstance(file_path, str) else () + names = (name,) if (name := hook_input.file_path(values)) else () edits = values.get("edits") if isinstance(edits, list): names += tuple( diff --git a/agents/hooks/guard_file.py b/agents/hooks/guard_file.py index a0267e9b..5778f137 100755 --- a/agents/hooks/guard_file.py +++ b/agents/hooks/guard_file.py @@ -14,6 +14,7 @@ import audit_log from command_policy import regex_matches +import hook_input def policy_path(environment: str, name: str, directory: Path) -> Path: @@ -97,8 +98,9 @@ def content_text(mode: str, payload: dict) -> str: if mode == "prompt": return payload.get("prompt") or "" if mode == "write": - return (values.get("content") or "") + (values.get("new_string") or "") - path = values.get("file_path") or "" + return "".join(values.get(key) or "" for key in ("content", "new_string", "new_source")) + # Grep may name a directory; only a single file target can be scanned before the tool runs. + path = hook_input.file_path(values) if not path or not Path(path).is_file(): return "" with Path(path).open("rb") as stream: @@ -141,7 +143,7 @@ def scan_content(mode: str, payload: dict, directory: Path) -> None: tool = ( "UserPromptSubmit" if mode == "prompt" else "Read" if mode == "read" else payload.get("tool_name") or "Write" ) - summary = "" if mode == "prompt" else values.get("file_path") or "" + summary = "" if mode == "prompt" else hook_input.file_path(values) audit_log.blocked( tool, summary, @@ -177,9 +179,7 @@ def check(mode: str, payload: dict, directory: Path = ROOT) -> int: return 0 values = payload.get("tool_input") or {} tool = payload.get("tool_name") or tool - value = ( - values.get("command") or "" if mode == "commit" else values.get("file_path") or values.get("path") or "" - ) + value = values.get("command") or "" if mode == "commit" else hook_input.file_path(values) if not value: return 0 reason = ( diff --git a/agents/hooks/lib/hook_input.py b/agents/hooks/lib/hook_input.py index 21a6c9fe..6727d008 100644 --- a/agents/hooks/lib/hook_input.py +++ b/agents/hooks/lib/hook_input.py @@ -29,6 +29,12 @@ ".sty": "tex", } EDIT_TOOLS = frozenset({"Write", "Edit", "MultiEdit", "NotebookEdit", "apply_patch"}) +FILE_FIELDS = ("file_path", "path", "notebook_path") + + +def file_path(values: dict) -> str: + """Return the single file a tool names, whichever provider field carries it.""" + return next((value for key in FILE_FIELDS if isinstance(value := values.get(key), str) and value), "") def _cwd(payload: dict) -> Path: @@ -74,9 +80,8 @@ def _input_names(payload: dict) -> tuple[str, ...]: if tool and tool not in EDIT_TOOLS: return () names = list(patch_input.paths(patch_body(values, tool))) - file_path = values.get("file_path") or values.get("path") - if isinstance(file_path, str): - names.append(file_path) + if name := file_path(values): + names.append(name) edits = values.get("edits") if isinstance(edits, list): names.extend( diff --git a/agents/hooks/lib/provider_adapter.py b/agents/hooks/lib/provider_adapter.py new file mode 100644 index 00000000..70b0b953 --- /dev/null +++ b/agents/hooks/lib/provider_adapter.py @@ -0,0 +1,309 @@ +"""Translate provider hook payloads at the boundary to shared hook implementations. + +Each provider adapter declares only a Profile of provider facts. Payload normalization, +guard dispatch, and CLI validation live here so the providers cannot drift apart. +""" + +from collections.abc import Callable, Mapping +from dataclasses import dataclass, field +import json +import os +from pathlib import Path +import sys + +import audit_log +import guard_command +import guard_file +import guard_git +import hook_dispatch +import hook_input +import lint_format +import patch_input +import secret_path_policy +import shell_syntax +import verification_session + + +SECRET_PATH_RULE = "guard_secret_paths.sh" +SHELL_MODES = ("allowed", "forbidden", "git", "commit") +PATH_MODES = ("command", "patch") +AUDIT_KINDS = ("tool", "compaction") +CONTENT_MODES = ("prompt", "read", "write", "apply-patch") +NO_ARGUMENT: tuple[str, ...] = () +ANY_ARGUMENT = None +Formatter = Callable[[str, dict], dict] + + +def denial(result: dict) -> dict | None: + """Return the block decision for a shared Claude-shaped denial, if the result is one.""" + specific = result.get("hookSpecificOutput") + if not isinstance(specific, dict) or specific.get("permissionDecision") != "deny": + return None + return {"decision": "block", "reason": specific.get("permissionDecisionReason") or "Blocked by hook policy"} + + +def system_message(event: str, result: dict) -> dict: + """Format a shared result for providers that surface a top-level systemMessage.""" + message = result.get("systemMessage") + return denial(result) or ({"systemMessage": message} if message else result) + + +def additional_context(event: str, result: dict) -> dict: + """Format a shared result for providers that surface messages as additionalContext.""" + message = result.get("systemMessage") + context = {"hookSpecificOutput": {"hookEventName": event, "additionalContext": message}} + return denial(result) or (context if message else result) + + +def no_cwd() -> str: + return "" + + +@dataclass(frozen=True) +class Profile: + """Provider facts the shared adapter needs; every behavior derived from them is shared. + + `events` maps verification events to native event names and enables `native`. + `compaction_events` maps native event names to shared audit event names and enables `audit`. + `manifest_env` names the manifest override and enables `dispatch`. + """ + + name: str + tools: Mapping[str, str] = field(default_factory=dict) + events: Mapping[str, str] = field(default_factory=dict) + compaction_events: Mapping[str, str] = field(default_factory=dict) + formatter: Formatter = system_message + content_modes: tuple[str, ...] = CONTENT_MODES + cwd_fallback: Callable[[], str] = os.getcwd + manifest_env: str = "" + manifest_default: str = "" + + +class Adapter: + """Run one provider's hook commands against the shared hook implementations.""" + + def __init__(self, profile: Profile) -> None: + self.profile = profile + + def main(self, arguments: list[str]) -> int: + kind, rest = (arguments[0], arguments[1:]) if arguments else ("", []) + if not self._accepts(kind, rest): + print(f"Usage: hook_adapter.py <{self._usage()}>", file=sys.stderr) + return 1 + try: + return self.dispatch(kind, rest, self._payload()) + except (OSError, ValueError, TypeError, AttributeError) as error: + print(f"BLOCKED: {error}", file=sys.stderr) + return 2 + + def dispatch(self, kind: str, arguments: list[str], payload: dict) -> int: + if kind == "lint": + return self.lint(payload) + if kind == "harness": + return self.harness(payload) + commands = { + "native": self.native, + "audit": self.audit, + "dispatch": self.fan_out, + "shell": self.shell, + "paths": self.check_paths, + "content": self.content, + } + return commands[kind](arguments[0], payload) + + def normalized(self, payload: dict) -> dict: + """Copy the provider payload, adding the shared fields and canonical tool name.""" + result = dict(payload) + result["tool_input"] = tool_input(payload) + result.setdefault("cwd", self.profile.cwd_fallback()) + result.setdefault("prompt", payload.get("text") or "") + result["provider"] = self.profile.name + if canonical := self.profile.tools.get(payload.get("tool_name") or ""): + result["tool_name"] = canonical + return result + + def translated(self, payload: dict, tool: str, values: dict) -> dict: + """Forward only the fields a shared guard reads, under the shared tool name.""" + return { + "tool_name": tool, + "tool_input": values, + "session_id": payload.get("session_id") or "", + "cwd": payload.get("cwd") or self.profile.cwd_fallback(), + "provider": self.profile.name, + } + + def native(self, event: str, payload: dict) -> int: + context = self.normalized(payload) + try: + result = verification_session.dispatch(self.profile.name, event, context) + except (OSError, ValueError, TypeError, KeyError) as error: + result = verification_session.failure(event, context, error) + output = self.profile.formatter(self.profile.events[event], result) + print(json.dumps(output, ensure_ascii=True, separators=(",", ":"))) + return 0 + + def audit(self, kind: str, payload: dict) -> int: + context = self.normalized(payload) + if kind == "compaction": + context = self._compaction_context(context) + if context is not None: + audit_log.append_record( + audit_log.record(kind, context, self.profile.name), cwd=Path(context["cwd"] or ".") + ) + return 0 + + def fan_out(self, event: str, payload: dict) -> int: + return hook_dispatch.run(self._manifest(), event, payload) + + def shell(self, mode: str, payload: dict) -> int: + values = {"command": command_text(self.normalized(payload)["tool_input"])} + command = self.translated(payload, "Bash", values) + if mode == "git": + return guard_git.check(command) + if mode == "commit": + return guard_file.check("commit", command, shared_directory()) + return guard_command.check(mode, command, shared_directory()) + + def lint(self, payload: dict) -> int: + """Report shared quality diagnostics using the PostToolUse JSON contract.""" + lint_format.emit(lint_format.diagnostics(self.normalized(payload))) + return 0 + + def harness(self, payload: dict) -> int: + for name in edit_targets(self.normalized(payload)): + absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) + values = {"file_path": absolute} + if status := guard_file.check( + "harness", self.translated(payload, "apply_patch", values), shared_directory() + ): + return status + return 0 + + def check_paths(self, mode: str, payload: dict) -> int: + rules = secret_path_policy.load(secret_policy_path()) + context = self.normalized(payload) + candidates = edit_targets(context) if mode == "patch" else shell_words(command_text(context["tool_input"])) + for value in candidates: + if rule := secret_path_policy.blocked_rule(value, rules): + deny_secret_path(mode, value, rule, context) + return 0 + + def content(self, mode: str, payload: dict) -> int: + context = self.normalized(payload) + if mode != "apply-patch": + return guard_file.check(mode, context, shared_directory()) + body = hook_input.patch_body(context["tool_input"], context.get("tool_name") or "apply_patch") + values = {"content": patch_input.added_text(body)} + return guard_file.check("write", self.translated(payload, "Edit", values), shared_directory()) + + def _compaction_context(self, context: dict) -> dict | None: + """Name a native compaction event as shared audit expects, or drop an unknown event.""" + event = self.profile.compaction_events.get(context.get("hook_event_name") or "") + return None if event is None else context | {"hook_event_name": event} + + def _manifest(self) -> Path: + default = Path.home() / self.profile.manifest_default + return Path(os.environ.get(self.profile.manifest_env, str(default))) + + def _payload(self) -> dict: + payload = json.load(sys.stdin) + if not isinstance(payload, dict): + raise TypeError("hook payload must be an object") + cwd = payload.get("cwd") or self.profile.cwd_fallback() + if isinstance(cwd, str) and cwd: + os.chdir(cwd) + return payload + + def _accepts(self, kind: str, rest: list[str]) -> bool: + commands = self._commands() + if kind not in commands or any(value in {"-h", "--help"} for value in rest): + return False + choices = commands[kind] + if choices == NO_ARGUMENT: + return not rest + return len(rest) == 1 and (choices is ANY_ARGUMENT or rest[0] in choices) + + def _commands(self) -> dict[str, tuple[str, ...] | None]: + """Map each accepted command to its argument choices.""" + commands: dict[str, tuple[str, ...] | None] = {} + if self.profile.events: + commands["native"] = tuple(self.profile.events) + if self.profile.compaction_events: + commands["audit"] = AUDIT_KINDS + if self.profile.manifest_env: + commands["dispatch"] = ANY_ARGUMENT + return commands | { + "shell": SHELL_MODES, + "lint": NO_ARGUMENT, + "harness": NO_ARGUMENT, + "paths": PATH_MODES, + "content": self.profile.content_modes, + } + + def _usage(self) -> str: + return "|".join(usage_entry(kind, choices) for kind, choices in self._commands().items()) + + +def shared_directory() -> Path: + return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" + + +def secret_policy_path() -> Path: + default = shared_directory() / "rules/secret_path_policy.json" + return Path(os.environ.get("AGENT_SECRET_PATH_POLICY", str(default))) + + +def tool_input(payload: dict) -> object: + """Copy the tool arguments; a non-object is kept so the consuming check rejects it.""" + values = payload.get("tool_input") + if values is None: + # Hermes carries tool arguments in `args`. + values = payload.get("args") + if not isinstance(values, dict): + return {} if values is None else values + result = dict(values) + if name := hook_input.file_path(values): + result["file_path"] = name + return result + + +def command_text(values: dict) -> str: + return values.get("command") or values.get("cmd") or "" + + +def edit_targets(context: dict) -> list[str]: + values = context["tool_input"] + body = hook_input.patch_body(values, context.get("tool_name")) + return [name for name in (hook_input.file_path(values), *patch_input.paths(body)) if name] + + +def shell_words(command: str) -> list[str]: + """Return every argument and redirection word, plus the values of assignment words.""" + try: + commands = shell_syntax.parse(command) + except shell_syntax.UnsupportedSyntax as error: + raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error + words = [word for item in commands for word in (*item.arguments, *item.redirections)] + return words + [word.partition("=")[2] for word in words if "=" in word] + + +def deny_secret_path(mode: str, value: str, rule: dict, context: dict) -> None: + command = mode == "command" + audit_log.blocked( + "Bash" if command else "apply_patch", + command_text(context["tool_input"]) if command else value, + f"{rule['reason']}: {value}", + SECRET_PATH_RULE, + context.get("session_id") or "", + payload=context, + targets=() if command else (value,), + ) + raise ValueError( + f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" + ) + + +def usage_entry(kind: str, choices: tuple[str, ...] | None) -> str: + if choices is ANY_ARGUMENT: + return f"{kind} " + return f"{kind} {'/'.join(choices)}" if choices else kind diff --git a/agents/pi/hooks/hook_adapter.py b/agents/pi/hooks/hook_adapter.py index 7e74d9bd..3bd66887 100755 --- a/agents/pi/hooks/hook_adapter.py +++ b/agents/pi/hooks/hook_adapter.py @@ -1,229 +1,42 @@ #!/usr/bin/env -S python3 -IB """Translate pi hook payloads at the boundary to shared hook implementations.""" -import json -import os from pathlib import Path import sys SCRIPT = Path(__file__).resolve() +# Deployed adapters live in ~/./hooks beside ~/.claude/hooks; the source tree keeps agents/hooks. COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name.startswith(".") else "hooks") sys.path.insert(0, str(COMMON)) sys.path.insert(0, str(COMMON / "lib")) -import audit_log -import guard_command -import guard_git -import guard_file -import hook_dispatch -import hook_input -import lint_format -import patch_input -import secret_path_policy -import shell_syntax -import verification_session - -CANONICAL_TOOLS = { - "bash": "Bash", - "read": "Read", - "write": "Write", - "edit": "Edit", - "multi_edit": "MultiEdit", - "notebook_edit": "NotebookEdit", - "apply_patch": "apply_patch", - "webfetch": "WebFetch", - "web_search": "WebSearch", -} - -NATIVE_EVENTS = { - "session-start": "session_start", - "pre-tool-use": "tool_call", - "session-end": "session_shutdown", -} - - -def shared_directory() -> Path: - return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" - - -def manifest_path() -> Path: - return Path(os.environ.get("AGENT_HARNESS_PI_MANIFEST", str(Path.home() / ".pi/agent/hooks.json"))) - - -def normalized(payload: dict) -> dict: - """Preserve the provider event while supplying the fields shared checks require.""" - result = dict(payload) - values = result.get("tool_input") - result["tool_input"] = dict(values) if isinstance(values, dict) else {} - result["tool_input"].setdefault("file_path", result["tool_input"].get("path") or "") - result.setdefault("cwd", os.getcwd()) - result.setdefault("prompt", result.get("text") or "") - result["provider"] = "pi" - if canonical := CANONICAL_TOOLS.get(result.get("tool_name") or ""): - result["tool_name"] = canonical - return result - - -def pi_result(event: str, result: dict) -> dict: - """Convert the shared Claude-shaped decision into pi's blocking contract.""" - specific = result.get("hookSpecificOutput") - if isinstance(specific, dict) and specific.get("permissionDecision") == "deny": - reason = specific.get("permissionDecisionReason") or "Blocked by hook policy" - return {"decision": "block", "reason": reason} - if message := result.get("systemMessage"): - return {"systemMessage": message} - return result - - -def native(event: str, payload: dict) -> int: - normalized_payload = normalized(payload) - try: - result = verification_session.dispatch("pi", event, normalized_payload) - except (OSError, ValueError, TypeError, KeyError) as error: - result = verification_session.failure(event, normalized_payload, error) - print(json.dumps(pi_result(event, result), ensure_ascii=True, separators=(",", ":"))) - return 0 - - -def audit(kind: str, payload: dict) -> int: - context = normalized(payload) - entry = audit_log.record(kind, context, "pi") - cwd = context.get("cwd") or "." - audit_log.append_record(entry, cwd=Path(cwd)) - return 0 - - -def lint(payload: dict) -> int: - """Report shared quality diagnostics using the post-tool JSON contract.""" - lint_format.emit(lint_format.diagnostics(normalized(payload))) - return 0 - - -def harness(payload: dict) -> int: - values = normalized(payload).get("tool_input") or {} - candidates = [values.get("file_path") or values.get("path") or ""] - candidates += patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))) - for name in candidates: - if not name: - continue - absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) - if status := guard_file.check( - "harness", - normalized(payload) | {"tool_name": "apply_patch", "tool_input": {"file_path": absolute}}, - shared_directory(), - ): - return status - return 0 - - -def check_paths(mode: str, payload: dict) -> int: - policy = Path( - os.environ.get("AGENT_SECRET_PATH_POLICY", str(shared_directory() / "rules/secret_path_policy.json")) - ) - rules = secret_path_policy.load(policy) - values = normalized(payload).get("tool_input") or {} - command = values.get("command") or values.get("cmd") or "" - if mode == "command": - try: - commands = shell_syntax.parse(command) - except shell_syntax.UnsupportedSyntax as error: - raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error - candidates = [word for item in commands for word in (*item.arguments, *item.redirections)] - candidates += [word.partition("=")[2] for word in candidates if "=" in word] - else: - candidates = [ - values.get("file_path") or values.get("path") or "", - *patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))), - ] - context = normalized(payload) - for value in candidates: - if rule := secret_path_policy.blocked_rule(value, rules): - audit_log.blocked( - "Bash" if mode == "command" else "apply_patch", - command if mode == "command" else value, - f"{rule['reason']}: {value}", - "adapt_guard_secret_paths.sh", - payload.get("session_id") or "", - payload=context, - targets=() if mode == "command" else (value,), - ) - raise ValueError( - f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" - ) - return 0 - - -def content(mode: str, payload: dict) -> int: - if mode == "apply-patch": - values = normalized(payload).get("tool_input") or {} - body = hook_input.patch_body(values, normalized(payload).get("tool_name") or "apply_patch") - return guard_file.check( - "write", - normalized(payload) | {"tool_name": "Edit", "tool_input": {"content": patch_input.added_text(body)}}, - shared_directory(), - ) - return guard_file.check(mode, normalized(payload), shared_directory()) - - -def dispatch(kind: str, arguments: list[str], payload: dict) -> int: - values = normalized(payload).get("tool_input") or {} - if kind == "native": - return native(arguments[0], payload) - if kind == "audit": - return audit(arguments[0], payload) - if kind == "dispatch": - return hook_dispatch.run(manifest_path(), arguments[0], payload) - if kind == "shell": - command = normalized(payload) | { - "tool_name": "Bash", - "tool_input": {"command": values.get("command") or values.get("cmd") or ""}, - } - if arguments[0] in {"allowed", "forbidden"}: - return guard_command.check(arguments[0], command, shared_directory()) - if arguments[0] == "git": - return guard_git.check(command) - return guard_file.check("commit", command, shared_directory()) - if kind == "lint": - return lint(payload) - if kind == "harness": - return harness(payload) - if kind == "paths": - return check_paths(arguments[0], payload) - return content(arguments[0], payload) - - -def main(arguments: list[str]) -> int: - kind = arguments[0] if arguments else "" - rest = arguments[1:] - valid = ( - (kind == "native" and len(rest) == 1 and rest[0] in NATIVE_EVENTS) - or (kind == "audit" and len(rest) == 1 and rest[0] in {"tool", "compaction"}) - or (kind == "dispatch" and len(rest) == 1) - or (kind in {"lint", "harness"} and not rest) - or (kind == "shell" and len(rest) == 1 and rest[0] in {"allowed", "forbidden", "git", "commit"}) - or (kind == "paths" and len(rest) == 1 and rest[0] in {"command", "patch"}) - or (kind == "content" and len(rest) == 1 and rest[0] in {"prompt", "read", "write", "apply-patch"}) +import provider_adapter + +ADAPTER = provider_adapter.Adapter( + provider_adapter.Profile( + name="pi", + tools={ + "bash": "Bash", + "read": "Read", + "write": "Write", + "edit": "Edit", + "multi_edit": "MultiEdit", + "notebook_edit": "NotebookEdit", + "apply_patch": "apply_patch", + "webfetch": "WebFetch", + "web_search": "WebSearch", + }, + events={ + "session-start": "session_start", + "pre-tool-use": "tool_call", + "session-end": "session_shutdown", + }, + compaction_events={"session_before_compact": "PreCompact", "session_compact": "PostCompaction"}, + manifest_env="AGENT_HARNESS_PI_MANIFEST", + manifest_default=".pi/agent/hooks.json", ) - if not valid or any(value in {"-h", "--help"} for value in rest): - print( - "Usage: hook_adapter.py |shell allowed/forbidden/git/commit|lint|harness" - "|paths command/patch|content prompt/read/write/apply-patch>", - file=sys.stderr, - ) - return 1 - try: - payload = json.load(sys.stdin) - if not isinstance(payload, dict): - payload = {} - cwd = payload.get("cwd") - if isinstance(cwd, str) and cwd: - os.chdir(cwd) - return dispatch(kind, rest, payload) - except (OSError, ValueError, TypeError, AttributeError) as error: - print(f"BLOCKED: {error}", file=sys.stderr) - return 2 +) if __name__ == "__main__": - sys.exit(main(sys.argv[1:])) + sys.exit(ADAPTER.main(sys.argv[1:])) diff --git a/pyproject.toml b/pyproject.toml index 2571a5b7..7caadbef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,6 +29,7 @@ known_first_party = [ "lib", "lint_format", "patch_input", + "provider_adapter", "secret_path_policy", "shell_syntax", "session_gate", diff --git a/tests/agents/hooks/test_hook_boundary.py b/tests/agents/hooks/test_hook_boundary.py index 520e5961..1ed72acb 100644 --- a/tests/agents/hooks/test_hook_boundary.py +++ b/tests/agents/hooks/test_hook_boundary.py @@ -14,16 +14,17 @@ from tests.runtime import REPO_ROOT, load_script_module hook_adapter = load_script_module("agents/codex/hooks/hook_adapter.py", "codex_hook_adapter") +shared = hook_adapter.provider_adapter files = load_script_module("agents/hooks/guard_file.py", "guard_file") audit = load_script_module("agents/hooks/audit_log.py", "audit_log") def test_patch_paths_include_moves_and_deletions_once() -> None: patch = "*** Add File: a.py\n*** Update File: a.py\n*** Delete File: old.py\n*** Move to: new.py" - assert hook_adapter.patch_input.paths(patch) == ("a.py", "new.py", "old.py") + assert shared.patch_input.paths(patch) == ("a.py", "new.py", "old.py") -hook_input = hook_adapter.lint_format.hook_input +hook_input = shared.lint_format.hook_input def test_patch_body_requires_tool_or_format_evidence() -> None: @@ -59,7 +60,7 @@ def test_shell_commands_are_not_patch_targets() -> None: def test_scanner_receives_only_added_lines() -> None: - assert hook_adapter.patch_input.added_text("--- old\n+++ new\n context\n-old\n+new\n+more") == "new\nmore" + assert shared.patch_input.added_text("--- old\n+++ new\n context\n-old\n+new\n+more") == "new\nmore" @pytest.mark.parametrize( @@ -88,11 +89,13 @@ def test_scanner_receives_only_added_lines() -> None: ], ) def test_formatter_routes_supported_extensions(suffix: str, kind: str) -> None: - assert hook_adapter.EXTENSIONS[Path("file" + suffix).suffix] == kind + assert shared.lint_format.EXTENSIONS[Path("file" + suffix).suffix] == kind def test_translated_command_preserves_session_and_provider_context() -> None: - assert hook_adapter.translated({"session_id": "s", "untrusted": "ignored"}, "Bash", {"command": "git status"}) == { + assert hook_adapter.ADAPTER.translated( + {"session_id": "s", "untrusted": "ignored"}, "Bash", {"command": "git status"} + ) == { "tool_name": "Bash", "tool_input": {"command": "git status"}, "session_id": "s", @@ -193,11 +196,11 @@ def test_blocked_audit_rows_preserve_schema_and_append(tmp_path: Path, mocker: M ], ) def test_ast_command_boundaries_preserve_literals(source: str, expected: list[str]) -> None: - assert [command.raw for command in hook_adapter.shell_syntax.parse(source)] == expected + assert [command.raw for command in shared.shell_syntax.parse(source)] == expected def test_redirect_paths_are_inspected_separately_from_command_arguments() -> None: - command = hook_adapter.shell_syntax.parse("printf x > .env")[0] + command = shared.shell_syntax.parse("printf x > .env")[0] assert command.arguments == ("printf", "x") assert command.redirections == (".env",) @@ -209,16 +212,16 @@ def test_shell_translation_calls_shared_policy( ) -> None: monkeypatch.setenv("AGENT_HARNESS_ROOT", str(tmp_path)) target = ( - hook_adapter.guard_command + shared.guard_command if mode in {"allowed", "forbidden"} - else hook_adapter.guard_git + else shared.guard_git if mode == "git" - else hook_adapter.guard_file + else shared.guard_file ) check = Mock(return_value=2) monkeypatch.setattr(target, "check", check) payload = {"tool_input": {field: "git status"}, "session_id": "session-one"} - assert hook_adapter.dispatch("shell", [mode], payload) == 2 + assert hook_adapter.ADAPTER.dispatch("shell", [mode], payload) == 2 translated = { "tool_name": "Bash", "tool_input": {"command": "git status"}, @@ -235,17 +238,17 @@ def test_shell_translation_calls_shared_policy( def test_adapter_applies_payload_cwd_before_enforcement(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: monkeypatch.chdir(tmp_path.parent) payload = {"cwd": str(tmp_path), "tool_input": {"cmd": "git status"}} - monkeypatch.setattr(hook_adapter.sys, "stdin", io.StringIO(json.dumps(payload))) + monkeypatch.setattr(shared.sys, "stdin", io.StringIO(json.dumps(payload))) observed = [] - monkeypatch.setattr(hook_adapter.guard_git, "check", lambda _: observed.append(Path.cwd()) or 0) - assert hook_adapter.main(["shell", "git"]) == 0 + monkeypatch.setattr(shared.guard_git, "check", lambda _: observed.append(Path.cwd()) or 0) + assert hook_adapter.ADAPTER.main(["shell", "git"]) == 0 assert observed == [tmp_path] @pytest.mark.parametrize("raw", ["{", "null", "[]", '{"tool_input":true}']) def test_adapter_rejects_invalid_input(monkeypatch: pytest.MonkeyPatch, raw: str) -> None: - monkeypatch.setattr(hook_adapter.sys, "stdin", io.StringIO(raw)) - assert hook_adapter.main(["shell", "git"]) == 2 + monkeypatch.setattr(shared.sys, "stdin", io.StringIO(raw)) + assert hook_adapter.ADAPTER.main(["shell", "git"]) == 2 def test_lint_only_checks_existing_supported_patch_files(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: @@ -253,20 +256,20 @@ def test_lint_only_checks_existing_supported_patch_files(monkeypatch: pytest.Mon (tmp_path / "source.py").touch() (tmp_path / "unknown.bin").touch() check = Mock(return_value=()) - monkeypatch.setattr(hook_adapter.lint_format, "check_file", check) + monkeypatch.setattr(shared.lint_format, "check_file", check) patch = "\n".join(f"*** Update File: {name}" for name in ("source.py", "unknown.bin", "missing.py")) - assert hook_adapter.lint({"tool_input": {"command": patch}}) == 0 + assert hook_adapter.ADAPTER.lint({"tool_input": {"command": patch}}) == 0 check.assert_called_once_with("py", tmp_path / "source.py") @pytest.mark.parametrize( "module,mode,identifier", [ - (hook_adapter.guard_command, "allowed", "guard_allowed_commands.sh"), - (hook_adapter.guard_command, "forbidden", "guard_forbidden_commands.sh"), - (hook_adapter.guard_git, "", "guard_dangerous_git.sh"), - (hook_adapter.guard_file, "commit", "guard_secret_commit.sh"), - (hook_adapter.guard_file, "harness", "guard_harness_files.sh"), + (shared.guard_command, "allowed", "guard_allowed_commands.sh"), + (shared.guard_command, "forbidden", "guard_forbidden_commands.sh"), + (shared.guard_git, "", "guard_dangerous_git.sh"), + (shared.guard_file, "commit", "guard_secret_commit.sh"), + (shared.guard_file, "harness", "guard_harness_files.sh"), ], ) def test_malformed_guard_input_remains_auditable( diff --git a/tests/agents/hooks/test_tool_target_fields.py b/tests/agents/hooks/test_tool_target_fields.py new file mode 100644 index 00000000..4b9a2b53 --- /dev/null +++ b/tests/agents/hooks/test_tool_target_fields.py @@ -0,0 +1,69 @@ +"""NotebookEdit and Grep payload fields reach the shared file guards and lint targets.""" + +import json +from pathlib import Path + +import pytest + +from tests.runtime import CliRunner, load_script_module + + +SCRIPT = "agents/hooks/guard_file.py" +MARKER = "fixture-sensitive-marker" +hook_input = load_script_module("agents/hooks/lib/hook_input.py", "hook_input") +audit = load_script_module("agents/hooks/audit_log.py", "audit_log") + + +@pytest.fixture(autouse=True) +def policies(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + protected = tmp_path / "protected.json" + protected.write_text(json.dumps({"version": 1, "paths": ["~/.claude/settings.json"]})) + patterns = tmp_path / "patterns.json" + patterns.write_text(json.dumps({"fixture": {"pattern": MARKER, "message": "Fixture detected"}})) + monkeypatch.setenv("AGENT_PROTECTED_PATH_POLICY", str(protected)) + monkeypatch.setenv("AGENT_SECRET_CONTENT_PATTERNS", str(patterns)) + + +def notebook_edit(path: str, source: str = "x = 1") -> dict: + return {"tool_name": "NotebookEdit", "tool_input": {"notebook_path": path, "new_source": source}} + + +def test_notebook_edit_on_a_harness_file_is_blocked(run_cli: CliRunner) -> None: + result = run_cli(SCRIPT, "harness", payload=notebook_edit("~/.claude/settings.json")) + assert result.returncode == 2 + assert "agent harness boundary" in result.stderr + + +@pytest.mark.parametrize("source,blocked", [(MARKER, True), ("x = 1", False)]) +def test_notebook_edit_source_is_scanned(run_cli: CliRunner, source: str, blocked: bool) -> None: + result = run_cli(SCRIPT, "write", payload=notebook_edit("analysis.ipynb", source)) + assert result.returncode == 0 + if blocked: + assert json.loads(result.stdout)["hookSpecificOutput"]["permissionDecision"] == "deny" + else: + assert result.stdout == "" + + +@pytest.mark.parametrize("target,blocked", [("secret.txt", True), (".", False)]) +def test_grep_scans_a_single_file_target_only( + run_cli: CliRunner, isolated_project: Path, target: str, blocked: bool +) -> None: + (isolated_project / "secret.txt").write_text(MARKER + "\n") + payload = {"tool_name": "Grep", "tool_input": {"pattern": "x", "path": str(isolated_project / target)}} + result = run_cli(SCRIPT, "read", payload=payload) + assert result.returncode == 0 + if blocked: + assert json.loads(result.stdout)["hookSpecificOutput"]["permissionDecision"] == "deny" + else: + assert result.stdout == "" + + +def test_notebook_edit_target_reaches_lint(tmp_path: Path) -> None: + payload = notebook_edit("analysis.ipynb") | {"cwd": str(tmp_path)} + assert hook_input.target_paths(payload) == (tmp_path.resolve() / "analysis.ipynb",) + + +def test_notebook_edit_target_is_audited(tmp_path: Path) -> None: + entry = audit.record("tool", notebook_edit("analysis.ipynb") | {"cwd": str(tmp_path)}) + assert entry is not None + assert entry["targets"] == ["analysis.ipynb"] diff --git a/tests/agents/test_entry_points.py b/tests/agents/test_entry_points.py new file mode 100644 index 00000000..d75f2642 --- /dev/null +++ b/tests/agents/test_entry_points.py @@ -0,0 +1,33 @@ +"""Source entry points that providers execute directly are runnable scripts.""" + +import os +import subprocess + +import pytest + +from tests.runtime import REPO_ROOT + + +ENTRY_POINTS = ( + "agents/codex/hooks/hook_adapter.py", + "agents/devin/hooks/hook_adapter.py", + "agents/hermes/hooks/hook_adapter.py", + "agents/pi/hooks/hook_adapter.py", + "agents/claude/statusline/statusline.py", +) + + +@pytest.mark.parametrize("name", ENTRY_POINTS) +def test_entry_point_is_an_executable_script(name: str) -> None: + path = REPO_ROOT / name + assert os.access(path, os.X_OK), name + assert path.read_text().startswith("#!/usr/bin/env -S python3"), name + + +@pytest.mark.parametrize("name", ENTRY_POINTS) +def test_entry_point_is_executable_in_the_git_index(name: str) -> None: + # Nix builds the rendered harness from the Git tree, so the index mode is what ships. + result = subprocess.run( + ["git", "ls-files", "--stage", "--", name], cwd=REPO_ROOT, capture_output=True, text=True, check=True + ) + assert result.stdout.startswith("100755 "), result.stdout or name diff --git a/tests/agents/test_hook_generation.py b/tests/agents/test_hook_generation.py index 9da76065..66b3bdde 100644 --- a/tests/agents/test_hook_generation.py +++ b/tests/agents/test_hook_generation.py @@ -15,13 +15,29 @@ def test_generated_claude_lint_hook_uses_the_dispatcher_without_if(agent_harness path = tmp_path / "settings.json" agent_harness("generate-claude-settings", "--output", str(path)) settings = json.loads(path.read_text()) - groups = [group for group in settings["hooks"]["PostToolUse"] if group.get("matcher") == "Write|Edit"] + groups = [group for group in settings["hooks"]["PostToolUse"] if group.get("matcher") == "Write|Edit|NotebookEdit"] lint_hooks = [hook for group in groups for hook in group["hooks"] if "lint_format.py" in hook["command"]] assert len(lint_hooks) == 1 assert lint_hooks[0]["command"].endswith('lint_format.py"') assert "if" not in lint_hooks[0] +@pytest.mark.parametrize( + "matcher,mode", + [("Write|Edit|NotebookEdit", "harness"), ("Write|Edit|NotebookEdit", "write"), ("Read|Grep", "read")], +) +def test_generated_claude_file_guards_cover_every_file_tool( + agent_harness: CliRunner, tmp_path: Path, matcher: str, mode: str +) -> None: + path = tmp_path / "settings.json" + agent_harness("generate-claude-settings", "--output", str(path)) + groups = [ + group for group in json.loads(path.read_text())["hooks"]["PreToolUse"] if group.get("matcher") == matcher + ] + commands = [hook["command"] for group in groups for hook in group["hooks"]] + assert any(command.endswith(f'guard_file.py" {mode}') for command in commands) + + def test_generated_codex_lint_hook_matches_native_apply_patch_aliases( agent_harness: CliRunner, tmp_path: Path ) -> None: diff --git a/tests/agents/test_provider_adapters.py b/tests/agents/test_provider_adapters.py new file mode 100644 index 00000000..0fd66117 --- /dev/null +++ b/tests/agents/test_provider_adapters.py @@ -0,0 +1,128 @@ +"""Behavior every provider hook adapter shares, exercised through each real entry point.""" + +import json +from pathlib import Path +import shutil + +import pytest + +from tests.runtime import CliRunner, REPO_ROOT, load_script_module + + +PROVIDERS = ("codex", "devin", "hermes", "pi") + + +def script(provider: str) -> str: + return f"agents/{provider}/hooks/hook_adapter.py" + + +@pytest.fixture +def harness(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + home = tmp_path / "home" + root = tmp_path / "harness" + home.mkdir() + rules = root / ".claude/hooks/rules" + rules.mkdir(parents=True) + paths = [f"~/.{provider}/hooks/hook_adapter.py" for provider in PROVIDERS] + (rules / "protected_paths.json").write_text(json.dumps({"version": 1, "paths": paths})) + shutil.copyfile(REPO_ROOT / "agents/hooks/rules/secret_path_policy.json", rules / "secret_path_policy.json") + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("AGENT_HARNESS_ROOT", str(root)) + for name in ("AGENT_PROTECTED_PATH_POLICY", "AGENT_SECRET_PATH_POLICY", "DEVIN_PROJECT_DIR"): + monkeypatch.delenv(name, raising=False) + return root + + +@pytest.fixture +def state(isolated_project: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + # Audit logging refuses a state directory inside the working tree it describes. + path = isolated_project.parent / (isolated_project.name + "-state") + monkeypatch.setenv("XDG_STATE_HOME", str(path)) + return path + + +def audit_rows(state: Path) -> list[dict]: + logs = sorted((state / "agent-harness/audit").rglob("*.jsonl")) + return [json.loads(line) for path in logs for line in path.read_text().splitlines()] + + +@pytest.mark.parametrize("provider", PROVIDERS) +@pytest.mark.parametrize( + "case", + [("command", {"command": "cat .env"}), ("patch", {"file_path": "~/.ssh/config"})], +) +def test_secret_path_denial_is_audited_for_the_provider( + run_cli: CliRunner, harness: Path, state: Path, provider: str, case: tuple[str, dict] +) -> None: + mode, values = case + payload = {"session_id": "adapter-session", "tool_input": values} + result = run_cli(script(provider), "paths", mode, payload=payload) + assert result.returncode == 2 + [row] = audit_rows(state) + assert row["status"] == "blocked" + assert row["provider"] == provider + assert row["rule"] == "guard_secret_paths.sh" + + +@pytest.mark.parametrize("provider", ["devin", "pi"]) +def test_notebook_edits_are_checked_against_the_harness_boundary( + run_cli: CliRunner, harness: Path, provider: str +) -> None: + values = {"notebook_path": f"~/.{provider}/hooks/hook_adapter.py", "new_source": "x = 1"} + result = run_cli(script(provider), "harness", payload={"tool_name": "notebook_edit", "tool_input": values}) + assert result.returncode == 2 + assert "agent harness boundary" in result.stderr + + +SESSION_START = {"devin": "SessionStart", "hermes": "on_session_start"} + + +@pytest.mark.parametrize("provider", sorted(SESSION_START)) +@pytest.mark.parametrize("source,recorded", [("startup", False), ("", False), ("compact", True), ("resume", True)]) +def test_session_start_compaction_is_recorded_only_for_compact_or_resume( + run_cli: CliRunner, state: Path, provider: str, source: str, recorded: bool +) -> None: + payload = {"hook_event_name": SESSION_START[provider], "session_id": "adapter-session", "source": source} + result = run_cli(script(provider), "audit", "compaction", payload=payload) + assert result.returncode == 0 + rows = audit_rows(state) + assert len(rows) == int(recorded) + if recorded: + assert rows[0]["event"] == "SessionStart" + assert rows[0]["source"] == source + assert rows[0]["provider"] == provider + + +@pytest.mark.parametrize( + "provider,event,expected", + [ + ("devin", "PostCompaction", "PostCompaction"), + ("pi", "session_before_compact", "PreCompact"), + ("pi", "session_compact", "PostCompaction"), + ("devin", "", None), + ], +) +def test_native_compaction_events_are_recorded_under_the_shared_name( + run_cli: CliRunner, state: Path, provider: str, event: str, expected: str | None +) -> None: + payload = {"hook_event_name": event, "session_id": "adapter-session"} + result = run_cli(script(provider), "audit", "compaction", payload=payload) + assert result.returncode == 0 + assert [row["event"] for row in audit_rows(state)] == ([expected] if expected else []) + + +@pytest.mark.parametrize("provider", PROVIDERS) +def test_normalization_leaves_the_provider_payload_untouched(provider: str) -> None: + adapter = load_script_module(script(provider), f"{provider}_hook_adapter_under_test").ADAPTER + payload = {"tool_name": "write", "tool_input": {"path": "notes.txt"}, "args": {"path": "other.txt"}} + original = json.loads(json.dumps(payload)) + adapter.normalized(payload) + assert payload == original + + +@pytest.mark.parametrize("provider", PROVIDERS) +@pytest.mark.parametrize("raw", ["{", "null", "[]"]) +def test_non_object_payloads_are_rejected(run_cli: CliRunner, harness: Path, provider: str, raw: str) -> None: + result = run_cli(script(provider), "shell", "git", payload=raw) + assert result.returncode == 2 + assert "BLOCKED" in result.stderr diff --git a/tests/agents/test_python_runtime.py b/tests/agents/test_python_runtime.py index c161471e..652b4e23 100644 --- a/tests/agents/test_python_runtime.py +++ b/tests/agents/test_python_runtime.py @@ -10,46 +10,67 @@ from tests.runtime import CliRunner -@pytest.mark.integration -def test_rendered_hooks_load_adjacent_modules(agent_harness: CliRunner, isolated_project: Path) -> None: +ADAPTERS = (".codex/hooks", ".devin/hooks", ".hermes/hooks", ".pi/hooks") + + +@pytest.fixture +def rendered(agent_harness: CliRunner, isolated_project: Path) -> Path: prefix = isolated_project / "rendered" agent_harness("install", "--prefix", str(prefix), "--runtime-root", str(prefix)) - for name in ( - ".claude/hooks/guard_command.py", - ".claude/hooks/lint_format.py", - ".claude/hooks/lib/shell_syntax.py", - ".codex/hooks/hook_adapter.py", - ".claude/statusline/statusline.py", - ): - assert (prefix / name).is_file(), name - environment = os.environ | {"AGENT_HARNESS_ROOT": str(prefix)} + return prefix + + +@pytest.fixture +def environment(rendered: Path) -> dict[str, str]: + result = os.environ | {"AGENT_HARNESS_ROOT": str(rendered)} for name in ("AGENT_COMMAND_PERMISSIONS", "AGENT_ALLOWED_COMMAND_RULES", "AGENT_FORBIDDEN_COMMAND_RULES"): - environment.pop(name, None) - lint = prefix / ".claude/hooks/lint_format.py" - result = subprocess.run( - [str(lint), "py"], - input=json.dumps({"tool_input": {}}), + result.pop(name, None) + return result + + +def run( + executable: Path, arguments: list[str], payload: dict, environment: dict[str, str] +) -> subprocess.CompletedProcess: + return subprocess.run( + [str(executable), *arguments], + input=json.dumps(payload), env=environment, text=True, capture_output=True, timeout=10, check=False, ) + + +@pytest.mark.integration +def test_rendered_hooks_load_adjacent_modules(rendered: Path, environment: dict[str, str]) -> None: + for name in ( + ".claude/hooks/guard_command.py", + ".claude/hooks/lint_format.py", + ".claude/hooks/lib/shell_syntax.py", + ".claude/hooks/lib/provider_adapter.py", + ".claude/statusline/statusline.py", + *(f"{directory}/hook_adapter.py" for directory in ADAPTERS), + ): + assert (rendered / name).is_file(), name + result = run(rendered / ".claude/hooks/lint_format.py", ["py"], {"tool_input": {}}, environment) assert result.returncode == 0 assert result.stdout == result.stderr == "" - executable = prefix / ".codex/hooks/hook_adapter.py" - for arguments, payload, status, message in ( + + +@pytest.mark.integration +@pytest.mark.parametrize("directory", ADAPTERS) +@pytest.mark.parametrize( + "case", + [ (["shell", "allowed"], {"tool_input": {"cmd": "git status"}}, 0, ""), (["paths", "patch"], {"tool_input": {"file_path": ".env.local"}}, 2, "secret path policy matched"), - ): - result = subprocess.run( - [str(executable), *arguments], - input=json.dumps(payload), - env=environment, - text=True, - capture_output=True, - timeout=10, - check=False, - ) - assert result.returncode == status, result.stderr - assert message in result.stderr + ], +) +def test_rendered_provider_adapters_run_shared_guards( + rendered: Path, environment: dict[str, str], directory: str, case: tuple[list[str], dict, int, str] +) -> None: + arguments, payload, status, message = case + result = run(rendered / directory / "hook_adapter.py", arguments, payload, environment) + assert result.returncode == status, result.stderr + assert message in result.stderr From 3b3f345e8ed7ce9ea78b53980752d3513dcf8c6c Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:59:25 +0900 Subject: [PATCH 02/14] fix(agents): tighten sandbox escapes and secret path rules Disallow unsandboxed commands in Claude; allow the uv cache and run gh outside the sandbox. Narrow key and token read denials to credential-shaped names so source files stay readable. Require approval for docker build, drop removed Codex features, and declare git as a runtime. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) --- agents/claude/settings.json | 6 ++- agents/codex/config.toml | 6 --- agents/command_permissions.json | 20 ++++----- agents/hooks/rules/allowed_commands.json | 1 - agents/hooks/rules/secret_path_policy.json | 34 ++++++++++++++- agents/manifest.json | 1 + tests/agents/claude/test_settings.py | 12 ++++++ tests/agents/hooks/test_secret_path_policy.py | 43 +++++++++++++++++++ tests/agents/test_command_permission_sync.py | 6 +++ 9 files changed, 108 insertions(+), 21 deletions(-) create mode 100644 tests/agents/hooks/test_secret_path_policy.py diff --git a/agents/claude/settings.json b/agents/claude/settings.json index 241ad126..1358eabe 100644 --- a/agents/claude/settings.json +++ b/agents/claude/settings.json @@ -45,10 +45,11 @@ "sandbox": { "enabled": true, "autoAllowBashIfSandboxed": false, - "allowUnsandboxedCommands": true, + "allowUnsandboxedCommands": false, "filesystem": { "allowWrite": [ - "$HOME/.cache/nix" + "$HOME/.cache/nix", + "$HOME/.cache/uv" ] }, "network": { @@ -58,6 +59,7 @@ }, "excludedCommands": [ "docker", + "gh", "watchman" ] }, diff --git a/agents/codex/config.toml b/agents/codex/config.toml index d8e1bdec..78b7fd00 100644 --- a/agents/codex/config.toml +++ b/agents/codex/config.toml @@ -44,9 +44,3 @@ enabled = false [features] hooks = true -remote_connections = true -external_migration = true - -[features.multi_agent_v2] -min_wait_timeout_ms = 120000 -default_wait_timeout_ms = 120000 diff --git a/agents/command_permissions.json b/agents/command_permissions.json index 746eaa29..063e4286 100644 --- a/agents/command_permissions.json +++ b/agents/command_permissions.json @@ -1019,32 +1019,27 @@ { "decision": "allow", "prefix": ["docker", "ps"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", "prefix": ["docker", "images"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", "prefix": ["docker", "logs"], - "justification": "Allow read-only container inspection and image builds." - }, - { - "decision": "allow", - "prefix": ["docker", "build"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", "prefix": ["docker", "compose", "ps"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", "prefix": ["docker", "compose", "logs"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", @@ -1381,6 +1376,11 @@ "prefix": ["esa", "post", "update"], "justification": "Ask before publishing or triggering work that is visible to others." }, + { + "decision": "ask", + "prefix": ["docker", "build"], + "justification": "Ask before building images; build steps run arbitrary commands outside the sandbox." + }, { "decision": "ask", "prefix": ["docker", "run"], diff --git a/agents/hooks/rules/allowed_commands.json b/agents/hooks/rules/allowed_commands.json index ed7589d4..23b3289f 100644 --- a/agents/hooks/rules/allowed_commands.json +++ b/agents/hooks/rules/allowed_commands.json @@ -144,7 +144,6 @@ "^docker ps( [^;&|<>$`]+)?$", "^docker images( [^;&|<>$`]+)?$", "^docker logs( [^;&|<>$`]+)?$", - "^docker build( [^;&|<>$`]+)?$", "^docker compose ps( [^;&|<>$`]+)?$", "^docker compose logs( [^;&|<>$`]+)?$", "^kubectl get( [^;&|<>$`]+)?$", diff --git a/agents/hooks/rules/secret_path_policy.json b/agents/hooks/rules/secret_path_policy.json index a67dadc8..eece3529 100644 --- a/agents/hooks/rules/secret_path_policy.json +++ b/agents/hooks/rules/secret_path_policy.json @@ -7,15 +7,45 @@ "reason": "Environment files may contain credentials." }, { - "pattern": "**/*key*", + "pattern": "**/*.key", + "access": ["read"], + "reason": "Key files may contain private key material." + }, + { + "pattern": "**/*_key", "access": ["read"], "reason": "Key-named files may contain credentials." }, { - "pattern": "**/*token*", + "pattern": "**/*key*.json", + "access": ["read"], + "reason": "Key-named JSON files may contain credentials." + }, + { + "pattern": "**/*.pem", + "access": ["read"], + "reason": "PEM files may contain private key material." + }, + { + "pattern": "**/*.p12", + "access": ["read"], + "reason": "PKCS#12 bundles contain private key material." + }, + { + "pattern": "**/*.token", + "access": ["read"], + "reason": "Token files may contain credentials." + }, + { + "pattern": "**/*_token", "access": ["read"], "reason": "Token-named files may contain credentials." }, + { + "pattern": "**/*token*.json", + "access": ["read"], + "reason": "Token-named JSON files may contain credentials." + }, { "pattern": "**/.netrc", "access": ["read"], diff --git a/agents/manifest.json b/agents/manifest.json index 4c86629f..4fa53ee7 100644 --- a/agents/manifest.json +++ b/agents/manifest.json @@ -2,6 +2,7 @@ "version": 1, "runtime_commands": [ "bash", + "git", "jq", "python3", "shfmt", diff --git a/tests/agents/claude/test_settings.py b/tests/agents/claude/test_settings.py index b856f0ee..e4a501ac 100644 --- a/tests/agents/claude/test_settings.py +++ b/tests/agents/claude/test_settings.py @@ -12,3 +12,15 @@ def test_agent_git_commands_run_without_the_sandboxed_fsmonitor_daemon() -> None environment[f"GIT_CONFIG_KEY_{index}"]: environment[f"GIT_CONFIG_VALUE_{index}"] for index in range(count) } assert configured["core.fsmonitor"] == "false" + + +def test_model_cannot_escape_the_sandbox_on_its_own() -> None: + sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] + assert sandbox["enabled"] is True + assert sandbox["allowUnsandboxedCommands"] is False + + +def test_verification_and_github_tools_work_without_escaping_the_sandbox() -> None: + sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] + assert "$HOME/.cache/uv" in sandbox["filesystem"]["allowWrite"] + assert "gh" in sandbox["excludedCommands"] diff --git a/tests/agents/hooks/test_secret_path_policy.py b/tests/agents/hooks/test_secret_path_policy.py new file mode 100644 index 00000000..a889958e --- /dev/null +++ b/tests/agents/hooks/test_secret_path_policy.py @@ -0,0 +1,43 @@ +"""Secret path rules deny credential-shaped files without denying ordinary source code.""" + +import pytest + +from tests.runtime import REPO_ROOT, load_script_module + + +policy = load_script_module("agents/hooks/lib/secret_path_policy.py", "secret_path_policy") +RULES = policy.load(REPO_ROOT / "agents/hooks/rules/secret_path_policy.json") + + +@pytest.mark.parametrize( + "path", + [ + ".env", + "config/github.token", + "config/api_token", + "auth/token.json", + "certs/server.key", + "certs/server.pem", + "certs/client.p12", + "keys/api_key.json", + "home/.netrc", + "credentials.json", + "id_ed25519", + ], +) +def test_credential_paths_are_denied(path: str) -> None: + assert policy.blocked_rule(path, RULES) is not None + + +@pytest.mark.parametrize( + "path", + [ + ".venv/lib/python3.14/site-packages/pygments/token.py", + "src/tokenizer.rs", + "src/keyboard.ts", + "lib/monkey_patch.py", + "docs/token_design.md", + ], +) +def test_source_code_paths_are_readable(path: str) -> None: + assert policy.blocked_rule(path, RULES) is None diff --git a/tests/agents/test_command_permission_sync.py b/tests/agents/test_command_permission_sync.py index ef636e21..27b836f2 100644 --- a/tests/agents/test_command_permission_sync.py +++ b/tests/agents/test_command_permission_sync.py @@ -119,6 +119,12 @@ def test_obsolete_nonfrozen_and_transient_prefix_is_absent(prefix: list[str]) -> assert all(rule["prefix"] != prefix for rule in POLICY["rules"]) +@pytest.mark.parametrize("prefix", [("docker", "build"), ("docker", "run"), ("docker", "compose", "up")]) +def test_container_execution_requires_approval(prefix: tuple[str, ...]) -> None: + assert prefix in ASK + assert prefix not in ALLOW + + def test_justifications_are_provider_neutral() -> None: provider_specific = [rule for rule in POLICY["rules"] if re.search(r"\b(Codex|Claude)\b", rule["justification"])] assert provider_specific == [] From 312efafd5e74287d0415080c51ba454d3043d061 Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Wed, 23 Sep 2026 18:59:50 +0900 Subject: [PATCH 03/14] fix(nix): repair codex commit alias and drop unguarded opencode codex exec no longer accepts --full-auto; the alias only needs a read-only sandbox. Remove opencode, which had no harness hooks or permissions, and check both contracts. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) --- nix/checks.nix | 3 +++ nix/home/default.nix | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/nix/checks.nix b/nix/checks.nix index aff5b0f1..cfa3dda1 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -110,6 +110,9 @@ in "devin-cli" "codex" ]; + no-unguarded-agent-packages = !(builtins.elem "opencode" homeNames); + commit-message-alias-avoids-removed-codex-flag = + !(lib.hasInfix "--full-auto" home.programs.git.settings.alias.cc); shared-agent-input = inputs ? llm-agents && !(inputs ? nix-claude-code) diff --git a/nix/home/default.nix b/nix/home/default.nix index dc62bdfd..bf94ef14 100644 --- a/nix/home/default.nix +++ b/nix/home/default.nix @@ -217,7 +217,6 @@ in devinCliPackage herdrCompatibleCodex hermesAgentPackage - unstable.opencode unstable.pi-coding-agent # Nix tooling @@ -338,7 +337,7 @@ in receive.fsckObjects = true; status.short = true; status.branch = true; - alias.cc = "!f() { tmpf=$(mktemp) && codex exec --full-auto -o \"$tmpf\" 'Review the staged diff and generate a Conventional Commits message. Output ONLY the commit message, nothing else.' && git commit -F \"$tmpf\"; rm -f \"$tmpf\"; }; f"; + alias.cc = "!f() { tmpf=$(mktemp) && codex exec -s read-only -o \"$tmpf\" 'Review the staged diff and generate a Conventional Commits message. Output ONLY the commit message, nothing else.' && git commit -F \"$tmpf\"; rm -f \"$tmpf\"; }; f"; alias.wtd = "wt -D"; }; }; From 8d0779584d7551df3201d7fdc02f918ebc8f14ed Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Wed, 23 Sep 2026 19:00:11 +0900 Subject: [PATCH 04/14] fix(explain-visually): detect SVG, embed, track, and refresh resource loads Flag srcset only for non-data candidates and meta refresh or image inputs only when they load. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../explain-visually/scripts/verify_page.py | 36 ++++++++++++++++--- .../explain-visually/test_verify_page.py | 30 ++++++++++++++++ 2 files changed, 62 insertions(+), 4 deletions(-) diff --git a/agents/skills/explain-visually/scripts/verify_page.py b/agents/skills/explain-visually/scripts/verify_page.py index 4bcb8a42..5ed97167 100644 --- a/agents/skills/explain-visually/scripts/verify_page.py +++ b/agents/skills/explain-visually/scripts/verify_page.py @@ -34,6 +34,11 @@ r"(?:\bfrom\s+|\bimport\s*\(\s*|\bimport\s+|\bfetch\s*\(\s*)[\"']([^\"']+)[\"']", re.IGNORECASE, ) +SRCSET_URL_PATTERN = re.compile(r"[\s,]*(\S*[^\s,])(?:,+|\s[^,]*,?|$)") +REFRESH_URL_PATTERN = re.compile( + r"^\s*[\d.]*\s*[;,]?\s*(?:url\s*=\s*)?([\"']?)(.*?)\1\s*$", + re.IGNORECASE | re.DOTALL, +) BROWSER_COMMANDS = ( "google-chrome", "google-chrome-stable", @@ -49,14 +54,23 @@ RESOURCE_ATTRIBUTES = { "audio": ("src",), "base": ("href",), + "embed": ("src",), "iframe": ("src",), + "image": ("href", "xlink:href"), "img": ("src", "srcset"), "link": ("href",), "object": ("data",), "script": ("src",), "source": ("src", "srcset"), + "track": ("src",), + "use": ("href", "xlink:href"), "video": ("poster", "src"), } +# Tags that load a resource only when a qualifying attribute has the given value. +CONDITIONAL_RESOURCE_ATTRIBUTES = { + "input": ("type", "image", "src"), + "meta": ("http-equiv", "refresh", "content"), +} @dataclass(frozen=True, slots=True) @@ -103,9 +117,9 @@ def handle_starttag(self, tag: str, attrs: list[tuple[str, str | None]]) -> None content = attributes.get("content") if name is not None and content is not None: self.metadata[name] = content.strip() - for attribute in RESOURCE_ATTRIBUTES.get(tag, ()): + for attribute in _resource_attributes(tag, attributes): value = attributes.get(attribute) - if value is not None and (attribute == "srcset" or _loads_external_resource(value)): + if value is not None and any(map(_is_external_reference, _referenced_urls(attribute, value))): self.external_resources.append(f"{tag}[{attribute}]={value}") style = attributes.get("style") if style is not None: @@ -193,8 +207,22 @@ def viewport_height(dom: str) -> tuple[int, str | None]: return bounded_height, None -def _loads_external_resource(value: str) -> bool: - return _is_external_reference(value) +def _resource_attributes(tag: str, attributes: dict[str, str | None]) -> tuple[str, ...]: + condition = CONDITIONAL_RESOURCE_ATTRIBUTES.get(tag) + if condition is None: + return RESOURCE_ATTRIBUTES.get(tag, ()) + qualifier, expected_value, resource_attribute = condition + is_loading = (attributes.get(qualifier) or "").strip().lower() == expected_value + return (resource_attribute,) if is_loading else () + + +def _referenced_urls(attribute: str, value: str) -> tuple[str, ...]: + if attribute == "srcset": + return tuple(SRCSET_URL_PATTERN.findall(value)) + if attribute == "content": + match = REFRESH_URL_PATTERN.match(value) + return (match.group(2),) if match is not None else (value,) + return (value,) def _external_css_resources(styles: Iterable[str]) -> tuple[str, ...]: diff --git a/tests/agents/skills/explain-visually/test_verify_page.py b/tests/agents/skills/explain-visually/test_verify_page.py index 83e769d7..c1c77151 100644 --- a/tests/agents/skills/explain-visually/test_verify_page.py +++ b/tests/agents/skills/explain-visually/test_verify_page.py @@ -46,6 +46,18 @@ def test_validate_html_accepts_self_contained_page_with_external_source_link() - '', '', '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', + '', ], ) def test_validate_html_rejects_externally_loaded_resources(resource: str) -> None: @@ -73,6 +85,24 @@ def test_validate_html_allows_embedded_resources_and_source_examples() -> None: assert verify_page.validate_html(html) == () +@pytest.mark.parametrize( + "resource", + [ + 'diagram', + '', + '', + '', + '', + '', + '', + ], +) +def test_validate_html_allows_embedded_or_non_loading_references(resource: str) -> None: + html = VALID_HTML.replace("
", f"
{resource}") + + assert verify_page.validate_html(html) == () + + def test_validate_html_rejects_unresolved_template_tokens() -> None: errors = verify_page.validate_html(VALID_HTML.replace("Example explanation", "{{TITLE}}")) From 01d92c7beb77b4a967229f71ad36c724e1311626 Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Wed, 23 Sep 2026 19:00:49 +0900 Subject: [PATCH 05/14] docs(skills): resolve ownership overlaps and stale guidance Let models load project-setup, which other skills name as the setup owner. Create PRs with an explicit body and move adr triggers into its description. Give Issue body, glossary, and completion rules a single owning skill. Drop generic or drifted content; align python, rust, bash, and actions guidance with tools. Replace history-bound skill layout tests with contracts over every skill. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) --- agents/skill_rendering.json | 12 -- agents/skills/adr/SKILL.md | 44 +---- agents/skills/bash-style/SKILL.md | 12 +- .../bash-style/references/bats_testing.md | 4 +- .../bash-style/references/script_template.md | 19 +- agents/skills/domain-modeling/SKILL.md | 3 +- agents/skills/git-workflow/SKILL.md | 3 +- .../git-workflow/references/delivery.md | 19 +- .../git-workflow/references/stacked_prs.md | 3 +- agents/skills/github-actions-style/SKILL.md | 15 +- agents/skills/nix-dotfiles/SKILL.md | 72 +------ agents/skills/project-setup/SKILL.md | 6 +- .../python-style/references/code_style.md | 22 +-- .../skills/python-style/references/tooling.md | 6 +- .../rust-style/references/code_style.md | 8 - .../skills/rust-style/references/tooling.md | 1 - agents/skills/rust-style/references/types.md | 10 +- agents/skills/source-of-truth/SKILL.md | 17 +- agents/skills/tsdd/SKILL.md | 31 +-- agents/skills/tsdd/references/enforcement.md | 11 ++ tests/agents/test_workflow_skill_layout.py | 186 +++++++++++++----- 21 files changed, 250 insertions(+), 254 deletions(-) create mode 100644 agents/skills/tsdd/references/enforcement.md diff --git a/agents/skill_rendering.json b/agents/skill_rendering.json index 28099766..d77bf315 100644 --- a/agents/skill_rendering.json +++ b/agents/skill_rendering.json @@ -26,18 +26,6 @@ "allow_implicit_invocation": false } } - }, - "project-setup": { - "claude": { - "frontmatter": { - "disable-model-invocation": true - } - }, - "codex": { - "openai": { - "allow_implicit_invocation": false - } - } } } } diff --git a/agents/skills/adr/SKILL.md b/agents/skills/adr/SKILL.md index ca4dd7c1..f80f4a31 100644 --- a/agents/skills/adr/SKILL.md +++ b/agents/skills/adr/SKILL.md @@ -1,8 +1,10 @@ --- name: adr description: > - Use to decide whether a broad design decision needs an Architecture Decision Record (ADR), - or to create, review, update, supersede, or deprecate ADRs. + Use to create, review, update, supersede, or deprecate ADRs under docs/adr/, or when a change + selects a library, framework, database, protocol, hosting, or tool among real alternatives; + picks an architectural pattern; sets a durable constraint not obvious from code; or reverses + a previous decision. Not for local implementation details understandable from the code. --- # Architecture Decision Records @@ -13,22 +15,9 @@ Use this together with `tsdd` when the work involves both executable behavior an ## Core Rule -When a change introduces or reverses a broad project decision, create or update an ADR under `docs/adr/` as part of the same task. - -When Decision Test identifies a broad decision requiring an ADR, record its rationale before -finishing the work rather than leaving it as an optional follow-up. - -## When To Load - -Load this skill before making or reviewing changes involving: - -- Architecture Decision Records, decision records, ADRs, or `docs/adr/`. -- Library, framework, database, protocol, hosting, or tooling selection where real alternatives exist. -- Architectural patterns such as server-rendered vs SPA, sessions vs tokens, monolith vs services, sync vs async, polling vs events, or generated vs handwritten artifacts. -- Durable constraints that are not obvious from code, such as "no runtime dependencies", "Nix owns this configuration", or "requirement intent remains independent of executable test evidence". -- Reversing, superseding, deprecating, or intentionally rejecting a previous decision. - -Do not load it for a local implementation detail that can be understood from the code in under a minute. +When a change introduces or reverses a broad project decision that the Decision Test identifies, +create or update an ADR under `docs/adr/` in the same task, before finishing the work rather than +as an optional follow-up. ## Decision Test @@ -155,23 +144,8 @@ Use the longer template only when a Y-Statement would be too compressed: ## Workflow -For decision-relevant implementation tasks: - -1. Load `tsdd` and any language-specific skill required by the code. -2. Establish the executable behavior with tests when the change has behavior. -3. Inspect existing ADRs before choosing or recording the decision. -4. Implement the code change. -5. Create or supersede the ADR before finishing. -6. Confirm the relevant verification results. Follow `tsdd`'s Automatic Verification policy for execution needs and reuse of successful hook evidence. -7. Report the ADR path and the verification result. - -For documentation-only decision tasks: - -1. Inspect existing ADRs. -2. Decide whether this is a new decision, a duplicate, or a supersession. -3. Create or update only the necessary ADR files. -4. Confirm the repository's markdown or generated-artifact check results when available. Follow `tsdd`'s Automatic Verification policy for execution needs and reuse of successful hook evidence. -5. Report the ADR path. +After [Repository Inspection](#repository-inspection), create or supersede only the necessary ADR +files before finishing, then report the ADR path. ## Non-Goals diff --git a/agents/skills/bash-style/SKILL.md b/agents/skills/bash-style/SKILL.md index c0ebe3d4..870c889d 100644 --- a/agents/skills/bash-style/SKILL.md +++ b/agents/skills/bash-style/SKILL.md @@ -38,11 +38,11 @@ set -x ## usage Function -Every script must define a `usage` function that prints documentation to stderr and exits with failure. Use heredoc + redirect inside the function body: +Every script must define a `usage` function that prints documentation. Use a heredoc inside the function body: ```sh function usage() { - cat <&2 + cat <&2` and `exit 1`. ## Constants Declare constants with `readonly`. Names use `SCREAMING_SNAKE_CASE`: ```sh -readonly INPUT_DIR="../data/input" +readonly INPUT_DIR="$SCRIPT_DIR/../data/input" readonly MAX_RETRY=3 ``` @@ -88,8 +88,6 @@ Always quote the right-hand side — values may contain spaces or special charac | Variables | `snake_case` | `input_file="..."` | | Functions | `snake_case` | `function parse_args()` | | Local variables | `_snake_case` (leading underscore) | `local _tmp_dir` | -| Files | `snake_case` | `lint_format.sh` | -| Directories | `kebab-case` | `claude-scripts/` | Do not start names with a digit. diff --git a/agents/skills/bash-style/references/bats_testing.md b/agents/skills/bash-style/references/bats_testing.md index 0f0a2dab..9a13d97d 100644 --- a/agents/skills/bash-style/references/bats_testing.md +++ b/agents/skills/bash-style/references/bats_testing.md @@ -1,6 +1,6 @@ # Testing with bats -Test shell scripts with [bats](https://github.com/bats-core/bats-core) (Bash Automated Testing System). Available tools: `bats`, `shellcheck`, `shfmt`. +Test shell scripts with [bats](https://github.com/bats-core/bats-core) (Bash Automated Testing System). ### Directory Layout @@ -37,7 +37,7 @@ setup() { @test "descriptive lowercase sentence" { run bash "$SCRIPT" --help - [ "$status" -eq 1 ] + [ "$status" -eq 0 ] [[ "$output" == *"Usage:"* ]] } ``` diff --git a/agents/skills/bash-style/references/script_template.md b/agents/skills/bash-style/references/script_template.md index 86024ad4..97a1b0f7 100644 --- a/agents/skills/bash-style/references/script_template.md +++ b/agents/skills/bash-style/references/script_template.md @@ -6,7 +6,7 @@ set -euCo pipefail # Resolve the working directory according to the task; see the parent skill. function usage() { - cat <&2 + cat <&2 + exit 1 + ;; + esac + local _input="$1" echo "processing: $_input" diff --git a/agents/skills/domain-modeling/SKILL.md b/agents/skills/domain-modeling/SKILL.md index 59cac1e5..0afcaaa5 100644 --- a/agents/skills/domain-modeling/SKILL.md +++ b/agents/skills/domain-modeling/SKILL.md @@ -52,7 +52,8 @@ Use `source-of-truth` when durable placement, duplication, or conflicting author judgment. Reuse existing authoritative material; do not automatically create `CONTEXT.md` or a context map when the first term is clarified. -When a glossary is warranted, follow its existing format. Keep definitions concise but sufficient +Add a short glossary only when code cannot communicate the distinctions to every relevant +audience. When a glossary is warranted, follow its existing format. Keep definitions concise but sufficient to distinguish concepts, identify the applicable context, and note easily confused terms where useful. Include domain-specific meanings, not a general programming tutorial. Do not copy implementation details or behavioral specifications into the glossary. diff --git a/agents/skills/git-workflow/SKILL.md b/agents/skills/git-workflow/SKILL.md index 6658fe93..a885adc2 100644 --- a/agents/skills/git-workflow/SKILL.md +++ b/agents/skills/git-workflow/SKILL.md @@ -19,9 +19,8 @@ number and plan URL from issue-workflow and associate them with the branch or PR - Inspect Git state before edits: current branch, `git status --porcelain=v1`, and recent commit style when commit messages will be written. - Never overwrite, reset, clean, or discard user changes unless the user explicitly asked for that exact destructive action. - Do not push directly to the default / protected branch, and rewrite published history only with the leased force push described in [Delivery](references/delivery.md). Ordinary implementation and PR-creation requests do not authorize merging; merge only when explicitly requested and permitted by repository and runtime rules. -- Complete requested implementation, relevant verification, and fixes for problems caused by the change. Do not stop at the first implementation or first Green when required work remains. - Commit, push, and create pull requests only within the user's request or an explicit standing authorization. Permission to execute a command is not authorization to perform that action for the task. Do not repeat confirmation for steps already covered by the authorized workflow; runtime approval requirements still apply. -- When commits are authorized, group them by reviewable intent. Multiple TSDD cycles may belong to one commit; do not force a commit for every cycle. Both TSDD paths end Green. +- When commits are authorized, group them by reviewable intent. Multiple TSDD cycles may belong to one commit; do not force a commit for every cycle. - Keep branch names and commit subjects aligned with the primary intent of the change, not with filenames. Review-only requests authorize inspection and findings, not edits or delivery. A request to fix a bug diff --git a/agents/skills/git-workflow/references/delivery.md b/agents/skills/git-workflow/references/delivery.md index fbe02319..d9799d99 100644 --- a/agents/skills/git-workflow/references/delivery.md +++ b/agents/skills/git-workflow/references/delivery.md @@ -10,7 +10,7 @@ For implementation tasks: - `git fetch origin` - optionally `git rebase origin/` before the first push when the feature branch should be refreshed onto the latest base - `git push -u origin ` - - `gh pr create -f --base ` + - `gh pr create --base --title "" --body-file <file>`, with a [pull request body](#pull-request-body) 6. Stop before merge; merging is a human decision unless the user explicitly asks for it. When the user asks, `gh pr merge` prompts for approval, and that prompt is the authorization step. For explicit PR requests: @@ -18,7 +18,8 @@ For explicit PR requests: 1. If the user asked for a stacked or red-green pull request, follow [Stacked pull requests](stacked_prs.md) instead of steps 2 and 3. 2. Confirm the target base branch if it is not obvious from the repo default. 3. Follow [Local Verification Before Delivery](#local-verification-before-delivery), then push the feature branch with `git push -u origin <branch>`. -4. Create a normal PR with `gh pr create -f --base <base>`. +4. Create a normal PR with `gh pr create --base <base> --title "<title>" --body-file <file>`, with a + [pull request body](#pull-request-body). For rebase and force-push: @@ -27,6 +28,20 @@ For rebase and force-push: - Interactive rebase is outside the default workflow. - After rebasing a branch this task already pushed, publish it with `git push --force-with-lease --force-if-includes origin <branch>`. The lease refuses to overwrite commits that were pushed elsewhere, and `--force-if-includes` refuses when a background fetch hid them. Never use bare `--force`, `+refspec` pushes, or any force push to the default branch. If the lease is rejected, stop and ask the user. +## Pull Request Body + +Write the title as a Conventional Commits subject stating the primary intent, and keep the body +short. Do not use `-f`/`--fill`; commit messages do not carry this content. The body must include: + +- A few lines on what actually changed. +- `Closes #N` only when the PR fully resolves the Issue; otherwise `Refs #N`. +- A link to the plan comment, when one exists. +- Verification evidence: what ran and its result, or what could not run. +- Material departures from the plan, or none. + +For what the PR body owns relative to the Issue and plan, follow +[Information Placement](../../source-of-truth/SKILL.md#information-placement). + ## Local Verification Before Delivery 1. Complete required local verification before delivery without waiting for Stop. diff --git a/agents/skills/git-workflow/references/stacked_prs.md b/agents/skills/git-workflow/references/stacked_prs.md index 425da34e..07f44eba 100644 --- a/agents/skills/git-workflow/references/stacked_prs.md +++ b/agents/skills/git-workflow/references/stacked_prs.md @@ -29,7 +29,8 @@ before publishing, as `tsdd` requires for Red. 4. `gh stack add fix/<subject>` creates layer 2 on top of layer 1 and checks it out. 5. Remove the marker and its comment, implement the fix, and commit as `fix(<scope>): ...`. 6. `gh stack submit --auto` pushes both branches and opens two linked draft pull requests. Set - each title and body with `gh pr edit`, then mark them ready with `gh pr ready`. + each title and body with `gh pr edit` following [Pull request body](delivery.md#pull-request-body), + then mark them ready with `gh pr ready`. 7. Report the stack with `gh stack view --short`. Rules: diff --git a/agents/skills/github-actions-style/SKILL.md b/agents/skills/github-actions-style/SKILL.md index 71d8614e..185a917a 100644 --- a/agents/skills/github-actions-style/SKILL.md +++ b/agents/skills/github-actions-style/SKILL.md @@ -7,7 +7,7 @@ description: > # GitHub Actions Coding Conventions -Claude already knows the basic structure of GitHub Actions (events, workflows, jobs, steps). Focus on the following patterns that are easy to overlook but important for security and reliability. +These patterns are easy to overlook but important for security and reliability. ## 1. Permissions — Whitelist Approach @@ -27,28 +27,27 @@ Common permission keys: `contents`, `issues`, `pull-requests`, `packages`, `id-t ## 2. Version Pinning -Tags like `@v4` can be moved or deleted by the action author — the same tag may point to different code over time. Pin to an exact version tag or commit SHA. +Tags such as `@v4` or `@v4.2.1` can be moved or deleted by the action author, so the same tag may +point to different code over time. Pin every action to a full commit SHA and record the version in +a comment for readability and update tooling. ```yaml # Avoid — tag can move - uses: actions/checkout@v4 -# Recommended — exact version tag -- uses: actions/checkout@v4.2.1 - -# High security — immutable SHA (add the tag as a comment for readability) +# Recommended — immutable SHA with the version as a comment - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.1 ``` ## 3. Script Injection Prevention -GitHub context values like PR titles, issue bodies, and branch names come from untrusted external sources. Interpolating them directly into `run:` allows an attacker to inject arbitrary shell commands. Always pass them through `env:` instead. +GitHub context values like PR titles, issue bodies, and branch names come from untrusted external sources. Interpolating them directly into `run:` allows an attacker to inject arbitrary shell commands. Always pass them through `env:` instead, and quote the variable in the script. This is not escaping: the value never becomes part of the script text. ```yaml # Dangerous — PR title injected directly into shell - run: echo "${{ github.event.pull_request.title }}" -# Safe — value passed as environment variable (shell-escaped automatically) +# Safe — the shell reads the value from a variable; it is never interpolated into the script - env: TITLE: ${{ github.event.pull_request.title }} run: echo "$TITLE" diff --git a/agents/skills/nix-dotfiles/SKILL.md b/agents/skills/nix-dotfiles/SKILL.md index 4af304c0..d0be0027 100644 --- a/agents/skills/nix-dotfiles/SKILL.md +++ b/agents/skills/nix-dotfiles/SKILL.md @@ -130,36 +130,11 @@ and Atuin modules disable generated Zsh integration for this reason. ## Dotfile Symlinks -### Editable symlinks (mkOutOfStoreSymlink) - -```nix -let - link = path: config.lib.file.mkOutOfStoreSymlink "${dotfilesDir}/${path}"; -in -{ - home.file = { - ".zshrc".source = link "zsh/.zshrc"; - ".config/nvim".source = link "nvim"; # directory symlink - # ... etc - }; -} -``` - -Read `home.file` and `xdg.configFile` in `nix/home/default.nix` for each target's source. Entries -using `link` point at editable working-tree files. Agent deployment follows the managed-source -lifecycle above. - -Use `mkOutOfStoreSymlink` for files edited frequently — changes apply immediately without rebuild. - -### Nix-generated files - -```nix -home.file.".config/zsh/nix-plugins.zsh".text = '' - source ${pkgs.zsh-autosuggestions}/share/zsh-autosuggestions/zsh-autosuggestions.zsh -''; -``` - -Use `.text` when the file content depends on Nix store paths (e.g. plugin source paths that change on every nixpkgs update). +Read `home.file` and `xdg.configFile` in `nix/home/default.nix` for each target's source. +Entries using the `link` helper (`mkOutOfStoreSymlink`) point at editable working-tree files; +use it for frequently edited files, since changes apply without a rebuild. Use a generated +`.text` file only when its content depends on Nix store paths. Agent deployment follows the +managed-source lifecycle above. ## Activation Hooks @@ -189,37 +164,6 @@ that require the primary user's context. Preserve those execution boundaries whe ## Homebrew (via nix-homebrew) -```nix -nix-homebrew = { - enable = true; - user = username; - autoMigrate = true; -}; - -homebrew = { - enable = true; - onActivation = { - autoUpdate = true; - upgrade = true; - cleanup = "uninstall"; # remove unlisted casks/brews on rebuild - }; - casks = [ ... ]; # GUI apps - taps = [ ... ]; # third-party repos - brews = [ ... ]; # formulae not in nixpkgs -}; -``` - -`cleanup = "uninstall"` means any cask or brew removed from the list will be uninstalled on the next `darwin-rebuild switch`. This keeps the machine declarative. - -## Key Patterns - -| Pattern | Usage | -| --------------------------------------------- | ----------------------------------------------------------------- | -| `with pkgs;` | Avoids repeating `pkgs.` in `home.packages` list | -| `unstable.xxx` | Package from `nixpkgs-unstable` (passed via `extraSpecialArgs`) | -| `input.packages.${system}.default` | Package from a third-party flake input | -| `link "path"` | Helper for `mkOutOfStoreSymlink` (defined in `let` block) | -| `${pkgs.xxx}` in `.text` | Embeds Nix store paths into generated files | -| `enableZshIntegration = false` | Keep shell initialization in the linked `.zshrc` where configured | -| `\|\| true` in activation | Prevents non-fatal errors from aborting rebuild | -| `homebrew.onActivation.cleanup = "uninstall"` | Declarative cask management | +Read `nix-homebrew` and `homebrew` in `nix/darwin/default.nix` for the current settings and +lists. `onActivation.cleanup = "uninstall"` uninstalls any cask or brew removed from those lists +on the next `darwin-rebuild switch`, so remove an app by deleting its entry. diff --git a/agents/skills/project-setup/SKILL.md b/agents/skills/project-setup/SKILL.md index b0fc10de..f4bad797 100644 --- a/agents/skills/project-setup/SKILL.md +++ b/agents/skills/project-setup/SKILL.md @@ -1,8 +1,10 @@ --- name: project-setup description: > - Set up per-project Nix development environments and create or configure GitHub repositories - using this user's templates and repo commands. For requested setup, not routine code edits. + Use only when the user requests creating a GitHub repository, applying standard GitHub + settings to one, or setting up a per-project Nix devShell or development environment, using + this user's templates and repo commands. Not for ordinary code edits or other work in + existing projects. --- # Project Setup diff --git a/agents/skills/python-style/references/code_style.md b/agents/skills/python-style/references/code_style.md index 6ca5d44a..d653a6f6 100644 --- a/agents/skills/python-style/references/code_style.md +++ b/agents/skills/python-style/references/code_style.md @@ -7,19 +7,9 @@ - Don't use `global`, `nonlocal` (not explicit enough) - Use built-in generics (e.g., `tuple`, `list`, `dict`) instead of `typing.Tuple`, `typing.List`, `typing.Dict` -## Strings +## Formatting -### Quote Usage - -- String contains `'` → use `"` -- String contains `"` → use `'` -- f-strings (variable substitution) → use `"` -- `raise` statements → use `"` (normal sentences use `'`) - -### Operators - -- Separate operators and operands by one space -- When using 2+ operators, omit spaces around `*`, `/`, `//`, `%`, `**` (higher precedence) +Let `ruff format` decide quotes and operator spacing; do not hand-format against it. ## Logging @@ -108,17 +98,17 @@ class MyClass: # Function level -def function_name(arg1, arg2): +def function_name(arg1: int, arg2: str) -> bool: """Function functionality summary. (Detailed function functionality.) Args: - arg1 (type): Argument description - arg2 (type): Argument description + arg1: Argument description + arg2: Argument description Returns/Yields: - type: Return value description + Return value description Raises: ErrorType: Error description diff --git a/agents/skills/python-style/references/tooling.md b/agents/skills/python-style/references/tooling.md index 750a6ac7..0a9a40ea 100644 --- a/agents/skills/python-style/references/tooling.md +++ b/agents/skills/python-style/references/tooling.md @@ -4,7 +4,7 @@ - Use only `uv` for package management, don't use `pip` - Install dependencies using `uv sync` -- Run tools using `uv run {tool}` +- Run verification tools using `uv run --frozen {tool}` so checks never rewrite `uv.lock` - Baseline tooling dependencies belong in template-defined dependency groups, not ad hoc commands - Add project-specific dependencies using `uv add {package}` - Add project-specific tool dependencies using `uv add --group <group> {package}` @@ -28,7 +28,7 @@ ## Static Type Checking - ty should already be provided by the project's `typecheck` dependency group -- Run type checks using `uv run ty check` -- Check specific paths using `uv run ty check src tests` +- Run type checks using `uv run --frozen ty check` +- Check specific paths using `uv run --frozen ty check src tests` - Use `uv run ty server` only for editor or language-server integration - Do not introduce another type checker unless the existing project is already standardized on it diff --git a/agents/skills/rust-style/references/code_style.md b/agents/skills/rust-style/references/code_style.md index e4007291..f297f260 100644 --- a/agents/skills/rust-style/references/code_style.md +++ b/agents/skills/rust-style/references/code_style.md @@ -10,13 +10,6 @@ ## Naming Conventions -- Modules and files: `snake_case`. -- Functions, methods, and variables: `snake_case`. -- Types, traits, and enum variants: `UpperCamelCase`. -- Constants and statics: `SCREAMING_SNAKE_CASE`. -- Generic type parameters: short `UpperCamelCase` names such as `T`, `E`, `P`. -- Lifetimes: short lowercase names such as `'a`; use descriptive names only when they clarify an unusual relationship. -- Cargo feature names: `kebab-case`. - Avoid `manager`, `helper`, `util`, and `common` unless the name is already established locally. ### Module Naming @@ -36,7 +29,6 @@ - Avoid `as` aliases unless they remove ambiguity or follow a local convention. - Prefer `crate::` for crate-local imports and `super::` for parent-module test imports. - Keep trait imports close to the code that needs method resolution. -- Do not use glob imports in production modules except for explicit prelude modules. ## Whitespace and Line Breaks diff --git a/agents/skills/rust-style/references/tooling.md b/agents/skills/rust-style/references/tooling.md index 0ca6e92e..c055326f 100644 --- a/agents/skills/rust-style/references/tooling.md +++ b/agents/skills/rust-style/references/tooling.md @@ -43,7 +43,6 @@ tests/ - Let `rustfmt` define formatting. Do not hand-format around rustfmt. - Keep files focused and cohesive. -- Prefer modules of roughly 200-500 lines. Split when multiple responsibilities appear. - Keep public items before private helpers when it improves scanning. - Use `mod.rs` only when the existing project already uses that style; otherwise prefer `module_name.rs` plus `module_name/child.rs`. - Put one top-level concept per file when the concept has real behavior. diff --git a/agents/skills/rust-style/references/types.md b/agents/skills/rust-style/references/types.md index 2799b953..e3825a12 100644 --- a/agents/skills/rust-style/references/types.md +++ b/agents/skills/rust-style/references/types.md @@ -24,11 +24,17 @@ Value Object pattern: #[derive(Debug, Clone, PartialEq, Eq)] pub struct SkillName(String); +#[derive(Debug, thiserror::Error)] +pub enum SkillNameError { + #[error("skill name must not be empty")] + Empty, +} + impl SkillName { - pub fn parse(value: impl Into<String>) -> anyhow::Result<Self> { + pub fn parse(value: impl Into<String>) -> Result<Self, SkillNameError> { let value = value.into(); if value.is_empty() { - anyhow::bail!("skill name must not be empty"); + return Err(SkillNameError::Empty); } Ok(Self(value)) } diff --git a/agents/skills/source-of-truth/SKILL.md b/agents/skills/source-of-truth/SKILL.md index d4fb64e3..a088e019 100644 --- a/agents/skills/source-of-truth/SKILL.md +++ b/agents/skills/source-of-truth/SKILL.md @@ -48,11 +48,14 @@ rationale, API reference, user guidance, migration guidance, or an operational r ## Information Placement -- **Issue body** → this task's purpose, scope, acceptance criteria, and requirement sources. -- **Plan comment** → this task's implementation approach, major steps, and important assumptions. - A plan is not the authority for requirements; existing formal specifications and user - requirements retain their authority under this skill's conflict policy. -- **PR body** → actual changes, verification results, and material departures from the plan. +- **Issue body** → this task's requirements; its contents follow + [Issue Resolution](../issue-workflow/SKILL.md#issue-resolution). +- **Plan comment** → this task's implementation approach; its contents follow + [Plan Comments](../issue-workflow/SKILL.md#plan-comments). A plan is not the authority for + requirements; existing formal specifications and user requirements retain their authority under + this skill's conflict policy. +- **PR body** → actual changes, verification results, and material departures from the plan; + its format follows [Pull Request Body](../git-workflow/references/delivery.md#pull-request-body). Link these artifacts to each other instead of manually maintaining the same details in all three. - **Requirement purpose, source, scope, or accepted risk** → user request, issue or reproduced defect, public interface, external standard, product record, or behavior intentionally preserved @@ -79,8 +82,8 @@ rationale, API reference, user guidance, migration guidance, or an operational r - **Agent navigation** → the repository-defined entry map described under Agent Entry Maps. Domain vocabulary is cross-cutting rather than a separate document by default. Within a bounded -context, use the same terms in code, types, tests, diagrams, and prose. Add a short glossary only -when code cannot communicate the distinctions to every relevant audience. +context, use the same terms in code, types, tests, diagrams, and prose. Glossary rules belong to +[domain-modeling](../domain-modeling/SKILL.md#preserve-useful-knowledge). ## Duplication Test diff --git a/agents/skills/tsdd/SKILL.md b/agents/skills/tsdd/SKILL.md index 3fc8cf80..ecd5af9b 100644 --- a/agents/skills/tsdd/SKILL.md +++ b/agents/skills/tsdd/SKILL.md @@ -95,7 +95,8 @@ merely because there is more than one assertion or example row. unclear names, mixed responsibilities, and unnecessary coupling. Refactor when a concrete improvement is warranted, then re-run the relevant gate. Do not manufacture edits or speculative abstractions merely to perform a Refactor step. -7. **Finish in a coherent verified state.** VCS recording and delivery are governed by the +7. **Finish in a coherent verified state.** Continue with the next slice while required work + remains; the first Green does not end the task. VCS recording and delivery are governed by the `git-workflow` skill. ### Work without a contract gap @@ -174,31 +175,7 @@ Direct execution of narrow tests for intermediate decisions is allowed. Report e and known coverage limits. Concrete CLI syntax and delivery ordering belong to [Git Delivery](../git-workflow/references/delivery.md#local-verification-before-delivery). -## Type-Driven Design - -Types carry compile-time invariants and constraints. They are not a second requirements source; -they make invalid states unrepresentable. - -### Patterns - -- **Value Objects** wrap primitives with domain constraints. Construct through a validating - boundary and keep fields immutable. -- **Branded or newtypes** prevent mixing structurally identical but semantically distinct values. -- **Result or Either types** force callers to handle expected success and failure paths. -- **Exhaustive sum types** make unhandled state-machine cases visible to the compiler. -- **Parse, do not validate repeatedly.** Turn raw boundary input into a constrained type once. - -Tests still cover behavior that types cannot express, including business rules, ordering, side -effects, integration, concurrency, and external-system interactions. - ## Enforcement Mechanisms -This section applies only when the user asks to establish or revise TSDD's development workflow. -Ordinary feature work uses existing mechanisms; it does not require creating hooks, wrappers, or PR -templates. Consider the following mechanisms to make the intended path observable: - -- A workflow or wrapper that supports selecting Red or Green-baseline paths without manufacturing - Red. Determining whether a real contract gap exists still requires judgment. -- Hooks and CI gates for tests, type checks, lint, formatting, and risk-specific evaluation. -- A PR template that records the requirement source, behavior slice, Red or baseline evidence, - final Green evidence, review needs, and ADR impact. +Only when the user asks to establish or revise TSDD's development workflow, read +[Enforcement mechanisms](references/enforcement.md). diff --git a/agents/skills/tsdd/references/enforcement.md b/agents/skills/tsdd/references/enforcement.md new file mode 100644 index 00000000..d9f261b8 --- /dev/null +++ b/agents/skills/tsdd/references/enforcement.md @@ -0,0 +1,11 @@ +# Enforcement Mechanisms + +This reference applies only when the user asks to establish or revise TSDD's development workflow. +Ordinary feature work uses existing mechanisms; it does not require creating hooks, wrappers, or PR +templates. Consider the following mechanisms to make the intended path observable: + +- A workflow or wrapper that supports selecting Red or Green-baseline paths without manufacturing + Red. Determining whether a real contract gap exists still requires judgment. +- Hooks and CI gates for tests, type checks, lint, formatting, and risk-specific evaluation. +- A PR template that records the requirement source, behavior slice, Red or baseline evidence, + final Green evidence, review needs, and ADR impact. diff --git a/tests/agents/test_workflow_skill_layout.py b/tests/agents/test_workflow_skill_layout.py index 1cdd9c03..482fa1fc 100644 --- a/tests/agents/test_workflow_skill_layout.py +++ b/tests/agents/test_workflow_skill_layout.py @@ -1,5 +1,10 @@ -"""Rendering and link integrity, not claims about model compliance with prose.""" +"""Skill source contracts: metadata, link integrity, and handoff reachability. +These tests check rendering and link integrity, not claims about model compliance with prose. +""" + +from collections import Counter +from collections.abc import Iterator import json from pathlib import Path import re @@ -10,7 +15,15 @@ SKILLS = REPO_ROOT / "agents/skills" -WORKFLOW_SKILLS = ("issue-workflow", "git-workflow", "source-of-truth", "tsdd") +SKILL_NAMES = sorted(path.name for path in SKILLS.iterdir() if path.is_dir()) +SKILL_RENDERING = REPO_ROOT / "agents/skill_rendering.json" +FENCED_CODE_PATTERN = re.compile(r"^(```|~~~).*?^\1\s*$", re.MULTILINE | re.DOTALL) +INLINE_CODE_PATTERN = re.compile(r"`[^`\n]*`") +LINK_PATTERN = re.compile(r"\[[^\]]*\]\(<?([^)\s>]+)>?(?:\s+\"[^\"]*\")?\)") +HEADING_PATTERN = re.compile(r"^#{1,6}\s+(.+?)\s*#*\s*$", re.MULTILINE) +FRONTMATTER_PATTERN = re.compile(r"\A---\n(.*?)\n---\n", re.DOTALL) +FRONTMATTER_KEY_PATTERN = re.compile(r"^([A-Za-z][\w-]*):[ \t]*(.*)$") +SKILL_HANDOFF_PATTERN = re.compile(r"^\.\./([^/]+)/SKILL\.md$") @pytest.mark.integration @@ -28,64 +41,137 @@ def test_issue_skill_renders_with_implicit_invocation(agent_harness: CliRunner, metadata = output / "issue-workflow/agents/openai.yaml" if metadata.exists(): assert not re.search(r"allow_implicit_invocation:\s*false", metadata.read_text()) - config = json.loads((REPO_ROOT / "agents/skill_rendering.json").read_text()) - settings = config["skills"].get("issue-workflow", {}) - assert settings.get("claude", {}).get("frontmatter", {}).get("disable-model-invocation") is not True - assert settings.get("codex", {}).get("openai", {}).get("allow_implicit_invocation") is not False - - -def test_workflow_local_links_resolve_and_retired_issue_reference_is_absent() -> None: - assert not (SKILLS / "git-workflow/references/issues.md").exists() - for name in WORKFLOW_SKILLS: - for path in (SKILLS / name).rglob("*.md"): - text = path.read_text() - for destination in re.findall(r"\[[^\]]+\]\(([^)]+)\)", text): - filename = destination.split("#", 1)[0] - if not filename or "://" in filename: - continue - assert (path.parent / filename).is_file(), (path, destination) - assert not filename.endswith("/issues.md"), (path, destination) - - -CHANGED_SKILLS = ("herdr-delegation", "tsdd", "github-actions-style") + assert not _is_model_invocation_disabled("issue-workflow") @pytest.mark.integration @pytest.mark.parametrize("provider", ["claude", "codex"]) -@pytest.mark.parametrize("skill", CHANGED_SKILLS) -def test_changed_skills_render_with_valid_frontmatter( - agent_harness: CliRunner, tmp_path: Path, provider: str, skill: str +def test_every_skill_renders_with_its_name_and_description( + agent_harness: CliRunner, tmp_path: Path, provider: str ) -> None: output = tmp_path / provider agent_harness("generate-skills", "--provider", provider, "--output", str(output)) - text = (output / skill / "SKILL.md").read_text() - frontmatter = text.split("---", 2)[1] - assert re.search(rf"^name:\s*{re.escape(skill)}\s*$", frontmatter, re.MULTILINE) - assert re.search(r"^description:\s*\S", frontmatter, re.MULTILINE) + for skill in SKILL_NAMES: + frontmatter = _frontmatter((output / skill / "SKILL.md").read_text()) + assert frontmatter.get("name") == skill, skill + assert frontmatter.get("description"), skill -@pytest.mark.parametrize("skill", CHANGED_SKILLS) -def test_changed_skill_links_resolve(skill: str) -> None: - for path in (SKILLS / skill).rglob("*.md"): - for destination in re.findall(r"\[[^\]]+\]\(([^)]+)\)", path.read_text()): - filename = destination.split("#", 1)[0] - if not filename or "://" in filename: - continue - assert (path.parent / filename).is_file(), (path, destination) +@pytest.mark.parametrize("skill", SKILL_NAMES) +def test_skill_frontmatter_names_its_directory_and_describes_it(skill: str) -> None: + frontmatter = _frontmatter((SKILLS / skill / "SKILL.md").read_text()) + assert frontmatter.get("name") == skill + assert frontmatter.get("description") -@pytest.mark.integration -def test_generated_github_actions_skill_drops_only_the_blacksmith_section( - agent_harness: CliRunner, tmp_path: Path -) -> None: - output = tmp_path / "claude" - agent_harness("generate-skills", "--provider", "claude", "--output", str(output)) - rendered = (output / "github-actions-style/SKILL.md").read_text() - assert "blacksmith" not in rendered.lower() - assert "blacksmith" not in (SKILLS / "github-actions-style/SKILL.md").read_text().lower() - assert "## 7. Cache Design" in rendered + +@pytest.mark.parametrize("skill", SKILL_NAMES) +def test_skill_relative_links_resolve_to_existing_files_and_headings(skill: str) -> None: + broken_links = [ + (str(path.relative_to(SKILLS)), destination) + for path, destination in _relative_links(SKILLS / skill) + if not _link_resolves(path, destination) + ] + + assert broken_links == [] + + +@pytest.mark.parametrize("skill", SKILL_NAMES) +def test_skill_handoff_links_name_existing_skills(skill: str) -> None: + missing_skills = sorted(target for target in _linked_skills(SKILLS / skill) if target not in SKILL_NAMES) + + assert missing_skills == [] + + +@pytest.mark.parametrize("skill", SKILL_NAMES) +def test_skill_handoff_targets_allow_model_invocation(skill: str) -> None: + disabled_targets = sorted( + target + for target in _handoff_targets(SKILLS / skill) + if target != skill and _is_model_invocation_disabled(target) + ) + + assert disabled_targets == [] + + +def _frontmatter(text: str) -> dict[str, str]: + match = FRONTMATTER_PATTERN.match(text) + assert match is not None, "SKILL.md must start with YAML frontmatter" + fields: dict[str, list[str]] = {} + current_key: str | None = None + for line in match.group(1).splitlines(): + key_match = FRONTMATTER_KEY_PATTERN.match(line) + if key_match is not None: + current_key = key_match.group(1) + fields[current_key] = [key_match.group(2)] + elif current_key is not None and line[:1].isspace(): + fields[current_key].append(line) + return {key: _scalar(parts) for key, parts in fields.items()} + + +def _scalar(parts: list[str]) -> str: + head, *continuation = parts + words = continuation if head.strip() in {">", "|", ">-", "|-"} else [head, *continuation] + return " ".join(word.strip() for word in words).strip().strip("\"'") + + +def _relative_links(skill_directory: Path) -> Iterator[tuple[Path, str]]: + for path in sorted(skill_directory.rglob("*.md")): + for destination in LINK_PATTERN.findall(_prose(path.read_text())): + if not re.match(r"^[a-z][a-z0-9+.-]*:", destination, re.IGNORECASE): + yield path, destination + + +def _link_resolves(source: Path, destination: str) -> bool: + filename, _, anchor = destination.partition("#") + target = (source.parent / filename).resolve() if filename else source + if not target.is_file(): + return False + return not anchor or target.suffix != ".md" or anchor in _heading_anchors(target) + + +def _heading_anchors(path: Path) -> set[str]: + counts: Counter[str] = Counter() + anchors: set[str] = set() + for heading in HEADING_PATTERN.findall(_prose(path.read_text(), keep_inline_code=True)): + slug = _github_slug(heading) + anchors.add(f"{slug}-{counts[slug]}" if counts[slug] else slug) + counts[slug] += 1 + return anchors + + +def _github_slug(heading: str) -> str: + return re.sub(r"[^\w\- ]", "", heading.strip().lower()).replace(" ", "-") + + +def _handoff_targets(skill_directory: Path) -> set[str]: + return _linked_skills(skill_directory) | _skills_named_in_inline_code(skill_directory) + + +def _linked_skills(skill_directory: Path) -> set[str]: + return { + match.group(1) + for _, destination in _relative_links(skill_directory) + if (match := SKILL_HANDOFF_PATTERN.match(destination.partition("#")[0])) is not None + } + + +def _skills_named_in_inline_code(skill_directory: Path) -> set[str]: + return { + code.strip("`") + for path in skill_directory.rglob("*.md") + for code in INLINE_CODE_PATTERN.findall(_prose(path.read_text(), keep_inline_code=True)) + if code.strip("`") in SKILL_NAMES + } + + +def _is_model_invocation_disabled(skill: str) -> bool: + settings = json.loads(SKILL_RENDERING.read_text())["skills"].get(skill, {}) + claude_disabled = settings.get("claude", {}).get("frontmatter", {}).get("disable-model-invocation") is True + codex_disabled = settings.get("codex", {}).get("openai", {}).get("allow_implicit_invocation") is False + return claude_disabled or codex_disabled -def test_no_review_or_ponytail_skill_was_added() -> None: - names = {path.name for path in SKILLS.iterdir() if path.is_dir()} - assert not any("review" in name or "ponytail" in name for name in names) +def _prose(text: str, *, keep_inline_code: bool = False) -> str: + without_fences = FENCED_CODE_PATTERN.sub("", text) + return without_fences if keep_inline_code else INLINE_CODE_PATTERN.sub("", without_fences) From 40fb97f87fffaf5d49014d7163072281ea86a9dd Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Wed, 23 Sep 2026 19:19:22 +0900 Subject: [PATCH 06/14] docs(agents): replace function line and nesting quotas with an intent rule The 2-4 line and one-level nesting quotas contradicted the intent-based extraction rule. They pushed agents toward over-splitting functions. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- agents/AGENTS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/agents/AGENTS.md b/agents/AGENTS.md index 9e7313b7..81e43ce9 100644 --- a/agents/AGENTS.md +++ b/agents/AGENTS.md @@ -26,7 +26,8 @@ Existing project style takes precedence over these rules. - Keep classes and modules single-purpose, cohesive, loosely coupled, and minimally public. - Name classes by responsibility; order methods public-to-private; use DTOs at component boundaries. - Keep domain-specific enums and exception classes near their owning class; use one exception class per domain failure concept unless it is shared across modules. -- Keep functions small and single-purpose: prefer 0-3 arguments, one abstraction level, 2-4 lines when practical, short variable lifetimes, guard clauses, and at most one indentation level; split only when the extracted name clarifies intent. +- Keep functions single-purpose at one abstraction level: prefer 0-3 arguments, short variable lifetimes, and guard clauses over nested conditionals; keep nesting to about two levels. +- Extract a function when its name states intent the inline code does not; do not split merely to shorten a function. - Split duplicated logic, control structures, mixed responsibilities, and command/query behavior into named functions or objects. - Name one concept with one word; name command functions for their side effects and query functions for the value they return. - Let name length match scope size; include units, trust/safety attributes, and boolean prefixes where they clarify meaning. From 1acf5d83a66f562c705a7bafb602234313a1fcce Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Wed, 23 Sep 2026 20:37:40 +0900 Subject: [PATCH 07/14] fix(agents): skip languages whose changes are mapped to other tests Embedded assets such as TypeScript bridge files tested by Rust no longer demand a JS runner. Unmapped sources of that language still require their own runner and fail closed. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- agents/hooks/lib/check_selection.py | 9 +++++++++ tests/agents/hooks/test_check_selection.py | 20 ++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/agents/hooks/lib/check_selection.py b/agents/hooks/lib/check_selection.py index 606b82a1..c1ea0045 100644 --- a/agents/hooks/lib/check_selection.py +++ b/agents/hooks/lib/check_selection.py @@ -193,6 +193,8 @@ def language_plan(root: Path, rules: dict, changed: tuple[str, ...]) -> tuple[li affected = changed_language(rule, changed) or triggered if not affected and not explicit[language]: continue + if not triggered and not explicit[language] and delegated(rule, changed, covered, language): + continue markers = rule.get("project_markers", ()) if language == "javascript_typescript": markers = ("package.json",) @@ -248,6 +250,13 @@ def language_plan(root: Path, rules: dict, changed: tuple[str, ...]) -> tuple[li return invocations, errors +def delegated(rule: dict, changed: tuple[str, ...], covered: dict[str, set[str]], language: str) -> bool: + """Whether every changed source of a language is explicitly verified by another language's tests.""" + sources = {path for path in changed if any(path.endswith(extension) for extension in rule["source_extensions"])} + others = set().union(*(files for name, files in covered.items() if name != language)) + return bool(sources) and sources <= others + + def matching_targets(root: Path, language: str, rule: dict, changed: tuple[str, ...]) -> tuple[set[str], set[str]]: """Return each language's matched tests and the sources that resolve to none.""" if language == "rust": diff --git a/tests/agents/hooks/test_check_selection.py b/tests/agents/hooks/test_check_selection.py index 7fab4048..6b68de97 100644 --- a/tests/agents/hooks/test_check_selection.py +++ b/tests/agents/hooks/test_check_selection.py @@ -56,6 +56,26 @@ def test_javascript_without_a_test_command_is_not_success(tmp_path: Path) -> Non selection.language_plan(tmp_path, rules, ("src/file.ts",)) +def test_sources_mapped_to_another_language_run_only_that_language(tmp_path: Path) -> None: + rules = project(tmp_path, {"src/assets/**": ["src/bridges.rs", "tests/artifacts.rs"]}) + (tmp_path / "package.json").write_text(json.dumps({"devDependencies": {"@openai/codex": "1.0.0"}})) + touch_all(tmp_path, ["Cargo.toml", "src/bridges.rs", "tests/artifacts.rs"]) + commands, errors = selection.language_plan(tmp_path, rules, ("src/assets/pi/hook_bridge.ts",)) + assert not errors + assert [item.arguments for item in commands] == [ + ("cargo", "test", "bridges", "--quiet"), + ("cargo", "test", "--test", "artifacts", "--quiet"), + ] + + +def test_unmapped_sources_beside_mapped_ones_still_require_their_runner(tmp_path: Path) -> None: + rules = project(tmp_path, {"src/assets/**": ["tests/artifacts.rs"]}) + (tmp_path / "package.json").write_text("{}") + touch_all(tmp_path, ["Cargo.toml", "tests/artifacts.rs"]) + with pytest.raises(selection.UnknownTestCommand): + selection.language_plan(tmp_path, rules, ("src/assets/bridge.ts", "src/app.ts")) + + PYTEST = ("uv", "run", "--frozen", "pytest", "--no-header", "-q") From 98114183fb6c41483359f44a83d3cb7439d3958a Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:46:21 +0900 Subject: [PATCH 08/14] docs(agents): restore type-driven design rules in coding guidelines Keep parse-once boundaries, failures as values, exhaustive sum types, and the limit of types. These were lost when the tsdd Type-Driven Design section was removed. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- agents/AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/agents/AGENTS.md b/agents/AGENTS.md index 81e43ce9..4ac71166 100644 --- a/agents/AGENTS.md +++ b/agents/AGENTS.md @@ -39,3 +39,5 @@ Existing project style takes precedence over these rules. - Prefer Collection Objects for domain collections with invariants; do not expose mutable raw collections - Use Entities only when stable identity matters across state changes - Use classification objects/enums when categories or state transitions have domain rules +- Parse raw input into constrained types once at the boundary; do not re-validate the same value downstream +- Make invalid states unrepresentable where the language allows: return expected failures as values (Result or an equivalent) and handle sum types exhaustively. Types do not replace tests for business rules, ordering, side effects, or integration From 0a6363910727cc6d75eeceaed4ffe0524e991a8f Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:59:21 +0900 Subject: [PATCH 09/14] fix(codex): keep external migration and enable multi-agent v2 external_migration is still read by codex 0.155.1, so restore it instead of dropping it. Enable multi_agent_v2 so its wait timeouts take effect; remote_connections stays removed. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- agents/codex/config.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/agents/codex/config.toml b/agents/codex/config.toml index 78b7fd00..516aa75a 100644 --- a/agents/codex/config.toml +++ b/agents/codex/config.toml @@ -44,3 +44,9 @@ enabled = false [features] hooks = true +external_migration = true + +[features.multi_agent_v2] +enabled = true +min_wait_timeout_ms = 120000 +default_wait_timeout_ms = 120000 From dab5ac92452d4fed19dde088ef543b41ed0d4844 Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:08:27 +0900 Subject: [PATCH 10/14] fix(claude): use expandable home paths and run herdr outside the sandbox Claude does not expand $HOME in sandbox paths, so the nix and uv cache allowances never applied. Run herdr outside the sandbox like gh, since its socket is denied and escaping is now disabled. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- agents/claude/settings.json | 5 +++-- tests/agents/claude/test_settings.py | 11 ++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/agents/claude/settings.json b/agents/claude/settings.json index 1358eabe..28a084ab 100644 --- a/agents/claude/settings.json +++ b/agents/claude/settings.json @@ -48,8 +48,8 @@ "allowUnsandboxedCommands": false, "filesystem": { "allowWrite": [ - "$HOME/.cache/nix", - "$HOME/.cache/uv" + "~/.cache/nix", + "~/.cache/uv" ] }, "network": { @@ -60,6 +60,7 @@ "excludedCommands": [ "docker", "gh", + "herdr", "watchman" ] }, diff --git a/tests/agents/claude/test_settings.py b/tests/agents/claude/test_settings.py index e4a501ac..707a137e 100644 --- a/tests/agents/claude/test_settings.py +++ b/tests/agents/claude/test_settings.py @@ -20,7 +20,12 @@ def test_model_cannot_escape_the_sandbox_on_its_own() -> None: assert sandbox["allowUnsandboxedCommands"] is False -def test_verification_and_github_tools_work_without_escaping_the_sandbox() -> None: +def test_verification_and_agent_tools_work_without_escaping_the_sandbox() -> None: sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] - assert "$HOME/.cache/uv" in sandbox["filesystem"]["allowWrite"] - assert "gh" in sandbox["excludedCommands"] + assert {"~/.cache/nix", "~/.cache/uv"} <= set(sandbox["filesystem"]["allowWrite"]) + assert {"gh", "herdr"} <= set(sandbox["excludedCommands"]) + + +def test_sandbox_paths_use_the_home_prefix_the_sandbox_expands() -> None: + sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] + assert [path for path in sandbox["filesystem"]["allowWrite"] if "$" in path] == [] From 450e79d593e9247ee52734c655df4bce0fd58119 Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:48:53 +0900 Subject: [PATCH 11/14] fix(agents): let sandboxed verification run and gate herdr pane control Allow the verification state directory and dprint cache so verify runs without escaping. Ask before herdr commands that drive other panes; send-text, send-keys, and prompt were not covered. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- agents/claude/settings.json | 4 +++- agents/command_permissions.json | 14 ++++++++++++-- tests/agents/claude/test_settings.py | 7 ++++++- tests/agents/test_command_permission_sync.py | 15 +++++++++++++++ 4 files changed, 36 insertions(+), 4 deletions(-) diff --git a/agents/claude/settings.json b/agents/claude/settings.json index 28a084ab..7598f79c 100644 --- a/agents/claude/settings.json +++ b/agents/claude/settings.json @@ -49,7 +49,9 @@ "filesystem": { "allowWrite": [ "~/.cache/nix", - "~/.cache/uv" + "~/.cache/uv", + "~/Library/Caches/dprint", + "~/.local/state/agent-harness" ] }, "network": { diff --git a/agents/command_permissions.json b/agents/command_permissions.json index 063e4286..b7a7db36 100644 --- a/agents/command_permissions.json +++ b/agents/command_permissions.json @@ -1323,7 +1323,12 @@ }, { "decision": "ask", - "prefix": ["herdr", "pane", "send"], + "prefix": ["herdr", "pane", "send-text"], + "justification": "Ask before controlling other panes; their commands run outside this policy." + }, + { + "decision": "ask", + "prefix": ["herdr", "pane", "send-keys"], "justification": "Ask before controlling other panes; their commands run outside this policy." }, { @@ -1343,7 +1348,12 @@ }, { "decision": "ask", - "prefix": ["herdr", "agent", "send"], + "prefix": ["herdr", "agent", "send-keys"], + "justification": "Ask before controlling other panes; their commands run outside this policy." + }, + { + "decision": "ask", + "prefix": ["herdr", "agent", "prompt"], "justification": "Ask before controlling other panes; their commands run outside this policy." }, { diff --git a/tests/agents/claude/test_settings.py b/tests/agents/claude/test_settings.py index 707a137e..b5e60f90 100644 --- a/tests/agents/claude/test_settings.py +++ b/tests/agents/claude/test_settings.py @@ -22,7 +22,12 @@ def test_model_cannot_escape_the_sandbox_on_its_own() -> None: def test_verification_and_agent_tools_work_without_escaping_the_sandbox() -> None: sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] - assert {"~/.cache/nix", "~/.cache/uv"} <= set(sandbox["filesystem"]["allowWrite"]) + assert { + "~/.cache/nix", + "~/.cache/uv", + "~/Library/Caches/dprint", + "~/.local/state/agent-harness", + } <= set(sandbox["filesystem"]["allowWrite"]) assert {"gh", "herdr"} <= set(sandbox["excludedCommands"]) diff --git a/tests/agents/test_command_permission_sync.py b/tests/agents/test_command_permission_sync.py index 27b836f2..a0e5edb4 100644 --- a/tests/agents/test_command_permission_sync.py +++ b/tests/agents/test_command_permission_sync.py @@ -125,6 +125,21 @@ def test_container_execution_requires_approval(prefix: tuple[str, ...]) -> None: assert prefix not in ALLOW +@pytest.mark.parametrize( + "prefix", + [ + ("herdr", "pane", "run"), + ("herdr", "pane", "send-text"), + ("herdr", "pane", "send-keys"), + ("herdr", "agent", "start"), + ("herdr", "agent", "prompt"), + ("herdr", "agent", "send-keys"), + ], +) +def test_driving_other_terminal_panes_requires_approval(prefix: tuple[str, ...]) -> None: + assert prefix in ASK + + def test_justifications_are_provider_neutral() -> None: provider_specific = [rule for rule in POLICY["rules"] if re.search(r"\b(Codex|Claude)\b", rule["justification"])] assert provider_specific == [] From 888db758488e94b470231a5b4a285ebed039579e Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:49:18 +0900 Subject: [PATCH 12/14] fix(nix): use lefthook 2.x so git hooks run inside agent sandboxes lefthook 1.13.6 always ran hook commands under a pty, which the Claude and Codex sandboxes deny. Take lefthook from nixpkgs-unstable in home and the devShell, and check the minimum version. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- flake.nix | 7 ++++--- nix/checks.nix | 7 +++++++ nix/home/default.nix | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/flake.nix b/flake.nix index bc2163cb..922f82c5 100644 --- a/flake.nix +++ b/flake.nix @@ -217,9 +217,10 @@ in { default = shellPkgs.mkShell { - packages = with shellPkgs; [ - commitlint - lefthook + packages = [ + shellPkgs.commitlint + # lefthook before 2.1.6 runs hooks under a pty, which agent sandboxes deny. + nixpkgs-unstable.legacyPackages.${sys}.lefthook ]; }; } diff --git a/nix/checks.nix b/nix/checks.nix index cfa3dda1..de2faa84 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -111,6 +111,13 @@ in "codex" ]; no-unguarded-agent-packages = !(builtins.elem "opencode" homeNames); + # lefthook before 2.1.6 runs hook commands under a pty, which agent sandboxes deny. + sandbox-compatible-git-hooks = + let + lefthookVersions = versions "lefthook" home.home.packages; + in + lefthookVersions != [ ] + && builtins.all (version: lib.versionAtLeast version "2.1.6") lefthookVersions; commit-message-alias-avoids-removed-codex-flag = !(lib.hasInfix "--full-auto" home.programs.git.settings.alias.cc); shared-agent-input = diff --git a/nix/home/default.nix b/nix/home/default.nix index bf94ef14..35829cd4 100644 --- a/nix/home/default.nix +++ b/nix/home/default.nix @@ -197,7 +197,7 @@ in autocorrect commitlint dprint - lefthook + unstable.lefthook ls-lint # prettierd: used for markdown because dprint-plugin-markdown hardcodes 2-space list indent. # tabWidth:4 in ~/.prettierrc gives 4-space list nesting to match Obsidian. From c656219615da8e5205e645af373855e66b5654c2 Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Mon, 28 Sep 2026 16:51:56 +0900 Subject: [PATCH 13/14] fix(agents): exempt excluded commands with arguments and keep .envrc readable Glob-less excludedCommands entries match only the bare command, so gh and herdr stayed sandboxed. The .env* read denial also matched .envrc, which broke nix evaluation in the sandbox. Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- agents/claude/settings.json | 8 ++++---- agents/hooks/rules/secret_path_policy.json | 7 ++++++- tests/agents/claude/test_settings.py | 7 ++++++- tests/agents/hooks/test_secret_path_policy.py | 2 ++ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/agents/claude/settings.json b/agents/claude/settings.json index 7598f79c..81502b0b 100644 --- a/agents/claude/settings.json +++ b/agents/claude/settings.json @@ -60,10 +60,10 @@ ] }, "excludedCommands": [ - "docker", - "gh", - "herdr", - "watchman" + "docker *", + "gh *", + "herdr *", + "watchman *" ] }, "effortLevel": "xhigh", diff --git a/agents/hooks/rules/secret_path_policy.json b/agents/hooks/rules/secret_path_policy.json index eece3529..8ab7b1ff 100644 --- a/agents/hooks/rules/secret_path_policy.json +++ b/agents/hooks/rules/secret_path_policy.json @@ -2,7 +2,12 @@ "version": 1, "rules": [ { - "pattern": ".env*", + "pattern": ".env", + "access": ["read", "write"], + "reason": "Environment files may contain credentials." + }, + { + "pattern": ".env.*", "access": ["read", "write"], "reason": "Environment files may contain credentials." }, diff --git a/tests/agents/claude/test_settings.py b/tests/agents/claude/test_settings.py index b5e60f90..3a537e4a 100644 --- a/tests/agents/claude/test_settings.py +++ b/tests/agents/claude/test_settings.py @@ -28,7 +28,12 @@ def test_verification_and_agent_tools_work_without_escaping_the_sandbox() -> Non "~/Library/Caches/dprint", "~/.local/state/agent-harness", } <= set(sandbox["filesystem"]["allowWrite"]) - assert {"gh", "herdr"} <= set(sandbox["excludedCommands"]) + assert {"gh *", "herdr *"} <= set(sandbox["excludedCommands"]) + + +def test_excluded_commands_match_invocations_with_arguments() -> None: + sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] + assert [command for command in sandbox["excludedCommands"] if not command.endswith(" *")] == [] def test_sandbox_paths_use_the_home_prefix_the_sandbox_expands() -> None: diff --git a/tests/agents/hooks/test_secret_path_policy.py b/tests/agents/hooks/test_secret_path_policy.py index a889958e..4e9fb310 100644 --- a/tests/agents/hooks/test_secret_path_policy.py +++ b/tests/agents/hooks/test_secret_path_policy.py @@ -13,6 +13,7 @@ "path", [ ".env", + ".env.local", "config/github.token", "config/api_token", "auth/token.json", @@ -33,6 +34,7 @@ def test_credential_paths_are_denied(path: str) -> None: "path", [ ".venv/lib/python3.14/site-packages/pygments/token.py", + ".envrc", "src/tokenizer.rs", "src/keyboard.ts", "lib/monkey_patch.py", From 38ed91aed7600f947dd8a9175116904c62b6e7c3 Mon Sep 17 00:00:00 2001 From: furedea <132188853+furedea@users.noreply.github.com> Date: Mon, 28 Sep 2026 16:52:59 +0900 Subject: [PATCH 14/14] fix(claude): require approval for every sandbox escape Re-enable unsandboxed retries for commit, push, and nix, which cannot run in the sandbox. Add an ask rule for dangerouslyDisableSandbox so auto mode never escapes without the user. Bump agent-harness so source permission rules survive generation (furedea/agent-harness#140). Refs #148 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- .../hooks/rules/related_test_extensions.json | 2 ++ agents/claude/settings.json | 7 +++-- flake.lock | 6 ++-- tests/agents/claude/test_settings.py | 3 +- tests/agents/hooks/test_sandbox_escape.py | 31 +++++++++++++++++++ 5 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 tests/agents/hooks/test_sandbox_escape.py diff --git a/.agents/hooks/rules/related_test_extensions.json b/.agents/hooks/rules/related_test_extensions.json index 09706332..c72d6ee4 100644 --- a/.agents/hooks/rules/related_test_extensions.json +++ b/.agents/hooks/rules/related_test_extensions.json @@ -37,6 +37,7 @@ ], "agents/claude/settings.json": [ "tests/agents/claude/test_settings.py", + "tests/agents/hooks/test_sandbox_escape.py", "tests/agents/hooks/test_allowed_command.py", "tests/agents/hooks/test_command_cli.py", "tests/agents/test_notification.py" @@ -51,6 +52,7 @@ "tests/nix/configuration.bats" ], "agents/hooks.json": [ + "tests/agents/hooks/test_sandbox_escape.py", "tests/agents/test_notification.py", "tests/agents/hooks/test_verification_rule.py", "tests/agents/test_python_runtime.py", diff --git a/agents/claude/settings.json b/agents/claude/settings.json index 81502b0b..466b2ae6 100644 --- a/agents/claude/settings.json +++ b/agents/claude/settings.json @@ -10,7 +10,10 @@ "GIT_CONFIG_VALUE_0": "false" }, "permissions": { - "defaultMode": "auto" + "defaultMode": "auto", + "ask": [ + "Bash(dangerouslyDisableSandbox:true)" + ] }, "model": "fable", "preferredNotifChannel": "auto", @@ -45,7 +48,7 @@ "sandbox": { "enabled": true, "autoAllowBashIfSandboxed": false, - "allowUnsandboxedCommands": false, + "allowUnsandboxedCommands": true, "filesystem": { "allowWrite": [ "~/.cache/nix", diff --git a/flake.lock b/flake.lock index cf05ff07..0cf58552 100644 --- a/flake.lock +++ b/flake.lock @@ -10,11 +10,11 @@ ] }, "locked": { - "lastModified": 1789749985, - "narHash": "sha256-KbCFtPbooIHdevHWPRG+vQ8ChTCsBjiIPnM9pagJUfM=", + "lastModified": 1790580263, + "narHash": "sha256-n5/aBgN7YdnLRx7bcgW+HXLxKyN5EblBA8fFNVNTA68=", "owner": "furedea", "repo": "agent-harness", - "rev": "67fc439e817368d365338d879091d314f0467ced", + "rev": "9d9a8a31257487aa2cd474812c13ae821cc98a63", "type": "github" }, "original": { diff --git a/tests/agents/claude/test_settings.py b/tests/agents/claude/test_settings.py index 3a537e4a..d5b98acb 100644 --- a/tests/agents/claude/test_settings.py +++ b/tests/agents/claude/test_settings.py @@ -14,10 +14,9 @@ def test_agent_git_commands_run_without_the_sandboxed_fsmonitor_daemon() -> None assert configured["core.fsmonitor"] == "false" -def test_model_cannot_escape_the_sandbox_on_its_own() -> None: +def test_commands_run_sandboxed_by_default() -> None: sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] assert sandbox["enabled"] is True - assert sandbox["allowUnsandboxedCommands"] is False def test_verification_and_agent_tools_work_without_escaping_the_sandbox() -> None: diff --git a/tests/agents/hooks/test_sandbox_escape.py b/tests/agents/hooks/test_sandbox_escape.py new file mode 100644 index 00000000..82d6ad23 --- /dev/null +++ b/tests/agents/hooks/test_sandbox_escape.py @@ -0,0 +1,31 @@ +"""Leaving the sandbox is possible only through a human approval, even in auto mode.""" + +import json +from pathlib import Path + +import pytest + +from tests.runtime import CliRunner + + +ESCAPE_RULE = "Bash(dangerouslyDisableSandbox:true)" + + +@pytest.fixture(scope="module") +def generated_settings(agent_harness: CliRunner, tmp_path_factory: pytest.TempPathFactory) -> dict: + path = tmp_path_factory.mktemp("claude-sandbox") / "settings.json" + agent_harness("generate-claude-settings", "--output", str(path)) + return json.loads(Path(path).read_text()) + + +@pytest.mark.integration +def test_sandbox_escape_is_available(generated_settings: dict) -> None: + assert generated_settings["sandbox"]["enabled"] is True + assert generated_settings["sandbox"]["allowUnsandboxedCommands"] is True + + +@pytest.mark.integration +def test_every_sandbox_escape_asks_the_user(generated_settings: dict) -> None: + permissions = generated_settings["permissions"] + assert ESCAPE_RULE in permissions["ask"] + assert ESCAPE_RULE not in permissions.get("allow", [])