Skip to content

loop PRs carry no evidence, so the reviewer is the first human to read the change #73

Closed

Description

@Alex1990

What

docs/agents/ops.md defines the loop PR contract as identity only:

Loop PR = head branch loop/<issueNo>-*, body Closes #<n> and loop-task: #<n>.

Nothing requires the author to state intent, proof, risk tier, or where human
attention is needed.

Why it matters

Review's job changed once agents write the code: the reasoning is discarded together
with the diff, so the reviewer has to reconstruct intent that was never written down.
Addy Osmani's PR Contract is the cheap fix, and pushing that reconstruction back to
the author is what keeps review time from ballooning (Faros: median review duration
up 441.5% at high AI adoption).

Proposal

Require four items in the loop PR body, and make their absence a blocking finding:

  1. What/why — intent in one or two sentences.
  2. Proof it works — test output, manual steps, logs.
  3. Risk tier + AI role — which parts the agent produced, what breaks if wrong.
  4. Review focus — the one or two places human judgment is actually needed.

Files: docs/agents/ops.md (the contract), skills/review/SKILL.md (a PR missing
them is request-changes before any line-level review), skills/verify/SKILL.md
(the run record carries the same evidence). Norms layer → proposal PR.

Acceptance

  • A newly opened loop PR body contains the four sections.
  • Review returns request-changes when they are absent.

References

Activity

  1. github-actions commented on Sep 21, 2026

    @github-actions
    Contributor

    Loop triage — feature / ready-for-agent (confidence: high)

    Confirmed the gap against the tree, not just the report:

    • docs/norms/ops.md:92-93 defines a loop PR by identity only: head branch loop/<n>-*, body Closes #<n> and loop-task: #<n>.
    • The maker prompt (scripts/loop/shared/agent.mjs:299-305) shows push.body as Closes #<n>\n\nloop-task: #<n>\n\n<what and why> and requires only that it pass the two-marker loop-PR test, so even "what and why" is optional in practice.
    • The host enforces only the markers: ensureLoopPrBody (scripts/loop/shared/state.mjs:373-381) appends Closes #<id>/loop-task: #<id> when absent, and loopTaskOf (scripts/loop/shared/route.mjs:35-38) recognises a loop PR by head ref + loop-task:. Its own comment (state.mjs:369-372) calls this "a structural contract, so the host enforces it rather than trusting the model to remember it" — but nothing checks for evidence.
    • skills/review/SKILL.md:22 says "Review the diff, not the intent", and defines no finding for a missing intent/evidence section, so the reviewer is the first reader who must reconstruct why the change exists.
    • skills/verify/SKILL.md:24 records gate outputs in the local run log, not in the PR body the reviewer reads.

    Why this is loop work. The fix is a norms-layer proposal (docs/norms/ops.md, skills/review/SKILL.md, skills/verify/SKILL.md) plus the maker/PR-body prompt in scripts/loop/shared/agent.mjs. docs/norms/ops.md permits host changes as loop PRs (the owner's review is the checker) and norms-layer changes as proposal PRs. No .github/workflows/** edit is needed, so the job token can push; the ruleset keeps the loop from merging its own work.

    Two notes for the implementer:

    1. The issue names docs/agents/ops.md; that directory was renamed to docs/norms/ in Rename docs/agents to docs/norms #69 / PR chore(norms): rename docs/agents to docs/norms #70. Implement against docs/norms/ops.md.
    2. Acceptance Support PutSymlink request #1 ("a newly opened loop PR body contains the four sections") is a host-enforced structural contract, not something prose alone can guarantee: the maker composes push.body and the host accepts it if the two markers are present. Carry the four sections into the push.body example (agent.mjs:301-305) and add a host-side check (e.g. extend ensureLoopPrBody/planPush), matching the existing pattern. Acceptance 希望支持 STS #2 is the blocking pre-review request-changes gate in skills/review/SKILL.md (a missing section must not be an L3 nit). Keep the existing Closes #<n> / loop-task: #<n> markers so acceptance accounting keeps working.

    Suggested branch loop/73-pr-contract, typed chore(norms) (and chore(loop) for the host/prompt change) so nothing consumer-visible enters CHANGELOG.md. Labelling ready-for-agent.

  2. added
    ready-for-humanRequires a human: implementation, merge of a loop PR, or a release call
    and removed
    ready-for-agentFully specified, an AFK agent may handle it unattended
    on Sep 21, 2026
  3. github-actions commented on Sep 21, 2026

    @github-actions
    Contributor

    Loop feature — proposal PR opened (loop/73-pr-contract)

    Implemented #73. The loop PR contract now requires four author sections — What/why, Proof it works, Risk tier + AI role, Review focus — alongside the identity markers.

    • validatePush (scripts/loop/entry.mjs) refuses a pr-opened whose body is missing any section, so a newly opened loop PR always carries all four; the maker prompt renders the exact template from PR_CONTRACT_SECTIONS (scripts/loop/shared/state.mjs).
    • skills/review/SKILL.md adds a blocking PR-contract gate: a missing/empty section is request-changes before any line-level review.
    • skills/verify/SKILL.md collects the same evidence for the run record.

    This is a norms-layer proposal plus a loop host change, so the loop will not merge it — the owner's review is the checker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    ready-for-humanRequires a human: implementation, merge of a loop PR, or a release call

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions