merge queue: checking #840 on main (7978864)

engine/hooks/pr-schema-gate/README.md

@@ -1,10 +1,23 @@
 # pr-schema-gate
 
-Keeps PR text in the repo's own style without blocking anything. In any repo
-that has `scripts/create-pr.mjs`, when a shell tool call writes PR text
-directly, the hook checks that text with the repo's own
-`scripts/validate-pr-body.mjs` and tells the agent the result. The command
-always runs.
+Keeps PR text in the repo's own style without blocking anything. When a
+shell tool call writes PR text directly, the hook checks that text with the
+repo's own validator and tells the agent the result. The command always runs.
+
+## Which repos are in scope
+
+The repo is the directory holding `.git` (a directory, or a worktree's
+`.git` file), found by walking up from the command's working directory. Its
+validator is the first of these that exists:
+
+1. `scripts/validate-pr-body.mjs` (Invoker)
+2. `engine/skills/draft-pr/scripts/validate-pr-body.mjs` (catstack)
+
+A repo with either is fully in scope. A PR-publishing command (`gh pr create`,
+`gh pr edit` with a body, `gh api` on `pulls` with a body, `mergify stack
+push`) in a git repo with neither reports UNCHECKED, naming both paths it
+looked for. It is never silent. Outside any git repo the hook says nothing.
+`scripts/create-pr.mjs` plays no part in scope.
 
 ## What counts as a direct PR text write
 
@@ -31,11 +44,18 @@ Three outcomes, never two:
 |---|---|---|
 | clean | the validator exits 0 | nothing |
 | failed | the validator exits 1 | `pr-schema-gate: the PR text in <file> does not follow this repo's PR style ... The command is not blocked.` plus the validator's error lines (up to 20) |
-| unchecked | inline or piped text, a missing or unreadable file, no validator, `node` missing, a crash (any other exit code), a timeout (3s), or a command the parser cannot read | `pr-schema-gate: could not check this PR text against the repo's PR style: <reason>. The command is not blocked.` |
+| unchecked | inline or piped text, a missing or unreadable file, no validator at either path, `node` missing, a crash (any other exit code), a timeout (3s), an exit 0 that states no verdict and prints UNCHECKED/SKIPPED/not-installed, or a command the parser cannot read | `pr-schema-gate: could not check this PR text against the repo's PR style: <reason>. The command is not blocked.` |
 
 An unchecked write is never reported as clean. The rules live only in the
 repo's validator, so this hook carries no copy of them to drift.
 
+A run that prints `PR body validation passed.` has judged the body, so it is
+clean even when the same run names a sub-check it skipped. The catstack
+validator says `Summary reading grade unchecked: ...` for a Summary too short
+to grade and still accepts the body; reading that note as a vacuous pass told
+the agent an accepted body was unchecked and left an owed stack follow-up
+armed.
+
 Claude Code gets the message as `additionalContext` on its `Bash` tool.
 Every harness also gets it on stderr. Whether Cursor and Codex show a
 stderr line from an exit-0 `preToolUse` hook to the agent is unverified.
@@ -45,6 +65,8 @@ stderr line from an exit-0 `preToolUse` hook to the agent is unverified.
 `mergify stack push` publishes PRs with a bare body. The push is told which
 follow-up is owed (`node scripts/create-pr.mjs ... --update-existing`, or a
 direct body write whose file passes the validator) and arms a pending flag.
+In a repo with no validator the push reports UNCHECKED instead and arms
+nothing.
 A later push while the flag is armed repeats the reminder. Either follow-up
 clears it; an unchecked or failing direct write does not.
 
@@ -68,7 +90,9 @@ The hook never blocks, so every failure fails open, and says so:
   with no local checkout under
   `PR_SCHEMA_GATE_CHECKOUTS_ROOT` (default `~/Documents/GitHub`): out of
   scope;
-- a repo with no `scripts/create-pr.mjs`: out of scope.
+- a directory outside any git repo: out of scope;
+- an unparseable command in a repo with no validator: silent, since it may
+  not be a PR write at all.
 
 There is no escape hatch because there is nothing to escape.
 

engine/hooks/pr-schema-gate/claude_pretooluse.py

@@ -16,15 +16,18 @@
 
 from detect import (  # noqa: E402
     UNPARSEABLE_MESSAGE,
+    VALIDATOR_RELATIVE_PATHS,
     check_body_file,
     classify_pr_text_write,
     clear_pending,
+    find_validator,
     followup_message,
     is_create_pr_followup,
     is_stack_push,
     mark_pending,
     read_pending,
     scope_root,
+    stack_push_unchecked_message,
     style_message,
 )
 from shell_model import parse_commands, shell_call_from_tool_input  # noqa: E402
