Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .claude/skills/fix-issue/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ node scripts/ds-bot-patches.mjs generate && node scripts/ds-bot-patches.mjs chec

## 6. Merge and cleanup

- Merge only when all hold **on the same tip**: review PASS (and implementer↔reviewer agreement, or a maintainer waiver of one side), required checks green, maintainer's go. Tip moved → start over from the review.
- Merge only when all hold **on the same tip**: review PASS (and implementer↔reviewer agreement, or a maintainer waiver of one side), required checks green, and either the maintainer's go or the standing T0/T1 delegation below. Tip moved → start over from the review.
- **Standing delegation (T0/T1 only).** An agent may enable auto-merge itself when `node .github/scripts/risk-gate.mjs --pr <n>` reports **T0 or T1**, the `reviewed tip` report shows a PASS at the current head by a login in `.github/agent/review-policy.json` (never the PR's own author), and `tests-ok` is green. Then `gh pr merge --auto --squash --match-head-commit <sha>`. T2, T3 and any pull request without a PASS at its tip wait for the maintainer.
- Then: fetch and fast-forward main, delete the branch locally and remotely, remove the worktree, and carry follow-up issues (not TODOs).

## Filing (before the fix)
Expand Down
10 changes: 10 additions & 0 deletions .claude/skills/review-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,13 @@ Host / delivery changes: note smoke as **Ran** or **Not run**. Record skipped su
5. Explicit statement that this is not merge authority

Do not fill contributor checkboxes or flip draft status unless asked.

## Recording the PASS

Post the report as a **pull request comment** (or the review body) only when a human asks for it, and
only from the reviewer's own login: `.github/scripts/review-gate.mjs` reads the newest Verifier block
that names the head, from a login in `.github/agent/review-policy.json`, and the pull request's own
author never counts. Keep the block intact — `Tip SHA:`, `Ran:`, `Not run:`, `Verdict:` — because a
missing `Ran:` command or `Not run:` line is reported as incomplete evidence, and a PASS at an older
sha stops counting the moment the tip moves. Posting it is not merge authority: see AGENTS.md step 6
for when the standing T0/T1 delegation lets an agent merge.
5 changes: 5 additions & 0 deletions .github/agent/review-policy.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"version": 1,
"note": "Who may record a PASS that counts, for the `reviewed tip` report (Phase 1: report only; see docs/autonomous-dev.md). A login listed here is not a merge right: the report is read when a tip is judged, and a T0/T1 auto-merge needs it under the standing delegation in AGENTS.md step 6. The pull request's own author never counts, even if listed. Add the reviewer agent's login here when the delegation lands. Editing this file is itself T3.",
"reviewers": ["hsliuustc0106", "Birfy"]
}
172 changes: 172 additions & 0 deletions .github/scripts/review-gate.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
// The reviewed tip of a pull request (Phase 1: report only; see docs/autonomous-dev.md).
//
// PR=12 GH_REPO=owner/repo GH_TOKEN=... node review-gate.mjs # what the workflow runs
// node review-gate.mjs --pr 12 [--repo owner/repo] # by hand
// node review-gate.mjs --pr 12 --sha <head> # judge another tip
//
// It answers one question: does this pull request's current head carry an independent
// Verifier PASS? `decide` is the whole rule:
// - the report is a pull request comment or a review body by a login listed in
// .github/agent/review-policy.json's `reviewers` (the file the base branch holds);
// - it is not the pull request author's own report: no self-review;
// - it carries the Verifier block review-pr already writes, with a `Tip SHA:` that is the
// head being judged and a `Verdict:`;
// - the verdict is PASS, the report lists a command it `Ran:`, and it says what was
// `Not run:` (a report that was not run is not evidence).
// The newest qualifying report that names this tip decides, so a later FAIL at the same tip
// vetoes an earlier PASS. A tip that moves past the report is not reviewed: the old tip is
// not a prefix of the new head and stops counting.
//
// The pull request is read only through the GitHub API; no PR code is checked out or run.
import { readFileSync, appendFileSync } from 'node:fs'
import { execFileSync } from 'node:child_process'
import { fileURLToPath } from 'node:url'

