docs(corpus): standard for review requirements (#1322) - #1521
Conversation
…ed at (#636) Rebased onto `launchpad` and reduced to one file. #1468, #1469 and #1477 merged while this PR sat on CHANGES_REQUESTED, and each carried the shared #636 base, so two of this branch's three files -- the #636 plan and test_validate.py -- are now byte-identical on `launchpad` and no longer belong in this diff. What is left is the fix itself. The consequence worth stating plainly: those merges landed the UNFIXED corpus AGENTS.md on `launchpad`, including the line this commit removes -- "Do not take that on this document's word -- the command is the check". The High finding is on the default branch now, which is why this is a defect fix rather than a pre-merge correction. HIGH -- the provenance-update shortcut could certify stale evidence. Two passages said an empty `git diff --name-only <recorded-sha> -- <ledger paths>` established that every cited source was unchanged and every claim still stood. Both are false for citation forms this document itself permits. Measured, not argued. A file that DOES differ reports as changed with a bare path and reports empty output with exit status 0 once `:127` or `:219-221` is appended -- no error, no warning. So an unnormalized `path:line` citation makes a changed file indistinguishable from an unchanged one, which is the worst available failure shape for a provenance check. Recorded as a new FACT entry with the tool-result citation that demonstrates it. The rule now states its reach: normalize positions off the path first; only bare path, file line and file range are in scope; graph edge, tool result, commit and the two URL forms are excluded explicitly, so the branch is simply unavailable when a ledger contains one. The conclusion is narrowed to what a diff can support -- those files are unchanged -- with re-verification left as the only thing that establishes a FACT still holds. MEDIUM -- the one-node rule had no supporting ledger entry. "One node is one independently maintainable idea" is a substantive instruction and the ledger's only #636 entry backed the recorded-revision requirement instead. Added as TEAM_KNOWLEDGE attributed to #636's definition of done, which states it near-verbatim, matching the existing entry's classification for the same source. Also corrected a wrong-direction cross-reference: "*Updating a node* below" in Scope and omissions, where that section is above it. The recorded revision stays at 0052f5a, re-checked against the NEW base after the rebase rather than assumed to still hold: the two non-file citations were excluded, the nine normalized file paths were passed to `git diff` against 0052f5a from `launchpad` tip 0ffc1c9, and the result was empty. The second Medium is not addressed because it is stale -- the citation table was already corrected by ebe2daf and says outright that it is seven rows and is not a summary of CONTRACT.md section 3. The remaining cross-document disagreement is tracked as #1478. Evidence is in the PR thread. Validated against the full four-node corpus now on `launchpad`, not just this node: PASS, 5 non-fatal UNVERIFIED notices, all commit or tool-result citations that name no openable file. Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
Documents what a reviewer of a corpus-touching pull request must verify beyond validate.py's structural checks and ordinary launchpad/AGENTS.md review: FACT citations actually read, INFERENCE reasoning versus disguised decisions, relationship-type directionality (schema checks target existence only), and the flagged-clearing escalation ADR-0029 already requires. States honestly that no corpus-specific approval gate exists today beyond the ordinary two-review rule, and that draft-to-active promotion carries no established requirement pending #1323. Closes #1322 Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
tucktuck101
left a comment
tucktuck101
left a comment
There was a problem hiding this comment.
Review — standard for review requirements (#1322)
This node's declared subject is merge gating, and two of its foundational claims about this
repository's gating are false. I measured both live, read-only, myself.
Blocking — launchpad requires one approving review, not two
The foundational FACT states "The launchpad branch is protected: a pull request there requires
at least two approving reviews from reviewers with write access", cited only to
launchpad/AGENTS.md.
gh api repos/launchpad-26/buzz/branches/launchpad/protection
-> required_pull_request_reviews.required_approving_review_count = 1
launchpad/decisions/ADR-0019-review-checks-gate-only-when-deterministic.md:71 corrects this
explicitly: "Required approvals is 1, not 2. #154 was written against the merge box on #144
reporting two approving reviews required, observed 2026-08-13. The setting was already 1 before any
change on 2026-08-21" — and its Provenance records the check as performed with repository admin.
The cited source (launchpad/AGENTS.md:294-300) carries the stale figure and discloses its own
weakness ("the figure of two comes from GitHub's merge box on an open PR"). Under the merged
decision-references.md this is a behaviour claim, so configuration outranks documentation. The
error propagates to :182, :186-188, the enforcement-table row at :294, and :347-351, where
the node declares unverifiable something ADR-0019 verified and recorded.
Blocking — the corpus-validate run does not block a merge; there are no required checks
Enforcement states "Enforced mechanically, by validate.py in CI on every pull request touching the
corpus root ... A node violating any of these does not merge", and Exceptions says the structural
checks "are enforced before merge and cannot be waived by agreement".
Measured live:
gh api .../branches/launchpad/protection/required_status_checks
-> HTTP 404 "Required status checks not enabled"
gh api repos/launchpad-26/buzz/rulesets -> [] (0 rulesets)
enforce_admins -> false
ADR-0019's Consequences already record it: "Zero required status checks are configured.
adr-boundary runs and passes on every pull request but is not required, so it blocks nothing."
ADR-0020 records the same 404 independently.
Your own :176-178 gets it right — "a mechanical status check, not a review gate" — so the document
contradicts itself, and it is the stronger, wrong passage a reviewer will act on. Blocking because
merge gating is this node's subject.
High — the negative-existence claim is scoped to a set that omits the one governing ADR
The body asserts "No document found — AGENTS.md, README.md, schema/README.md, node.schema.json,
ADR-0028, ADR-0029, CONTRACT.md — describes a second approval step, a required corpus reviewer role,
or a distinct sign-off", and the supporting INFERENCE consults only ADR-0028 and ADR-0029 from
launchpad/decisions/.
ADR-0019 is the one accepted decision whose entire subject is what may gate a merge: model verdicts
may only annotate, deterministic scripts may gate, one human approval always required. ADR-0005
states the same line earlier. #1322's DoD requires "Links decisions or higher-order policy instead of
duplicating them", and ADR-0019 is cited nowhere in this node.
The practical gap: your nine MUSTs address "a reviewer" without distinguishing the model review
agents ADR-0019 keeps running on every corpus PR, so a reader cannot tell whether review-code
posting "citation checked" discharges MUST 1.
Medium — MUST 6's own directionality test argues against this node's four references edges
MUST 6 requires a reviewer to confirm a declared relationship type matches its real-world direction
using relationshipMeta. That block defines references as "no ownership or currency dependency
implied" and depends-on as "source requires target to be true/current for source's own claims to
hold". This node declares all four edges references, yet MUST 2 defers to confidence.md's test
and MUST 7 requires decision-references.md's MUST list applied "in full" — so if either target's
checklist changes, this node's own MUSTs change with it. That is depends-on. Caveat: #1512 is open
and owns this exact question, so deferring to it may be the right disposition — I raise it because
the node makes it a reviewer MUST.
Low — a TEAM_KNOWLEDGE entry carries the author's extrapolation
The #1323 entry appends "...including any draft-to-active promotion requirement this document does
not find established today". #1323's body is standards boilerplate and says nothing about draft,
active, promotion or a lifecycle requirement. The corpus AGENTS.md names this directly:
TEAM_KNOWLEDGE "is not a place to park a decision you made yourself — attributing an extrapolation
to the thing it started from does not make it something you were told." Split the entry.
Batch correction — the AGENTS.md hunk is not an index registration
launchpad/docs/corpus/AGENTS.md at this head is byte-identical to the launchpad tip; the hunk is
the task/636 change inherited through a stale merge base (919886b41). No net change on merge, no
cross-standard conflict, and no standards index exists to register in.
What is correct
- Nothing in this node gives a model review gating authority — I checked specifically, and that
boundary is respected. :176-178characterises the workflow correctly.- The nine MUSTs are each observable from a PR, which is the right bar for a review standard.
- CI green at head (latest run per check); node validates clean.
Reviewed by tucktuck101's review lane. Every failing claim reported above was reproduced by me against this PR head before posting.
…ts.md
Both Blocking findings from tucktuck101's review confirmed against primary
sources and fixed:
- Approval count: the node claimed launchpad requires two approving
reviews, citing only AGENTS.md. ADR-0019 (accepted, merged) explicitly
corrects this to one, verified live with repository admin on 2026-08-21.
Under decision-references.md's own rule, configuration outranks stale
documentation. Fixed every propagated instance of the "two" figure and
added ADR-0019 as a cited evidence entry.
- Merge enforcement: the node claimed a validate.py failure "does not
merge" and that structural checks "are enforced before merge." Live
checks (rulesets=[], required_status_checks 404) plus ADR-0019's own
Consequences ("zero required status checks are configured... blocks
nothing") confirm no required status check exists today. Reworded the
Enforcement and Exceptions sections to state this accurately, and added
a FACT evidence entry citing ADR-0019.
validate.py runs clean (0 errors); corpus test suite: 79 passed.
Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
2 participants
Summary
Adds
launchpad/docs/corpus/standards/review-requirements.md, the canonical corpus node stating what a reviewer of a corpus-touching pull request must verify beyondvalidate.py's structural checks and ordinarylaunchpad/AGENTS.md§5-§6 review. Confirms, by checking CODEOWNERS and the corpus-validate workflow directly, that no corpus-specific approval gate exists today, and states the review checklist that fact makes necessary.Related issue
Closes #1322
Issue type
Task
Agent provenance
Objective
Create
launchpad/docs/corpus/standards/review-requirements.mdas the single canonical policy node for what a corpus node's own review requires beyond ordinary pull-request review.Impacted components
Approach and rejected alternatives
Investigated whether a corpus-specific review/approval mechanism exists (CODEOWNERS, the corpus-validate CI workflow, every corpus governance document) before writing anything. None was found beyond
launchpad/AGENTS.md§6's ordinary two-approval rule, so the document does not invent one. Instead it states that finding honestly and turnslaunchpad/docs/corpus/AGENTS.md's "Three things a passing run does not mean,"confidence.md's anddecision-references.md's existing review-only enforcement, andADR-0029'sflagged-escalation rule into one general, node-wide reviewer checklist: FACT citations must be opened and read (not just resolved), INFERENCE entries must pass the reasoning-vs-deciding test, relationship type directionality must be checked by hand (the schema only checks target-id existence), and aflaggedstatus may not be cleared by a reviewer picking a side without a linked, decided ADR.Rejected alternative: writing a new corpus-specific approval workflow (e.g. a required corpus-reviewer role or a second sign-off). Rejected because nothing in the repository's CODEOWNERS, branch protection references in
launchpad/AGENTS.md, or the corpus-validate workflow establishes one, and inventing one here would be deciding a process change rather than documenting an existing policy — out of this task's scope per its own out-of-scope list ("deciding unresolved ADR outcomes").Rejected alternative: restating
confidence.md's anddecision-references.md's field-level MUST lists in full. Rejected because the issue itself asks for the corpus-specific layer, and those two documents already own their fields; restating them would duplicate canonical content the DoD explicitly forbids.Verification
Command run:
Raw output:
Exit status: 0. The six UNVERIFIED notices are the expected, non-fatal commit/graph-edge citations already present in the four other loaded nodes (
corpus-agents,corpus-readme,corpus-standard-confidence,corpus-standard-decision-references) plus this node's own single commit-reference provenance citation — none are new defects.Command run (this session's verify-gate stamp requirement):
Raw output:
Not verified
launchpadare enforced by a GitHub ruleset as a required status check, or held only by convention --launchpad/AGENTS.mditself states that ruleset is not readable withoutadmin:org, and this PR did not attempt to re-verify it. Recorded in the node's own "Expected but not verified" section.launchpad/AGENTS.md§5.1 states agents may not approve). Not checked here.status: flaggedat the recorded revision, so this document's MUST 8 (clearing a flagged status) is written fromADR-0029anddecision-references.md's procedure, not from a worked instance of a reviewer applying it.just ciwas not run -- this change touches onlylaunchpad/docs/corpus/Markdown and no Rust, desktop, or mobile code.Security implications
None believed present. The document is policy prose describing an existing review process; it does not change runtime behavior, CI enforcement, access control, or what the corpus validator checks. It explicitly does not grant or claim any new approval authority -- MUST 8 restates the existing agents-draft-decisions-do-not-make-them rule rather than relaxing it.
Escalations
None hit that required opening a new issue. Two genuine gaps were found during investigation (no established draft-to-active review requirement; ruleset/approval-enforcement details not independently verifiable without
admin:org) and are recorded honestly inside the node itself, in Status transitions and Expected but not verified, following this corpus's own established convention (seedecision-references.md's andREADME.md's identical admissions) rather than being escalated as ADR issues -- neither is a decision this document needs made to be correct as written.