From 00851e0e1d58ff2a2509bf303840ef6554b6464d Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Fri, 11 Sep 2026 22:58:29 -0700 Subject: [PATCH] Gate the publishing act, not the subagent prompt agent-routing-guard decided from regexes over the spawn prompt. "carrying commits" parsed as an action because the noun test required a determiner, so deleting the word cleared the block without changing what the subagent would do; and its override check read only text blocks, so an answer given through AskUserQuestion never counted as approval. publish-act-guard reads the command the tool is about to run, which is a typed field, and blocks only when all three hold: the caller is a subagent, the command performs a publishing act at command position, and a liveness probe finds a reachable Invoker owner. No live owner means the subagent publishes, which is what the user asked for. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01UbWT7AQ3sJY49m13QPcom4 Change-Id: I53e191f4a21c16d42a1a345741108ead187f7687 --- docs/ecosystem.md | 1 + engine/hooks/agent-routing-guard/README.md | 105 ----- .../claude.agent.hook.json | 16 - .../claude_pretooluse_agent.py | 39 -- engine/hooks/agent-routing-guard/detect.py | 325 -------------- .../blocks_eight_publishing_subagents.json | 15 - .../fixtures/blocks_merge_the_stack.json | 15 - .../fixtures/silent_invoker_cli_absent.json | 15 - .../silent_nested_subagent_split.json | 16 - .../fixtures/silent_read_only_research.json | 15 - .../silent_user_said_do_it_locally.json | 15 - .../silent_user_said_dont_use_invoker.json | 15 - .../silent_verification_subagent.json | 15 - .../agent-routing-guard/tests/test_hooks.py | 396 ------------------ engine/hooks/publish-act-guard/README.md | 53 +++ .../hooks/publish-act-guard/claude.hook.json | 16 + .../publish-act-guard/claude_pretooluse.py | 41 ++ engine/hooks/publish-act-guard/detect.py | 280 +++++++++++++ .../install_claude_hook.py | 25 +- .../publish-act-guard/tests/test_detect.py | 123 ++++++ install.sh | 4 +- tests/test_install.py | 22 +- 22 files changed, 549 insertions(+), 1018 deletions(-) delete mode 100644 engine/hooks/agent-routing-guard/README.md delete mode 100644 engine/hooks/agent-routing-guard/claude.agent.hook.json delete mode 100644 engine/hooks/agent-routing-guard/claude_pretooluse_agent.py delete mode 100644 engine/hooks/agent-routing-guard/detect.py delete mode 100644 engine/hooks/agent-routing-guard/tests/fixtures/blocks_eight_publishing_subagents.json delete mode 100644 engine/hooks/agent-routing-guard/tests/fixtures/blocks_merge_the_stack.json delete mode 100644 engine/hooks/agent-routing-guard/tests/fixtures/silent_invoker_cli_absent.json delete mode 100644 engine/hooks/agent-routing-guard/tests/fixtures/silent_nested_subagent_split.json delete mode 100644 engine/hooks/agent-routing-guard/tests/fixtures/silent_read_only_research.json delete mode 100644 engine/hooks/agent-routing-guard/tests/fixtures/silent_user_said_do_it_locally.json delete mode 100644 engine/hooks/agent-routing-guard/tests/fixtures/silent_user_said_dont_use_invoker.json delete mode 100644 engine/hooks/agent-routing-guard/tests/fixtures/silent_verification_subagent.json delete mode 100644 engine/hooks/agent-routing-guard/tests/test_hooks.py create mode 100644 engine/hooks/publish-act-guard/README.md create mode 100644 engine/hooks/publish-act-guard/claude.hook.json create mode 100755 engine/hooks/publish-act-guard/claude_pretooluse.py create mode 100644 engine/hooks/publish-act-guard/detect.py rename engine/hooks/{agent-routing-guard => publish-act-guard}/install_claude_hook.py (59%) mode change 100644 => 100755 create mode 100644 engine/hooks/publish-act-guard/tests/test_detect.py diff --git a/docs/ecosystem.md b/docs/ecosystem.md index be121482..f39bd384 100644 --- a/docs/ecosystem.md +++ b/docs/ecosystem.md @@ -67,6 +67,7 @@ again. | `thrash-reflect-automate` | skill — FAIL → reflect → automate | | `auto-pr` | hook | | `bug-complaint-leak` | hook | +| `publish-act-guard` | hook | | `categorical-scope-guard` | hook (PreToolUse on `Bash`; blocks a status-narrowed mutation when the live turn said all/every/each) | | `cat-mode-default` | hook (UserPromptSubmit + PreToolUse on `Agent`; applies `cat-mode` on work turns and subagent prompts when `CATSTACK_CAT_MODE_DEFAULT=1`) | | `demo-freeze` | hook | diff --git a/engine/hooks/agent-routing-guard/README.md b/engine/hooks/agent-routing-guard/README.md deleted file mode 100644 index cc0792b9..00000000 --- a/engine/hooks/agent-routing-guard/README.md +++ /dev/null @@ -1,105 +0,0 @@ -# agent-routing-guard - -PreToolUse hook on the **Agent** tool: refuses a subagent spawn whose prompt -carries publication work while `invoker-cli` is on PATH. Exits 2 with a -message naming the routing rule and the `invoker-plan-to-invoker` skill. - -The question it asks is the *vehicle*, not the work. Nothing here judges the -task; it only refuses to let parallel subagents be the thing that commits, -pushes, merges, and opens PRs on a machine where Invoker is installed. - -## Fires when both hold - -1. The Agent payload's `tool_input.prompt` uses a publication verb as an - **action**: commit, push, merge, or open / make / create / raise / file / - submit / land a PR. -2. `invoker-cli` resolves on `PATH`. - -`invoker-cli` on PATH is a **proxy** for the routing rule's own condition, -which is written in terms of Invoker's MCP tools being available. A PreToolUse -payload cannot see the harness's MCP tool list, so PATH is the observable -stand-in — the machine has Invoker installed. The two can disagree: Invoker -installed but its MCP server not connected still blocks. The block is -clearable in one line by the user, which is the right cost for that gap. - -## Stays silent when - -- `invoker-cli` is absent — there is nothing to route to, and a subagent is - the only vehicle available. -- The prompt is read-only research or verification with no publication verb. - The same words as nouns do not count: "read the last commit", "summarize - each PR", "run the suite against the merge commit" all pass. A determiner - in front of the word ("the", "a", "each", "last", "which", a number) is - what marks it as a noun. -- The verb is negated or sits inside a hyphenated name. "Do not edit, create, - commit, or push anything", "no need to open a PR", and the skill name - `principle-push-not-poll` all pass. The negation only reaches a bare list - of words ending at the verb, so "Don't forget to commit and push" and "Do - not edit the tests, commit the fix" still block, and so does `force-push`. -- The payload carries `agent_id` — a subagent splitting its own slice is - executing a route somebody already chose, so re-asking there would block - work Invoker may itself be running. -- The user's current message says "do it locally" or "don't use invoker" - (and the near phrasings: "keep it local", "no invoker", "without invoker"). - -## The override check fails closed - -This is the one deliberate fail-closed path, and it is worth stating plainly. -The override lives in the user's current message, which the hook reads from -the transcript. That read has **three** outcomes, not two: - -| Outcome | What happens | -| --- | --- | -| The message carries an override | silent | -| The message carries no override | block, with the routing message | -| The message could not be read | block, with its own message naming the reason | - -"Could not be read" means: no `transcript_path` in the payload, an unreadable -file, a file over `TRANSCRIPT_SIZE_CAP_BYTES` (32 MB), or no user line in it. -An override that cannot be read is not an override — collapsing that outcome -into "no override was given" would be wrong in the loud direction, and -collapsing it into "an override was given" would silently restore the -incident this exists to close. The block costs one turn and its message says -exactly how to clear it: route through `invoker-plan-to-invoker`, or have the -user restate the local override in this turn. - -Everything else fails open: a payload that will not parse, a missing -`tool_input`, a promptless call, and any unexpected error in the detector all -let the spawn through (the last of those says so on stderr rather than -swallowing itself). - -Only the **tail** of the user's message is scanned for the override. A long -message is usually a paste whose quoted text can contain "do not use Invoker" -as somebody else's dialogue; a live directive sits at the edge of what was -just typed. Same bound, and the same reason, as `scope-lock`. - -## Incident this closes - -One session spawned eight subagents, each of which produced a PR-worthy -commit, with `invoker-cli` on PATH and the user having asked three separate -times to route through Invoker. No hook guarded it. The only PreToolUse hook -matching the Agent tool, `engine/hooks/cat-mode-default`, injects cat-mode -into the subagent's prompt — it improves how the subagent works and never -asks whether a subagent should have been the runner. - -## Files - -- `detect.py` — verb detection with the noun guard, `invoker-cli` PATH - resolution, the three-outcome override read, and `decide()`. -- `claude_pretooluse_agent.py` — Claude `PreToolUse` entrypoint; exit 2 with - the refusal on stderr. -- `claude.agent.hook.json` — Claude `PreToolUse` fragment (matcher `Agent`). -- `install_claude_hook.py` — idempotent `settings.json` merge, never an - overwrite. -- `tests/fixtures/*.json` — whole scenarios (invoker on PATH or not, the - user's current message, the Agent payload) with the expected outcome. - -## Install - -`./install.sh` from the repo root, then restart Claude Code. - -## Tests - -```sh -python3 -m unittest discover -s engine/hooks/agent-routing-guard/tests -v -``` diff --git a/engine/hooks/agent-routing-guard/claude.agent.hook.json b/engine/hooks/agent-routing-guard/claude.agent.hook.json deleted file mode 100644 index cd9a94c2..00000000 --- a/engine/hooks/agent-routing-guard/claude.agent.hook.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "hooks": { - "PreToolUse": [ - { - "matcher": "Agent", - "hooks": [ - { - "type": "command", - "command": "python3 $HOME/.claude/hooks/agent-routing-guard/claude_pretooluse_agent.py", - "timeout": 10 - } - ] - } - ] - } -} diff --git a/engine/hooks/agent-routing-guard/claude_pretooluse_agent.py b/engine/hooks/agent-routing-guard/claude_pretooluse_agent.py deleted file mode 100644 index e920fe9b..00000000 --- a/engine/hooks/agent-routing-guard/claude_pretooluse_agent.py +++ /dev/null @@ -1,39 +0,0 @@ -#!/usr/bin/env python3 -"""Claude Code PreToolUse (Agent): refuse a subagent spawn that carries -publication work while invoker-cli is on PATH. - -Exits 2 with the refusal on stderr, which is how Claude Code turns a -PreToolUse hook into a block. Fail-open on a payload that will not parse and -on any unexpected error in the detector; the one deliberate fail-closed path -is the local-override check, which lives in detect.py and blocks with its own -message when the user's current message could not be read. -""" -from __future__ import annotations - -import json -import os -import sys - -sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) - -from detect import decide - - -def main() -> None: - try: - payload = json.load(sys.stdin) - except (json.JSONDecodeError, OSError, ValueError): - return - try: - refusal = decide(payload if isinstance(payload, dict) else {}) - except Exception as exc: - sys.stderr.write(f"agent-routing-guard: check did not run ({exc}); allowing the spawn\n") - return - if refusal is None: - return - sys.stderr.write(refusal + "\n") - sys.exit(2) - - -if __name__ == "__main__": - main() diff --git a/engine/hooks/agent-routing-guard/detect.py b/engine/hooks/agent-routing-guard/detect.py deleted file mode 100644 index c8fc9317..00000000 --- a/engine/hooks/agent-routing-guard/detect.py +++ /dev/null @@ -1,325 +0,0 @@ -"""agent-routing-guard: refuse an Agent spawn that carries publication work -while Invoker is available. - -The incident this closes: one session spawned eight subagents, each of which -produced a PR-worthy commit, with `invoker-cli` on PATH and the user having -asked three separate times to route through Invoker. Nothing guarded the -vehicle -- the only PreToolUse hook matching the Agent tool -(engine/hooks/cat-mode-default) injects cat-mode into the subagent's prompt -and never asks whether a subagent was the right runner at all. - -Fires when both hold: - -1. The Agent payload's prompt carries a publication verb -- commit, push, - merge, or open/make/create/raise/file/submit/land a PR. The verb has to - read as an action: a determiner in front of it ("the last commit", "a - PR-worthy commit", "each merge") marks a noun, which is how a read-only - research or verification prompt mentions the same words. A negated verb - ("Do not edit, create, commit, or push anything") and a verb inside a - hyphenated name ("principle-push-not-poll", "auto-merge") do not count - either; a read-only scanner's prompt uses both. -2. `invoker-cli` resolves on PATH. That is a proxy for the routing rule's own - condition -- Invoker's MCP tools being available -- which a PreToolUse - payload cannot see. PATH is the observable stand-in and is named as such - wherever this hook reports. - -Stays silent when `invoker-cli` is absent (nothing to route to), when the -prompt has no publication verb, when the payload carries `agent_id` (a -subagent splitting its own work is not the routing decision -- rule 3 was -already answered by whoever spawned it), and when the user's current message -says "do it locally" or "don't use invoker". - -THE OVERRIDE CHECK FAILS CLOSED, deliberately, and it is the one part of -this hook that does. The override lives in the user's current message, which -this reads from the transcript. Three outcomes, not two: the message says -override (silent), the message says nothing (block), or the message could -not be read -- no transcript path, an unreadable or over-cap file, no user -line in it -- which blocks with its own message naming the reason. An -override that cannot be read is not an override; treating "unreadable" as -"no override was given" would be the safe direction only if the hook were -optional, and treating it as "an override was given" would silently restore -the incident. The block costs a turn and names how to clear it. Everything -else here fails open: a payload that will not parse, a missing tool_input, -and a promptless call are all left alone. -""" -from __future__ import annotations - -import json -import os -import re -import shutil - -AGENT_TOOL_NAMES = frozenset({"Agent", "Task"}) -INVOKER_CLI = "invoker-cli" -ROUTING_SKILL = "invoker-plan-to-invoker" - -TRANSCRIPT_SIZE_CAP_BYTES = 32 * 1024 * 1024 -OVERRIDE_SCAN_TAIL_CHARS = 400 - -OVERRIDE_PRESENT = "override" -OVERRIDE_ABSENT = "none" -OVERRIDE_UNCHECKED = "unchecked" - -ACTION_VERB_RES = ( - ("commit", re.compile(r"(?i)\bcommit(?:s|ted|ting)?\b")), - ("push", re.compile(r"(?i)\bpush(?:es|ed|ing)?\b")), - ("merge", re.compile(r"(?i)\bmerg(?:e|es|ed|ing)\b")), -) - -PR_ACTION_RE = re.compile( - r"(?i)\b(?:open|opens|opening|make|makes|making|create|creates|creating" - r"|raise|raises|raising|file|files|filing|submit|submits|submitting" - r"|land|lands|landing|ship|ships|shipping)\s+" - r"(?:(?:a|an|the|one|another|its|your|their|each|every|\d+)\s+)?" - r"(?:draft\s+)?(?:prs?|pull\s+requests?)\b" -) - -NOUN_CONTEXT_RE = re.compile( - r"(?i)\b(?:the|a|an|this|that|these|those|each|every|one|two|three|\d+" - r"|last|latest|previous|first|newest|next|head|initial|merge|which|whose" - r"|its|his|her|their|your|my|our)\s+(?:\w+[\s-]+){0,2}\Z" -) - -NEGATION_CUE_RE = re.compile(r"(?i)(? str: - for key in ("tool_name", "toolName", "tool", "name"): - value = payload.get(key) - if isinstance(value, str) and value.strip(): - return value.strip() - return "" - - -def transcript_path(payload: dict) -> str: - for key in ("transcript_path", "transcriptPath"): - value = payload.get(key) - if isinstance(value, str) and value.strip(): - return value.strip() - return "" - - -def spawned_by_subagent(payload: dict) -> bool: - """True when the caller is itself a subagent. - - Rule 3 is a decision about the parent's work, taken once. A subagent that - fans its own slice out further is executing a route that was already - chosen, so re-asking there would block work Invoker may itself be running. - """ - for key in SUBAGENT_ID_KEYS: - value = payload.get(key) - if isinstance(value, str) and value.strip(): - return True - return False - - -def _is_noun_use(text: str, start: int) -> bool: - return bool(NOUN_CONTEXT_RE.search(text[:start])) - - -def _is_negated(text: str, start: int) -> bool: - """True when the verb sits inside the scope of a negation in its clause. - - The scope is what "Do not edit, create, commit, or push" and "no need to - open a PR" share: the cue, a few lead words ("git", "need to", "ever"), - then a bare list of single words ending right at the verb. Anything else - between the cue and the verb -- "Don't forget to commit", "Do not edit - the tests, commit the fix" -- ends the negation, so the verb still counts. - """ - breaks = list(CLAUSE_BREAK_RE.finditer(text, 0, start)) - clause = text[breaks[-1].end():start] if breaks else text[:start] - cues = list(NEGATION_CUE_RE.finditer(clause)) - if not cues: - return False - chunks = [chunk.lower().split() for chunk in LIST_SEPARATOR_RE.split(clause[cues[-1].end():])] - *items, tail = chunks - if not all(word in NEGATION_LEAD_WORDS for word in tail): - return False - return all( - words and all(word in NEGATION_LEAD_WORDS for word in words[:-1]) for words in items - ) - - -def _is_hyphen_joined(text: str, start: int, end: int) -> bool: - """True for a verb inside a hyphenated name (principle-push-not-poll, - merge-clone, auto-merge). force-push and re-push are still verbs.""" - before = text[:start] - if HYPHEN_BEFORE_RE.search(before) and not HYPHEN_VERB_PREFIX_RE.search(before): - return True - return bool(HYPHEN_AFTER_RE.match(text, end)) - - -def _counts_as_action(text: str, start: int, end: int) -> bool: - return not (_is_hyphen_joined(text, start, end) or _is_negated(text, start)) - - -def publication_verbs(prompt: str) -> list[str]: - """The publication verbs this prompt uses as actions, deduped and ordered.""" - text = prompt or "" - found: list[str] = [] - for label, pattern in ACTION_VERB_RES: - for match in pattern.finditer(text): - if _is_noun_use(text, match.start()): - continue - if not _counts_as_action(text, match.start(), match.end()): - continue - found.append(label) - break - if any( - _counts_as_action(text, match.start(), match.end()) - for match in PR_ACTION_RE.finditer(text) - ): - found.append("open a PR") - return found - - -def invoker_available(path: str | None = None) -> bool: - return bool(shutil.which(INVOKER_CLI, path=path)) - - -def _text_content(data: dict) -> str: - message = data.get("message") - content = message.get("content") if isinstance(message, dict) else data.get("content") - if isinstance(content, str): - return content - if isinstance(content, list): - parts = [] - for block in content: - if isinstance(block, str): - parts.append(block) - elif isinstance(block, dict) and block.get("type") in ("text", "output_text"): - parts.append(str(block.get("text") or "")) - return "\n".join(parts) - return "" - - -def _is_human_user_line(data: dict) -> bool: - if data.get("type") != "user": - return False - text = _text_content(data) - return bool(text.strip()) and not text.lstrip().startswith("<") - - -def last_human_message(path: str) -> tuple[str, str]: - """(text, reason). A non-empty reason means the message could not be read - and the caller must treat the override as unchecked, never as absent.""" - if not path: - return "", "no transcript path in the hook payload" - try: - size = os.path.getsize(path) - except OSError as exc: - return "", f"transcript unreadable: {exc}" - if size > TRANSCRIPT_SIZE_CAP_BYTES: - return "", ( - f"transcript is {size} bytes, over the " - f"{TRANSCRIPT_SIZE_CAP_BYTES}-byte scan cap" - ) - try: - with open(path, encoding="utf-8", errors="replace") as handle: - lines = handle.readlines() - except OSError as exc: - return "", f"transcript unreadable: {exc}" - for raw in reversed(lines): - try: - data = json.loads(raw) - except (json.JSONDecodeError, TypeError): - continue - if isinstance(data, dict) and _is_human_user_line(data): - return _text_content(data), "" - return "", "no user message in the transcript" - - -def override_state(path: str) -> tuple[str, str]: - """One of OVERRIDE_PRESENT / OVERRIDE_ABSENT / OVERRIDE_UNCHECKED, plus - the reason when the check could not run. - - Only the tail of the message is scanned. A long paste is a log or a - transcript whose quoted text can contain "do not use Invoker" as somebody - else's dialogue; a live directive sits at the edge of what was just typed. - """ - text, reason = last_human_message(path) - if reason: - return OVERRIDE_UNCHECKED, reason - window = text[-OVERRIDE_SCAN_TAIL_CHARS:] - if any(pattern.search(window) for pattern in LOCAL_OVERRIDE_RES): - return OVERRIDE_PRESENT, "" - return OVERRIDE_ABSENT, "" - - -def block_message(verbs: list[str]) -> str: - return BLOCK_MESSAGE.format(verbs=", ".join(verbs), skill=ROUTING_SKILL) - - -def unchecked_message(verbs: list[str], reason: str) -> str: - return UNCHECKED_MESSAGE.format( - verbs=", ".join(verbs), reason=reason, skill=ROUTING_SKILL - ) - - -def decide(payload: dict) -> str | None: - """The refusal to print, or None to let the spawn through.""" - if not isinstance(payload, dict): - return None - if tool_name(payload) not in AGENT_TOOL_NAMES: - return None - tool_input = payload.get("tool_input") or payload.get("toolInput") - if not isinstance(tool_input, dict): - return None - prompt = tool_input.get("prompt") - if not isinstance(prompt, str) or not prompt.strip(): - return None - if spawned_by_subagent(payload): - return None - verbs = publication_verbs(prompt) - if not verbs: - return None - if not invoker_available(): - return None - state, reason = override_state(transcript_path(payload)) - if state == OVERRIDE_PRESENT: - return None - if state == OVERRIDE_UNCHECKED: - return unchecked_message(verbs, reason) - return block_message(verbs) diff --git a/engine/hooks/agent-routing-guard/tests/fixtures/blocks_eight_publishing_subagents.json b/engine/hooks/agent-routing-guard/tests/fixtures/blocks_eight_publishing_subagents.json deleted file mode 100644 index 2f9fa74e..00000000 --- a/engine/hooks/agent-routing-guard/tests/fixtures/blocks_eight_publishing_subagents.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "why": "the measured incident: one of eight subagents, each told to land its own PR-worthy commit, spawned while invoker-cli was on PATH and the user had asked three times for Invoker", - "expect": "blocks", - "invoker_on_path": true, - "user_message": "keep going through the list", - "payload": { - "hook_event_name": "PreToolUse", - "tool_name": "Agent", - "tool_input": { - "description": "Fix the retry budget", - "subagent_type": "general-purpose", - "prompt": "Work in your own worktree on the retry-budget fix, run the suite, then commit and push the branch and open a PR against main. Report the PR number." - } - } -} diff --git a/engine/hooks/agent-routing-guard/tests/fixtures/blocks_merge_the_stack.json b/engine/hooks/agent-routing-guard/tests/fixtures/blocks_merge_the_stack.json deleted file mode 100644 index ea82c7ec..00000000 --- a/engine/hooks/agent-routing-guard/tests/fixtures/blocks_merge_the_stack.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "why": "merge is a publication verb on its own; a subagent told to merge is the routing decision, not a research task", - "expect": "blocks", - "invoker_on_path": true, - "user_message": "get the stack landed", - "payload": { - "hook_event_name": "PreToolUse", - "tool_name": "Agent", - "tool_input": { - "description": "Land the admin-bypass stack", - "subagent_type": "general-purpose", - "prompt": "Babysit the admin-bypass stack: watch CI on each head, fix what breaks, then merge each PR bottom to top once it is green." - } - } -} diff --git a/engine/hooks/agent-routing-guard/tests/fixtures/silent_invoker_cli_absent.json b/engine/hooks/agent-routing-guard/tests/fixtures/silent_invoker_cli_absent.json deleted file mode 100644 index cf2d5658..00000000 --- a/engine/hooks/agent-routing-guard/tests/fixtures/silent_invoker_cli_absent.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "why": "no invoker-cli on PATH means there is nothing to route to; the subagent is the only vehicle available", - "expect": "silent", - "invoker_on_path": false, - "user_message": "keep going through the list", - "payload": { - "hook_event_name": "PreToolUse", - "tool_name": "Agent", - "tool_input": { - "description": "Fix the retry budget", - "subagent_type": "general-purpose", - "prompt": "Work in your own worktree on the retry-budget fix, run the suite, then commit and push the branch and open a PR against main. Report the PR number." - } - } -} diff --git a/engine/hooks/agent-routing-guard/tests/fixtures/silent_nested_subagent_split.json b/engine/hooks/agent-routing-guard/tests/fixtures/silent_nested_subagent_split.json deleted file mode 100644 index 06c6e97e..00000000 --- a/engine/hooks/agent-routing-guard/tests/fixtures/silent_nested_subagent_split.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "why": "the payload carries agent_id, so this is a subagent splitting its own slice; rule 3 was already answered by whoever spawned it", - "expect": "silent", - "invoker_on_path": true, - "user_message": "keep going through the list", - "payload": { - "hook_event_name": "PreToolUse", - "tool_name": "Agent", - "agent_id": "agent_01HQ8VZ3", - "tool_input": { - "description": "Second half of the slice", - "subagent_type": "general-purpose", - "prompt": "Take the second half of the file list, apply the same rename, then commit and push it to the shared branch." - } - } -} diff --git a/engine/hooks/agent-routing-guard/tests/fixtures/silent_read_only_research.json b/engine/hooks/agent-routing-guard/tests/fixtures/silent_read_only_research.json deleted file mode 100644 index 7640d62a..00000000 --- a/engine/hooks/agent-routing-guard/tests/fixtures/silent_read_only_research.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "why": "read-only research that names commits and PRs as nouns, never as actions; delegating this is exactly what subagents are for", - "expect": "silent", - "invoker_on_path": true, - "user_message": "where does the retry budget come from?", - "payload": { - "hook_event_name": "PreToolUse", - "tool_name": "Agent", - "tool_input": { - "description": "Find the retry budget", - "subagent_type": "Explore", - "prompt": "Find where the retry budget is configured. Read the last commit that touched it and the PR that introduced it, and report file:line for each. Change nothing." - } - } -} diff --git a/engine/hooks/agent-routing-guard/tests/fixtures/silent_user_said_do_it_locally.json b/engine/hooks/agent-routing-guard/tests/fixtures/silent_user_said_do_it_locally.json deleted file mode 100644 index a8509cd6..00000000 --- a/engine/hooks/agent-routing-guard/tests/fixtures/silent_user_said_do_it_locally.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "why": "the user's current message overrides the routing rule in so many words", - "expect": "silent", - "invoker_on_path": true, - "user_message": "forget the workflow, just do it locally and push when it is green", - "payload": { - "hook_event_name": "PreToolUse", - "tool_name": "Agent", - "tool_input": { - "description": "Fix the retry budget", - "subagent_type": "general-purpose", - "prompt": "Work in your own worktree on the retry-budget fix, then commit and push the branch and open a PR against main." - } - } -} diff --git a/engine/hooks/agent-routing-guard/tests/fixtures/silent_user_said_dont_use_invoker.json b/engine/hooks/agent-routing-guard/tests/fixtures/silent_user_said_dont_use_invoker.json deleted file mode 100644 index 844c52f1..00000000 --- a/engine/hooks/agent-routing-guard/tests/fixtures/silent_user_said_dont_use_invoker.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "why": "the other spelling of the same override", - "expect": "silent", - "invoker_on_path": true, - "user_message": "don't use invoker for this one, it is a two-line change", - "payload": { - "hook_event_name": "PreToolUse", - "tool_name": "Agent", - "tool_input": { - "description": "Fix the retry budget", - "subagent_type": "general-purpose", - "prompt": "Fix the retry budget in your own worktree, then commit and push the branch and open a PR against main." - } - } -} diff --git a/engine/hooks/agent-routing-guard/tests/fixtures/silent_verification_subagent.json b/engine/hooks/agent-routing-guard/tests/fixtures/silent_verification_subagent.json deleted file mode 100644 index 22ce70e6..00000000 --- a/engine/hooks/agent-routing-guard/tests/fixtures/silent_verification_subagent.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "why": "verification with no publication verb; the merge and the commit it names are both nouns", - "expect": "silent", - "invoker_on_path": true, - "user_message": "did that actually fix it?", - "payload": { - "hook_event_name": "PreToolUse", - "tool_name": "Agent", - "tool_input": { - "description": "Verify the fix", - "subagent_type": "general-purpose", - "prompt": "Check whether the flake is gone: run the suite ten times against the merge commit and paste the pass lines. Do not change any file." - } - } -} diff --git a/engine/hooks/agent-routing-guard/tests/test_hooks.py b/engine/hooks/agent-routing-guard/tests/test_hooks.py deleted file mode 100644 index f53b313f..00000000 --- a/engine/hooks/agent-routing-guard/tests/test_hooks.py +++ /dev/null @@ -1,396 +0,0 @@ -#!/usr/bin/env python3 -"""Tests for the agent-routing-guard PreToolUse (Agent) hook. - -Run: python3 -m unittest discover -s engine/hooks/agent-routing-guard/tests -v - -Every fixture under fixtures/ is a whole scenario: whether invoker-cli is on -PATH, what the user's current message says, and the Agent payload. Tests run -the real entrypoint with PATH pointed at a temp directory, so whether the -developer actually has invoker-cli installed never changes a result. -""" -from __future__ import annotations - -import io -import json -import os -import stat -import sys -import tempfile -import unittest -from contextlib import redirect_stderr, redirect_stdout -from unittest.mock import patch - -HOOK_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) -FIXTURE_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures") -sys.path.insert(0, HOOK_DIR) - -import claude_pretooluse_agent # noqa: E402 -import detect # noqa: E402 -import install_claude_hook # noqa: E402 - -INCIDENT_PROMPT = ( - "Work in your own worktree on the retry-budget fix, run the suite, then commit " - "and push the branch and open a PR against main. Report the PR number." -) - - -def load_fixture(name: str) -> dict: - with open(os.path.join(FIXTURE_DIR, name), encoding="utf-8") as handle: - return json.load(handle) - - -def fixture_names() -> list[str]: - return sorted(n for n in os.listdir(FIXTURE_DIR) if n.endswith(".json")) - - -class Sandbox: - """A PATH with or without invoker-cli, plus a transcript to read the - user's current message from.""" - - def __init__(self, invoker_on_path: bool = True) -> None: - self.tmp = tempfile.TemporaryDirectory() - self.bin_dir = os.path.join(self.tmp.name, "bin") - os.makedirs(self.bin_dir) - if invoker_on_path: - self.install_invoker() - - def install_invoker(self) -> str: - path = os.path.join(self.bin_dir, detect.INVOKER_CLI) - with open(path, "w", encoding="utf-8") as handle: - handle.write("#!/bin/sh\nexit 0\n") - os.chmod(path, os.stat(path).st_mode | stat.S_IXUSR) - return path - - def transcript(self, user_message: str | None, extra_lines: list[str] | None = None) -> str: - path = os.path.join(self.tmp.name, "transcript.jsonl") - with open(path, "w", encoding="utf-8") as handle: - handle.write(json.dumps({"type": "assistant", "message": {"content": [ - {"type": "text", "text": "on it"}]}}) + "\n") - if user_message is not None: - handle.write(json.dumps({"type": "user", "message": { - "role": "user", "content": user_message}}) + "\n") - for line in extra_lines or []: - handle.write(line + "\n") - return path - - def environ(self) -> dict: - return {"PATH": self.bin_dir, "HOME": self.tmp.name} - - def cleanup(self) -> None: - self.tmp.cleanup() - - -def run_entrypoint(payload: dict, environ: dict) -> tuple[bool, str]: - """(blocked, stderr). Blocked means the hook exited 2, which is how - Claude Code refuses the tool call.""" - err = io.StringIO() - out = io.StringIO() - with patch.object(sys, "stdin", io.StringIO(json.dumps(payload))): - with patch.dict(os.environ, environ, clear=True): - with redirect_stdout(out), redirect_stderr(err): - try: - claude_pretooluse_agent.main() - except SystemExit as exc: - return exc.code == 2, err.getvalue() - return False, err.getvalue() - - -class FixtureCase(unittest.TestCase): - def run_fixture(self, name: str) -> tuple[dict, bool, str]: - fixture = load_fixture(name) - box = Sandbox(invoker_on_path=fixture["invoker_on_path"]) - try: - payload = dict(fixture["payload"]) - payload["transcript_path"] = box.transcript(fixture["user_message"]) - blocked, message = run_entrypoint(payload, box.environ()) - finally: - box.cleanup() - return fixture, blocked, message - - def test_every_fixture_is_an_agent_payload_with_a_stated_expectation(self) -> None: - names = fixture_names() - self.assertGreaterEqual(len(names), 8, names) - for name in names: - fixture = load_fixture(name) - self.assertEqual(fixture["payload"]["tool_name"], "Agent", name) - self.assertIn(fixture["expect"], ("blocks", "silent"), name) - self.assertTrue(fixture["why"].strip(), name) - self.assertTrue(any(load_fixture(n)["expect"] == "blocks" for n in names)) - self.assertTrue(any(load_fixture(n)["expect"] == "silent" for n in names)) - - def test_every_fixture_matches_its_stated_expectation(self) -> None: - for name in fixture_names(): - with self.subTest(fixture=name): - fixture, blocked, message = self.run_fixture(name) - self.assertEqual(blocked, fixture["expect"] == "blocks", f"{name}: {message}") - - def test_the_block_names_the_routing_rule_and_the_invoker_skill(self) -> None: - _fixture, blocked, message = self.run_fixture("blocks_eight_publishing_subagents.json") - self.assertTrue(blocked, message) - self.assertIn("execution routing rule 3", message) - self.assertIn(detect.ROUTING_SKILL, message) - self.assertIn("commit", message) - self.assertIn("push", message) - - def test_the_nested_split_fixture_blocks_once_agent_id_is_removed(self) -> None: - fixture = load_fixture("silent_nested_subagent_split.json") - box = Sandbox() - try: - payload = dict(fixture["payload"]) - payload["transcript_path"] = box.transcript(fixture["user_message"]) - self.assertFalse(run_entrypoint(payload, box.environ())[0]) - payload.pop("agent_id") - blocked, message = run_entrypoint(payload, box.environ()) - finally: - box.cleanup() - self.assertTrue(blocked, message) - - def test_a_silent_fixture_prints_nothing_at_all(self) -> None: - _fixture, blocked, message = self.run_fixture("silent_read_only_research.json") - self.assertFalse(blocked) - self.assertEqual(message, "") - - -class VerbCase(unittest.TestCase): - def test_action_verbs_are_detected(self) -> None: - cases = { - "commit and push when the suite is green": ["commit", "push"], - "open a PR against main": ["open a PR"], - "make a draft PR for each slice": ["open a PR"], - "merge it once CI is green": ["merge"], - "git push -u origin HEAD": ["push"], - "land the PRs bottom to top": ["open a PR"], - } - for prompt, expected in cases.items(): - self.assertEqual(detect.publication_verbs(prompt), expected, prompt) - - def test_the_same_words_as_nouns_do_not_count(self) -> None: - for prompt in ( - "read the last commit and say what changed", - "find the commit that broke the build", - "summarize each PR in the stack", - "run the suite against the merge commit", - "which PR introduced this?", - "report the first push that failed", - ): - self.assertEqual(detect.publication_verbs(prompt), [], prompt) - - def test_negated_verbs_do_not_count(self) -> None: - for prompt in ( - "READ-ONLY task. Do not edit, create, commit, or push anything.", - "Scan the transcripts. Don't commit or push.", - "Never git push; report back instead.", - "There is no need to open a PR for this.", - "Neither edit nor merge anything.", - "Finish without committing, pushing, or merging.", - ): - self.assertEqual(detect.publication_verbs(prompt), [], prompt) - - def test_hyphenated_names_do_not_count(self) -> None: - for prompt in ( - "Follow principle-push-not-poll. Scan the combined transcripts and list each blocked spawn.", - "Apply principle-push-not-poll and report the combined findings.", - "Explain how merge-clone works in the repo.", - "Check whether auto-merge is enabled on the repo settings.", - ): - self.assertEqual(detect.publication_verbs(prompt), [], prompt) - - def test_real_publishing_requests_around_negation_still_fire(self) -> None: - cases = { - "commit and push the fix, then open a PR": ["commit", "push", "open a PR"], - "Don't forget to commit and push the fix.": ["commit", "push"], - "Do not edit the tests, commit and push the fix.": ["commit", "push"], - "No need to open a PR, just commit and push.": ["commit", "push"], - "Don't stop until you commit and push.": ["commit", "push"], - "Do not open a PR; merge it once CI is green.": ["merge"], - "Use principle-push-not-poll, then commit and push.": ["commit", "push"], - "force-push the branch after the rebase": ["push"], - "re-push the branch": ["push"], - } - for prompt, expected in cases.items(): - self.assertEqual(detect.publication_verbs(prompt), expected, prompt) - - -class OverrideCase(unittest.TestCase): - def setUp(self) -> None: - self.box = Sandbox() - - def tearDown(self) -> None: - self.box.cleanup() - - def payload(self, transcript: str) -> dict: - return { - "hook_event_name": "PreToolUse", - "tool_name": "Agent", - "transcript_path": transcript, - "tool_input": {"description": "fix", "prompt": INCIDENT_PROMPT}, - } - - def test_override_phrasings_all_allow_the_spawn(self) -> None: - for message in ( - "do it locally", - "just do this locally please", - "don't use invoker", - "do not use Invoker for this", - "no invoker, we are debugging", - "keep it local", - ): - transcript = self.box.transcript(message) - blocked, _ = run_entrypoint(self.payload(transcript), self.box.environ()) - self.assertFalse(blocked, message) - - def test_an_override_quoted_deep_inside_a_paste_does_not_count(self) -> None: - pasted = "do not use invoker\n" + ("filler log line\n" * 400) + "carry on with the plan" - transcript = self.box.transcript(pasted) - blocked, _ = run_entrypoint(self.payload(transcript), self.box.environ()) - self.assertTrue(blocked) - - def test_blocks_when_the_transcript_is_missing(self) -> None: - payload = self.payload(os.path.join(self.box.tmp.name, "gone.jsonl")) - blocked, message = run_entrypoint(payload, self.box.environ()) - self.assertTrue(blocked) - self.assertIn("could not be checked", message) - self.assertIn(detect.ROUTING_SKILL, message) - - def test_blocks_when_the_payload_carries_no_transcript_path(self) -> None: - payload = self.payload("") - payload.pop("transcript_path") - blocked, message = run_entrypoint(payload, self.box.environ()) - self.assertTrue(blocked) - self.assertIn("no transcript path", message) - - def test_blocks_when_the_transcript_is_too_large_to_scan(self) -> None: - transcript = self.box.transcript("do it locally") - with patch.object(detect, "TRANSCRIPT_SIZE_CAP_BYTES", 4): - state, reason = detect.override_state(transcript) - self.assertEqual(state, detect.OVERRIDE_UNCHECKED) - self.assertIn("scan cap", reason) - - def test_blocks_when_the_transcript_holds_no_user_message(self) -> None: - transcript = self.box.transcript(None) - blocked, message = run_entrypoint(self.payload(transcript), self.box.environ()) - self.assertTrue(blocked) - self.assertIn("no user message", message) - - def test_a_malformed_transcript_line_is_skipped_not_treated_as_the_message(self) -> None: - transcript = self.box.transcript("do it locally", extra_lines=["{not json", ""]) - blocked, message = run_entrypoint(self.payload(transcript), self.box.environ()) - self.assertFalse(blocked, message) - - def test_an_injected_system_turn_is_not_the_users_message(self) -> None: - injected = json.dumps({"type": "user", "message": { - "role": "user", "content": "do it locally"}}) - transcript = self.box.transcript("carry on", extra_lines=[injected]) - blocked, _ = run_entrypoint(self.payload(transcript), self.box.environ()) - self.assertTrue(blocked) - - -class SilenceCase(unittest.TestCase): - def setUp(self) -> None: - self.box = Sandbox() - - def tearDown(self) -> None: - self.box.cleanup() - - def test_a_non_agent_tool_is_never_touched(self) -> None: - payload = { - "tool_name": "Bash", - "transcript_path": self.box.transcript("go"), - "tool_input": {"command": "git commit -am wip", "prompt": INCIDENT_PROMPT}, - } - blocked, message = run_entrypoint(payload, self.box.environ()) - self.assertFalse(blocked) - self.assertEqual(message, "") - - def test_an_agent_call_with_no_prompt_is_allowed(self) -> None: - payload = { - "tool_name": "Agent", - "transcript_path": self.box.transcript("go"), - "tool_input": {"description": "x"}, - } - self.assertFalse(run_entrypoint(payload, self.box.environ())[0]) - - def test_read_only_scanner_prompts_are_allowed_through_the_entrypoint(self) -> None: - for prompt in ( - "READ-ONLY task. Do not edit, create, commit, or push anything.", - "Follow principle-push-not-poll. Scan the combined transcripts and list each blocked spawn.", - ): - payload = { - "tool_name": "Agent", - "transcript_path": self.box.transcript("go"), - "tool_input": {"prompt": prompt}, - } - blocked, message = run_entrypoint(payload, self.box.environ()) - self.assertFalse(blocked, prompt) - self.assertEqual(message, "", prompt) - - def test_a_publishing_prompt_still_blocks_through_the_entrypoint(self) -> None: - payload = { - "tool_name": "Agent", - "transcript_path": self.box.transcript("go"), - "tool_input": {"prompt": "commit and push the fix, then open a PR"}, - } - blocked, message = run_entrypoint(payload, self.box.environ()) - self.assertTrue(blocked) - self.assertIn("commit, push, open a PR", message) - - def test_invoker_absent_allows_the_spawn_without_reading_the_transcript(self) -> None: - box = Sandbox(invoker_on_path=False) - try: - payload = { - "tool_name": "Agent", - "transcript_path": os.path.join(box.tmp.name, "gone.jsonl"), - "tool_input": {"prompt": INCIDENT_PROMPT}, - } - blocked, message = run_entrypoint(payload, box.environ()) - finally: - box.cleanup() - self.assertFalse(blocked) - self.assertEqual(message, "") - - -class FailOpenCase(unittest.TestCase): - def test_malformed_stdin_allows_the_spawn(self) -> None: - err = io.StringIO() - with patch.object(sys, "stdin", io.StringIO("not json")): - with redirect_stderr(err): - claude_pretooluse_agent.main() - self.assertEqual(err.getvalue(), "") - - def test_a_detector_error_is_reported_and_fails_open(self) -> None: - err = io.StringIO() - payload = {"tool_name": "Agent", "tool_input": {"prompt": INCIDENT_PROMPT}} - with patch.object(sys, "stdin", io.StringIO(json.dumps(payload))): - with patch.object(claude_pretooluse_agent, "decide", side_effect=RuntimeError("boom")): - with redirect_stderr(err): - claude_pretooluse_agent.main() - self.assertIn("check did not run (boom)", err.getvalue()) - - -class InstallerCase(unittest.TestCase): - def test_the_fragment_is_a_pretooluse_hook_on_the_agent_tool(self) -> None: - with open(install_claude_hook.FRAGMENT_PATH, encoding="utf-8") as handle: - fragment = json.load(handle) - [entry] = fragment["hooks"]["PreToolUse"] - self.assertEqual(entry["matcher"], "Agent") - self.assertEqual( - entry["hooks"][0]["command"], - "python3 $HOME/.claude/hooks/agent-routing-guard/claude_pretooluse_agent.py", - ) - - def test_merge_is_idempotent_and_keeps_other_hooks(self) -> None: - with open(install_claude_hook.FRAGMENT_PATH, encoding="utf-8") as handle: - fragment = json.load(handle) - settings = {"hooks": {"PreToolUse": [ - {"matcher": "Agent", "hooks": [{"type": "command", "command": "python3 other.py"}]}]}} - settings, changed = install_claude_hook.merge_hook(settings, fragment) - self.assertTrue(changed) - settings, changed_again = install_claude_hook.merge_hook(settings, fragment) - self.assertFalse(changed_again) - commands = [h["command"] for e in settings["hooks"]["PreToolUse"] for h in e["hooks"]] - self.assertEqual(commands.count("python3 other.py"), 1) - self.assertEqual(sum(install_claude_hook.MARKER in c for c in commands), 1) - - -if __name__ == "__main__": - unittest.main() diff --git a/engine/hooks/publish-act-guard/README.md b/engine/hooks/publish-act-guard/README.md new file mode 100644 index 00000000..4afb40b4 --- /dev/null +++ b/engine/hooks/publish-act-guard/README.md @@ -0,0 +1,53 @@ +# publish-act-guard + +Refuses a publishing command issued from inside a subagent while a live Invoker +owner is reachable, so publishing work routes through Invoker instead of +fanning out across parallel subagents. + +## Fires on + +A shell-like PreToolUse call whose command, parsed at command position, performs +one of: `git push`, `gh pr create|merge|ready`, a mutating `gh api` call against +`/pulls`, `mergify stack push`, `create-pr.mjs`, `safe-stack-push.mjs --execute`. +All three must hold: the caller is a subagent, the act is one of those, and the +liveness probe reports a reachable owner. + +## Silent on + +- The main session — routing is the parent's decision, taken once. +- Every read-only neighbour: `gh pr view`, `gh pr checks`, `gh api ... --jq`, + `git log`, a filename argument such as `cat scripts/safe-stack-push.mjs`, and + a mention of a command inside a quoted string or grep pattern. +- Any dry run: `git push --dry-run`, `safe-stack-push.mjs` without `--execute`. +- No reachable owner — if Invoker cannot take the work, the subagent publishes. + +## Replaces agent-routing-guard + +The predecessor classified the spawn prompt with regexes over prose. "carrying +commits" parsed as an action because its noun test required a determiner, and +deleting the word cleared the block without changing what the subagent would do. +Its override check read only text blocks, so an answer given through +`AskUserQuestion` never counted as approval. This hook reads the command, which +is a typed field, and never the prompt. + +## Fail direction + +- Payload will not parse, non-shell tool, no command string: open, reason on stderr. +- Caller is not a subagent, command is not a publishing act: open, reason recorded + (set `PUBLISH_ACT_GUARD_DEBUG=1` to see it). +- Liveness unreadable (probe timed out or errored): open, and the block message + says UNCHECKED so the report has to name it. +- Owner reachable and a subagent is publishing: closed. + +## Block message and escape hatch + +The refusal names the act and points at the `invoker-plan-to-invoker` skill. It +clears on its own when no live owner answers. The user clears it deliberately by +saying "do it locally" or "don't use invoker", which routes the work here rather +than to Invoker. + +## Liveness probe + +`invoker-cli query capacity --output json`, 8s timeout, cached 120s in +`$TMPDIR/publish-act-guard-liveness.json`. An injected probe (tests) bypasses the +cache entirely. diff --git a/engine/hooks/publish-act-guard/claude.hook.json b/engine/hooks/publish-act-guard/claude.hook.json new file mode 100644 index 00000000..33f97512 --- /dev/null +++ b/engine/hooks/publish-act-guard/claude.hook.json @@ -0,0 +1,16 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "python3 $HOME/.claude/hooks/publish-act-guard/claude_pretooluse.py", + "timeout": 12 + } + ] + } + ] + } +} diff --git a/engine/hooks/publish-act-guard/claude_pretooluse.py b/engine/hooks/publish-act-guard/claude_pretooluse.py new file mode 100755 index 00000000..05219b46 --- /dev/null +++ b/engine/hooks/publish-act-guard/claude_pretooluse.py @@ -0,0 +1,41 @@ +#!/usr/bin/env python3 +"""Claude/Cursor PreToolUse: refuse a publishing command issued from inside a +subagent while a live Invoker owner is reachable. + +Exits 2 with the refusal on stderr, which is how Claude Code turns a +PreToolUse hook into a block. Fails open on a payload that will not parse and +on any unexpected detector error; the liveness read has its own UNCHECKED +branch that allows the command and names the reason. +""" +from __future__ import annotations + +import json +import os +import sys + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +from detect import decide + + +def main() -> None: + try: + payload = json.load(sys.stdin) + except (json.JSONDecodeError, OSError, ValueError) as exc: + sys.stderr.write(f"publish-act-guard: payload did not parse ({exc}); allowing\n") + return + try: + refusal = decide(payload if isinstance(payload, dict) else {}) + except Exception as exc: + sys.stderr.write(f"publish-act-guard: check did not run ({exc}); allowing\n") + return + if refusal is None: + return + sys.stderr.write(refusal + "\n") + if refusal.startswith("publish-act-guard: UNCHECKED"): + return + sys.exit(2) + + +if __name__ == "__main__": + main() diff --git a/engine/hooks/publish-act-guard/detect.py b/engine/hooks/publish-act-guard/detect.py new file mode 100644 index 00000000..19a2d83a --- /dev/null +++ b/engine/hooks/publish-act-guard/detect.py @@ -0,0 +1,280 @@ +"""Refuse a publishing command run from inside a subagent while a live +Invoker owner is reachable. + +The predecessor, agent-routing-guard, classified the spawn prompt with +regexes over prose: "carrying commits" parsed as an action because the noun +test required a determiner, and deleting the word cleared the block without +changing what the subagent would do. This detector never reads prose. It +parses the command the tool is about to run, which is a typed field, and +decides from three facts: the act, the caller, and whether Invoker can take +the work. + +Fail directions, one per read: + - unparsable payload, non-shell tool, empty command, non-subagent caller, + command that is not a publishing act: open, with the reason recorded. + - Invoker liveness could not be determined: open, and the reason is + printed, because a probe that cannot run must not hold up publishing + when Invoker may itself be down. + - Invoker reachable and a subagent is publishing: closed. +""" +from __future__ import annotations + +import json +import os +import re +import shlex +import subprocess +import sys +import time + +INVOKER_CLI = "invoker-cli" +ROUTING_SKILL = "invoker-plan-to-invoker" +LIVENESS_TTL_SECONDS = 120 +LIVENESS_TIMEOUT_SECONDS = 8 +LIVENESS_CACHE_PATH = os.path.join( + os.environ.get("TMPDIR", "/tmp"), "publish-act-guard-liveness.json" +) +DEBUG_ENV = "PUBLISH_ACT_GUARD_DEBUG" + +SUBAGENT_ID_KEYS = ("subagent_id", "subagentId", "agent_id", "agentId", "sub_agent_id") + +SHELL_LIKE_TOOL_NAMES = frozenset({ + "Bash", "bash", "shell", "Shell", "exec", "exec_command", + "run_terminal_cmd", "local_shell", "run_command", "shell_call", +}) + +LIVE = "live" +DOWN = "down" +UNCHECKED = "unchecked" + +BLOCK_MESSAGE = ( + "publish-act-guard: this subagent is about to run a publishing command " + "({act}) while a live Invoker owner is reachable.\n" + "Publishing work routes through Invoker, not through parallel subagents: " + "follow the installed {skill} skill, then submit the plan.\n" + "This gate reads the command, never the prompt. It goes quiet on its own " + "when no live owner answers, and the user clears it by saying " + "\"do it locally\" or \"don't use invoker\"." +) + + +def _silent(reason: str) -> None: + if os.environ.get(DEBUG_ENV): + sys.stderr.write(f"publish-act-guard: silent ({reason})\n") + return None + + +def _silent_text(reason: str) -> str: + _silent(reason) + return "" + + +def _silent_state(reason: str) -> None: + _silent(reason) + return None + + +def _warn(reason: str) -> None: + sys.stderr.write(f"publish-act-guard: {reason}\n") + + +def tool_name(payload: dict) -> str: + for key in ("tool_name", "toolName", "tool", "name"): + value = payload.get(key) + if isinstance(value, str) and value.strip(): + return value.strip() + return "" + + +def command_text(payload: dict) -> str: + tool_input = payload.get("tool_input") or payload.get("toolInput") or {} + if not isinstance(tool_input, dict): + return _silent_text("tool_input is not an object") + for key in ("command", "cmd", "script", "input"): + value = tool_input.get(key) + if isinstance(value, str) and value.strip(): + return value + return "" + + +def in_subagent(payload: dict) -> bool: + return any( + isinstance(payload.get(key), str) and payload[key].strip() + for key in SUBAGENT_ID_KEYS + ) + + +def _words(command: str) -> list[str]: + try: + return [word for word in shlex.split(command) if word] + except ValueError as exc: + _warn(f"command did not lex ({exc}); falling back to whitespace split") + return re.findall(r"[^\s'\"]+", command) + + +SHELL_SEPARATORS = frozenset({"&&", "||", "|", ";", "(", ")", "{", "}", "&"}) +COMMAND_PREFIXES = frozenset({"sudo", "env", "time", "timeout", "nohup", "xargs", "command", "nice"}) +DRY_RUN_FLAGS = frozenset({"--dry-run", "-n", "--dryrun"}) + + +def _command_heads(words: list[str]) -> list[list[str]]: + """Split argv into command invocations, skipping separators, env + assignments, and wrapper prefixes, so only a real command head is + matched. A filename passed to cat is never a command head.""" + commands: list[list[str]] = [] + current: list[str] = [] + for word in words: + if word in SHELL_SEPARATORS: + if current: + commands.append(current) + current = [] + continue + current.append(word) + if current: + commands.append(current) + heads = [] + for command in commands: + index = 0 + while index < len(command): + word = command[index] + if "=" in word and not word.startswith("-") and index == 0: + index += 1 + continue + if os.path.basename(word) in COMMAND_PREFIXES: + index += 1 + while index < len(command) and command[index].startswith("-"): + index += 1 + continue + break + if index < len(command): + heads.append(command[index:]) + return heads + + +def publishing_act(command: str) -> str | None: + """Name the publishing act this command performs, or None. + + Parses argv at command position, so a mention of "push" in a message, a + path, a grep pattern, or a file argument is silent by construction. A + dry run publishes nothing and is silent too. + """ + for argv in _command_heads(_words(command)): + base = os.path.basename(argv[0]) + rest = argv[1:] + if any(flag in DRY_RUN_FLAGS for flag in rest): + continue + heads = [item for item in rest if not item.startswith("-")] + if base == "git" and heads[:1] == ["push"]: + return "git push" + if base == "gh" and heads[:2] in (["pr", "create"], ["pr", "merge"], ["pr", "ready"]): + return f"gh {heads[0]} {heads[1]}" + if base == "gh" and heads[:1] == ["api"] and _writes_pull_request(rest): + return "gh api pull-request write" + if base == "mergify" and heads[:2] == ["stack", "push"]: + return "mergify stack push" + if base in ("node", "npx") and heads[:1]: + script = os.path.basename(heads[0]) + if script == "create-pr.mjs": + return "create-pr.mjs" + if script == "safe-stack-push.mjs" and "--execute" in rest: + return "safe-stack-push.mjs" + if base == "create-pr.mjs": + return "create-pr.mjs" + if base == "safe-stack-push.mjs" and "--execute" in rest: + return "safe-stack-push.mjs" + return None + + +def _writes_pull_request(rest: list[str]) -> bool: + joined = " ".join(rest) + mutating = any(flag in rest for flag in ("-X", "--method")) and re.search( + r"(?i)\b(POST|PATCH|PUT)\b", joined + ) + return bool(mutating and re.search(r"/pulls\b", joined)) + + +def _cache_read(now: float) -> str | None: + try: + with open(LIVENESS_CACHE_PATH, encoding="utf-8") as handle: + cached = json.load(handle) + except FileNotFoundError: + return _silent_state("no liveness cache yet") + except (PermissionError, json.JSONDecodeError, OSError) as exc: + _warn(f"liveness cache unreadable ({type(exc).__name__}: {exc}); probing again") + return None + stamp = cached.get("at") + state = cached.get("state") + if not isinstance(stamp, (int, float)) or state not in (LIVE, DOWN): + _warn("liveness cache malformed; probing again") + return None + if now - stamp > LIVENESS_TTL_SECONDS: + return None + return state + + +def _cache_write(state: str, now: float) -> None: + try: + with open(LIVENESS_CACHE_PATH, "w", encoding="utf-8") as handle: + json.dump({"state": state, "at": now}, handle) + except (PermissionError, OSError) as exc: + _warn(f"liveness cache not written ({type(exc).__name__}: {exc})") + + +def invoker_state(runner=None, now: float | None = None) -> tuple[str, str]: + """LIVE / DOWN / UNCHECKED, plus the reason when the probe could not run.""" + stamp = time.time() if now is None else now + if runner is None: + cached = _cache_read(stamp) + if cached is not None: + return cached, "" + run = runner or _probe + try: + code = run() + except FileNotFoundError: + return DOWN, "" + except (OSError, subprocess.SubprocessError) as exc: + return UNCHECKED, f"{type(exc).__name__}: {exc}" + if code is None: + return UNCHECKED, "liveness probe timed out" + state = LIVE if code == 0 else DOWN + if runner is None: + _cache_write(state, stamp) + return state, "" + + +def _probe() -> int | None: + try: + completed = subprocess.run( + [INVOKER_CLI, "query", "capacity", "--output", "json"], + capture_output=True, + timeout=LIVENESS_TIMEOUT_SECONDS, + ) + except subprocess.TimeoutExpired: + _warn("liveness probe timed out") + return None + return completed.returncode + + +def decide(payload: dict, runner=None) -> str | None: + """The refusal to print, or None to let the command run.""" + if not isinstance(payload, dict): + return _silent("payload is not an object") + if tool_name(payload) not in SHELL_LIKE_TOOL_NAMES: + return _silent("tool is not shell-like") + command = command_text(payload) + if not command.strip(): + return _silent("no command string in tool_input") + if not in_subagent(payload): + return _silent("caller is the main session, not a subagent") + act = publishing_act(command) + if act is None: + return _silent("command performs no publishing act") + state, reason = invoker_state(runner=runner) + if state == DOWN: + return _silent(f"no live Invoker owner; {act} may proceed here") + if state == UNCHECKED: + return ( + f"publish-act-guard: UNCHECKED: could not tell whether a live Invoker owner " + f"is reachable ({reason}); allowing {act}. Say so in the report." + ) + return BLOCK_MESSAGE.format(act=act, skill=ROUTING_SKILL) diff --git a/engine/hooks/agent-routing-guard/install_claude_hook.py b/engine/hooks/publish-act-guard/install_claude_hook.py old mode 100644 new mode 100755 similarity index 59% rename from engine/hooks/agent-routing-guard/install_claude_hook.py rename to engine/hooks/publish-act-guard/install_claude_hook.py index b72980c3..e0f3a18b --- a/engine/hooks/agent-routing-guard/install_claude_hook.py +++ b/engine/hooks/publish-act-guard/install_claude_hook.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 -"""Merge agent-routing-guard's Claude PreToolUse (Agent) hook into -~/.claude/settings.json without disturbing any other hook.""" +"""Merge publish-act-guard's Claude PreToolUse (Bash) hook into +~/.claude/settings.json, and drop the retired agent-routing-guard entry it +replaces, without disturbing any other hook.""" from __future__ import annotations import json @@ -8,19 +9,25 @@ HERE = os.path.dirname(os.path.abspath(__file__)) SETTINGS_PATH = os.path.expanduser("~/.claude/settings.json") -FRAGMENT_PATH = os.path.join(HERE, "claude.agent.hook.json") -MARKER = "agent-routing-guard/claude_pretooluse_agent.py" +FRAGMENT_PATH = os.path.join(HERE, "claude.hook.json") +MARKER = "publish-act-guard/claude_pretooluse.py" +RETIRED_MARKER = "agent-routing-guard/claude_pretooluse_agent.py" -def _is_ours(entry: dict) -> bool: - return any(MARKER in hook.get("command", "") for hook in entry.get("hooks", [])) +def _has_marker(entry: dict, marker: str) -> bool: + return any(marker in hook.get("command", "") for hook in entry.get("hooks", [])) def merge_hook(settings: dict, fragment: dict) -> tuple[dict, bool]: entry_list = settings.setdefault("hooks", {}).setdefault("PreToolUse", []) new_entries = fragment["hooks"]["PreToolUse"] before = json.dumps(entry_list, sort_keys=True) - entry_list[:] = [e for e in entry_list if not _is_ours(e)] + new_entries + kept = [ + entry + for entry in entry_list + if not _has_marker(entry, MARKER) and not _has_marker(entry, RETIRED_MARKER) + ] + entry_list[:] = kept + new_entries return settings, json.dumps(entry_list, sort_keys=True) != before @@ -35,14 +42,14 @@ def main() -> None: settings, changed = merge_hook(settings, fragment) if not changed: - print("ok claude PreToolUse agent-routing-guard already up to date") + print("ok claude PreToolUse publish-act-guard already up to date") return os.makedirs(os.path.dirname(SETTINGS_PATH), exist_ok=True) with open(SETTINGS_PATH, "w") as handle: json.dump(settings, handle, indent=2) handle.write("\n") - print("link claude PreToolUse agent-routing-guard merged") + print("link claude PreToolUse publish-act-guard merged") print(" (restart Claude Code to pick up the change)") diff --git a/engine/hooks/publish-act-guard/tests/test_detect.py b/engine/hooks/publish-act-guard/tests/test_detect.py new file mode 100644 index 00000000..2e1dd5e3 --- /dev/null +++ b/engine/hooks/publish-act-guard/tests/test_detect.py @@ -0,0 +1,123 @@ +import os +import sys +import unittest + +sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..")) + +import detect + + +def payload(command, *, subagent=True, tool="Bash"): + data = {"tool_name": tool, "tool_input": {"command": command}} + if subagent: + data["subagent_id"] = "a4b7b959ce73515c9" + return data + + +LIVE = lambda: 0 +DOWN = lambda: 1 +TIMEOUT = lambda: None + + +class PublishingActs(unittest.TestCase): + def test_real_session_commands_are_acts(self): + cases = { + "git push -q -u origin HEAD": "git push", + "git push --force-with-lease origin 7fbbd80:refs/heads/experiment/wf-1/repair": "git push", + "gh pr create -R EdbertChan/catstack --fill": "gh pr create", + "gh pr merge 439 -R EdbertChan/catstack --squash --match-head-commit 70b29a7": "gh pr merge", + "mergify stack push": "mergify stack push", + "node scripts/create-pr.mjs --title x --base master --body-file /tmp/b.md": "create-pr.mjs", + "node scripts/safe-stack-push.mjs --execute": "safe-stack-push.mjs", + "gh api -X POST repos/o/r/pulls -f title=x": "gh api pull-request write", + } + for command, expected in cases.items(): + with self.subTest(command=command): + self.assertEqual(detect.publishing_act(command), expected) + + def test_near_neighbours_stay_silent(self): + for command in ( + "git push --dry-run origin HEAD", + "grep -rn 'git push' scripts/", + "echo 'remember to gh pr create later'", + "gh pr view 12143 -R Neko-Catpital-Labs/Invoker --json state", + "gh pr checks 12143 -R Neko-Catpital-Labs/Invoker", + "gh api repos/o/r/pulls/12143 --jq .title", + "git log --oneline origin/master", + "cat scripts/safe-stack-push.mjs", + ): + with self.subTest(command=command): + self.assertIsNone(detect.publishing_act(command)) + + def test_dry_run_push_is_not_an_act(self): + self.assertIsNone(detect.publishing_act("git push --dry-run origin HEAD")) + + +class Decide(unittest.TestCase): + def test_blocks_subagent_publish_when_owner_is_live(self): + refusal = detect.decide(payload("git push -u origin HEAD"), runner=LIVE) + self.assertIsNotNone(refusal) + self.assertIn("git push", refusal) + self.assertIn("publish-act-guard", refusal) + + def test_allows_when_no_live_owner(self): + self.assertIsNone(detect.decide(payload("git push -u origin HEAD"), runner=DOWN)) + + def test_allows_main_session(self): + self.assertIsNone( + detect.decide(payload("git push -u origin HEAD", subagent=False), runner=LIVE) + ) + + def test_allows_non_publishing_command(self): + self.assertIsNone(detect.decide(payload("pnpm run check:types"), runner=LIVE)) + + def test_allows_non_shell_tool(self): + self.assertIsNone( + detect.decide(payload("git push -u origin HEAD", tool="Write"), runner=LIVE) + ) + + def test_unreadable_liveness_reports_and_allows(self): + refusal = detect.decide(payload("mergify stack push"), runner=TIMEOUT) + self.assertIsNotNone(refusal) + self.assertIn("UNCHECKED", refusal) + self.assertIn("mergify stack push", refusal) + + def test_prompt_wording_cannot_trigger_or_clear_the_gate(self): + wording = payload("echo 'this subagent is carrying commits and will push a PR'") + self.assertIsNone(detect.decide(wording, runner=LIVE)) + real = payload("git push -u origin HEAD") + self.assertIsNotNone(detect.decide(real, runner=LIVE)) + + +class LivenessStates(unittest.TestCase): + def setUp(self): + self.cache = detect.LIVENESS_CACHE_PATH + detect.LIVENESS_CACHE_PATH = self.cache + ".test" + if os.path.exists(detect.LIVENESS_CACHE_PATH): + os.unlink(detect.LIVENESS_CACHE_PATH) + + def tearDown(self): + if os.path.exists(detect.LIVENESS_CACHE_PATH): + os.unlink(detect.LIVENESS_CACHE_PATH) + detect.LIVENESS_CACHE_PATH = self.cache + + def test_three_outcomes(self): + self.assertEqual(detect.invoker_state(runner=LIVE)[0], detect.LIVE) + self.assertEqual(detect.invoker_state(runner=DOWN)[0], detect.DOWN) + state, reason = detect.invoker_state(runner=TIMEOUT, now=0.0) + self.assertEqual(state, detect.UNCHECKED) + self.assertTrue(reason) + + def test_injected_probe_never_reads_or_writes_the_cache(self): + detect.invoker_state(runner=LIVE) + self.assertFalse(os.path.exists(detect.LIVENESS_CACHE_PATH)) + + def test_missing_cli_reads_as_down(self): + def missing(): + raise FileNotFoundError("invoker-cli") + + self.assertEqual(detect.invoker_state(runner=missing)[0], detect.DOWN) + + +if __name__ == "__main__": + unittest.main() diff --git a/install.sh b/install.sh index 146dddcd..d6d383bc 100755 --- a/install.sh +++ b/install.sh @@ -256,7 +256,7 @@ link_item "scratchpad-collision" "$REPO_DIR/engine/hooks/scratchpad-collision" " link_item "ui-input-guard" "$REPO_DIR/engine/hooks/ui-input-guard" "$HOME/.claude/hooks/ui-input-guard" link_item "hook-freshness" "$REPO_DIR/engine/hooks/hook-freshness" "$HOME/.claude/hooks/hook-freshness" link_item "gh-write-verification" "$REPO_DIR/engine/hooks/gh-write-verification" "$HOME/.claude/hooks/gh-write-verification" -link_item "agent-routing-guard" "$REPO_DIR/engine/hooks/agent-routing-guard" "$HOME/.claude/hooks/agent-routing-guard" +link_item "publish-act-guard" "$REPO_DIR/engine/hooks/publish-act-guard" "$HOME/.claude/hooks/publish-act-guard" link_item "categorical-scope-guard" "$REPO_DIR/engine/hooks/categorical-scope-guard" "$HOME/.claude/hooks/categorical-scope-guard" echo "--- git pre-push hooks (init.templateDir and this clone) ---" @@ -361,7 +361,7 @@ python3 "$REPO_DIR/engine/hooks/playbook-router/install_claude_hook.py" python3 "$REPO_DIR/engine/hooks/restated-constraint/install_claude_hook.py" python3 "$REPO_DIR/engine/hooks/named-verb-guard/install_claude_hook.py" python3 "$REPO_DIR/engine/hooks/gh-write-verification/install_claude_hook.py" -python3 "$REPO_DIR/engine/hooks/agent-routing-guard/install_claude_hook.py" +python3 "$REPO_DIR/engine/hooks/publish-act-guard/install_claude_hook.py" python3 "$REPO_DIR/engine/hooks/categorical-scope-guard/install_claude_hook.py" echo "--- subagent-inheritance: every Stop hook above also fires on SubagentStop; a manifest opts out with subagent_stop.inherit=false + reason ---" diff --git a/tests/test_install.py b/tests/test_install.py index 279ae205..32bb2b08 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -240,18 +240,30 @@ def test_external_claim_gate_linked_and_bash_pretooluse_wired_for_claude(self): ) self.assertEqual(result.returncode, expected, result.stderr) - def test_agent_routing_guard_linked_and_agent_pretooluse_wired_for_claude(self): - target = os.path.join(self.fake_home, ".claude", "hooks", "agent-routing-guard") + def test_publish_act_guard_linked_and_bash_pretooluse_wired_for_claude(self): + target = os.path.join(self.fake_home, ".claude", "hooks", "publish-act-guard") self.assertTrue(os.path.islink(target), target) - self.assertEqual(os.readlink(target), hook_src("agent-routing-guard")) + self.assertEqual(os.readlink(target), hook_src("publish-act-guard")) with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: settings = json.load(handle) entries = [ entry for entry in settings["hooks"]["PreToolUse"] - if any("agent-routing-guard/claude_pretooluse_agent.py" in hook["command"] for hook in entry["hooks"]) + if any("publish-act-guard/claude_pretooluse.py" in hook["command"] for hook in entry["hooks"]) ] self.assertEqual(len(entries), 1, entries) - self.assertEqual(entries[0]["matcher"], "Agent") + self.assertEqual(entries[0]["matcher"], "Bash") + + def test_retired_agent_routing_guard_is_gone(self): + self.assertFalse( + os.path.exists(os.path.join(self.fake_home, ".claude", "hooks", "agent-routing-guard")) + ) + with open(os.path.join(self.fake_home, ".claude", "settings.json")) as handle: + settings = json.load(handle) + stale = [ + entry for entry in settings["hooks"]["PreToolUse"] + if any("agent-routing-guard" in hook["command"] for hook in entry["hooks"]) + ] + self.assertEqual(stale, [], stale) def test_categorical_scope_guard_linked_and_bash_pretooluse_wired_for_claude(self): target = os.path.join(self.fake_home, ".claude", "hooks", "categorical-scope-guard")