feat(skills): approve a work through a pull request

motoko commented

2026-10-17 18:23:55 +02:00

Collaborator

Why

Recommendation 2. You asked twice to approve a piece of work once and judge it as a PR, but no gate offered that.

This PR is narrower than the original plan for a "light lane". A draft review found that lane's savings came only from dropping the security and documentation reviews, which have to stay, and from merging a gate that small changes already merge. You chose to narrow the work, and the cost goal moves to recommendation 3, trimming AGENTS.md.

What changed

Approve through a PR. At a work's first gate, get-approval offers to approve the work through a pull request. Choosing it passes every later gate, and deliver opens the PR.
- design still asks its decisions.
- For a branch that fixes a pre-existing security flaw, the choice doesn't name the PR, so you're told about the disclosure first.
- The offer only appears in run-change, and only when no go-without-approval instruction already covers the gate.
One reviewer for prose-only changes. document-reviewer reviews them alone and takes on change-reviewer's two checks, drift from the work file and an oversized diff, so no check is lost.
One proof of a reproduction. If diagnose already proved a reproduction on main and it lands unchanged, execute-plan doesn't prove it again.

How to verify

bash tests/verify-agent-config.sh (252), bash tests/check-work-folder.sh (46), .agents/hooks/shellcheck.sh repo, prettier --check '**/*.md', scripts/verify-agent-config.sh and git diff --check main all pass on the tip. No test covers the workflows' prose.

Not run yet:

The change, security and documentation reviews of this branch.
The eval comparison between main and this branch, which was still running when I opened this. I'll post both results here.

Considerations

The document-feature eval agent usually adds pin tests alongside the README, as the policy asks. That makes the change not prose-only, so the reviewer saving may rarely apply in that case.
Two choices are mine, made on your instruction to go ahead: document-reviewer taking over drift and diff size when it reviews alone, and when the PR option is offered.

## Why Recommendation 2. You asked twice to approve a piece of work once and judge it as a PR, but no gate offered that. This PR is narrower than the original plan for a "light lane". A draft review found that lane's savings came only from dropping the security and documentation reviews, which have to stay, and from merging a gate that small changes already merge. You chose to narrow the work, and the cost goal moves to recommendation 3, trimming `AGENTS.md`. ## What changed - **Approve through a PR.** At a work's first gate, `get-approval` offers to approve the work through a pull request. Choosing it passes every later gate, and `deliver` opens the PR. - `design` still asks its decisions. - For a branch that fixes a pre-existing security flaw, the choice doesn't name the PR, so you're told about the disclosure first. - The offer only appears in `run-change`, and only when no go-without-approval instruction already covers the gate. - **One reviewer for prose-only changes.** `document-reviewer` reviews them alone and takes on `change-reviewer`'s two checks, drift from the work file and an oversized diff, so no check is lost. - **One proof of a reproduction.** If `diagnose` already proved a reproduction on main and it lands unchanged, `execute-plan` doesn't prove it again. ## How to verify `bash tests/verify-agent-config.sh` (252), `bash tests/check-work-folder.sh` (46), `.agents/hooks/shellcheck.sh repo`, `prettier --check '**/*.md'`, `scripts/verify-agent-config.sh` and `git diff --check main` all pass on the tip. No test covers the workflows' prose. **Not run yet:** - The change, security and documentation reviews of this branch. - The eval comparison between main and this branch, which was still running when I opened this. I'll post both results here. ## Considerations - The document-feature eval agent usually adds pin tests alongside the README, as the policy asks. That makes the change not prose-only, so the reviewer saving may rarely apply in that case. - Two choices are mine, made on your instruction to go ahead: `document-reviewer` taking over drift and diff size when it reviews alone, and when the PR option is offered.

motoko added 1 commit

2026-10-17 18:23:59 +02:00

feat(skills): approve a work through a pull request 4a5f1b156e

Let get-approval offer, at a work's first gate, to approve the work
through a pull request, which then passes its later gates and opens
that pull request, except for a branch fixing a pre-existing security
flaw. Run document-reviewer alone, with change-reviewer's drift and
diff checks, on a change only to prose, and skip a second proof of a
reproduction diagnose already proved.

motoko commented

