diff --git a/prompts/_shared/interface-contract-rules.md b/prompts/_shared/interface-contract-rules.md new file mode 100644 index 0000000..44d84c4 --- /dev/null +++ b/prompts/_shared/interface-contract-rules.md @@ -0,0 +1,43 @@ +A change that publishes an interface — an MCP tool's schema and description, an +endpoint's params, a serializer's payload, a webhook contract — is read by a caller who +cannot see the implementation and can only do what the interface tells them. Check the +interface against the implementation, not only the implementation against itself. + +1. Every required input must be obtainable through the interface itself: + - Grep for the values the implementation reads out of the incoming payload, and check + that the interface's own schema, description, or documented requirements name each + one. An input the implementation requires but the interface never names makes the + documented path fail every time it is followed, and the caller has no way to + discover what is missing. + - Pay particular attention to values the existing UI supplies out of band: a hidden + field, a landing-page widget, a session value, an id resolved by an earlier page. + Those are exactly the ones a new non-browser caller cannot produce, and the ones an + author working from the browser flow is least likely to notice are missing. + - When one operation tells callers to take an input from another ("exactly as X + enumerates them", "pass the id returned by Y"), open that other operation and + confirm it actually produces it. A cross-reference to something that does not exist + reads as complete and is not. + +2. Failing on a missing or unusable input must name the input: + - An interface whose only answer to a missing required value is a generic error, a + 500, or a rescued "something went wrong" leaves the caller nothing to correct. Ask + that the error name the value, and where one exists, how to obtain it. + +3. Values that cross a serialization boundary must be read the way they arrive: + - Where a payload is parsed from JSON, read back from a jsonb column, pulled off a + queue, or handed over by an SDK, check the key type on both sides. A hash written + with symbol keys and read with string keys (or the reverse) does not raise — the + read returns nil, the value silently reads as absent, and the code takes its + "nothing was supplied" branch on a fully supplied payload. + - Tests that build the payload themselves cannot catch this, because the test picks + the key type the implementation already expects. When a diff adds or changes a + handler for an externally supplied payload, look for a test that drives it through + the real boundary — a request spec posting real JSON, not a direct call to the + handler with a hand-built hash — and flag its absence as a genuine gap rather than a + style preference. + +4. A filter that silently drops unrecognized entries hides both kinds of mistake: + - `select` / `reject` / `filter_map` against a list of known names drops a newly added + name as quietly as a bad one, so an omission looks identical to a deliberate + exclusion. When a diff adds one or depends on one, check what it drops today rather + than what the comment beside it says it drops. diff --git a/prompts/claude-synthesize-thesis-first.md b/prompts/claude-synthesize-thesis-first.md index ed4557a..57ea983 100644 --- a/prompts/claude-synthesize-thesis-first.md +++ b/prompts/claude-synthesize-thesis-first.md @@ -37,11 +37,14 @@ Synthesize a single review decision for pull request #{{PR}}. 9. Check that in-app navigational links use React Router's Link rather than a raw `` tag, per these rules: {{@prompts/_shared/navigation-rules.md}} -10. Validate which of Codex's findings are real (discard false positives), add any genuine +10. When the diff publishes or changes an interface an outside caller drives, check it + against these rules: + {{@prompts/_shared/interface-contract-rules.md}} +11. 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. -11. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one +12. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task, placeholder, or deferred work); 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 14ea29e..ddde264 100644 --- a/prompts/claude-synthesize.md +++ b/prompts/claude-synthesize.md @@ -37,11 +37,14 @@ Synthesize a single review decision for pull request #{{PR}}. 9. Check that in-app navigational links use React Router's Link rather than a raw `` tag, per these rules: {{@prompts/_shared/navigation-rules.md}} -10. Validate which of Codex's findings are real (discard false positives), add any genuine +10. When the diff publishes or changes an interface an outside caller drives, check it + against these rules: + {{@prompts/_shared/interface-contract-rules.md}} +11. 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. -11. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one +12. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task, placeholder, or deferred work); 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 a490f53..2f72fd9 100644 --- a/prompts/codex-first-pass.md +++ b/prompts/codex-first-pass.md @@ -31,7 +31,10 @@ verbatim and handed to a second reviewer, so do not add conversational preamble. 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. Report concrete issues — bugs, regressions, security problems, member-privacy +9. When the diff publishes or changes an interface an outside caller drives, check it + against these rules: + {{@prompts/_shared/interface-contract-rules.md}} +10. Report concrete issues — bugs, regressions, security problems, member-privacy violations, incomplete tasks/half-measures/placeholders/deferred work, 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