[no-ticket] Add verification and automation review rules - #35
Open
bockets wants to merge 1 commit into
Open
Conversation
A review of BiggerPockets/biggerpockets#30384 approved a scheduled job that could never do its work, and stated the defect was correct: the job declared issues: read, the /cwa check reads comments on a PR-backed issue, that endpoint also needs pull-requests: read, and the summary recorded the scope as correctly declared. The finding that would have caught it — a swallowed curl -sf failure — was retired as parity with sibling workflows and as self-healing, reasoning that held for the DELETE call it was written about and not for the comments call it was applied to. verification-rules.md governs claims and dismissals: no affirmative correctness claim without a check that could have failed, parity is evidence about regression rather than about correctness, and a retired finding names its call site instead of generalizing. automation-rules.md gives CI and unattended-job diffs the domain block the other four shared rules already give app code: surfaced failures on external calls, declared scopes checked against the endpoint, a fail-closed guard that separates nothing-to-do from could-not-tell, and a success signal that means something. Both Claude arms and the Codex first pass get the automation step, and both arms get the verification rules on the validate step, so the arms stay byte-identical through their shared steps and remain comparable.
bockets
marked this pull request as ready for review
August 21, 2026 14:33
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[no-ticket]
BiggerPockets/biggerpockets#30384 shipped a scheduled review-app sweeper that has never expired a single app. The job declared
issues: read, but the/cwaoverride check reads comments on a PR-backed issue, which also requirespull-requests: read. Every sweep since the merge 403s,curl -sfturns that into an empty string, and the fail-closed guard keeps every app. Fixed in BiggerPockets/biggerpockets#30502.The review approved it, and two specific moves in that review are what these rules address.
It reported "
issues: readpermission is correctly declared for the new job". That established apermissions:key was present, not that its scope covered the endpoint the script calls — and the unchecked half was the entire defect. A confident wrong conclusion is worse than silence, because it closes the question for later readers.It also retired the finding that would have caught this. The first pass raised the swallowed
curl -sffailure as a Medium; the approving pass retired it as parity withcleanup-review-apps.yml/destroy-review-app.ymland as self-healing, since a failed delete leaves the appcreatedand the next sweep retries. That reasoning is correct about the DELETE call it was written about, and got applied to the whole script. On the comments call the same construct is not self-healing: it disables the feature permanently and silently.prompts/_shared/verification-rules.md
Governs how claims are stated and how findings are retired: no affirmative correctness claim without a check that could have come back the other way (and for a declared scope, permission, credential, or version constraint, the check is against the consuming API's documented requirements, not a sibling file); repository parity is evidence about whether something is a regression and never about whether it is a bug; a retired finding names its exact call site and the conditions its reasoning depends on.
Injected into step 10 of both Claude arms, where findings are validated and discarded.
prompts/_shared/automation-rules.md
CI, workflow, and unattended-job diffs currently have no domain block — privacy, perf, parsing, and navigation are all app-code rules, so a workflow-only diff has nothing to anchor on. This one covers external calls whose failures must be surfaced rather than swallowed, declared token scopes checked against the endpoint (including that a GitHub Actions
permissions:block replaces the default set, and that PR comments needpull-requestsdespite theissuespath), fail-closed guards that must distinguish "nothing to do" from "could not tell", and success signals that mean something.Added as a new step to both Claude arms and to the Codex first pass.
Effect on the arm comparison
Both arms change identically and stay byte-identical through their shared steps, so
experiment_split_percentresults remain comparable across the roll. Prompt versions are content-derived, so both bump automatically.Ran the
validate-prompts.ymlchecks locally against this branch: registry config valid, resolver deterministic for a fixed PR, no unresolved markers in either resolved prompt, both templates include-expanded, both arms reachable and resolving, assignment stable.