2026-10-17 18:30:59 +02:00

Author

Collaborator

Eval comparison, this branch against main (ae346e7), one attempt per case:

All 6 cases pass on both.
document-feature: $1.11 against $2.52 (−56%). Both runs changed only README.md. On main the agent made five reviewer calls: a docs audit, then two rounds of change review plus document review. On this branch it made two: the audit and one document review.
Total: $8.88 against $8.50. The other cases each swing between −$0.30 and +$1.16, which is normal run-to-run noise for single attempts.

The change, security and documentation reviews of this branch have not run yet.

Eval comparison, this branch against main (`ae346e7`), one attempt per case: - All 6 cases pass on both. - document-feature: $1.11 against $2.52 (−56%). Both runs changed only `README.md`. On main the agent made five reviewer calls: a docs audit, then two rounds of change review plus document review. On this branch it made two: the audit and one document review. - Total: $8.88 against $8.50. The other cases each swing between −$0.30 and +$1.16, which is normal run-to-run noise for single attempts. The change, security and documentation reviews of this branch have not run yet.

motoko added 2 commits

2026-10-17 18:43:31 +02:00

fixup! feat(skills): approve a work through a pull request 8b0268fd8e

fixup! feat(skills): approve a work through a pull request 784af38ea8

motoko commented

2026-10-17 18:43:40 +02:00

Author

Collaborator

Reviews done: change, security and documentation, in two rounds. The fixes are pushed as two fixup! commits.

Fixed:

Security exception missing (high, raised by all three reviewers). A branch that fixes a pre-existing security flaw could have been opened as a PR automatically, disclosing the flaw. deliver now skips the automatic PR for such a branch, and get-approval points its promise at the same exception in AGENTS.md.
Dropped checks (high). The document reviewer couldn't tell when change-reviewer was skipped, so its drift and diff-size checks could silently drop. Its handoff now says so.
Others' PRs. change-reviewer is now skipped only on a work's own prose-only branch, so others' PRs keep their correctness checks.
Smaller fixes:
- the gate records in the work template;
- the diagnosis template now records which commit a reproduction ran on;
- the workflow pages and charts.

In round 2 every finding was confirmed closed, with no new medium or high finding. All checks pass on 784af38.

Left for you:

Before merging: the two fixup commits need folding, which means a force-push of this branch. Say if I should do it. The eval numbers were measured on the original commit; the fixes change wording only, so I don't think a rerun is needed.
Optional: reword the commit body to say "prose for people" instead of "prose". This can happen in the same fold.

Reviews done: change, security and documentation, in two rounds. The fixes are pushed as two `fixup!` commits. Fixed: - **Security exception missing (high, raised by all three reviewers).** A branch that fixes a pre-existing security flaw could have been opened as a PR automatically, disclosing the flaw. `deliver` now skips the automatic PR for such a branch, and `get-approval` points its promise at the same exception in `AGENTS.md`. - **Dropped checks (high).** The document reviewer couldn't tell when `change-reviewer` was skipped, so its drift and diff-size checks could silently drop. Its handoff now says so. - **Others' PRs.** `change-reviewer` is now skipped only on a work's own prose-only branch, so others' PRs keep their correctness checks. - **Smaller fixes:** - the gate records in the work template; - the diagnosis template now records which commit a reproduction ran on; - the workflow pages and charts. In round 2 every finding was confirmed closed, with no new medium or high finding. All checks pass on `784af38`. Left for you: - **Before merging:** the two fixup commits need folding, which means a force-push of this branch. Say if I should do it. The eval numbers were measured on the original commit; the fixes change wording only, so I don't think a rerun is needed. - **Optional:** reword the commit body to say "prose for people" instead of "prose". This can happen in the same fold.

motoko force-pushed feat/pull-request-approval from 784af38ea8 to dc129e7f4e

2026-10-17 18:46:01 +02:00

Compare

claudiomaradonna merged commit dc129e7f4e into main

2026-10-17 18:47:52 +02:00

claudiomaradonna deleted branch feat/pull-request-approval

2026-10-17 18:47:57 +02:00

Rows
Columns

feat(skills): approve a work through a pull request #136

Why

What changed

How to verify

Considerations