const SHORT = 12

// The last Verifier block in one body: a comment may hold an earlier review that a later
// one replaced, and only the last block is the verdict the reviewer stands behind.
export function parseReport(body) {
const lines = String(body ?? '').replace(/[*`]/g, '').split('\n')
let start = -1
for (let i = 0; i < lines.length; i++) {
if (/^Tip SHA\s*:/i.test(lines[i].trim())) start = i
}
const out = { tip: null, verdict: null, ran: [], hasNotRun: false }
const from = start < 0 ? 0 : start
for (let i = from; i < lines.length; i++) {
const line = lines[i].trim()
let m = /^Tip SHA\s*:\s*(.+)$/i.exec(line)
if (m) {
const hex = /[0-9a-f]{7,40}/i.exec(m[1])
out.tip = hex ? hex[0].toLowerCase() : null
}
m = /^Verdict\s*:\s*(PASS|FAIL|INC)\b/i.exec(line)
if (m) out.verdict = m[1].toUpperCase()
m = /^Ran\s*:\s*(.*)$/i.exec(line)
if (m) {
out.ran = []
if (m[1].trim() && m[1].trim() !== '-') out.ran.push(m[1].trim())
for (let j = i + 1; j < lines.length; j++) {
const next = lines[j].trim()
if (/^[-•]\s*\S/.test(next)) out.ran.push(next)
else if (next === '' && out.ran.length) break
else if (next !== '') break
}
}
if (/^Not run\s*:/i.test(line)) out.hasNotRun = true
}
return out
}

const login = (r) => String(r.login ?? '').toLowerCase()
const where = (r) => `${r.kind === 'review' ? 'review' : 'comment'} by ${r.login}`

// reports: [{ login, body, at, kind, id }] from the comments and review bodies of one PR.
export function decide({ head, author, reports, policy }) {
const headSha = String(head ?? '').toLowerCase()
const authorLogin = String(author ?? '').toLowerCase()
const reviewers = new Set((policy?.reviewers ?? []).map((l) => String(l).toLowerCase()))
const reasons = []
const judged = []

for (const r of reports ?? []) {
const parsed = parseReport(r.body)
if (!parsed.tip && !parsed.verdict) continue // not a Verifier report
const entry = { login: r.login, verdict: parsed.verdict, tip: parsed.tip, at: r.at, kind: r.kind, parsed, counts: false }
judged.push(entry)
if (!parsed.tip) { reasons.push(`${where(r)}: no Tip SHA`); continue }
if (!headSha.startsWith(parsed.tip)) {
reasons.push(`${where(r)}: names ${parsed.tip.slice(0, SHORT)}, not the tip ${headSha.slice(0, SHORT)}`)
continue
}
if (login(r) === authorLogin) { reasons.push(`${where(r)}: the pull request's own author is not a reviewer`); continue }
if (!reviewers.has(login(r))) { reasons.push(`${where(r)}: not a reviewer listed in .github/agent/review-policy.json`); continue }
entry.counts = true
}

const counting = judged.filter((e) => e.counts)
if (!counting.length) {
const reason = reviewers.size
? reasons.length ? reasons.join('; ') : `no Verifier report names the tip ${headSha.slice(0, SHORT)}`
: 'no reviewer is listed in .github/agent/review-policy.json'
return { ok: false, verdict: null, by: null, tip: null, reason, judged }
}

// Newest first: the reviewer's latest word at this tip decides. Equal timestamps fall back to
// the order the API returned them in (oldest first), so the later comment still wins.
const last = counting
.map((e, i) => ({ e, i, t: Date.parse(e.at ?? 0) || 0 }))
.sort((a, b) => a.t - b.t || a.i - b.i)
.at(-1).e
const { parsed } = last
if (last.verdict === 'PASS' && !parsed.ran.length) {
return { ok: false, verdict: 'PASS', by: last.login, tip: last.tip, reason: `${last.login}'s PASS lists no command it ran`, judged }
}
if (last.verdict === 'PASS' && !parsed.hasNotRun) {
return { ok: false, verdict: 'PASS', by: last.login, tip: last.tip, reason: `${last.login}'s PASS has no "Not run:" line`, judged }
}
if (last.verdict !== 'PASS') {
return { ok: false, verdict: last.verdict, by: last.login, tip: last.tip, reason: `${last.login} says ${last.verdict ?? 'nothing'} at this tip`, judged }
}
return { ok: true, verdict: 'PASS', by: last.login, tip: last.tip, reason: `${last.login}'s PASS names this tip`, judged }
}

export function render(result, { pr, head, author } = {}) {
const out = [
`### Reviewed tip: ${result.ok ? `PASS at ${head.slice(0, SHORT)}` : 'not reviewed'}`,
'',
result.ok
? `${result.by} passed this tip.`
: `No independent PASS names this tip: ${result.reason}.`,
'',
`Pull request #${pr}, author ${author}, head \`${String(head).slice(0, SHORT)}\`.`,
]
const seen = result.judged.filter((e) => e.counts || e.verdict)
if (seen.length) out.push('', 'Verifier reports read:', ...seen.slice(0, 10).map((e) => `- ${e.login} (${e.kind}): ${e.verdict ?? 'no verdict'} at ${(e.tip ?? 'no tip').slice(0, SHORT)}${e.counts ? '' : ' (not counted)'}`))
out.push('', '_Report only (Phase 1): this does not block, label, approve or merge anything. Rules: `.github/agent/review-policy.json`; see [`docs/autonomous-dev.md`](../../docs/autonomous-dev.md)._')
return out.join('\n')
}

export function loadPolicy(path) {
return JSON.parse(readFileSync(path, 'utf8'))
}

const gh = (...a) => execFileSync('gh', a, { encoding: 'utf8', maxBuffer: 256 << 20 })
const list = (path) => gh('api', '--paginate', '--jq', '.[]', path).split('\n').filter(Boolean).map((l) => JSON.parse(l))

function arg(name) {
const i = process.argv.indexOf(name)
return i > 0 ? process.argv[i + 1] : undefined
}

function main() {
const policyPath = arg('--policy') || process.env.REVIEW_POLICY || fileURLToPath(new URL('../agent/review-policy.json', import.meta.url))
const repo = arg('--repo') || process.env.GH_REPO
if (!repo) throw new Error('set GH_REPO or pass --repo owner/name')
const pr = arg('--pr') || process.env.PR
if (!pr) throw new Error('set PR or pass --pr N')

const pull = JSON.parse(gh('api', `repos/${repo}/pulls/${pr}`))
const head = arg('--sha') || pull.head?.sha
const author = pull.user?.login
if (!head) throw new Error(`pull request ${pr} has no head sha`)

const reports = [
...list(`repos/${repo}/issues/${pr}/comments?per_page=100`).map((c) => ({ kind: 'comment', login: c.user?.login, body: c.body, at: c.created_at, url: c.html_url })),
...list(`repos/${repo}/pulls/${pr}/reviews?per_page=100`)
.filter((v) => (v.body ?? '').trim())
.map((v) => ({ kind: 'review', login: v.user?.login, body: v.body, at: v.submitted_at, url: v.html_url })),
]

const result = decide({ head, author, reports, policy: loadPolicy(policyPath) })
console.log(`review-gate (report only): PR #${pr} tip ${String(head).slice(0, SHORT)} — ${result.ok ? `PASS by ${result.by}` : result.reason}`)
const text = render(result, { pr, head, author })
if (process.env.GITHUB_STEP_SUMMARY) appendFileSync(process.env.GITHUB_STEP_SUMMARY, text + '\n')
else console.log('\n' + text)
process.exit(result.ok ? 0 : 1)
}

if (process.argv[1] === fileURLToPath(import.meta.url)) {
try { main() } catch (e) { console.error(`review-gate: ${e.message}`); process.exit(1) }
}
155 changes: 155 additions & 0 deletions .github/scripts/review-gate.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
// review-gate: what counts as a reviewed tip, and the shape of the report-only workflow.
import { test } from 'node:test'
import assert from 'node:assert/strict'
import { readFileSync } from 'node:fs'
import { decide, loadPolicy, parseReport, render } from './review-gate.mjs'

const HEAD = 'a4d506a1b2c3d4e5f60718293a4b5c6d7e8f9012'
const OLD = '175328d0c1b2a3d4e5f60718293a4b5c6d7e8f90'
const policy = { reviewers: ['codex-agent', 'hsliuustc0106'] }

const report = ({ tip = HEAD, verdict = 'PASS', ran = [' - npm run verify → pass'], notRun = true } = {}) =>
[
`Tip SHA: ${tip}`,
'Ran:',
...ran,
...(notRun ? ['Not run:', ' - smoke — host code unchanged'] : []),
'OpenCode↔Codex: agree PASS',
`Verdict: ${verdict}`,
].join('\n')

const comment = (login, body = report(), at = '2026-10-18T01:00:00Z', kind = 'comment') => ({ login, body, at, kind })

test('a PASS by a listed reviewer that names the tip counts', () => {
const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('codex-agent')], policy })
assert.equal(r.ok, true)
assert.equal(r.by, 'codex-agent')
assert.match(r.reason, /PASS names this tip/)
})

