From 91669b139bd4fbba7459e7cedd57a20a9e676d56 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 05:58:11 +0000 Subject: [PATCH] feat(pm): the relay requests review from every GOVERNED_APPROVERS account on a Tier H pull, and CODEOWNERS routes every Tier H surface to both A seat-driven Tier H pull is opened draft and stays draft until the approval, and GitHub requests a code owner only at ready_for_review (measured on this repository's own Tier H pulls: the CODEOWNERS request arrived one second after the ready flip, never at the draft create). CODEOWNERS listed docs/adr/ -> @hotlong alone, and SKILL.md's "request review from both approvers" was prose no op performed. So an approver waited on a pull nobody told him about. - fleet-write/ops.mjs: pr_create and pr_ready carry governedReview; governedReviewRequests spells the two conditional writes (requested_reviewers, issue-assignees); pr_ready spends issues for the assignee fallback. - fleet-write/execute.mjs: after a governedReview row lands, read the pull's whole file list (every page, a cap that reds as a truncated subset), classify through the register (governedPathsIn + landingTierOf, scoped to payload.repo), and on a Tier H hit request every GOVERNED_APPROVERS account not already requested, assigned or holding a submitted review; the pull's own author is assigned instead (a request naming it is a 422). Tier S, ungoverned and covered pulls send nothing and say so; an unreadable list, pull or review list, a truncated walk or a refused write is a FAILED action, never a quiet skip. Self-test: a new battery of 29 cases; fixtures of the pr ops now declare the files read. - fleet-write/validate.mjs: pins the two rows, the two endpoints against the refused families, and the three alsoSpends rows. - .github/CODEOWNERS: every Tier H register row and the enforcement chain route to @os-zhuang @hotlong; the header names GOVERNED_APPROVERS as the list it mirrors. - check-governed-queue-guard.mjs: codeownersMirrorVerdict reads the file and the self-test reds when a Tier H row is missing, an owner set drifts from the constant, or a Tier S row is routed (a pin inside an existing self-test, no new gate). - pm-dispatch SKILL.md step 3 and rest-channel.md point at the op (net 0 lines each). Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01M82feuetLzrCiBz41BJb7L --- .claude/skills/pm-dispatch/SKILL.md | 8 +- .../pm-dispatch/references/rest-channel.md | 2 +- .github/CODEOWNERS | 67 +++-- scripts/pm/check-governed-queue-guard.mjs | 114 ++++++- scripts/pm/fleet-write/execute.mjs | 284 +++++++++++++++++- scripts/pm/fleet-write/ops.mjs | 82 ++++- scripts/pm/fleet-write/validate.mjs | 12 +- 7 files changed, 525 insertions(+), 44 deletions(-) diff --git a/.claude/skills/pm-dispatch/SKILL.md b/.claude/skills/pm-dispatch/SKILL.md index fc47414f6ac..3e1597d0b30 100644 --- a/.claude/skills/pm-dispatch/SKILL.md +++ b/.claude/skills/pm-dispatch/SKILL.md @@ -195,10 +195,10 @@ PM 的工作是循环:选卡 → 认领 → 派发 → 收集 → 复核 → 报 - PR 上的标签 = 待维护者审阅,不入六态;与请审同为等人批清单,随获批或撤回判决离开。 - 批准判定单源 = 队列守卫常量 `GOVERNED_APPROVERS`:授权账户 APPROVED 即算,⛔ 不卡 `commit_id`。 - 批准后再推亦不过期;席位落地 = 过落地前检、清标、ready、auto-merge,踢出/变基同法。 -- ③ 在 draft PR 上向两个授权批准账户 `os-zhuang` 与 `hotlong` 都 request review,主动推。 -- PR 作者身份即两账户之一的席位时,对该账户请审必失败(author-identity 422)。 -- 该账户改为把 PR assign 给它替代通知,另一账户照常请审;轮次报告点名说明走了兜底。 -- 请审走免碰 draft 位的 REST 专用路,ready/draft 走中继 op;MCP 兜底已拒;端点见 rest-channel。 +- ③ 请审是中继 op:`pr_create`/`pr_ready` 命中 Tier H 即向 `GOVERNED_APPROVERS` 全员请审。 +- 已请审/已指派/已评审者跳过(零写);PR 作者即其一时请审必 422,op 改为 assign 该账户替代。 +- 席位只核对 run 摘要 governed review 行,缺口才手补(免碰 draft 位 REST 路)并在轮次报告点名。 +- ready/draft 走中继 op;MCP 兜底已拒;端点见 rest-channel。 - ④ 轮次报告单列 awaiting a human merge。 - skills 车道自有 PR:纯代码面如 `scripts/pm/` 由本席按达档自审(清单不减)后落地。 - 受管面两层:Tier S = `.claude/**` 全树,余皆 Tier H 等人批;S 经达档复核 PASS 后 ready → 入队。 diff --git a/.claude/skills/pm-dispatch/references/rest-channel.md b/.claude/skills/pm-dispatch/references/rest-channel.md index 4d9d1a0ce5c..e2c97743116 100644 --- a/.claude/skills/pm-dispatch/references/rest-channel.md +++ b/.claude/skills/pm-dispatch/references/rest-channel.md @@ -44,7 +44,7 @@ - ✓ 建卡带标签 `POST .../issues` · 改正文 `PATCH .../issues/{n}` · 认领 `POST .../issues/{n}/assignees`。 - ✓ 该 `PATCH` 带 `state` 关卡/重开,`state_reason` 交付 `completed`、撤单 `not_planned`,走裸 REST。 - 请求体走文件(`-d @file`)或引号定界 heredoc(`<<'EOF'`),⛔ 永不内联双引号串。 -- ✓ 请求复审 `POST .../pulls/{n}/requested_reviewers` · 开 PR `POST .../pulls` 带 `draft=true`。 +- ✓ 请审 `POST .../pulls/{n}/requested_reviewers`(Tier H 归中继 op)· 开 PR `POST .../pulls` 带 `draft=true`。 - ✓ `origin/main` 合进 PR head:中继 op `pr_update_branch`(`pull` + 40 位 `expected_head_sha`),真合并提交。 - ✓ 落地经中继 `node scripts/pm/fleet-write/dispatch.mjs --repo O/N --actions-file F`;`--via dispatch` 同载。 - ✓ 启动 run 走 op `workflow_dispatch`:解锁物是白名单 run 才启;入表只经 workflow 自己的 PR。 diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index dc04e996a2d..15d3fffd682 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1,31 +1,52 @@ -# .github/CODEOWNERS — governance routing (#6785; ruling #6741). +# .github/CODEOWNERS — governance routing: every Tier H governed surface to +# every authorized approver. # # The ruling this file helps enforce (maintainer, verbatim): # 「adr 只能由维护者自己确认,人工合并,ai 不得擅自合并。」 # -# Routing docs/adr/ to the maintainer does two things: every PR touching an -# ADR requests his review automatically, and a ruleset / branch-protection -# rule "require review from Code Owners" has an entry to bind to (Half 2 of -# #6785 — a repo-settings change only the maintainer can apply; the -# button-by-button instructions are in the PR that added this file). +# The owner list on EVERY row mirrors `GOVERNED_APPROVERS` in +# scripts/pm/check-governed-queue-guard.mjs — the one list of accounts whose +# APPROVED review lands a Tier H surface (「os-zhuang hotlong 批准算数」). The +# rows mirror the Tier H rows of `GOVERNED_SURFACES` in +# scripts/pm/check-governed-merges.mjs. This file is static and can import +# neither, so the two stay in step the only way a static file can: +# `node scripts/pm/check-governed-queue-guard.mjs --self-test` reads this file +# (`codeownersMirrorVerdict`) and reds when a Tier H row has no line here, when +# any line names fewer or other owners than that constant, or when a Tier S row +# (`.claude/**`, which lands on a seat's review of record) is routed here. +# Change the constant or the register first; this file follows in the same PR. # -# The per-PR CI half that used to sit beside this file (`ADR maintainer -# approval`) retired under the 2026-08-18 ruling (#9495): a human merge IS -# the review record for a governed surface, the pre-merge line of defense is -# seat discipline (no AI seat flips ready / enqueues / arms auto-merge on a -# governed PR — AGENTS.md Prime Directive #14), and detection moved to the -# report-only post-merge audit, scripts/pm/check-governed-merges.mjs, whose -# header carries the incident history that used to live in the gate script. +# What routing buys, and what it does not: GitHub requests a code owner's +# review when a pull is opened READY or marked ready for review. A DRAFT open +# requests nobody — measured on this repository's own Tier H pulls, where the +# request arrived at `ready_for_review` and never at the draft create. Every +# seat-driven Tier H pull is opened draft and stays draft until the approval, +# so the fleet relay requests the same accounts itself at `pr_create` and +# `pr_ready` (scripts/pm/fleet-write/ops.mjs, "The governed-review follow-up"); +# this file is the platform half — for a pull opened ready or readied by hand — +# and the entry a ruleset's "require review from Code Owners" can bind to. A +# code owner without write access is ignored by GitHub without a word: +# `GET /repos/{owner}/{repo}/codeowners/errors` reads this file's own verdict. # -# `hotlong` is verified against the repo history, not assumed: 2,153 commits -# on main are authored as 50353452+hotlong@users.noreply.github.com — GitHub's -# {id}+{login} noreply address form ties the login to the account. +# Both logins are verified, not assumed: `hotlong` authors the bulk of main's +# history as 50353452+hotlong@users.noreply.github.com (GitHub's {id}+{login} +# noreply form ties the login to the account); `os-zhuang` is named by the +# ruling above and has readied and reviewed this repository's pulls. # -# The enforcement chain itself is routed the same way, deliberately: a PR -# that edits the audit script or this file is a governance change — without -# these entries the docs/adr/ rule could be lifted by editing the enforcers -# instead of the ADRs. +# The enforcement chain is routed the same way, deliberately: a PR that edits +# the audit script, the approver constant or this file is a governance change — +# without these entries the rule could be lifted by editing the enforcers +# instead of the surfaces. -/docs/adr/ @hotlong -/.github/CODEOWNERS @hotlong -/scripts/pm/check-governed-merges.mjs @hotlong +# Tier H governed surfaces — the register's rows with tier H +/docs/adr/ @os-zhuang @hotlong +/skills/ @os-zhuang @hotlong +/AGENTS.md @os-zhuang @hotlong +/CLAUDE.md @os-zhuang @hotlong +/docs/NORTH-STAR.md @os-zhuang @hotlong +/.github/workflows/ @os-zhuang @hotlong + +# The enforcement chain +/.github/CODEOWNERS @os-zhuang @hotlong +/scripts/pm/check-governed-merges.mjs @os-zhuang @hotlong +/scripts/pm/check-governed-queue-guard.mjs @os-zhuang @hotlong diff --git a/scripts/pm/check-governed-queue-guard.mjs b/scripts/pm/check-governed-queue-guard.mjs index 48d37c3563f..2c3d1e2a7fe 100644 --- a/scripts/pm/check-governed-queue-guard.mjs +++ b/scripts/pm/check-governed-queue-guard.mjs @@ -559,6 +559,7 @@ const SELF_TEST_BATTERIES = Object.freeze({ '⭐ the 2026-09-20 ruling: a certified PURE REGENERATION carries the record to the queued head': 13, 'the per-row repository scope (ruling B2, 2026-10-13)': 14, '⭐ B′ with C: the record stands at the tier its FACE maps to, read off the committed paths': 8, + 'the CODEOWNERS mirror: every Tier H register row routes to every GOVERNED_APPROVERS account, nothing else is routed, and the file says so': 12, }); // DELETING an entry silences that battery's floor exactly as effectively as @@ -567,7 +568,8 @@ const SELF_TEST_BATTERIES = Object.freeze({ // 5770886272 on #19061, letter B): the ordinary direction, one battery, one row. // Raised 23 → 24 with the queued pull-back battery: one battery, one row. // Raised 25 → 26 with the B′-with-C face battery (ruling record 6159870311): one battery, one row. -const SELF_TEST_BATTERY_FLOOR = 26; +// Raised 26 → 27 with the CODEOWNERS mirror battery: one battery, one row. +const SELF_TEST_BATTERY_FLOOR = 27; // The key an assertion is filed under when no battery is open. It is not a // declared battery, so it reds by the same set difference rather than silently @@ -618,6 +620,84 @@ export const EVENT_PULL_REQUEST = 'pull_request'; */ export const GOVERNED_APPROVERS = Object.freeze(['os-zhuang', 'hotlong']); +// ── the CODEOWNERS mirror — the platform half of "both approvers are asked" ─ + +/** + * `.github/CODEOWNERS` is the platform's own review router: GitHub requests a + * code owner's review when a pull is opened READY or marked ready for review. + * It is a static file — it can import neither `GOVERNED_SURFACES` nor + * `GOVERNED_APPROVERS` — so the two stay in step the only way a static file + * can: this predicate reads it and the self-test reds when it drifts. ⛔ It is + * a pin inside this existing self-test, not a gate, a check-run or a patrol + * row: the fleet's own request on the draft open is the relay's + * governed-review follow-up (`fleet-write/ops.mjs`), and the gap both close is + * what the H43 patrol row keeps reading. + * + * `codeownersRows` parses the file's effective lines — a `#` comment to end of + * line stripped, blank lines dropped — into `{ pattern, owners, line }`. + * `codeownersPatternFor` spells the one CODEOWNERS pattern a register row maps + * to: its prefix or exact path, root-anchored. Both pure. + */ +export function codeownersRows(text) { + const rows = []; + String(text ?? '') + .split('\n') + .forEach((raw, i) => { + const line = raw.replace(/#.*$/, '').trim(); + if (line === '') return; + const [pattern, ...owners] = line.split(/\s+/); + rows.push({ pattern, owners, line: i + 1 }); + }); + return rows; +} + +export function codeownersPatternFor(surface) { + return `/${surface.prefix ?? surface.exact}`; +} + +/** + * The verdict: `{ ok, problems, tierHPatterns }`. `ok` holds only when + * - every Tier H row of the register that governs `repo` has a CODEOWNERS + * line at exactly its pattern, naming exactly `@` + every account in + * `approvers` (order-free; nobody else, nobody fewer); + * - every other line — the enforcement chain's rows — names exactly that + * same owner set, so an enforcer cannot be routed to a subset; + * - no line routes a Tier S row's pattern: Tier S lands on a seat's review of + * record, and a code-owner request there would summon the maintainer for a + * landing that is not his; + * - the file's prose names `GOVERNED_APPROVERS`, so a reader finds the + * constant the owner list mirrors. + * Every problem is a sentence naming the row or line; the list is whole, never + * the first hit. Pure. + */ +export function codeownersMirrorVerdict({ text, approvers = GOVERNED_APPROVERS, repo = SELF_REPO_ID } = {}) { + const rows = codeownersRows(text); + const wanted = [...approvers].map((a) => `@${a}`.toLowerCase()).sort(); + const sameOwners = (owners) => JSON.stringify([...owners].map((o) => o.toLowerCase()).sort()) === JSON.stringify(wanted); + const problems = []; + // The rows that govern THIS repository, read from the register (ruling B2: the question has a repository in it). + const governing = governedSurfacesFor(repo); + const tierH = governing.filter((s) => s.tier === GOVERNED_TIER_H); + const tierHPatterns = tierH.map(codeownersPatternFor); + for (const surface of tierH) { + const pattern = codeownersPatternFor(surface); + const row = rows.find((r) => r.pattern === pattern); + if (!row) { + problems.push(`Tier H row \`${surface.id}\` (${surface.glob}) has no CODEOWNERS line at \`${pattern}\` — a pull touching it summons nobody when opened ready`); + continue; + } + if (!sameOwners(row.owners)) problems.push(`CODEOWNERS line ${row.line} \`${pattern}\` names ${row.owners.join(' ') || 'nobody'}, not every GOVERNED_APPROVERS account (${wanted.join(' ')}) — a Tier H pull then waits on an approver who is never asked`); + } + for (const row of rows) { + if (tierHPatterns.includes(row.pattern)) continue; + const tierS = governing.find((s) => s.tier === GOVERNED_TIER_S && codeownersPatternFor(s) === row.pattern); + if (tierS) problems.push(`CODEOWNERS line ${row.line} routes \`${row.pattern}\`, a Tier S row (\`${tierS.id}\`) that lands on a seat's review of record — a code-owner request there asks the maintainer for a landing that is not his`); + else if (!sameOwners(row.owners)) problems.push(`CODEOWNERS line ${row.line} \`${row.pattern}\` (an enforcement-chain row) names ${row.owners.join(' ') || 'nobody'}, not every GOVERNED_APPROVERS account (${wanted.join(' ')})`); + } + if (!/GOVERNED_APPROVERS/.test(String(text ?? ''))) problems.push('the file never names `GOVERNED_APPROVERS` — its header must say which constant the owner list mirrors'); + return { ok: problems.length === 0, problems, tierHPatterns, rows }; +} + // ── the landing TIER: Tier S lands on a review of record, Tier H on an approval ─ /** @@ -4529,6 +4609,38 @@ export async function selfTest() { assert('⭐ a-record-on-an-OLDER-head-CARRIES-when-the-head-moved-ONLY-on-prose-faces-outside-the-review-surface', proseCarried.exitCode === EXIT_CLEAR && proseCarried.entries[0].record.state === 'stands' && proseCarried.entries[0].record.carriedHops === 1, JSON.stringify(proseCarried.entries[0].record)); const proseCode = await tierRun({ comments: onOldHead, runGit: qgit(qProseCode), baseRef: QBASE }); assert('⛔ a-CODE-path-moved-beside-the-prose-REFUSES-exactly-as-an-uncarried-old-head-does', proseCode.exitCode === EXIT_REFUSED_UNAPPROVED && proseCode.entries[0].record.state === 'absent', JSON.stringify(proseCode.entries[0].record)); + // ── the CODEOWNERS mirror ───────────────────────────────────────────────── + // + // The platform half of "both approvers are asked": `.github/CODEOWNERS` is a + // static file, so this is the ONE place that proves it mirrors the register's + // Tier H rows and `GOVERNED_APPROVERS`. The file on disk is read here; the + // negative controls drive the predicate with fixtures. + battery('the CODEOWNERS mirror: every Tier H register row routes to every GOVERNED_APPROVERS account, nothing else is routed, and the file says so'); + { + const onDisk = readFileSync(join(repoRoot, '.github', 'CODEOWNERS'), 'utf8'); + const live = codeownersMirrorVerdict({ text: onDisk }); + assert('⭐ the-file-on-disk-mirrors-the-register-and-the-approver-list', live.ok, live.problems.join(' | ')); + assert('every-Tier-H-row-that-governs-this-repository-is-a-pattern-the-file-carries', live.tierHPatterns.length === governedSurfacesFor(REPO).filter((s) => s.tier === GOVERNED_TIER_H).length && live.tierHPatterns.every((p) => live.rows.some((r) => r.pattern === p)), JSON.stringify(live.tierHPatterns)); + assert('the-patterns-are-the-register-rows-root-anchored-prefix-or-exact', live.tierHPatterns.join() === '/docs/adr/,/skills/,/AGENTS.md,/CLAUDE.md,/docs/NORTH-STAR.md,/.github/workflows/', live.tierHPatterns.join()); + assert('every-line-on-disk-names-every-approver-spelled-as-a-login', live.rows.every((r) => GOVERNED_APPROVERS.every((a) => r.owners.includes(`@${a}`))) && live.rows.length >= live.tierHPatterns.length, JSON.stringify(live.rows)); + assert('the-parser-strips-comments-and-blank-lines-and-splits-owners', JSON.stringify(codeownersRows('# c\n\n/docs/adr/ @a @b # trailing\n /x.md @c\n')) === JSON.stringify([{ pattern: '/docs/adr/', owners: ['@a', '@b'], line: 3 }, { pattern: '/x.md', owners: ['@c'], line: 4 }])); + const owners = GOVERNED_APPROVERS.map((a) => `@${a}`).join(' '); + const full = (extra = '') => `# mirrors GOVERNED_APPROVERS\n${live.tierHPatterns.map((p) => `${p} ${owners}`).join('\n')}\n${extra}`; + assert('a-fixture-carrying-exactly-the-Tier-H-rows-with-every-owner-passes', codeownersMirrorVerdict({ text: full() }).ok); + const one = codeownersMirrorVerdict({ text: full().replace(`/docs/adr/ ${owners}`, `/docs/adr/ @${GOVERNED_APPROVERS[1]}`) }); + assert('⛔ one-approver-on-the-adr-row-is-a-problem-naming-the-row-and-the-missing-account', !one.ok && one.problems.length === 1 && one.problems[0].includes('`/docs/adr/`') && one.problems[0].includes(`@${GOVERNED_APPROVERS[0]}`), one.problems.join(' | ')); + const missing = codeownersMirrorVerdict({ text: full().split('\n').filter((l) => !l.startsWith('/skills/')).join('\n') }); + assert('⛔ a-Tier-H-row-with-no-line-is-a-problem-naming-the-row', !missing.ok && missing.problems.length === 1 && missing.problems[0].includes('`skills-catalog`'), missing.problems.join(' | ')); + const tierS = codeownersMirrorVerdict({ text: full(`/.claude/ ${owners}\n`) }); + assert('⛔ routing-the-Tier-S-row-is-a-problem-a-seat-record-landing-summons-nobody', !tierS.ok && tierS.problems.length === 1 && tierS.problems[0].includes('`claude-tree`'), tierS.problems.join(' | ')); + const enforcer = codeownersMirrorVerdict({ text: full(`/.github/CODEOWNERS @${GOVERNED_APPROVERS[1]}\n`) }); + assert('⛔ an-enforcement-chain-row-routed-to-a-subset-is-a-problem', !enforcer.ok && enforcer.problems.length === 1 && enforcer.problems[0].includes('enforcement-chain'), enforcer.problems.join(' | ')); + const silent = codeownersMirrorVerdict({ text: full().replace('# mirrors GOVERNED_APPROVERS\n', '') }); + assert('⛔ a-file-that-never-names-the-constant-is-a-problem', !silent.ok && silent.problems.length === 1 && silent.problems[0].includes('GOVERNED_APPROVERS'), silent.problems.join(' | ')); + const other = codeownersMirrorVerdict({ text: onDisk, approvers: ['someone-else'] }); + assert('⭐ the-approver-list-is-READ-not-restated-another-list-reds-the-file-on-disk', !other.ok && other.problems.length >= live.tierHPatterns.length, String(other.problems.length)); + } + // ── The floor: every declared battery RAN, and ran its cases (#13489) ──── // // Evaluated after every battery has had its chance and BEFORE the verdict, so diff --git a/scripts/pm/fleet-write/execute.mjs b/scripts/pm/fleet-write/execute.mjs index ad70de3180a..c660b170c9e 100644 --- a/scripts/pm/fleet-write/execute.mjs +++ b/scripts/pm/fleet-write/execute.mjs @@ -146,6 +146,33 @@ * platform lists them (`resultOf`). Both legs are paced like every other * write verb. * + * ## The governed-review follow-up — after `pr_create` and after `pr_ready` + * + * Once a `governedReview` row's own requests have landed (`GOVERNED_REVIEW_OPS` + * in `ops.mjs`, its header section of this name: `pr_create` at the number the + * CREATE answered — ⛔ never one the request named — and `pr_ready` at the pull + * the action names), `requestGovernedReview` reads the pull's file list in + * full (`readPullFiles`: every page, a cap that reds as a truncated subset) and + * hands the paths to `governedReviewPlan`, a pure function over the register: + * `governedPathsIn` scoped to `payload.repo` through `repoIdForSlug`, then + * `landingTierOf` over the matched slice — a Tier H hit is the only answer that + * sends anything; a Tier S or ungoverned diff, or a target outside + * `GOVERNED_REPOS`, is ONE row saying so and zero writes. On a Tier H hit it + * reads the pull (its author, pending `requested_reviewers`, `assignees`) and + * its submitted reviews, and the plan is the `GOVERNED_APPROVERS` accounts + * NOT in that union — requested, assigned or reviewed, the coverage the H43 + * patrol reads — so a re-run, a `pr_ready` after a `pr_create`, or the + * platform's own CODEOWNERS request landing first costs no write. The pull's + * own author is never requested (GitHub answers 422) — a missing approver + * who IS the author is ASSIGNED instead, the documented fallback. The writes + * are what `governedReviewRequests` spells, judged by `requestLanded` as the + * legs above are (`requested` against the pull's pending requests, `assigned` + * against its assignees), one row each, paced like every write verb. An + * unreadable file list, pull or review list, a truncated walk, or a refused + * write is a FAILED action — the pull is open or ready, the row says what was + * not sent, and the stroke stops there as on any failure: silence here is the + * defect this follow-up exists to close, so it is never a quiet skip. + * * ## The step summary — what a person reads back * * `$GITHUB_STEP_SUMMARY` gets one table per run: request id · sender and its @@ -195,11 +222,14 @@ import process from 'node:process'; import { fileURLToPath } from 'node:url'; import { isEntrypoint } from '../../invoked-as.mjs'; +// The register and the approver list, READ — never restated here (the ops.mjs header, "The governed-review follow-up"). +import { GOVERNED_TIER_H, GOVERNED_TIER_S, governedPathsIn, landingTierOf, pullPathsFrom, repoIdForSlug } from '../check-governed-merges.mjs'; +import { GOVERNED_APPROVERS } from '../check-governed-queue-guard.mjs'; import { scrub } from '../fleet-token.mjs'; import { classifyHttp } from '../label-write.mjs'; import { announceBatch, EXIT_WRITE_PACE_REFUSED, fmtDuration, isWriteMethod, noteResponse, paceFilePath, paceWrite, releaseWriteLease, unrecordableText } from '../write-pace.mjs'; import { ANNOTATED_OPS, parseRelayAnnotation, RELAY_ANNOTATION_PREFIX, relayAnnotationMessage, strokeBudgetRefusal, strokePacing } from './dispatch.mjs'; -import { OPS, PERMISSIONS, PR_LANDED_STATE, answerPlaceholder, transferRemedy } from './ops.mjs'; +import { GOVERNED_REVIEW_FILES_PAGE_CAP, GOVERNED_REVIEW_FILES_PAGE_SIZE, GOVERNED_REVIEW_OPS, OPS, PERMISSIONS, PR_LANDED_STATE, answerPlaceholder, governedReviewRequests, transferRemedy } from './ops.mjs'; import { PAYLOAD_ENV, refusalText, tokenRepositoriesOf, validatePayload } from './validate.mjs'; const SELF_PATH = fileURLToPath(import.meta.url); @@ -284,6 +314,14 @@ export function requestLanded(req, { status, json } = {}) { return { ok: false, why: `HTTP ${status}, but the answer lists ${listed.length ? listed.join(', ') : 'no assignees'} — ${missing.join(', ')} not assigned (GitHub ignores a login the repository cannot assign without a word: it must be a collaborator the repository can assign)` }; } } + // The governed-review request (header): the answer is the pull with its PENDING requests; a login sent and not listed did not land. + if (Array.isArray(req.requested)) { + const listed = reviewerLogins(json); + const missing = req.requested.filter((login) => !listed.some((l) => l.toLowerCase() === login.toLowerCase())); + if (missing.length) { + return { ok: false, why: `HTTP ${status}, but the answer lists ${listed.length ? listed.join(', ') : 'no requested reviewers'} — ${missing.join(', ')} not requested (the endpoint answers the pull with its pending review requests, so a login it lacks was not written; read the pull on the board before anything else)` }; + } + } // The labels leg (header): the answer is the pull's whole label set; a name sent and not listed did not land. if (Array.isArray(req.labelled)) { const listed = labelNames(json); @@ -308,6 +346,56 @@ export function labelNames(json) { return Array.isArray(json) ? json.map((l) => (typeof l?.name === 'string' ? l.name : null)).filter(Boolean) : []; } +/** The logins a pull answer lists as PENDING review requests (`requested_reviewers`), in the platform's order; `[]` when none. Pure. */ +export function reviewerLogins(json) { + return Array.isArray(json?.requested_reviewers) ? json.requested_reviewers.map((u) => (typeof u?.login === 'string' ? u.login : null)).filter(Boolean) : []; +} + +/** The logins that have SUBMITTED a review in a reviews answer — any state but `PENDING` (unsubmitted); `[]` when none. Pure. */ +export function reviewedLogins(json) { + return Array.isArray(json) ? json.filter((r) => r?.state !== 'PENDING').map((r) => (typeof r?.user?.login === 'string' ? r.user.login : null)).filter(Boolean) : []; +} + +/** + * The governed-review plan for ONE pull (header: "The governed-review follow-up"). Pure — the executor reads + * the file list, the pull and its reviews, hands them in, and sends exactly what comes back: + * + * repoId the register id of `repo`, or null for a target outside `GOVERNED_REPOS` (no row governs it); + * matched the governed slice of `paths` (`governedPathsIn`), for the row's wording; + * tier `H` when the slice lands on an authorized approval, `S` when every hit is Tier S, null when no + * governed row is hit — read through `landingTierOf` over the MATCHED slice, never over nothing + * (an empty slice answers H there, fail-closed, which is the wrong answer for "send a request"); + * covered the approvers already requested, assigned or holding a submitted review — the H43 union; + * toRequest on a Tier H hit, the approvers not covered and not the pull's author; + * toAssign on a Tier H hit, the author when it is an uncovered approver (a request naming it is a 422). + * + * Logins are matched as GitHub matches them, case-insensitively; the answer keeps `approvers`' own spelling and order. + */ +export function governedReviewPlan({ paths, repo, author = null, requested = [], assigned = [], reviewed = [], approvers = GOVERNED_APPROVERS } = {}) { + const repoId = repoIdForSlug(repo); + const matched = repoId ? governedPathsIn(Array.isArray(paths) ? paths : [], repoId) : []; + const tier = matched.length === 0 ? null : landingTierOf(matched); + const lower = (list) => (Array.isArray(list) ? list : []).map((l) => String(l).toLowerCase()); + const have = new Set([...lower(requested), ...lower(assigned), ...lower(reviewed)]); + const list = Array.isArray(approvers) ? approvers : []; + const covered = list.filter((a) => have.has(a.toLowerCase())); + const missing = tier === GOVERNED_TIER_H ? list.filter((a) => !have.has(a.toLowerCase())) : []; + const isAuthor = (a) => typeof author === 'string' && a.toLowerCase() === author.toLowerCase(); + return { repoId, matched, tier, covered, toRequest: missing.filter((a) => !isAuthor(a)), toAssign: missing.filter(isAuthor) }; +} + +/** The words the follow-up's verdict row prints for a plan — what was hit, who is covered, what will be sent. Pure. */ +export function governedReviewText(plan, { repo, files, approvers = GOVERNED_APPROVERS } = {}) { + const hits = plan.matched.map((s) => `\`${s.glob}\`×${s.files.length}`).join(', '); + if (plan.repoId === null) return `governed review: ${repo} is outside GOVERNED_REPOS, so no register row governs it — ${files} file(s), no review requested`; + if (plan.tier === null) return `governed review: no governed row hit across ${files} file(s) — no review requested`; + if (plan.tier === GOVERNED_TIER_S) return `governed review: Tier S (${hits}) lands on a seat's review of record — no review requested`; + const named = (logins) => logins.map((l) => `\`${l}\``).join(', '); + const sending = [plan.toRequest.length ? `requesting ${named(plan.toRequest)}` : '', plan.toAssign.length ? `assigning the author ${named(plan.toAssign)} (a request naming the author is a 422)` : ''].filter(Boolean).join(' and '); + if (!sending) return `governed review: Tier H (${hits}) — ${named(approvers)} already requested, assigned or reviewed; nothing to send`; + return `governed review: Tier H (${hits}) — ${plan.covered.length ? `${named(plan.covered)} already covered; ` : ''}${sending}`; +} + /** * A descriptor carrying `fromAnswer` filled from its action's FIRST request's * answer — the create's (header): `{ req }` with the placeholder replaced by @@ -410,6 +498,8 @@ export function resultOf(req, json, status = null) { if (typeof json.html_url === 'string') bits.push(json.html_url); // The assignees leg's row prints the assignees as the platform lists them (header). if (Array.isArray(req.assigned)) bits.push(`· assignees ${assigneeLogins(json).join(', ') || 'none'}`); + // The governed-review request's row prints the pull's pending requests as the platform lists them (header). + if (Array.isArray(req.requested)) bits.push(`· review requested from ${reviewerLogins(json).join(', ') || 'none'}`); return bits.join(' ') || 'ok'; } @@ -558,6 +648,73 @@ async function graphqlVariables(req, payload, api, t) { return { variables: { issueId: issue.json.node_id, repositoryId: target.json.node_id } }; } +/** + * A pull's file list, every page (header: "The governed-review follow-up"). Returns `{ files, pages }`, or + * `{ fail: { status, why } }` for a page that did not answer 200 with an array, or a walk that would need + * more than `GOVERNED_REVIEW_FILES_PAGE_CAP` pages — a truncated list is a SUBSET and can hide a governed hit, + * so it is never read as "nothing governed". Never throws. + */ +async function readPullFiles(repo, pull, api, t) { + const files = []; + for (let page = 1; page <= GOVERNED_REVIEW_FILES_PAGE_CAP; page++) { + const r = await rest(api, `/repos/${repo}/pulls/${pull}/files?per_page=${GOVERNED_REVIEW_FILES_PAGE_SIZE}&page=${page}`, {}, t); + if (r.status !== 200 || !Array.isArray(r.json)) { + return { fail: { status: r.status, why: `${r.call} -> HTTP ${r.status}${r.detail ? ` (${r.detail})` : ''}: the pull's file list could not be read, so its governed tier is unknown and no review was requested — read the pull on the board and request by hand if its diff touches a Tier H surface` } }; + } + files.push(...r.json); + if (r.json.length < GOVERNED_REVIEW_FILES_PAGE_SIZE) return { files, pages: page }; + } + return { fail: { status: 200, why: `the file-list walk hit its ${GOVERNED_REVIEW_FILES_PAGE_CAP}-page cap with ${files.length} file(s) collected and more pending — a truncated list is a SUBSET that can hide a governed hit, so no review was requested; read the pull on the board and request by hand` } }; +} + +/** + * The governed-review follow-up for ONE landed `governedReview` action (header): the reads, the plan, the writes + * — one row per verdict and per write pushed onto `rows`. Returns `{ ok: true }` or `{ fail: why }`, the sentence + * the caller records as the action's failure. Never throws on a status. + */ +async function requestGovernedReview({ index, op, pull, payload, api, t, log, rows }) { + const repo = payload.repo; + const token = t.token ?? ''; + const FILES = `GET /repos/${repo}/pulls/${pull}/files`; + const failed = (call, status, why) => { + const clean = scrub(why, [token]); + rows.push({ action: index, op, call, status, result: `FAILED: ${clean}` }); + return { fail: clean }; + }; + const read = await readPullFiles(repo, pull, api, t); + if (read.fail) return failed(FILES, read.fail.status, read.fail.why); + const { paths, renames } = pullPathsFrom(read.files); + let plan = governedReviewPlan({ paths, repo }); + const renamed = renames.length ? ` (${renames.length} rename(s): the old path counts too)` : ''; + if (plan.tier !== GOVERNED_TIER_H) { + const text = governedReviewText(plan, { repo, files: read.files.length }); + rows.push({ action: index, op, call: FILES, status: 200, result: text }); + log(` · action ${index} ${op}: ${text}${renamed}`); + return { ok: true }; + } + const pr = await rest(api, `/repos/${repo}/pulls/${pull}`, {}, t); + if (pr.status !== 200 || !pr.json || typeof pr.json !== 'object') { + return failed(pr.call, pr.status, `${pr.call} -> HTTP ${pr.status}${pr.detail ? ` (${pr.detail})` : ''}: the pull could not be re-read, so who is already requested, assigned or its author is unknown and no review was requested — its diff hits a Tier H surface; read it on the board and request by hand`); + } + const reviews = await rest(api, `/repos/${repo}/pulls/${pull}/reviews?per_page=100`, {}, t); + if (reviews.status !== 200 || !Array.isArray(reviews.json)) { + return failed(`GET /repos/${repo}/pulls/${pull}/reviews`, reviews.status, `${reviews.call} -> HTTP ${reviews.status}${reviews.detail ? ` (${reviews.detail})` : ''}: the pull's reviews could not be read, so who has already reviewed is unknown and no review was requested — its diff hits a Tier H surface; read it on the board and request by hand`); + } + plan = governedReviewPlan({ paths, repo, author: pr.json.user?.login ?? null, requested: reviewerLogins(pr.json), assigned: assigneeLogins(pr.json), reviewed: reviewedLogins(reviews.json) }); + const verdict = governedReviewText(plan, { repo, files: read.files.length }); + rows.push({ action: index, op, call: FILES, status: 200, result: verdict }); + log(` · action ${index} ${op}: ${verdict}${renamed}`); + for (const req of governedReviewRequests(repo, pull, { reviewers: plan.toRequest, assignees: plan.toAssign })) { + const r = await rest(api, req.path, { method: req.verb, body: req.body }, t); + const landed = requestLanded(req, r); + if (!landed.ok) return failed(r.call, r.status, landed.why); + const result = scrub(resultOf(req, r.json, r.status), [token]); + rows.push({ action: index, op, call: r.call, status: r.status, result }); + log(` ✓ action ${index} ${op}: ${r.call} -> HTTP ${r.status} · ${result}`); + } + return { ok: true }; +} + /** * The run. Returns `{ exit, rows, summary, lines, annotations }` — `annotations` * the notes this run emitted, in order; never throws on a status. @@ -681,6 +838,23 @@ export async function executeFleetWrite({ payload: raw, sender, token, api = DEF log(` ⚠ action ${i + 1} ${action.op}: no annotation — ${scrub(why, [token])}; a seat reading this run finds the card on the board instead.`); } } + // The governed-review follow-up (header): after the row's own requests landed, at the number the CREATE + // answered for pr_create — never one the request named — and at the pull the action names for pr_ready. + if (stoppedAt === null && GOVERNED_REVIEW_OPS.includes(action.op)) { + const pull = Number.isInteger(action.pull) ? action.pull : opener?.json?.number; + let followed; + if (!Number.isInteger(pull) || pull < 1) { + const why = `${opener?.call ?? 'the create'} answered HTTP ${opener?.status ?? '?'} without a positive integer number, so the governed-review follow-up could not run — read the pull on the board and request by hand if its diff touches a Tier H surface`; + rows.push({ action: i + 1, op: action.op, call: 'GET …/pulls/{number}/files', status: 'not sent', result: `FAILED: ${scrub(why, [token])}` }); + followed = { fail: why }; + } else { + followed = await requestGovernedReview({ index: i + 1, op: action.op, pull, payload, api, t, log, rows }); + } + if (followed.fail) { + stoppedAt = { action: i + 1, op: action.op, why: scrub(followed.fail, [token]) }; + log(` ✗ action ${i + 1} ${action.op}: ${stoppedAt.why}`); + } + } } const notAttempted = stoppedAt ? payload.actions.length - stoppedAt.action : 0; const failedAction = stoppedAt ? payload.actions[stoppedAt.action - 1] : null; @@ -714,8 +888,10 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'the workflow_dispatch: ONE POST to the dispatch endpoint of the allowlisted file with { ref: main } under the App token, a bodiless 204 lands as accepted, a 422 or 403 is a failed action, no other actions/* request leaves, no annotation': 7, 'the release.yml refresh: ONE POST to its dispatch endpoint carrying { ref: main, inputs: { refresh_version_pr: "true" } } under the App token, landing as accepted; force, false, absent, a string or any other input refused with ZERO requests, exit 2': 9, "the batch gap on the runner: a stroke carrying DEFAULT_BATCH_THRESHOLD or more content-creating actions is announced as ONE rule ⑤ batch before its first write — on an injected clock every write through the last content-creating one leaves a full DEFAULT_BATCH_GAP_MS after the one before, and the pacing is exactly what the seat budgets; below the threshold, after the batch and on a landing stroke rule ①'s gap; a stroke the job cannot hold refused with ZERO requests; an announcement the log cannot record refused before the first write": 10, + "the governed-review follow-up: after pr_create (at the number the create answered) and after pr_ready, the pull's file list is read in full; a Tier H hit requests review from every GOVERNED_APPROVERS account not already requested, assigned or reviewed — the pull's own author assigned instead, never requested; a Tier S or ungoverned diff sends nothing and says so; an unreadable or truncated list, an unreadable pull or review list, or a refused write is a FAILED action": 29, }); -const SELF_TEST_BATTERY_FLOOR = 17; +// Raised 17 → 18 with the governed-review follow-up battery: one battery, one row. +const SELF_TEST_BATTERY_FLOOR = 18; const UNATTRIBUTED_BATTERY = '(unattributed)'; const batteryCases = new Map(); @@ -822,11 +998,13 @@ export async function selfTest() { const pr = await run(base([{ op: 'pr_create', title: 'T', head: 'h', base: 'main' }, { op: 'issue_create', title: 'I', body: 'B', labels: ['l'] }]), { ...allowed, [`POST /repos/${REPO}/pulls`]: { status: 201, json: { number: 9, html_url: 'https://github.test/p/9' } }, + // The governed-review follow-up reads the created pull's files (its own battery below); an empty list is an ungoverned diff. + [`GET /repos/${REPO}/pulls/9/files`]: { status: 200, json: [] }, [`POST /repos/${REPO}/issues`]: { status: 201, json: { number: 10, html_url: 'https://github.test/i/10' } }, }, { file: paceFile }); t('pr_create sends draft: true whatever was asked', writes(pr.seen)[0].body, { title: 'T', head: 'h', base: 'main', draft: true }); t('issue_create sends title, body and the optional lists it was given', writes(pr.seen)[1].body, { title: 'I', body: 'B', labels: ['l'] }); - t('…and the rows carry the numbers', pr.rows.map((r) => r.result), ['#9 https://github.test/p/9', '#10 https://github.test/i/10']); + t('…and the rows carry the numbers', pr.rows.map((r) => r.result), ['#9 https://github.test/p/9', 'governed review: no governed row hit across 0 file(s) — no review requested', '#10 https://github.test/i/10']); } // ── the run ───────────────────────────────────────────────────────────── @@ -865,8 +1043,10 @@ export async function selfTest() { ...allowed, [`GET /repos/${REPO}/pulls/12`]: { status: 200, json: { number: 12, node_id: 'PR_kwDO' } }, 'POST /graphql': { status: 200, json: { data: { markPullRequestReadyForReview: { pullRequest: { number: 12, isDraft: false } } } } }, + // The governed-review follow-up reads the readied pull's files (its own battery below); an empty list is an ungoverned diff. + [`GET /repos/${REPO}/pulls/12/files`]: { status: 200, json: [] }, }, { file: paceFile }); - t('pr_ready reads the node id, then POSTs the mutation with it', [ready.exit, ready.seen.map((s) => s.call), ready.seen[2].body.variables], [EXIT_OK, [PERM, `GET /repos/${REPO}/pulls/12`, 'POST /graphql'], { id: 'PR_kwDO' }]); + t('pr_ready reads the node id, then POSTs the mutation with it — then the governed-review follow-up reads the files', [ready.exit, ready.seen.map((s) => s.call), ready.seen[2].body.variables], [EXIT_OK, [PERM, `GET /repos/${REPO}/pulls/12`, 'POST /graphql', `GET /repos/${REPO}/pulls/12/files`], { id: 'PR_kwDO' }]); t('…the query names the mutation and the row reads the answer', [ready.seen[2].body.query.includes('markPullRequestReadyForReview'), ready.rows[0].result], [true, '#12 · ready']); const merge = await run(base([{ op: 'automerge_enable', pull: 12 }]), { ...allowed, @@ -977,6 +1157,8 @@ export async function selfTest() { const ASSIGN = (n) => `POST /repos/${REPO}/issues/${n}/assignees`; const create = (assignees, extra = []) => base([{ op: 'pr_create', title: 'T', head: 'claude/issue-1-x', base: 'main', body: 'B', ...(assignees ? { assignees } : {}) }, ...extra]); const opened = { status: 201, json: { number: 22700, html_url: `https://github.test/${REPO}/pull/22700` } }; + // The governed-review follow-up reads the created pull's files after the legs land (its own battery below); an empty list is an ungoverned diff. + const NOFILES = { [`GET /repos/${REPO}/pulls/22700/files`]: { status: 200, json: [] } }; const assignedTo = (logins, n = 22700) => ({ status: 201, json: { number: n, html_url: `https://github.test/${REPO}/pull/${n}`, assignees: logins.map((login) => ({ login })) } }); const legs = OPS.pr_create.requests({ op: 'pr_create', title: 'T', head: 'h', base: 'main', assignees: ['marchtian'] }, REPO); @@ -984,10 +1166,10 @@ export async function selfTest() { t('the fill: the placeholder becomes the positive integer the previous answer carries at that field, and nothing else about the leg changes', fillFromAnswer(legs[1], { call: 'POST /x', status: 201, json: { number: 22700 } }).req, { ...legs[1], path: `/repos/${REPO}/issues/22700/assignees` }); t('…a previous answer without it — absent, a numeric string, zero, a float — or no previous answer at all, is a refusal to send that names the field', [{ json: {} }, { json: { number: '22700' } }, { json: { number: 0 } }, { json: { number: 1.5 } }, null].map((prev) => { const f = fillFromAnswer(legs[1], prev ? { call: 'POST /x', status: 201, ...prev } : null).fail; return typeof f === 'string' && f.includes('number'); }), [true, true, true, true, true]); - const ok = await run(create(['marchtian']), { ...allowed, [PULLS]: opened, [ASSIGN(22700)]: assignedTo(['marchtian']) }, { file: paceFile }); + const ok = await run(create(['marchtian']), { ...allowed, ...NOFILES, [PULLS]: opened, [ASSIGN(22700)]: assignedTo(['marchtian']) }, { file: paceFile }); t('the run: the create, then the assignee POST at the number the create ANSWERED, both under the App token — exit 0', [ok.exit, writes(ok.seen).map((s) => [s.call, s.body, s.auth === `Bearer ${TOKEN}`])], [EXIT_OK, [[PULLS, { title: 'T', head: 'claude/issue-1-x', base: 'main', body: 'B', draft: true }, true], [ASSIGN(22700), { assignees: ['marchtian'] }, true]]]); - t("…and the leg's row prints the pull and the assignees as the platform lists them", ok.rows.map((r) => [r.action, r.op, r.call, r.status, r.result]), [[1, 'pr_create', PULLS, 201, `#22700 https://github.test/${REPO}/pull/22700`], [1, 'pr_create', ASSIGN(22700), 201, `#22700 https://github.test/${REPO}/pull/22700 · assignees marchtian`]]); - const plain = await run(create(null), { ...allowed, [PULLS]: opened }, { file: paceFile }); + t("…and the leg's row prints the pull and the assignees as the platform lists them, the governed-review verdict row behind it", ok.rows.map((r) => [r.action, r.op, r.call, r.status, r.result]), [[1, 'pr_create', PULLS, 201, `#22700 https://github.test/${REPO}/pull/22700`], [1, 'pr_create', ASSIGN(22700), 201, `#22700 https://github.test/${REPO}/pull/22700 · assignees marchtian`], [1, 'pr_create', `GET /repos/${REPO}/pulls/22700/files`, 200, 'governed review: no governed row hit across 0 file(s) — no review requested']]); + const plain = await run(create(null), { ...allowed, ...NOFILES, [PULLS]: opened }, { file: paceFile }); t('no assignees, no second request: the create alone, as before', [plain.exit, writes(plain.seen).map((s) => s.call)], [EXIT_OK, [PULLS]]); const dropped = await run(create(['marchtian', 'nobody-here'], [{ op: 'comment', issue: 1, body: 'after' }]), { ...allowed, [PULLS]: opened, [ASSIGN(22700)]: assignedTo(['marchtian']), [`POST /repos/${REPO}/issues/1/comments`]: { status: 201, json: { id: 1 } } }); t('⛔ a 201 whose answer lists fewer logins than sent is a FAILED action naming the one not assigned — never a quiet partial — and the stroke stops there', [dropped.exit, dropped.rows[1]?.result.includes('nobody-here not assigned'), dropped.rows[1]?.result.includes('lists marchtian'), writes(dropped.seen).length, dropped.summary.includes('1 later action(s) NOT attempted')], [EXIT_ACTION_FAILED, true, true, 2, true], dropped.rows[1]?.result); @@ -1005,10 +1187,10 @@ export async function selfTest() { const both = OPS.pr_create.requests({ op: 'pr_create', title: 'T', head: 'h', base: 'main', assignees: ['marchtian'], labels: ['skip-changeset'] }, REPO); t('the table half: with labels the row carries a POST to the issue-labels path at the placeholder, fromAnswer number, the names as labelled — after the assignees leg when both are sent', both.map((r) => [r.verb, r.path, r.fromAnswer ?? null, r.assigned ?? null, r.labelled ?? null]), [['POST', `/repos/${REPO}/pulls`, null, null, null], ['POST', `/repos/${REPO}/issues/${answerPlaceholder('number')}/assignees`, 'number', ['marchtian'], null], ['POST', `/repos/${REPO}/issues/${answerPlaceholder('number')}/labels`, 'number', null, ['skip-changeset']]]); // The assignee answer here carries a DIFFERENT number: the label leg must still land on the pull the CREATE answered. - const labelled = await run(createWith({ assignees: ['marchtian'], labels: ['skip-changeset'] }), { ...allowed, [PULLS]: opened, [ASSIGN(22700)]: assignedTo(['marchtian'], 99999), [LABEL(22700)]: labelsOn(['skip-changeset', 'documentation']) }, { file: paceFile }); + const labelled = await run(createWith({ assignees: ['marchtian'], labels: ['skip-changeset'] }), { ...allowed, ...NOFILES, [PULLS]: opened, [ASSIGN(22700)]: assignedTo(['marchtian'], 99999), [LABEL(22700)]: labelsOn(['skip-changeset', 'documentation']) }, { file: paceFile }); t("the run: the create, the assignee POST, then the label POST — at the number the CREATE answered, not the assignee answer's — exit 0", [labelled.exit, writes(labelled.seen).map((s) => [s.call, s.body])], [EXIT_OK, [[PULLS, { title: 'T', head: 'claude/issue-1-x', base: 'main', body: 'B', draft: true }], [ASSIGN(22700), { assignees: ['marchtian'] }], [LABEL(22700), { labels: ['skip-changeset'] }]]]); t("…and the leg's row prints the labels as the platform lists them — more than sent (the Auto Label workflow's additive writes) is a landing, never a failure", labelled.rows[2], { action: 1, op: 'pr_create', call: LABEL(22700), status: 200, result: '2 label(s) now on the pull · labels skip-changeset, documentation' }); - const labelsOnly = await run(createWith({ labels: ['skip-changeset'] }), { ...allowed, [PULLS]: opened, [LABEL(22700)]: labelsOn(['skip-changeset']) }, { file: paceFile }); + const labelsOnly = await run(createWith({ labels: ['skip-changeset'] }), { ...allowed, ...NOFILES, [PULLS]: opened, [LABEL(22700)]: labelsOn(['skip-changeset']) }, { file: paceFile }); t('labels without assignees: the create, then the label POST alone', [labelsOnly.exit, writes(labelsOnly.seen).map((s) => s.call)], [EXIT_OK, [PULLS, LABEL(22700)]]); const unlabelled = await run(createWith({ labels: ['skip-changeset', 'no-such-label'] }, [{ op: 'comment', issue: 1, body: 'after' }]), { ...allowed, [PULLS]: opened, [LABEL(22700)]: labelsOn(['skip-changeset']), [`POST /repos/${REPO}/issues/1/comments`]: { status: 201, json: { id: 1 } } }); t('⛔ a 200 whose answer lacks a name sent is a FAILED action naming it — never a quiet partial — and the stroke stops there', [unlabelled.exit, unlabelled.rows[1]?.result.includes('no-such-label not applied'), unlabelled.rows[1]?.result.includes('lists skip-changeset'), writes(unlabelled.seen).length, unlabelled.summary.includes('1 later action(s) NOT attempted')], [EXIT_ACTION_FAILED, true, true, 2, true], unlabelled.rows[1]?.result); @@ -1018,6 +1200,83 @@ export async function selfTest() { t('a create whose answer carries no number: the label leg is NOT SENT, exit 5, the later action never attempted', [blankLabel.exit, blankLabel.rows.map((r) => r.status), writes(blankLabel.seen).map((s) => s.call)], [EXIT_ACTION_FAILED, [201, 'not sent'], [PULLS]]); } + // ── the governed-review follow-up ─────────────────────────────────────── + battery("the governed-review follow-up: after pr_create (at the number the create answered) and after pr_ready, the pull's file list is read in full; a Tier H hit requests review from every GOVERNED_APPROVERS account not already requested, assigned or reviewed — the pull's own author assigned instead, never requested; a Tier S or ungoverned diff sends nothing and says so; an unreadable or truncated list, an unreadable pull or review list, or a refused write is a FAILED action"); + { + const PULLS = `POST /repos/${REPO}/pulls`; + const N = 25050; + const FILES = `GET /repos/${REPO}/pulls/${N}/files`; + const PULL = `GET /repos/${REPO}/pulls/${N}`; + const REVIEWS = `GET /repos/${REPO}/pulls/${N}/reviews`; + const REQUEST = `POST /repos/${REPO}/pulls/${N}/requested_reviewers`; + const ASSIGN = `POST /repos/${REPO}/issues/${N}/assignees`; + // The approvers are READ from the constant, never restated: a change there moves every expectation here with it. + const [A1, A2] = GOVERNED_APPROVERS; + const URL = `https://github.test/${REPO}/pull/${N}`; + const opened = { status: 201, json: { number: N, html_url: URL } }; + const files = (...names) => ({ status: 200, json: names.map((f) => (typeof f === 'string' ? { filename: f } : f)) }); + const pullOf = ({ author = 'objectstack-fleet[bot]', requested = [], assignees = [] } = {}) => ({ status: 200, json: { number: N, node_id: 'PR_kwDO25050', user: { login: author }, requested_reviewers: requested.map((login) => ({ login })), assignees: assignees.map((login) => ({ login })) } }); + const reviewsBy = (...logins) => ({ status: 200, json: logins.map((login) => ({ state: 'APPROVED', user: { login } })) }); + const requestedAnswer = (logins) => ({ status: 201, json: { number: N, html_url: URL, requested_reviewers: logins.map((login) => ({ login })) } }); + const assignedAnswer = (logins) => ({ status: 201, json: { number: N, html_url: URL, assignees: logins.map((login) => ({ login })) } }); + const create = (extra = []) => base([{ op: 'pr_create', title: 'T', head: 'claude/issue-1-x', base: 'main', body: 'B' }, ...extra]); + const tierH = { [PULLS]: opened, [FILES]: files('docs/adr/0140-x.md', 'packages/core/src/a.ts'), [PULL]: pullOf(), [REVIEWS]: reviewsBy() }; + const pick = (plan) => [plan.tier, plan.toRequest, plan.toAssign, plan.covered]; + + // The table half. + t('the table half: GOVERNED_REVIEW_OPS is exactly pr_create and pr_ready — derived from the rows, never listed — and both spend issues (the assignee fallback) as alsoSpends', [GOVERNED_REVIEW_OPS, GOVERNED_REVIEW_OPS.every((op) => (OPS[op].alsoSpends ?? []).includes('issues'))], [['pr_create', 'pr_ready'], true]); + t('…governedReviewRequests spells ONE requested_reviewers POST carrying the logins as requested and ONE issue-assignees POST carrying them as assigned; an empty list spells nothing', [governedReviewRequests(REPO, N, { reviewers: [A1, A2], assignees: [A2] }).map((r) => [r.verb, r.path, r.body, r.requested ?? null, r.assigned ?? null]), governedReviewRequests(REPO, N), governedReviewRequests(REPO, N, { reviewers: [], assignees: [] })], [[['POST', `/repos/${REPO}/pulls/${N}/requested_reviewers`, { reviewers: [A1, A2] }, [A1, A2], null], ['POST', `/repos/${REPO}/issues/${N}/assignees`, { assignees: [A2] }, null, [A2]]], [], []]); + // The plan — pure, over the real register. + t('the plan: a Tier H path with nobody covered requests every GOVERNED_APPROVERS account, in its order, and assigns nobody', pick(governedReviewPlan({ paths: ['docs/adr/0140-x.md', 'README.md'], repo: REPO })), [GOVERNED_TIER_H, [A1, A2], [], []]); + t("…the pull's own author, when it is an uncovered approver, is ASSIGNED and never requested — the other approver is requested as before", pick(governedReviewPlan({ paths: ['AGENTS.md'], repo: REPO, author: A2 })), [GOVERNED_TIER_H, [A1], [A2], []]); + t('…an approver already requested, assigned or holding a submitted review is covered — the H43 union — and matched case-insensitively', pick(governedReviewPlan({ paths: ['skills/x/SKILL.md'], repo: REPO, author: A2, requested: [A1.toUpperCase()], assigned: [A2] })), [GOVERNED_TIER_H, [], [], [A1, A2]]); + t("…a submitted review covers its author's slot; a pending (unsubmitted) one does not", [reviewedLogins([{ state: 'COMMENTED', user: { login: 'x' } }, { state: 'PENDING', user: { login: 'y' } }]), governedReviewPlan({ paths: ['CLAUDE.md'], repo: REPO, reviewed: [A1] }).toRequest], [['x'], [A2]]); + t('…a Tier S diff (the .claude/** row alone) is tier S and sends nothing; an ungoverned diff is tier null; a mixed S+H diff is H', [governedReviewPlan({ paths: ['.claude/skills/pm-dispatch/SKILL.md'], repo: REPO }), governedReviewPlan({ paths: ['packages/core/src/a.ts'], repo: REPO }), governedReviewPlan({ paths: ['.claude/agents/os-dev.md', 'docs/NORTH-STAR.md'], repo: REPO })].map((p) => [p.tier, p.toRequest.length + p.toAssign.length]), [[GOVERNED_TIER_S, 0], [null, 0], [GOVERNED_TIER_H, 2]]); + t('…a target outside GOVERNED_REPOS has no governing row: repoId null, tier null, nothing to send', (({ repoId, tier, toRequest }) => [repoId, tier, toRequest])(governedReviewPlan({ paths: ['docs/adr/0001-x.md'], repo: 'objectstack-ai/elsewhere' })), [null, null, []]); + t('…the register is scoped per repository: a workflow path is Tier H on objectui and ungoverned on objectos', [governedReviewPlan({ paths: ['.github/workflows/ci.yml'], repo: 'objectstack-ai/objectui' }).tier, governedReviewPlan({ paths: ['.github/workflows/ci.yml'], repo: 'objectstack-ai/objectos' }).tier], [GOVERNED_TIER_H, null]); + t('…a rename OUT of a governed path counts by its old name (pullPathsFrom reads previous_filename)', governedReviewPlan({ paths: pullPathsFrom([{ filename: 'docs/agents.md', previous_filename: 'AGENTS.md' }]).paths, repo: REPO }).tier, GOVERNED_TIER_H); + t('…the approver list is INJECTED from GOVERNED_APPROVERS, never baked in: another list is requested in its place', governedReviewPlan({ paths: ['AGENTS.md'], repo: REPO, approvers: ['someone-else'] }).toRequest, ['someone-else']); + t('…the verdict words name the tier, the rows hit, who is covered and what is sent', [governedReviewText(governedReviewPlan({ paths: ['docs/adr/0140-x.md'], repo: REPO, requested: [A1] }), { repo: REPO, files: 1 }), governedReviewText(governedReviewPlan({ paths: ['x.ts'], repo: REPO }), { repo: REPO, files: 1 })], [`governed review: Tier H (\`docs/adr/**\`×1) — \`${A1}\` already covered; requesting \`${A2}\``, 'governed review: no governed row hit across 1 file(s) — no review requested']); + + // The run — pr_create. + const both = await run(create(), { ...allowed, ...tierH, [REQUEST]: requestedAnswer([A1, A2]) }); + t('⭐ pr_create on a Tier H diff: the create, then ONE requested_reviewers POST naming BOTH approvers, at the number the create ANSWERED, under the App token — exit 0', [both.exit, writes(both.seen).map((s) => [s.call, s.body, s.auth === `Bearer ${TOKEN}`])], [EXIT_OK, [[PULLS, { title: 'T', head: 'claude/issue-1-x', base: 'main', body: 'B', draft: true }, true], [REQUEST, { reviewers: [A1, A2] }, true]]], both.logs.join(' | ')); + t('…the file list, the pull and its reviews were read first, in that order, and the rows say the verdict and the landing', [both.seen.map((s) => s.call), both.rows.map((r) => [r.action, r.op, r.call, r.status, r.result])], [[PERM, PULLS, FILES, PULL, REVIEWS, REQUEST], [[1, 'pr_create', PULLS, 201, `#${N} ${URL}`], [1, 'pr_create', FILES, 200, `governed review: Tier H (\`docs/adr/**\`×1) — requesting \`${A1}\`, \`${A2}\``], [1, 'pr_create', REQUEST, 201, `#${N} ${URL} · review requested from ${A1}, ${A2}`]]]); + const none = await run(create([{ op: 'comment', issue: 1, body: 'after' }]), { ...allowed, [PULLS]: opened, [FILES]: files('packages/core/src/a.ts', 'README.md'), [`POST /repos/${REPO}/issues/1/comments`]: { status: 201, json: { id: 1 } } }); + t('a non-governed diff: the file list is read, NOTHING is requested, the row says so, and the later action runs', [none.exit, writes(none.seen).map((s) => s.call), none.rows[1]?.result, none.seen.some((s) => s.call === PULL)], [EXIT_OK, [PULLS, `POST /repos/${REPO}/issues/1/comments`], 'governed review: no governed row hit across 2 file(s) — no review requested', false]); + const tierS = await run(create(), { ...allowed, [PULLS]: opened, [FILES]: files('.claude/skills/pm-dispatch/SKILL.md') }); + t("a Tier S diff (.claude/** alone) requests nobody — it lands on a seat's review of record", [tierS.exit, writes(tierS.seen).map((s) => s.call), tierS.rows[1]?.result], [EXIT_OK, [PULLS], "governed review: Tier S (`.claude/**`×1) lands on a seat's review of record — no review requested"]); + const authored = await run(create(), { ...allowed, ...tierH, [PULL]: pullOf({ author: A2 }), [REQUEST]: requestedAnswer([A1]), [ASSIGN]: assignedAnswer([A2]) }); + t('⭐ the author-identity branch: a pull authored by an approver requests the OTHER approver and ASSIGNS the author — never a request naming the author', [authored.exit, writes(authored.seen).map((s) => [s.call, s.body]), authored.rows.map((r) => r.result).slice(1)], [EXIT_OK, [[PULLS, { title: 'T', head: 'claude/issue-1-x', base: 'main', body: 'B', draft: true }], [REQUEST, { reviewers: [A1] }], [ASSIGN, { assignees: [A2] }]], [`governed review: Tier H (\`docs/adr/**\`×1) — requesting \`${A1}\` and assigning the author \`${A2}\` (a request naming the author is a 422)`, `#${N} ${URL} · review requested from ${A1}`, `#${N} ${URL} · assignees ${A2}`]]); + const covered = await run(create(), { ...allowed, ...tierH, [PULL]: pullOf({ requested: [A1] }), [REVIEWS]: reviewsBy(A2) }); + t('⭐ idempotent: one approver already requested and the other already reviewed — ZERO writes beyond the create, the row says nothing to send, exit 0', [covered.exit, writes(covered.seen).map((s) => s.call), covered.rows[1]?.result], [EXIT_OK, [PULLS], `governed review: Tier H (\`docs/adr/**\`×1) — \`${A1}\`, \`${A2}\` already requested, assigned or reviewed; nothing to send`]); + const partial = await run(create(), { ...allowed, ...tierH, [PULL]: pullOf({ assignees: [A2] }), [REQUEST]: requestedAnswer([A1]) }); + t('…an assigned approver counts as covered, and only the missing one is requested', [partial.exit, writes(partial.seen).map((s) => [s.call, s.body]).slice(1)], [EXIT_OK, [[REQUEST, { reviewers: [A1] }]]]); + // The run — pr_ready. + const readyH = await run(base([{ op: 'pr_ready', pull: N }, { op: 'comment', issue: 1, body: 'after' }]), { ...allowed, [PULL]: pullOf(), 'POST /graphql': { status: 200, json: { data: { markPullRequestReadyForReview: { pullRequest: { number: N, isDraft: false } } } } }, [FILES]: files('skills/objectstack-ui/SKILL.md'), [REVIEWS]: reviewsBy(), [REQUEST]: requestedAnswer([A1, A2]), [`POST /repos/${REPO}/issues/1/comments`]: { status: 201, json: { id: 1 } } }); + t('⭐ pr_ready on a Tier H pull: the mutation lands, then the same follow-up at the pull the action names — both approvers requested, the later action still runs', [readyH.exit, readyH.seen.map((s) => s.call), writes(readyH.seen)[1]?.body], [EXIT_OK, [PERM, PULL, 'POST /graphql', FILES, PULL, REVIEWS, REQUEST, `POST /repos/${REPO}/issues/1/comments`], { reviewers: [A1, A2] }]); + const readyFailed = await run(base([{ op: 'pr_ready', pull: N }]), { ...allowed, [PULL]: pullOf(), 'POST /graphql': { status: 200, json: { data: { markPullRequestReadyForReview: { pullRequest: { number: N, isDraft: true } } } } }, [FILES]: files('docs/adr/0140-x.md') }); + t('a pr_ready that did not land runs no follow-up: no file list read, no request', [readyFailed.exit, readyFailed.seen.some((s) => s.call === FILES)], [EXIT_ACTION_FAILED, false]); + // The refusals. + const unread = await run(create([{ op: 'comment', issue: 1, body: 'after' }]), { ...allowed, [PULLS]: opened, [FILES]: { status: 503, json: { message: 'down' } }, [`POST /repos/${REPO}/issues/1/comments`]: { status: 201, json: { id: 1 } } }); + t('⛔ an unreadable file list is a FAILED action — the pull is open (its row 201), the follow-up row names the read, exit 5, the later action never attempted — never a quiet "not governed"', [unread.exit, unread.rows.map((r) => [r.status, r.result.startsWith('FAILED') ? r.result.includes('file list could not be read') : r.result]), writes(unread.seen).map((s) => s.call), unread.summary.includes('1 later action(s) NOT attempted')], [EXIT_ACTION_FAILED, [[201, `#${N} ${URL}`], [503, true]], [PULLS], true], unread.rows[1]?.result); + const page = (n) => files(...Array.from({ length: n }, (_, i) => `packages/core/src/f${i}.ts`)); + const twoPages = await run(create(), { ...allowed, [PULLS]: opened, [FILES]: [page(100), files('docs/adr/0140-x.md')], [PULL]: pullOf(), [REVIEWS]: reviewsBy(), [REQUEST]: requestedAnswer([A1, A2]) }); + t('the walk reads EVERY page: a governed path on the second page is found, and the pages were asked in order', [twoPages.exit, twoPages.seen.filter((s) => s.call === FILES).length, writes(twoPages.seen).map((s) => s.call)], [EXIT_OK, 2, [PULLS, REQUEST]]); + const truncated = await run(create(), { ...allowed, [PULLS]: opened, [FILES]: Array.from({ length: GOVERNED_REVIEW_FILES_PAGE_CAP }, () => page(100)) }); + t('⛔ a walk that would need more than the page cap is a FAILED action naming the cap — a truncated list is a subset, never "nothing governed"', [truncated.exit, truncated.seen.filter((s) => s.call === FILES).length, truncated.rows[1]?.result.includes(`${GOVERNED_REVIEW_FILES_PAGE_CAP}-page cap`)], [EXIT_ACTION_FAILED, GOVERNED_REVIEW_FILES_PAGE_CAP, true], truncated.rows[1]?.result); + const short = await run(create(), { ...allowed, ...tierH, [REQUEST]: requestedAnswer([A1]) }); + t('⛔ a 201 whose answer lists fewer reviewers than sent is a FAILED action naming the one not requested — never a quiet partial', [short.exit, short.rows[2]?.result.includes(`${A2} not requested`), short.rows[2]?.result.includes(`lists ${A1}`)], [EXIT_ACTION_FAILED, true, true], short.rows[2]?.result); + const refused = await run(create(), { ...allowed, ...tierH, [REQUEST]: { status: 422, json: { message: 'Reviews may only be requested from collaborators.' } } }); + t("a refused request is a FAILED action with the platform's sentence, the pull already open", [refused.exit, refused.rows.map((r) => r.status), refused.rows[2]?.result.includes('Reviews may only be requested from collaborators')], [EXIT_ACTION_FAILED, [201, 200, 422], true]); + const noPull = await run(create(), { ...allowed, ...tierH, [PULL]: { status: 503, json: { message: 'down' } } }); + const noReviews = await run(create(), { ...allowed, ...tierH, [REVIEWS]: { status: 503, json: { message: 'down' } } }); + t('an unreadable pull or review list on a Tier H hit is FAILED naming the read — never a request sent blind, never a quiet skip', [noPull.exit, noPull.rows[1]?.result.includes('could not be re-read'), noReviews.exit, noReviews.rows[1]?.result.includes('reviews could not be read'), [...noPull.seen, ...noReviews.seen].some((s) => s.call === REQUEST)], [EXIT_ACTION_FAILED, true, EXIT_ACTION_FAILED, true, false]); + const numberless = await run(create([{ op: 'comment', issue: 1, body: 'after' }]), { ...allowed, [PULLS]: { status: 201, json: { html_url: 'https://github.test/x' } }, [`POST /repos/${REPO}/issues/1/comments`]: { status: 201, json: { id: 1 } } }); + t('a create whose answer carries no number: the follow-up cannot run — FAILED, its row says not sent, no file list read, the later action never attempted', [numberless.exit, numberless.rows[1]?.status, numberless.rows[1]?.result.includes('without a positive integer number'), numberless.seen.some((s) => s.call.includes('/files')), writes(numberless.seen).map((s) => s.call)], [EXIT_ACTION_FAILED, 'not sent', true, false, [PULLS]]); + t('no annotation is emitted for the follow-up — neither row is an annotated op', [both.annotations, authored.logs.some((l) => l.startsWith('::notice'))], [[], false]); + } + // ── the transfer ──────────────────────────────────────────────────────── battery('the transfer: the sender gated on BOTH repositories, both node ids first, a pull or a moved card refused before the mutation, landing only on the target, the remedy on failure'); { @@ -1202,6 +1461,8 @@ export async function selfTest() { answers: { ...allowed, [`GET /repos/${REPO}/pulls/12`]: { status: 200, json: { number: 12, node_id: 'PR_kwDO' } }, + // The governed-review follow-up after pr_ready reads the pull's files (its own battery); an ungoverned diff adds no write, so the gap reading is unchanged. + [`GET /repos/${REPO}/pulls/12/files`]: { status: 200, json: [] }, 'POST /graphql': [ { status: 200, json: { data: { markPullRequestReadyForReview: { pullRequest: { number: 12, isDraft: false } } } } }, { status: 200, json: { data: { enablePullRequestAutoMerge: { pullRequest: { number: 12, autoMergeRequest: { enabledAt: 'now', mergeMethod: 'SQUASH' } } } } } }, @@ -1276,6 +1537,8 @@ export async function selfTest() { [`POST /repos/${REPO}/issues/1/labels`]: { status: 200, json: [{ name: 'x' }] }, [`PATCH /repos/${REPO}/issues/1`]: { status: 200, json: { number: 1, html_url: issueUrl(REPO, 1) } }, [`POST /repos/${REPO}/pulls`]: { status: 201, json: { number: 9, html_url: `https://github.test/${REPO}/pull/9` } }, + // The governed-review follow-up reads the created pull's files (its own battery); an empty list is an ungoverned diff, and it emits no note either. + [`GET /repos/${REPO}/pulls/9/files`]: { status: 200, json: [] }, }, { file: paceFile }); t('no other op emits one — a comment, labels, a patch and a NEW PULL REQUEST all landed, zero workflow commands, zero notes', [quiet.exit, notices(quiet), quiet.annotations], [EXIT_OK, [], []]); @@ -1371,7 +1634,8 @@ export async function selfTest() { console.log( `✓ fleet-write/execute self-test: ${cases.length} cases pass across ${declared.length} batteries — the sender gate from the target repo's answer, ` + 'every op as the request the table declares, stop at the first failure with later actions untouched, the idempotent label DELETE, the GraphQL ' + - 'ops behind a node-id read, a pull mutation landing only when its answer shows the state it asked for (armed or queued, off, ready, draft), a base sync confirmed by the head leaving its expected sha or measured as a no-op, an assignees leg sent to the number the create answered and landing only when its answer lists every login sent, a transfer gated on both repositories and landing only on its target, one summary row per request, ' + + 'ops behind a node-id read, a pull mutation landing only when its answer shows the state it asked for (armed or queued, off, ready, draft), a base sync confirmed by the head leaving its expected sha or measured as a no-op, an assignees leg sent to the number the create answered and landing only when its answer lists every login sent, ' + + 'a governed-review follow-up after pr_create and pr_ready that reads the pull\'s whole file list and, on a Tier H hit alone, requests every GOVERNED_APPROVERS account not already covered (the author assigned instead), zero writes when covered and a FAILED action when the list cannot be read whole, a transfer gated on both repositories and landing only on its target, one summary row per request, ' + "one annotation per created or moved card carrying the number the platform answered, read back by the reader's own parser, a stroke at or " + 'above the batch threshold announced as ONE rule ⑤ batch and paced at the batch gap (one the job cannot hold refused with zero requests), and a known ' + 'token that came back out of NO summary, log or error.', diff --git a/scripts/pm/fleet-write/ops.mjs b/scripts/pm/fleet-write/ops.mjs index 6ad2cb66976..33d690ac58c 100644 --- a/scripts/pm/fleet-write/ops.mjs +++ b/scripts/pm/fleet-write/ops.mjs @@ -84,11 +84,56 @@ * neither is the first request of its action; `execute.mjs --self-test` pins * the fill, the refusal to send and the landing. * + * ## The governed-review follow-up — `pr_create` and `pr_ready` on a Tier H diff + * + * A Tier H governed surface (`GOVERNED_SURFACES` rows with tier `H` in + * `check-governed-merges.mjs`: `docs/adr/**`, `skills/**`, `AGENTS.md`, + * `CLAUDE.md`, `docs/NORTH-STAR.md`, `.github/workflows/**`) lands only on an + * APPROVED review by an account in `GOVERNED_APPROVERS` + * (`check-governed-queue-guard.mjs`, the ONE list of those accounts). The + * platform summons a code owner only when a pull is opened READY or marked + * ready for review — measured on this repository's own Tier H pulls, the + * CODEOWNERS request arrived at `ready_for_review` and never at the draft + * create — and every relay `pr_create` is a draft (forced above), held draft + * until that approval. So without this follow-up a seat-driven Tier H pull + * waits on an approver nobody told. The rows `pr_create` and `pr_ready` + * therefore carry `governedReview: true`, and the executor, once the row's own + * requests have landed, reads the pull's file list in full + * (`GOVERNED_REVIEW_FILES_PAGE_SIZE` per page, at most + * `GOVERNED_REVIEW_FILES_PAGE_CAP` pages — the endpoint's own ceiling; a walk + * that would need more is a truncated SUBSET and a FAILED action, never a + * quiet "not governed"), asks the register (`governedPathsIn` + + * `landingTierOf`, scoped to `payload.repo` through `repoIdForSlug`) and, on a + * Tier H hit, sends what `governedReviewRequests` spells: ONE + * `POST /repos/{o}/{r}/pulls/{n}/requested_reviewers` naming every + * `GOVERNED_APPROVERS` account not already requested, assigned or holding a + * submitted review (the union the H43 patrol reads as covered — so a re-run, + * a `pr_ready` after a `pr_create`, or the platform's own CODEOWNERS request + * landing first costs zero writes), and — because GitHub answers 422 to a + * request naming the pull's own author — ONE + * `POST /repos/{o}/{r}/issues/{n}/assignees` for the author when the author is + * itself a missing approver. Each answer is judged as the legs above are: + * `requested` against the pull's pending requests, `assigned` against its + * assignees; a 2xx lacking a login sent is a FAILED action naming it. A Tier + * S diff (`.claude/**` alone — it lands on a seat's review of record) and an + * ungoverned diff send nothing, and the row says which. The reads are unpaced + * GETs; the writes are paced like every other write verb and are outside the + * rule ⑤ count (`strokePacing` counts a stroke's spelled requests; these two + * are conditional and at most two per pull). Both rows spend `issues: write` + * for the assignee fallback (`alsoSpends`), a literal grant on every stroke. + * `execute.mjs --self-test` pins the plan, the writes and both refusals; + * `validate.mjs --self-test` pins the two rows and the two endpoints against + * the refused families. ⛔ This is an ACTION, not a judge: no gate, check-run + * or patrol row is added by it — the H43 row in `check-half-states.mjs` keeps + * reading the gap it closes. + * * ## The permissions the relay's token is narrowed to * * Read from GitHub's own permission ledger for server-to-server tokens * (`github/docs`, `src/github-apps/data/fpt-2022-11-28/server-to-server-permissions.json`): - * every `/issues/**` write here is `issues: write`; `/pulls/**` and the + * every `/issues/**` write here is `issues: write` — `pr_create`'s assignees + * and labels legs and the governed-review follow-up's assignee fallback on + * `pr_create` and `pr_ready` spend it as `alsoSpends`; `/pulls/**` and the * ready / draft GraphQL mutations are `pull-requests: write`; the two * auto-merge mutations (`enablePullRequestAutoMerge` / * `disablePullRequestAutoMerge`) additionally require `contents: write` — @@ -493,8 +538,15 @@ export function transferRemedy(source, target) { * `alsoSpends` (optional) grants the row spends beside `permission` — * each a literal mint input, never a stroke-scoped one * (`validate.mjs --self-test` pins it): `pr_create`'s - * assignees and labels legs, LATER requests, and `automerge_enable` on - * a pull that changes `.github/workflows/**` (header above); + * assignees and labels legs, LATER requests, the governed-review + * follow-up's assignee fallback on `pr_create` and `pr_ready`, and + * `automerge_enable` on a pull that changes `.github/workflows/**` + * (header above); + * `governedReview` (optional) true for a row whose landed requests are followed + * by the governed-review follow-up (header above) — `pr_create` + * and `pr_ready`, the two moments a seat opens or readies a pull; + * the executor reads the pull's files and sends what + * `governedReviewRequests` spells, on a Tier H hit alone; * `requests` the request descriptors the executor issues, in order: * `{ verb, path, body?, idempotent404?, graphql?, updateBranch?, fromAnswer?, assigned?, labelled? }` * — a `graphql` descriptor names the mutation and the pull (or @@ -581,6 +633,8 @@ export const OPS = Object.freeze({ alsoSpends: Object.freeze(['issues']), required: Object.freeze(['title', 'head', 'base']), optional: Object.freeze(['body', 'assignees', 'labels']), + // The draft open is the moment the platform's CODEOWNERS is silent (header: "The governed-review follow-up"). + governedReview: true, // `draft: true` is FORCED — a seat cannot open a ready PR through the relay. requests: (a, repo) => [ { verb: 'POST', path: `/repos/${repo}/pulls`, body: { title: a.title, head: a.head, base: a.base, ...('body' in a ? { body: a.body } : {}), draft: true } }, @@ -611,10 +665,13 @@ export const OPS = Object.freeze({ optional: Object.freeze([]), requests: (a, repo) => [{ verb: 'PUT', path: `${pulls(repo, a.pull)}/update-branch`, body: { expected_head_sha: a.expected_head_sha }, updateBranch: { pull: a.pull, expectedHeadSha: a.expected_head_sha } }], }), + // `issues` is the assignee fallback of the governed-review follow-up (header), a literal grant on every stroke. pr_ready: Object.freeze({ permission: 'pull-requests', + alsoSpends: Object.freeze(['issues']), required: Object.freeze(['pull']), optional: Object.freeze([]), + governedReview: true, requests: (a) => [{ verb: 'POST', path: '/graphql', graphql: { mutation: 'markPullRequestReadyForReview', query: PR_MUTATIONS.pr_ready, pull: a.pull, landed: PR_LANDED_STATE.pr_ready } }], }), pr_draft: Object.freeze({ @@ -667,6 +724,25 @@ export const OPS = Object.freeze({ export const OP_NAMES = Object.freeze(Object.keys(OPS)); +/** The rows the governed-review follow-up runs after (header) — derived from the table, never listed. */ +export const GOVERNED_REVIEW_OPS = Object.freeze(OP_NAMES.filter((op) => OPS[op].governedReview === true)); +/** The page size the follow-up walks a pull's file list at — GitHub's maximum for `/pulls/{n}/files`. */ +export const GOVERNED_REVIEW_FILES_PAGE_SIZE = 100; +/** The most pages it walks: the endpoint lists at most 3,000 files, so a walk past this cap is a truncated SUBSET. */ +export const GOVERNED_REVIEW_FILES_PAGE_CAP = 30; +/** + * The requests the governed-review follow-up sends for ONE pull, spelled here so the validator's refused-family + * pin covers them: a `requested_reviewers` POST naming `reviewers` (its answer, the pull, must list every login + * sent — `requested`) and an issue-assignees POST naming `assignees` (its answer must list every login sent — + * `assigned`, the same judgment as `pr_create`'s assignees leg). An empty list sends nothing. Pure. + */ +export function governedReviewRequests(repo, pull, { reviewers = [], assignees = [] } = {}) { + return [ + ...(reviewers.length ? [{ verb: 'POST', path: `${pulls(repo, pull)}/requested_reviewers`, body: { reviewers: [...reviewers] }, requested: [...reviewers] }] : []), + ...(assignees.length ? [{ verb: 'POST', path: `${issues(repo, pull)}/assignees`, body: { assignees: [...assignees] }, assigned: [...assignees] }] : []), + ]; +} + /** * The families no row may ever reach — pinned by `validate.mjs --self-test` * over every path every op can spell. A future row that reaches one reds diff --git a/scripts/pm/fleet-write/validate.mjs b/scripts/pm/fleet-write/validate.mjs index fbb6d6976eb..fdde9a9c7aa 100644 --- a/scripts/pm/fleet-write/validate.mjs +++ b/scripts/pm/fleet-write/validate.mjs @@ -113,8 +113,10 @@ import { WORKFLOW_DISPATCH_INPUTS, WORKFLOW_DISPATCH_PATH_SHAPE, WORKFLOW_DISPATCH_REF, + GOVERNED_REVIEW_OPS, answerPlaceholder, dispatchInputsOnTheWire, + governedReviewRequests, } from './ops.mjs'; const SELF_PATH = fileURLToPath(import.meta.url); @@ -915,9 +917,15 @@ export async function selfTest() { const spec = OPS[op]; const a = { op }; for (const k of [...spec.required, ...spec.optional]) a[k] = sample[k]; - return spec.requests(a, 'objectstack-ai/objectstack').map((r) => ({ op, ...r })); + return [ + ...spec.requests(a, 'objectstack-ai/objectstack').map((r) => ({ op, ...r })), + // The governed-review follow-up's conditional writes (the ops.mjs header) — spelled by the table, sent by the executor after the row lands. + ...(spec.governedReview ? governedReviewRequests('objectstack-ai/objectstack', 7, { reviewers: ['u'], assignees: ['u'] }).map((r) => ({ op, ...r, governedReview: true })) : []), + ]; }); t('every op produces at least one request', everyRequest.length >= OP_NAMES.length); + t('the governed-review follow-up runs after exactly pr_create and pr_ready — derived from the rows, never listed — and its two writes are a POST to requested_reviewers carrying requested and a POST to the additive issue-assignees endpoint carrying assigned, both in the family pin above', [GOVERNED_REVIEW_OPS, everyRequest.filter((r) => r.governedReview).map((r) => [r.op, r.verb, r.path, r.requested ?? null, r.assigned ?? null])], [['pr_create', 'pr_ready'], [['pr_create', 'POST', `/repos/${TARGET_OWNER}/objectstack/pulls/7/requested_reviewers`, ['u'], null], ['pr_create', 'POST', `/repos/${TARGET_OWNER}/objectstack/issues/7/assignees`, null, ['u']], ['pr_ready', 'POST', `/repos/${TARGET_OWNER}/objectstack/pulls/7/requested_reviewers`, ['u'], null], ['pr_ready', 'POST', `/repos/${TARGET_OWNER}/objectstack/issues/7/assignees`, null, ['u']]]]); + t('…an empty list spells no request, and a governedReview row is never a row whose spelled requests change: pr_create and pr_ready keep the request lists they always had', [governedReviewRequests('o/r', 7), OPS.pr_create.requests({ op: 'pr_create', title: 't', head: 'h', base: 'b' }, 'o/r').length, OPS.pr_ready.requests({ op: 'pr_ready', pull: 7 }, 'o/r').length], [[], 1, 1]); t('⛔ no request path reaches a refused family', everyRequest.filter((r) => REFUSED_PATH_FAMILIES.some((re) => re.test(r.path))).map((r) => `${r.op} ${r.path}`), []); t('⛔ the table\'s one PUT is pr_update_branch\'s update-branch, an action endpoint — no PUT reaches /labels or /assignees, the whole-set replaces', [everyRequest.filter((r) => r.verb === 'PUT').map((r) => `${r.op} ${r.path}`), everyRequest.some((r) => r.verb === 'PUT' && /\/(labels|assignees)$/.test(r.path))], [[`pr_update_branch /repos/${TARGET_OWNER}/objectstack/pulls/7/update-branch`], false]); t('every verb is one of POST, PATCH, PUT, DELETE', everyRequest.every((r) => ['POST', 'PATCH', 'PUT', 'DELETE'].includes(r.verb))); @@ -930,7 +938,7 @@ export async function selfTest() { t('…and neither is the FIRST request of its action — the answer they read exists before either is sent', OP_NAMES.filter((op) => { const a = { op }; for (const k of [...OPS[op].required, ...OPS[op].optional]) a[k] = sample[k]; return OPS[op].requests(a, 'o/r')[0].fromAnswer !== undefined; }), []); t('…every such leg is a POST to the one additive issue endpoint for its list — never a PATCH or PUT, the whole-set replaces', legs.map((r) => [r.verb, /\/(assignees|labels)$/.test(r.path)]), [['POST', true], ['POST', true]]); t('…their placeholder is the one spelling the executor fills, spelled once, and no path without fromAnswer carries a brace', [answerPlaceholder('number'), everyRequest.filter((r) => r.fromAnswer === undefined && /[{}]/.test(r.path)).map((r) => `${r.op} ${r.path}`)], ['{number}', []]); - t('every grant a row ALSO spends is a literal mint input of PERMISSIONS, never a stroke-scoped one — so the mint widens nothing for it; pr_create\'s issues write and automerge_enable\'s workflows write are the two such rows', [OP_NAMES.flatMap((op) => OPS[op].alsoSpends ?? []).every((name) => name in PERMISSIONS && !STROKE_SCOPED_PERMISSIONS.includes(name)), OP_NAMES.filter((op) => OPS[op].alsoSpends).map((op) => [op, [...OPS[op].alsoSpends]])], [true, [['pr_create', ['issues']], ['automerge_enable', ['workflows']]]]); + t('every grant a row ALSO spends is a literal mint input of PERMISSIONS, never a stroke-scoped one — so the mint widens nothing for it; pr_create\'s and pr_ready\'s issues write (the legs and the governed-review assignee fallback) and automerge_enable\'s workflows write are the three such rows', [OP_NAMES.flatMap((op) => OPS[op].alsoSpends ?? []).every((name) => name in PERMISSIONS && !STROKE_SCOPED_PERMISSIONS.includes(name)), OP_NAMES.filter((op) => OPS[op].alsoSpends).map((op) => [op, [...OPS[op].alsoSpends]])], [true, [['pr_create', ['issues']], ['pr_ready', ['issues']], ['automerge_enable', ['workflows']]]]); t('pr_create without assignees or labels is the one create request it always was — no later leg', OPS.pr_create.requests({ op: 'pr_create', title: 't', head: 'h', base: 'b' }, 'o/r').length, 1); t('…with labels alone, the create then the labels leg; with assignees alone, the create then the assignees leg', [OPS.pr_create.requests({ op: 'pr_create', title: 't', head: 'h', base: 'b', labels: ['skip-changeset'] }, 'o/r').map((r) => r.labelled ?? r.assigned ?? 'create'), OPS.pr_create.requests({ op: 'pr_create', title: 't', head: 'h', base: 'b', assignees: ['u'] }, 'o/r').map((r) => r.labelled ?? r.assigned ?? 'create')], [['create', ['skip-changeset']], ['create', ['u']]]); t('labels_remove is one directed DELETE per name, URL-encoded, idempotent on 404', OPS.labels_remove.requests({ op: 'labels_remove', issue: 1, labels: ['a b', 'c'] }, 'o/r').map((r) => [r.verb, r.path, r.idempotent404]), [['DELETE', '/repos/o/r/issues/1/labels/a%20b', true], ['DELETE', '/repos/o/r/issues/1/labels/c', true]]);