@@ -66,7 +69,8 @@ def evaluate(payload: dict) -> list[str]:
     commands = parse_commands(call, session_cwd)
     if commands is None:
         base = call.workdir or session_cwd
-        return [UNPARSEABLE_MESSAGE] if scope_root(base, None) else []
+        root = scope_root(base, None)
+        return [UNPARSEABLE_MESSAGE] if root and find_validator(root) else []
 
     messages: list[str] = []
     for command in commands:
@@ -83,7 +87,8 @@ def evaluate(payload: dict) -> list[str]:
                 outcome, detail = check_body_file(root, write.body_file, write.cwd)
             if outcome == "clean":
                 clear_pending(root)
-            message = style_message(outcome, detail, write.body_file)
+            message = style_message(outcome, detail, write.body_file,
+                                    find_validator(root) or VALIDATOR_RELATIVE_PATHS[0])
             if message:
                 messages.append(message)
             continue
@@ -93,7 +98,11 @@ def evaluate(payload: dict) -> list[str]:
         if is_create_pr_followup(command):
             clear_pending(root)
         elif is_stack_push(command):
-            messages.append(followup_message(read_pending(root) is not None))
+            validator = find_validator(root)
+            if validator is None:
+                messages.append(stack_push_unchecked_message())
+                continue
+            messages.append(followup_message(read_pending(root) is not None, validator))
             mark_pending(root)
     return messages
 

engine/hooks/pr-schema-gate/detect.py

@@ -4,7 +4,7 @@
 command the shell will run), not on the raw payload text. A direct write is
 `gh pr create`, `gh pr edit` with a body flag, or `gh api` on a `pulls`
 endpoint with a `body=` field. When the text comes from a file, the hook