test('a review body counts the same as a comment', () => {
const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('codex-agent', report(), '2026-10-18T01:00:00Z', 'review')], policy })
assert.equal(r.ok, true)
})

test('a PASS that names an older tip does not count: the tip moved', () => {
const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('codex-agent', report({ tip: OLD }))], policy })
assert.equal(r.ok, false)
assert.match(r.reason, /not the tip/)
})

test('the pull request author never reviews their own tip, listed or not', () => {
const r = decide({ head: HEAD, author: 'codex-agent', reports: [comment('codex-agent')], policy })
assert.equal(r.ok, false)
assert.match(r.reason, /own author is not a reviewer/)
})

test('a PASS from a login outside the policy does not count', () => {
const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('someone-else')], policy })
assert.equal(r.ok, false)
assert.match(r.reason, /not a reviewer listed/)
})

test('the newest report at the tip decides: a later FAIL vetoes an earlier PASS', () => {
const reports = [
comment('codex-agent', report(), '2026-10-18T01:00:00Z'),
comment('hsliuustc0106', report({ verdict: 'FAIL' }), '2026-10-18T02:00:00Z'),
]
const r = decide({ head: HEAD, author: 'opencode-agent', reports, policy })
assert.equal(r.ok, false)
assert.equal(r.verdict, 'FAIL')
assert.match(r.reason, /hsliuustc0106 says FAIL/)
})

test('a later PASS replaces an earlier FAIL at the same tip', () => {
const reports = [
comment('codex-agent', report({ verdict: 'FAIL' }), '2026-10-18T01:00:00Z'),
comment('codex-agent', report(), '2026-10-18T03:00:00Z'),
]
const r = decide({ head: HEAD, author: 'opencode-agent', reports, policy })
assert.equal(r.ok, true)
})

test('equal timestamps: the later comment in API order decides', () => {
const at = '2026-10-18T01:00:00Z'
const reports = [
comment('codex-agent', report(), at),
comment('codex-agent', report({ verdict: 'FAIL' }), at),
]
const r = decide({ head: HEAD, author: 'opencode-agent', reports, policy })
assert.equal(r.ok, false, 'the FAIL that came later must not lose to an equal-timestamped PASS')
assert.equal(r.verdict, 'FAIL')
})

test('INC is not PASS', () => {
const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('codex-agent', report({ verdict: 'INC' }))], policy })
assert.equal(r.ok, false)
assert.equal(r.verdict, 'INC')
})

test('a PASS that lists no command it ran is not evidence', () => {
const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('codex-agent', report({ ran: [] }))], policy })
assert.equal(r.ok, false)
assert.match(r.reason, /lists no command it ran/)
})

test('a PASS with no "Not run:" line is incomplete evidence', () => {
const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('codex-agent', report({ notRun: false }))], policy })
assert.equal(r.ok, false)
assert.match(r.reason, /has no "Not run:" line/)
})

