diff --git a/prompts/_shared/automation-rules.md b/prompts/_shared/automation-rules.md new file mode 100644 index 0000000..573d7a1 --- /dev/null +++ b/prompts/_shared/automation-rules.md @@ -0,0 +1,26 @@ +When the diff adds or changes CI configuration, a GitHub Actions workflow, a scheduled +job, or any other unattended automation, review it as production code whose only user is +a log nobody reads. Flag each genuine problem with a file/line reference: +- Every external call must have its failure surfaced. A non-2xx response or a failed + request that is swallowed into an empty string or an ignored exit status turns a broken + job into a green one. `curl -sf ... || true`, an unchecked exit status before a success + log line, and a pipeline without `set -o pipefail` are all this defect. Require that the + status be captured and either fail the step or emit a `::warning::`/`::error::` + annotation naming what could not be reached. +- Check that the token scopes and permissions the job declares actually cover the API + calls it makes, against the endpoint's documented requirements rather than against a + sibling workflow. A too-narrow scope does not fail loudly; it returns 403 and the script + proceeds down whatever branch an empty response selects. GitHub Actions in particular: + a `permissions:` block replaces the default set rather than adding to it, and reading or + writing comments on a pull request needs `pull-requests` even though the endpoint path + says `issues`. +- A guard that fails closed must be able to tell "nothing to do" from "could not tell". A + conservative default is right, but if the safe branch is also what an outage, a + permission error, or an empty response selects, then the automation can no-op forever + while every run reports success. Require that the two cases log differently and that the + error case is visible without reading the log line by line. +- Judge whether the job's success signal means anything. A step that exits 0 on the path + where it did nothing, or that prints a success message before verifying the action took + effect, gives no way to notice the feature is dead. Say what the job should assert, or + what it should count, so that a run which accomplished nothing is distinguishable from a + run with nothing to accomplish. diff --git a/prompts/_shared/verification-rules.md b/prompts/_shared/verification-rules.md new file mode 100644 index 0000000..5be184b --- /dev/null +++ b/prompts/_shared/verification-rules.md @@ -0,0 +1,22 @@ +These rules govern how you report what you checked and how you retire a finding. They +exist because a review that states a wrong conclusion confidently is worse than one that +says nothing: it closes the question for every later reader. +- Do not write that something is correct, valid, sufficient, or safe unless you ran a + check that could have come back the other way. Confirming that a value is *present* is + not confirming that it is *right* — say "declares X" rather than "correctly declares X" + when presence is all you established. When the diff declares a token scope, permission, + credential, environment variable, version constraint, or config key, correctness means + checking it against the thing that consumes it: the API's own documented requirements, + the tool's schema, the reading code. A neighbouring file that declares something similar + is not that check. +- Matching a pattern that already exists in the repository is evidence about whether a + finding is a *regression*, never evidence about whether it is a *bug*. In a file the diff + adds there is no prior behavior to preserve, so parity cannot retire a finding — at most + it widens it. Write "the same problem exists in X", not "therefore it is not a problem", + and let the verdict rest on the impact, not the precedent. +- When you retire a finding — yours, Codex's, or one raised earlier in the thread — name + the exact call site and state the conditions under which your reasoning holds. Reasoning + that a failure is self-healing, tolerable, or cosmetic is a claim about one specific call + under specific conditions, not about a construct. If the same construct appears elsewhere + in the diff, judge each occurrence on its own; a dismissal that silently generalizes is + how a real defect ships with an approval attached. diff --git a/prompts/claude-synthesize-thesis-first.md b/prompts/claude-synthesize-thesis-first.md index 6f0b71f..fa845d7 100644 --- a/prompts/claude-synthesize-thesis-first.md +++ b/prompts/claude-synthesize-thesis-first.md @@ -34,11 +34,16 @@ Synthesize a single review decision for pull request #{{PR}}. 8. Check that in-app navigational links use React Router's Link rather than a raw `` tag, per these rules: {{@prompts/_shared/navigation-rules.md}} -9. Validate which of Codex's findings are real (discard false positives), add any genuine +9. When the diff touches CI config, a GitHub Actions workflow, or any unattended job, + review it per these rules: + {{@prompts/_shared/automation-rules.md}} +10. Validate which of Codex's findings are real (discard false positives), add any genuine issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's intent or clear scope creep — but give credit when the author went beyond the literal - acceptance criteria in a sound way rather than flagging it as non-compliant. -10. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one + acceptance criteria in a sound way rather than flagging it as non-compliant. Hold every + claim you make and every finding you retire to these rules: + {{@prompts/_shared/verification-rules.md}} +11. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one genuine, blocking issue; otherwise "approve". A change that exceeds the AC without breaking the ticket's intent is a reason to approve, not to block. diff --git a/prompts/claude-synthesize.md b/prompts/claude-synthesize.md index 32738b6..5703bac 100644 --- a/prompts/claude-synthesize.md +++ b/prompts/claude-synthesize.md @@ -34,11 +34,16 @@ Synthesize a single review decision for pull request #{{PR}}. 8. Check that in-app navigational links use React Router's Link rather than a raw `` tag, per these rules: {{@prompts/_shared/navigation-rules.md}} -9. Validate which of Codex's findings are real (discard false positives), add any genuine +9. When the diff touches CI config, a GitHub Actions workflow, or any unattended job, + review it per these rules: + {{@prompts/_shared/automation-rules.md}} +10. Validate which of Codex's findings are real (discard false positives), add any genuine issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's intent or clear scope creep — but give credit when the author went beyond the literal - acceptance criteria in a sound way rather than flagging it as non-compliant. -10. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one + acceptance criteria in a sound way rather than flagging it as non-compliant. Hold every + claim you make and every finding you retire to these rules: + {{@prompts/_shared/verification-rules.md}} +11. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one genuine, blocking issue; otherwise "approve". A change that exceeds the AC without breaking the ticket's intent is a reason to approve, not to block. diff --git a/prompts/codex-first-pass.md b/prompts/codex-first-pass.md index b892914..1e7f98b 100644 --- a/prompts/codex-first-pass.md +++ b/prompts/codex-first-pass.md @@ -28,7 +28,10 @@ verbatim and handed to a second reviewer, so do not add conversational preamble. 7. Check that in-app navigational links use React Router's Link rather than a raw `` tag, per these rules: {{@prompts/_shared/navigation-rules.md}} -8. Report concrete issues — bugs, regressions, security problems, member-privacy +8. When the diff touches CI config, a GitHub Actions workflow, or any unattended job, + review it per these rules: + {{@prompts/_shared/automation-rules.md}} +9. Report concrete issues — bugs, regressions, security problems, member-privacy violations, and genuine misses of the ticket's intent or clear scope creep — each with a file/line reference and a brief rationale. Do not list "doesn't match acceptance criteria" as an issue by itself; only raise it when the deviation harms the intent. If nothing is