-runs the repo's own `scripts/validate-pr-body.mjs` on that file and hands the
+runs the repo's own validator on that file and hands the
 result to the agent: silent when the text passes, the validator's error
 lines when it fails, and an explicit "could not check" with the reason when
 the check cannot run (inline text, a missing or unreadable file, no
@@ -16,8 +16,14 @@
 lands when a follow-up writes it. The push arms a bounded pending flag and
 reminds the agent which follow-up is owed; a later push while the flag is
 armed repeats the reminder. `scripts/create-pr.mjs`, or a direct body write
-whose file passes the validator, clears it. A repo with no
-scripts/create-pr.mjs is out of scope entirely.
+whose file passes the validator, clears it.
+
+The repo is the one the `.git` boundary marks. Its validator is the first of
+VALIDATOR_RELATIVE_PATHS that exists: `scripts/validate-pr-body.mjs`
+(Invoker), then `engine/skills/draft-pr/scripts/validate-pr-body.mjs`
+(catstack). A PR-publishing command in a repo with neither is reported as
+unchecked, never passed over in silence. Outside any git repo the hook says
+nothing.
 
 PreToolUse fires before the command, so the hook cannot see the push's exit
 status; pending is recorded when the push is let through. A push that then
@@ -47,11 +53,15 @@
 
 from shell_model import Command
 
-VALIDATOR_RELATIVE_PATH = os.path.join("scripts", "validate-pr-body.mjs")
+VALIDATOR_RELATIVE_PATHS = (
+    "scripts/validate-pr-body.mjs",
+    "engine/skills/draft-pr/scripts/validate-pr-body.mjs",
+)
 VALIDATOR_TIMEOUT_SECONDS = 3.0
 VALIDATOR_OUTPUT_MAX_LINES = 20
 VACUOUS_PASS_RE = re.compile(
     r"\bUNCHECKED\b|\bSKIPPED?\b|\bnot installed\b|\bno rules loaded\b", re.IGNORECASE)
+VALIDATOR_PASS_VERDICT_RE = re.compile(r"\bPR body validation passed\b", re.IGNORECASE)
 
 PENDING_TTL_SECONDS = 2 * 60 * 60
 STATE_DIR_ENV = "PR_SCHEMA_GATE_STATE_DIR"
@@ -61,13 +71,13 @@
 
 STYLE_FAILED_MESSAGE = (
     "pr-schema-gate: the PR text in {path} does not follow this repo's PR style "
-    "(scripts/validate-pr-body.mjs exited 1). The command is not blocked. Fix the "
+    "({validator} exited 1). The command is not blocked. Fix the "
     "file and write it to the PR again so the live PR matches:\n{details}"
 )
 STYLE_UNCHECKED_MESSAGE = (
     "pr-schema-gate: could not check this PR text against the repo's PR style: "
     "{reason}. The command is not blocked. Check it yourself with "
-    "`node scripts/validate-pr-body.mjs --body-file <file>`, or write it with "
+    "`node {validator} --body-file <file>`, or write it with "
     "`node scripts/create-pr.mjs`, which checks before writing."
 )
 UNPARSEABLE_MESSAGE = (
@@ -78,13 +88,13 @@
     "pr-schema-gate: '{cmd}' publishes PRs with a bare body. Follow up on each "
     "PR with `node scripts/create-pr.mjs --title \"...\" --base <branch> "
     "--body-file <file> --update-existing`, or a direct body write whose file "
-    "passes scripts/validate-pr-body.mjs. Either one clears this reminder."
+    "passes {validator}. Either one clears this reminder."
 )
 FOLLOWUP_STILL_OWED_MESSAGE = (
     "pr-schema-gate: the follow-up for the last '{cmd}' in this repository has "
     "not run yet, so those PRs may still have a bare body. The command is not "
     "blocked. Run `node scripts/create-pr.mjs ... --update-existing`, or a "
-    "direct body write whose file passes scripts/validate-pr-body.mjs."
+    "direct body write whose file passes {validator}."
 )
 
 GH_BODY_FILE_FLAGS = frozenset({"--body-file", "-F"})
@@ -238,34 +248,40 @@ def sibling_repo_dir(repo_spec: str) -> str | None:
     return candidate if os.path.isdir(candidate) else None
 
 
-def repo_root_with_create_pr_tool(start_dir: str) -> str | None:
-    """Walk up from start_dir; return the dir containing scripts/create-pr.mjs, or None.
-
-    Stops at a .git boundary (repo root) or filesystem root, whichever comes first.
-    """
+def git_root(start_dir: str) -> str | None:
+    """Walk up from start_dir to the dir holding `.git` (a directory, or a worktree's file), or None."""
     cur = os.path.abspath(start_dir) if start_dir else os.getcwd()
-    for _ in range(12):
-        if os.path.isfile(os.path.join(cur, "scripts", "create-pr.mjs")):
+    while True:
+        if os.path.exists(os.path.join(cur, ".git")):
             return cur
-        if os.path.isdir(os.path.join(cur, ".git")):
-            return None
         parent = os.path.dirname(cur)
         if parent == cur:
             return None
         cur = parent
+
+
+def find_validator(repo_root: str) -> str | None:
+    """The first of VALIDATOR_RELATIVE_PATHS present in repo_root, as that relative path, or None."""
+    for relative in VALIDATOR_RELATIVE_PATHS:
+        if os.path.isfile(os.path.join(repo_root, relative)):
+            return relative
     return None
 
 
+def no_validator_reason() -> str:
+    return "this repo has no PR validator (looked for " + " and ".join(VALIDATOR_RELATIVE_PATHS) + ")"
+
+
 def scope_root(cwd: str, repo_spec: str | None) -> str | None:
-    """The repo this command acts on, when that repo has scripts/create-pr.mjs.
+    """The git repo this command acts on.
 
     A `--repo` naming a repo with no local checkout resolves to None: out of
     scope, never a guess.
     """
     if repo_spec is not None:
         sibling = sibling_repo_dir(repo_spec)
-        return repo_root_with_create_pr_tool(sibling) if sibling else None
-    return repo_root_with_create_pr_tool(cwd)
+        return git_root(sibling) if sibling else None
+    return git_root(cwd)
 
 
 def check_body_file(repo_root: str, body_path: str | None, start_dir: str) -> tuple[str, str]:
@@ -274,6 +290,19 @@ def check_body_file(repo_root: str, body_path: str | None, start_dir: str) -> tu
     Three outcomes: "clean" (validator exit 0), "failed" (exit 1, detail is
     its error lines) and "unchecked" (the check could not run, detail is why).
     "unchecked" is never reported as clean.
+
+    Exit 0 is vacuous, and so unchecked, when the run states no verdict on the
+    body and prints an UNCHECKED/SKIPPED/not-installed line: the validator
+    never judged the text. Exit 0 that states VALIDATOR_PASS_VERDICT_RE is a
+    real pass even when the same run names a sub-check it skipped, which the
+    catstack validator does for a Summary too short to grade. Only a pass
+    verdict lifts the vacuous reading, so a "failed" banner beside exit 0
+    stays unchecked. Reading a skipped sub-check as a vacuous pass told the
+    agent a body the validator had accepted was unchecked, and left an owed
+    stack follow-up armed.
+
+    The whole output is scanned, not the first VALIDATOR_OUTPUT_MAX_LINES:
+    truncation shortens what the agent is shown, never what is judged.
     """
     if body_path is None:
         return "unchecked", "the PR text is inline or piped, not in a file the hook can read"
@@ -285,9 +314,10 @@ def check_body_file(repo_root: str, body_path: str | None, start_dir: str) -> tu
             fh.read(1)
     except (OSError, UnicodeDecodeError) as exc:
         return "unchecked", f"body file unreadable: {path}: {exc}"
-    validator = os.path.join(repo_root, VALIDATOR_RELATIVE_PATH)
-    if not os.path.isfile(validator):
-        return "unchecked", f"this repo has no {VALIDATOR_RELATIVE_PATH}"
+    relative = find_validator(repo_root)
+    if relative is None:
+        return "unchecked", no_validator_reason()
+    validator = os.path.join(repo_root, relative)
     try:
         proc = subprocess.run(
             ["node", validator, "--body-file", path],
@@ -300,23 +330,25 @@ def check_body_file(repo_root: str, body_path: str | None, start_dir: str) -> tu
         return "unchecked", "node is not on PATH, so the validator could not run"
     except subprocess.TimeoutExpired:
         return "unchecked", f"the validator timed out after {VALIDATOR_TIMEOUT_SECONDS:g}s"
-    lines = [line for line in (proc.stdout + "\n" + proc.stderr).splitlines() if line.strip()]
-    lines = lines[:VALIDATOR_OUTPUT_MAX_LINES]
+    output = [line for line in (proc.stdout + "\n" + proc.stderr).splitlines() if line.strip()]
+    lines = output[:VALIDATOR_OUTPUT_MAX_LINES]
     if proc.returncode == 0:
-        for line in lines:
-            if VACUOUS_PASS_RE.search(line):
-                return "unchecked", f"the validator exited 0 without checking: {line.strip()}"
+        if not any(VALIDATOR_PASS_VERDICT_RE.search(line) for line in output):
+            for line in output:
+                if VACUOUS_PASS_RE.search(line):
+                    return "unchecked", f"the validator exited 0 without checking: {line.strip()}"
         return "clean", ""
     if proc.returncode == 1:
         return "failed", "\n".join(lines)
     return "unchecked", f"the validator crashed (exit {proc.returncode}): " + " | ".join(lines[:3])
 
 
-def style_message(outcome: str, detail: str, body_path: str | None) -> str | None:
+def style_message(outcome: str, detail: str, body_path: str | None,
+                  validator: str = VALIDATOR_RELATIVE_PATHS[0]) -> str | None:
     if outcome == "failed":
-        return STYLE_FAILED_MESSAGE.format(path=body_path, details=detail)
+        return STYLE_FAILED_MESSAGE.format(path=body_path, details=detail, validator=validator)
     if outcome == "unchecked":
-        return STYLE_UNCHECKED_MESSAGE.format(reason=detail)
+        return STYLE_UNCHECKED_MESSAGE.format(reason=detail, validator=validator)
     return None
 
 
@@ -385,6 +417,13 @@ def clear_pending(repo_root: str) -> None:
         sys.stderr.write(f"pr-schema-gate: could not clear pending state at {path}: {exc}\n")
 
 
-def followup_message(already_owed: bool) -> str:
+def followup_message(already_owed: bool, validator: str = VALIDATOR_RELATIVE_PATHS[0]) -> str:
     template = FOLLOWUP_STILL_OWED_MESSAGE if already_owed else FOLLOWUP_OWED_MESSAGE
-    return template.format(cmd=STACK_PUSH_LABEL)
+    return template.format(cmd=STACK_PUSH_LABEL, validator=validator)
+
+
+def stack_push_unchecked_message() -> str:
+    return STYLE_UNCHECKED_MESSAGE.format(
+        reason=f"'{STACK_PUSH_LABEL}' publishes PRs and {no_validator_reason()}",
+        validator=VALIDATOR_RELATIVE_PATHS[0],
+    )