test('no report at all, and no reviewer configured, say so plainly', () => {
assert.match(decide({ head: HEAD, author: 'a', reports: [], policy }).reason, /no Verifier report names the tip/)
assert.match(decide({ head: HEAD, author: 'a', reports: [comment('codex-agent')], policy: { reviewers: [] } }).reason, /no reviewer is listed/)
})

test('parse: markdown bold, backticked shas and short shas all bind', () => {
const parsed = parseReport(['**Tip SHA:** `a4d506a`', 'Ran:', '- npm test', 'Not run:', '- smoke', '**Verdict: PASS**'].join('\n'))
assert.equal(parsed.tip, 'a4d506a')
assert.equal(parsed.verdict, 'PASS')
assert.deepEqual(parsed.ran, ['- npm test'])
assert.equal(parsed.hasNotRun, true)
})

test('parse: one body with two Verifier blocks is judged by the last block', () => {
const parsed = parseReport([report({ tip: OLD }), '', report({ tip: HEAD, verdict: 'FAIL' })].join('\n'))
assert.equal(parsed.tip, HEAD)
assert.equal(parsed.verdict, 'FAIL')
})

test('parse: a body that is not a Verifier report yields nothing to bind', () => {
const parsed = parseReport('LGTM, merging this')
assert.equal(parsed.tip, null)
assert.equal(parsed.verdict, null)
assert.deepEqual(parsed.ran, [])
})

test('render names the state and keeps the report-only promise', () => {
const r = decide({ head: HEAD, author: 'opencode-agent', reports: [comment('codex-agent')], policy })
const text = render(r, { pr: 215, head: HEAD, author: 'opencode-agent' })
assert.match(text, /Reviewed tip: PASS at a4d506a1b2c3/)
assert.match(text, /Report only \(Phase 1\)/)
})

test('the policy file lists at least one reviewer', () => {
const p = loadPolicy(new URL('../agent/review-policy.json', import.meta.url))
assert.ok(Array.isArray(p.reviewers) && p.reviewers.length > 0)
for (const login of p.reviewers) assert.match(login, /^[\w-]+$/)
})

test('workflow: pull_request, read-only, not required, acts on nothing', () => {
const wf = readFileSync(new URL('../workflows/review-gate.yml', import.meta.url), 'utf8')
const testYml = readFileSync(new URL('../workflows/test.yml', import.meta.url), 'utf8')
const triggers = wf.slice(wf.indexOf('\non:\n'), wf.indexOf('\nconcurrency:'))
const perms = wf.slice(wf.indexOf('\npermissions:\n'), wf.indexOf('\njobs:'))
assert.match(triggers, /^ {2}pull_request:\s*$/m)
assert.doesNotMatch(triggers, /pull_request_target/, 'a PR must not run with a write token or secrets')
assert.doesNotMatch(triggers, /workflow_dispatch|schedule/, 'a tip is judged when it changes, not on a clock')
assert.match(perms, /^permissions:\n(?: {2}[\w-]+: read\n)+$/m)
assert.doesNotMatch(perms, /: write/, 'the report never writes')
const step = wf.slice(wf.indexOf('- name: Judge the tip'))
assert.match(step, /continue-on-error: true/, 'report only: the judge step is neutral, so the check never turns red')
assert.doesNotMatch(wf, /gh pr merge|--approve|reviewDecision|gh api .*-X (POST|PATCH|DELETE)/)
assert.doesNotMatch(testYml, /review-gate/, 'it is not a required check or part of tests-ok')
})
Loading
Loading