diff --git a/.claude/skills/docs/acceptance-criteria-verification.md b/.claude/skills/docs/acceptance-criteria-verification.md index 9f93e5e26..3ff3f9aa3 100644 --- a/.claude/skills/docs/acceptance-criteria-verification.md +++ b/.claude/skills/docs/acceptance-criteria-verification.md @@ -25,7 +25,10 @@ Canonical owner for the acceptance-criteria verification procedure run during th 4. **Verify each AC item** against the proposed changes on the current PR head. -5. **Automatic tick before composing:** the reviewers of the `acceptance-criteria` and `pr-checklist` angles list the exact label of each checklist item they verified at this head in their findings artifact's `verifiedItems` field (defined in the review agent's structured findings artifact). Fan-in carries the deduplicated union into the durable findings ledger. For a `pre_approval_gate` post with `--findings-ledger`, `upsert-checkpoint-verdict.mjs` ticks exactly those labels before it composes the verdict. In `fanout_fanin` mode the ledger must first pass the fan-out angle-coverage checks. In every mode its provenance must be consistent and record a fresh (not carried) `acceptance-criteria` or `pr-checklist` review. Otherwise nothing is ticked. The poster flips each matching `- [ ]` to `- [x]` in the PR body's unchecked Acceptance criteria and Definition of done items. When the spec-of-record still has unticked AC items (`uncheckedAcItems`), it also ticks each linked issue body that the coordination load read and that carries unticked AC items. A label outside those unchecked lists is not ticked. A linked issue whose body the coordination load could not fetch is skipped and stays spec-of-record unavailable. The match is exact-label, and the tick never blanket-checks and never unchecks. Each changed body gets one edit. The poster then reloads the refinement artifact, so the verdict composition and the `clean` guards read the ticked bodies. A failed fetch or edit of a body being ticked fails closed, and no verdict is posted. The tick does not run for `draft_gate`. On a matrix-representation issue the issue-side tick is a no-op, and the PR body's derived checklist is the ticked surface. +5. **Automatic tick before composing:** the reviewers of the `acceptance-criteria` and `pr-checklist` angles list the exact label of each checklist item they verified at this head in their findings artifact's `verifiedItems` field (defined in the review agent's structured findings artifact). Fan-in carries the deduplicated union into the durable findings ledger. For a `pre_approval_gate` post with `--findings-ledger`, `upsert-checkpoint-verdict.mjs` computes the ticked bodies as a dry run for exactly those labels before it composes the verdict. In `fanout_fanin` mode the ledger must first pass the fan-out angle-coverage checks. In every mode its provenance must be consistent and record a fresh (not carried) `acceptance-criteria` or `pr-checklist` review. Otherwise nothing is ticked. The poster flips each matching `- [ ]` to `- [x]` in the PR body's unchecked Acceptance criteria and Definition of done items. When the spec-of-record still has unticked AC items (`uncheckedAcItems`), it also ticks each linked issue body that the coordination load read and that carries unticked AC items. A label outside those unchecked lists is not ticked. A verified label that an open judge act item (including one with an evidence-only remedy) names (by label text or by an `ac:` / `dod:` id) stays unticked. An id that does not resolve in the pinned spec, or whose text matches no unchecked PR row, skips every PR-body row of its category; the skip applies to PR-body rows only, not to linked issue bodies. Do not hand-tick a row left unticked for an open act item with `tick-verified-checkboxes`. A linked issue whose body the coordination load could not fetch is skipped and stays spec-of-record unavailable. The match is exact-label, and the tick never blanket-checks and never unchecks. Each changed body gets one edit, written only after every refusal check and directly before the verdict is posted. The verdict composition and the `clean` guards read the dry-run ticked bodies. A failed fetch or edit of a body being ticked fails closed, and no verdict is posted. A refusal changes no body. A failed post (HTTP error) after the write keeps the ticks, and a rerun is idempotent. The tick does not run for `draft_gate`. On a matrix-representation issue the issue-side tick is a no-op, and the PR body's derived checklist is the ticked surface. + + + **CHECKLIST-SCOPE-ONE-MODEL:** every PR-body tick-integrity check reads checklist scope from `checklistLineScopes` in `packages/core/src/loop/issue-refinement-artifact.mjs`: the fence-aware scope of a line is `ac`, `dod`, or none, set by its nearest Acceptance criteria or Definition of done heading, so a Definition of done section nested under Acceptance criteria is `dod`. The category-scoped tick, the live checklist preview and the `edit-pr` untick guard key a checkbox row by that scope plus its label. A heading rename inside one scope keeps a checked row. A heading edit that moves a checked row into or out of a scope counts as losing it and needs `--allow-untick`. A second fence scanner or section classifier MUST NOT decide that scope for the PR body. The linked-issue tick keeps its own `acOnly` section filter. **Manual tick paths:** the automatic tick needs `verifiedItems`, and only the fan-out path writes them (per-angle artifacts through `consolidate-fanin`). It also reads the unchecked PR-body lists, which the coordination state fills only for a tracker-backed PR with a linked issue (step 7). The tick reads `verifiedItems` only from a trusted ledger: consistent provenance that records a fresh `acceptance-criteria` or `pr-checklist` review, plus fan-out angle coverage in `fanout_fanin` mode. Two paths therefore tick nothing automatically. The first is a post that carries no trusted `verifiedItems`: a post without `--findings-ledger`, a light-mode `inline_single_agent` round (with or without a ledger), or a ledger that lacks a fresh `acceptance-criteria` or `pr-checklist` review or fails the `fanout_fanin` angle coverage. The second is a lightweight or issue-less PR. On those paths the operator ticks each verified item before posting with `node scripts/github/tick-verified-checkboxes.mjs --repo --issue --pr --verified ...`. Without a tracker-backed issue, the operator passes `--pr` alone. The CLI applies the same exact-label tick. On a fan-out post for a tracker-backed PR with a linked issue, the CLI is a manual correction tool only. diff --git a/changes/2898-tick-integrity.md b/changes/2898-tick-integrity.md new file mode 100644 index 000000000..2d3be92d9 --- /dev/null +++ b/changes/2898-tick-integrity.md @@ -0,0 +1,3 @@ +### Fixed + +- `upsert-checkpoint-verdict` ticks verified rows after all refusal checks, leaves act-named rows unticked; `edit-pr` refuses a silent untick without `--allow-untick` (#2898) diff --git a/packages/core/src/loop/issue-refinement-artifact.mjs b/packages/core/src/loop/issue-refinement-artifact.mjs index f908a3684..de801f744 100644 --- a/packages/core/src/loop/issue-refinement-artifact.mjs +++ b/packages/core/src/loop/issue-refinement-artifact.mjs @@ -1436,6 +1436,42 @@ export function checklistSectionLineIndices(body, { acOnly = false } = {}) { return indices; } +/** + * Per-line checklist scope of a body, indexed like `body.split(/\r?\n/)`: + * "ac" or "dod" for a line under an Acceptance criteria / Definition of done + * section (nearest matching ancestor wins, so a `### Definition of done` + * nested under `## Acceptance criteria` is "dod"), null for any other line, + * undefined for a line inside fenced code. One scope model for every + * tick-integrity consumer (CHECKLIST-SCOPE-ONE-MODEL). + * + * @param {string} body + * @returns {Array<"ac"|"dod"|null|undefined>} + */ +export function checklistLineScopes(body) { + const lines = typeof body === "string" ? body.split(/\r?\n/u) : []; + const scopes = new Array(lines.length).fill(undefined); + const sections = parseMarkdownSections(body); + const mark = (from, texts, scope) => { + let fence = null; + texts.forEach((text, k) => { + const step = stepFence(fence, text); + fence = step.fence; + if (!step.insideFence) scopes[from + k] = scope; + }); + }; + mark(0, lines.slice(0, sections[0]?.line ?? lines.length), null); + const stack = []; + for (const section of sections) { + while (stack.length > 0 && stack.at(-1).level >= section.level) stack.pop(); + const own = ACCEPTANCE_SECTION_PATTERNS.some((p) => p.test(section.name)) + ? "ac" + : DOD_SECTION_PATTERNS.some((p) => p.test(section.name)) ? "dod" : (stack.at(-1)?.scope ?? null); + stack.push({ level: section.level, scope: own }); + mark(section.line + 1, section.bodyLines, own); + } + return scopes; +} + /** * Decide what an enqueue caller should do with a refinement-artifact result, * so an un-refined item never lands in the Next Up pickup column in the first diff --git a/scripts/github/edit-pr.mjs b/scripts/github/edit-pr.mjs index 6d3350069..a13c79d78 100644 --- a/scripts/github/edit-pr.mjs +++ b/scripts/github/edit-pr.mjs @@ -2,7 +2,7 @@ import { buildParseError, formatCliError, isDirectCliRun } from "../_core-helpers.mjs"; import { parsePrNumber, requireTokenValue, resolveBodyOrFile, runChild } from "../_cli-primitives.mjs"; import { parseRepoSlug } from "@dev-loops/core/github/repo-slug"; -import { detectGrillEmbedHeading } from "@dev-loops/core/loop/issue-refinement-artifact"; +import { checklistLineScopes, detectGrillEmbedHeading } from "@dev-loops/core/loop/issue-refinement-artifact"; import { detectClosingKeyword, resolveExpectedIssueFromPrContext, @@ -37,6 +37,10 @@ At least one edit: --enforce-grill Opt-in GRILL-SUBLOOP-NO-EMBED-SYNTHESIS (#1628): refuse a body that embeds grill transcript/synthesis/Q&A headings. + --allow-untick Allow a body that unticks or removes a checked + \`[x]\` checkbox row (a flip to \`[ ]\`, a delete + or a reworded label; refused by default, naming + the rows). --allow-cross-issue Waive the branch-derived closing-reference mismatch guard for a deliberate cross-issue reference. By default a new body whose closing @@ -78,6 +82,7 @@ export function parseEditPrCliArgs(argv) { base: { type: "string" }, "enforce-grill": { type: "boolean" }, "allow-cross-issue": { type: "boolean" }, + "allow-untick": { type: "boolean" }, ...JQ_OUTPUT_PARSE_OPTIONS, }, allowPositionals: true, @@ -97,6 +102,7 @@ export function parseEditPrCliArgs(argv) { base: undefined, enforceGrill: false, allowCrossIssue: false, + allowUntick: false, jq: undefined, silent: false, }; @@ -185,6 +191,10 @@ export function parseEditPrCliArgs(argv) { options.enforceGrill = true; continue; } + if (token.name === "allow-untick") { + options.allowUntick = token.value === undefined || token.value === true || /^(?:true|1)$/iu.test(String(token.value)); + continue; + } if (token.name === "allow-cross-issue") { // Boolean flag: bare enables; parseArgs also accepts `=value` for a // boolean, so honor an explicit `=false`/`=0` (disable) rather than @@ -297,6 +307,32 @@ async function buildEditArgs(options) { return { args, edited }; } +// Labels of `[x]` rows in currentBody that nextBody no longer carries as `[x]`: +// unticked, deleted or reworded rows, across every checkbox in the body. Counts +// per label, so one lost occurrence of a duplicated label is reported. +// The row grammar matches CHECKBOX_RE in tick-verified-checkboxes.mjs and the gate's checklist parser: +// an optional blockquote prefix, then a [-*+] or N. or N) marker. +const CHECKBOX_ROW = /^\s*(?:>\s*)*(?:[-*+]|\d+[.)])\s+\[([ xX])\]\s+(.+?)\s*$/u; +function untickedRows(currentBody, nextBody) { + // Fence-aware and scope-keyed (CHECKLIST-SCOPE-ONE-MODEL): the key is the row's AC/DoD/other scope plus its label, from the + // same checklistLineScopes the tick path uses. A rename inside one scope keeps the row; a heading edit that moves a + // checked row into or out of a scope loses it. Rows inside fenced code have no scope and do not count. + const checked = (body) => { + const scopes = checklistLineScopes(body); + return body.split(/\r?\n/u).flatMap((line, i) => { + const m = scopes[i] === undefined ? null : CHECKBOX_ROW.exec(line); + return m && m[1] !== " " ? [{ key: `${scopes[i]}\u0000${m[2]}`, label: m[2] }] : []; + }); + }; + const remaining = new Map(); + for (const r of checked(nextBody)) remaining.set(r.key, (remaining.get(r.key) ?? 0) + 1); + return checked(currentBody).filter((r) => { + const left = remaining.get(r.key) ?? 0; + if (left > 0) remaining.set(r.key, left - 1); + return left === 0; + }).map((r) => r.label); +} + export async function editPr( options, { env = process.env, ghCommand = "gh", run = runChild, fetchPrContext = defaultFetchPrContext, waiverWriter = false, currentBody } = {}, @@ -330,6 +366,10 @@ export async function editPr( } const waiverRefusal = waiverLineChangeRefusal({ currentBody: baseline, nextBody: guardBody, action: "write" }); if (waiverRefusal) throw new Error(waiverRefusal); + const unticked = untickedRows(baseline, guardBody); + if (unticked.length > 0 && !options.allowUntick) { + throw new Error(`CHECKLIST-SCOPE-ONE-MODEL: refusing to untick or remove ${unticked.length} checked row(s) on PR #${options.pr} (unticked, deleted or reworded): ${unticked.map((l) => JSON.stringify(l)).join("; ")}. Edit from the current body, or pass --allow-untick for a deliberate change.`); + } } if (!options.allowCrossIssue && detectClosingKeyword(guardBody)) { const ctx = await readPrContext(); diff --git a/scripts/github/tick-verified-checkboxes.mjs b/scripts/github/tick-verified-checkboxes.mjs index edb1269c1..99dc1175d 100644 --- a/scripts/github/tick-verified-checkboxes.mjs +++ b/scripts/github/tick-verified-checkboxes.mjs @@ -62,7 +62,7 @@ const CHECKBOX_RE = /^(\s*)((?:>\s*)*)([-*+]|\d+[.)])\s+\[([ xX])\]\s+(.*?)(\r?) // an already-`[x]` verified label is neither flipped nor reported as unmatched. // Returns { body, flipped, unmatched } where flipped = labels changed this call // and unmatched = verified labels absent from the body's checklist. An optional -// `lineInScope(index)` limits the flip to the lines it accepts; the CLI omits it. +// `lineInScope(index, label)` limits the flip to the lines it accepts; the CLI omits it. export function tickVerifiedCheckboxes(body, verifiedLabels, lineInScope = () => true) { const verified = new Set( (verifiedLabels ?? []).map((l) => String(l).trim()).filter((l) => l.length > 0), @@ -71,11 +71,11 @@ export function tickVerifiedCheckboxes(body, verifiedLabels, lineInScope = () => const found = new Set(); const lines = String(body).split("\n"); const nextLines = lines.map((line, index) => { - if (!lineInScope(index)) return line; const m = line.match(CHECKBOX_RE); if (!m) return line; const [, indent, quote, marker, mark, rest, cr] = m; const label = rest.trim(); + if (!lineInScope(index, label)) return line; if (!verified.has(label)) return line; found.add(label); if (mark !== " ") return line; // already checked — leave it, idempotent diff --git a/scripts/github/upsert-checkpoint-verdict.mjs b/scripts/github/upsert-checkpoint-verdict.mjs index 82c0ecbf4..fd2cdd1b4 100755 --- a/scripts/github/upsert-checkpoint-verdict.mjs +++ b/scripts/github/upsert-checkpoint-verdict.mjs @@ -15,7 +15,7 @@ import { ghGraphql as runGhGraphql, ghJson as runGhJson } from "@dev-loops/core/ import { minimizeSupersededGateReviews } from "./_minimize-superseded-verdicts.mjs"; import { loadPrGateCoordinationContext, loadRefinementArtifact } from "../loop/detect-pr-gate-coordination-state.mjs"; import { fetchIssueBody } from "../loop/detect-issue-refinement-artifact.mjs"; -import { checklistSectionLineIndices, detectIssueRefinementArtifact } from "@dev-loops/core/loop/issue-refinement-artifact"; +import { checklistLineScopes, checklistSectionLineIndices, detectIssueRefinementArtifact } from "@dev-loops/core/loop/issue-refinement-artifact"; import { applyTick } from "./tick-verified-checkboxes.mjs"; import { editPr } from "./edit-pr.mjs"; import { editIssue } from "./edit-issue.mjs"; @@ -55,7 +55,7 @@ import { } from "./_gate-finding-surface.mjs"; import { fetchAllReviewThreads } from "./list-review-threads.mjs"; import { mergeSameDefectFindings, normalizeFindingSummary } from "./_gate-finding-text.mjs"; -import { stampSpecAuthorityIdentity } from "@dev-loops/core/loop/spec-authority"; +import { computeSpecDigest, extractSpecFromBody, normalizeCriterionText, stampSpecAuthorityIdentity } from "@dev-loops/core/loop/spec-authority"; import { readSpecAuthorityIdentity } from "../lib/spec-authority-stamp.mjs"; import { normalizeGate as normalizeGateShared, normalizeVerdict as normalizeVerdictShared } from "./_gate-names.mjs"; import { evaluatePrSizeBudget as realEvaluatePrSizeBudget } from "../loop/check-size-budget.mjs"; @@ -2327,7 +2327,8 @@ function anglesPassCoverage(entries, config, gate) { // ACCEPT-CRITERIA-VERIFY-AND-REFLECT automatic tick (pre_approval_gate with a // trusted --findings-ledger only): tick the ledger's reviewer-verified labels -// in the PR body's AC/DoD boxes and, for each linked issue whose unticked AC +// in the PR body's AC/DoD boxes (dry run first; writes are deferred until after +// every refusal) and, for each linked issue whose unticked AC // items the coordination load read, in that issue body. A label outside those // unchecked lists is dropped; an issue whose body the coordination load could // not fetch is skipped (it stays spec-of-record unavailable). Exact-label @@ -2337,7 +2338,57 @@ function anglesPassCoverage(entries, config, gate) { // labels no reviewer verified stay unchecked in it and still block. Only lines // inside the AC/DoD sections (the AC section for an issue body) are flipped, so // the same label under another heading stays unchecked. -async function tickReviewerVerifiedItems({ repo, pr, verifiedItems, coordinationContext }, { env, ghCommand, runChild }) { +// The tick computes the ticked artifact without editing and queues each edit in +// pendingWrites; each queued write re-fetches the live body and re-applies the +// tick to it, so a concurrent body edit is kept and the edit guards compare +// against the live body. namedCriteria (open act item criterion texts) are never +// ticked: a verified label that a criterion contains is skipped, and an `ac:` / +// `dod:` id skips the PR-body row whose label equals that criterion in the pinned +// spec, resolved only when the PR body's spec digest equals the pinned specDigest. +// Only an id the pinned spec cannot resolve (no pin, a different pinned spec, no such +// row), or whose text matches no unchecked PR row, skips every PR-body row of its category. +async function tickReviewerVerifiedItems({ repo, pr, verifiedItems: allVerified, coordinationContext, pendingWrites, namedCriteria = [], pinnedSpecDigest }, { env, ghCommand, runChild }) { + // A label is skipped when a normalized criterion equals it or contains it bounded by non-word characters or the string edge. + const flat = (t) => normalizeCriterionText(String(t).replaceAll("`", "")).toLowerCase().replace(/\.+$/u, ""); + const normalizedCriteria = namedCriteria.map(flat); + const verifiedItems = allVerified.filter((label) => { + const text = flat(label); + if (!text) return true; + const bounded = new RegExp(`(? bounded.test(c)); + }); + const inSections = (body, options) => { + const indices = checklistSectionLineIndices(body, options); + if (!options?.byCategory) return (index) => indices.has(index); + // Per-category labels: an AC row ticks only for an AC label, a DoD row only for a DoD label, so a duplicate label in the other category is unaffected. + // The nearest-heading scope puts a DoD section nested under Acceptance criteria in the DoD category. + const scopes = checklistLineScopes(body); + return (index, label) => indices.has(index) && options.byCategory[scopes[index]]?.has(label) === true; + }; + const tick = async (body, labels, write, liveIssue, options) => { + const result = await applyTick(body, labels, true, write, inSections(body, options)); + if (result.flipped.length > 0) { + // preview=true re-reads the live body and refuses when its ticked checkbox rows differ from the dry-run's (a concurrent row change the clean guards never saw); it writes nothing. + // Each row carries its checklist scope, so a concurrent heading edit that moves rows into or out of AC/DoD also refuses. + const checkboxRows = (b) => { + const scopes = checklistLineScopes(b); + return b.split(/\r?\n/u).flatMap((l, i) => /^\s*(?:>\s*)*(?:[-*+]|\d+[.)])\s+\[[ xX]\]\s/u.test(l) ? [`${scopes[i]}\u0000${l.trim()}`] : []); + }; + const dryRows = checkboxRows(result.body); + pendingWrites.push(async (preview = false) => { + const live = await fetchIssueBody({ repo, issue: liveIssue }, { env, ghCommand, runChild }); + if (preview) { + const liveRows = checkboxRows((await applyTick(live, labels, true, write, inSections(live, options))).body); + if (liveRows.length !== dryRows.length || liveRows.some((row, i) => row !== dryRows[i])) { + throw new Error(`The checklist of ${liveIssue === pr ? `PR ${pr}` : `issue ${liveIssue}`} changed after the gate read it; no verdict is posted and nothing was ticked. Re-run the gate against the current body.`); + } + return undefined; + } + return applyTick(live, labels, false, write, inSections(live, options)); + }); + } + return result; + }; const prData = coordinationContext.prData; if (typeof prData?.body !== "string") { throw new Error(`Cannot tick reviewer-verified items on PR ${pr}: the PR body is not a string.`); @@ -2347,21 +2398,44 @@ async function tickReviewerVerifiedItems({ repo, pr, verifiedItems, coordination const allowed = new Set(Array.isArray(list) ? list : []); return verifiedItems.filter((label) => allowed.has(String(label).trim())); }; - const inSections = (body, options) => { - const indices = checklistSectionLineIndices(body, options); - return (index) => indices.has(index); + // The PR body's spec stands in for the pinned spec only when its digest equals the pinned specDigest; otherwise (no pin, or the pin is another body such as the linked issue) no id resolves and the category skip applies. + const bodySpec = extractSpecFromBody(prData.body); + // computeSpecDigest throws on a body with no acceptance criteria; that body cannot match the pin. + const bodyDigest = pinnedSpecDigest === undefined ? undefined : (() => { try { return computeSpecDigest(bodySpec); } catch { return undefined; } })(); + const pinnedSpec = pinnedSpecDigest !== undefined && bodyDigest === pinnedSpecDigest + ? bodySpec + : { acceptanceCriteria: [], definitionOfDone: [] }; + const categories = { + ac: { spec: pinnedSpec.acceptanceCriteria, unchecked: artifact?.prBodyUncheckedAcItems ?? [] }, + dod: { spec: pinnedSpec.definitionOfDone, unchecked: artifact?.prBodyUncheckedDodItems ?? [] }, + }; + // Skips are scoped to the criterion's category: an AC id never suppresses a same-text DoD row. + const prSkipped = { ac: new Set(), dod: new Set() }; + for (const c of namedCriteria) { + for (const [, kind, index] of c.matchAll(/\b(ac|dod):(\d+)\b/giu)) { + const { spec, unchecked } = categories[kind.toLowerCase()]; + const text = spec[Number(index)] === undefined ? "" : normalizeCriterionText(spec[Number(index)]); + const row = unchecked.find((label) => normalizeCriterionText(label) === text); + for (const label of text && row !== undefined ? [row] : unchecked) prSkipped[kind.toLowerCase()].add(String(label).trim()); + } + } + const byCategory = { + ac: new Set(inList(artifact?.prBodyUncheckedAcItems).map((l) => String(l).trim()).filter((l) => !prSkipped.ac.has(l))), + dod: new Set(inList(artifact?.prBodyUncheckedDodItems).map((l) => String(l).trim()).filter((l) => !prSkipped.dod.has(l))), }; - const prTick = await applyTick(prData.body, inList([...(artifact?.prBodyUncheckedAcItems ?? []), ...(artifact?.prBodyUncheckedDodItems ?? [])]), false, (bodyFile) => - editPr({ repo, pr, bodyFile, addAssignees: [], removeAssignees: [] }, { env, ghCommand, run: runChild, currentBody: prData.body }), - inSections(prData.body)); + const prTick = await tick(prData.body, [...byCategory.ac, ...byCategory.dod], (bodyFile) => + editPr({ repo, pr, bodyFile, addAssignees: [], removeAssignees: [] }, { env, ghCommand, run: runChild }), + pr, { byCategory }); const issueLabels = inList(artifact?.uncheckedAcItems); const tickedIssueBodies = []; + const flippedIssueLabels = new Set(); if (Array.isArray(artifact?.uncheckedAcItems) && artifact.uncheckedAcItems.length > 0) { for (const issue of artifact.uncheckedAcIssues ?? []) { const issueBody = await fetchIssueBody({ repo, issue }, { env, ghCommand, runChild }); - const issueTick = await applyTick(issueBody, issueLabels, false, (bodyFile) => + const issueTick = await tick(issueBody, issueLabels, (bodyFile) => editIssue({ repo, issue, bodyFile, addAssignees: [], removeAssignees: [] }, { env, ghCommand, run: runChild }), - inSections(issueBody, { acOnly: true })); + issue, { acOnly: true }); + issueTick.flipped.forEach((label) => flippedIssueLabels.add(String(label).trim())); tickedIssueBodies.push(issueTick.body); } } @@ -2376,7 +2450,19 @@ async function tickReviewerVerifiedItems({ repo, pr, verifiedItems, coordination // drop them and let an unverified issue AC item pass the clean guard. // Union with the reload's list so the result is never smaller than either source. const local = tickedIssueBodies.flatMap((body) => detectIssueRefinementArtifact({ body }).uncheckedAcItems); - const uncheckedAcItems = [...new Set([...(reloaded.uncheckedAcItems ?? []), ...local])]; + // A dry run edited nothing, so the reload's issue rows still carry the labels this tick would flip: drop those, keep the rest. + // A linked issue only the reload read keeps its own unticked rows, even when they share a label this tick flips elsewhere. + const reloadOnlyLabels = new Set(); + const fetchedIssues = new Set(artifact.uncheckedAcIssues ?? []); + for (const issue of (reloaded.uncheckedAcIssues ?? []).filter((n) => !fetchedIssues.has(n))) { + const body = await fetchIssueBody({ repo, issue }, { env, ghCommand, runChild }); + detectIssueRefinementArtifact({ body }).uncheckedAcItems.forEach((label) => reloadOnlyLabels.add(String(label).trim())); + } + const reloadedItems = (reloaded.uncheckedAcItems ?? []).filter((label) => { + const text = String(label).trim(); + return !(flippedIssueLabels.has(text) && !reloadOnlyLabels.has(text)); + }); + const uncheckedAcItems = [...new Set([...reloadedItems, ...local])]; return { ...reloaded, uncheckedAcItems }; } @@ -2989,6 +3075,10 @@ export async function upsertCheckpointVerdict(options, { env = process.env, ghCo summary: (typeof f?.summary === "string" && f.summary.trim()) || "(unparseable)", judgeDisposition: typeof f?.judgeDisposition === "string" ? f.judgeDisposition.trim() : f?.judgeDisposition, ...(f?.remedyKind ? { remedyKind: f.remedyKind } : {}), + // Criterion references survive so the tick exclusion (namedCriteria) sees a reference supplied only in --findings-json. + ...(typeof f?.defectKey === "string" ? { defectKey: f.defectKey } : {}), + ...(typeof f?.judgeCriterion === "string" ? { judgeCriterion: f.judgeCriterion } : {}), + ...(Array.isArray(f?.checkedCriteria) ? { checkedCriteria: f.checkedCriteria } : {}), // Cluster and location survive the projection so the next-action count and fingerprints match the ledger's. ...(Number.isInteger(f?.clusterId) ? { clusterId: f.clusterId } : {}), ...(resolveFindingFile(f) ? { files: [resolveFindingFile(f)] } : {}), @@ -3024,22 +3114,33 @@ export async function upsertCheckpointVerdict(options, { env = process.env, ghCo throw new Error(`--verdict "clean" for ${options.gate} @ ${canonicalHeadSha} contradicts ${actItems.length} open judge act item(s) in --findings-json "${options.findingsJson}" (GATE-COMMENT-VERDICT-VALUES, skills/docs/gate-review-comment-contract.md; ADR 0089): ${actItems.map((f) => `[${f.severity}] ${f.summary}`).join("; ")}. Post "findings_present" or fix the act items first.`); } } - // Tick the reviewer-verified AC/DoD labels BEFORE composing, then read the + // Dry-run the reviewer-verified AC/DoD ticks before composing (writes are queued and applied after the last refusal), then read the // reloaded artifact everywhere below (blocker collection and the clean guards). // Only a ledger whose provenance records a fresh acceptance-criteria or // pr-checklist review is trusted to tick. In fanout_fanin mode the ledger // must also pass the angle-coverage checks, and so must --findings-json when // present: its throwing coverage check runs later, so an input it refuses // must tick nothing. + const pendingTickWrites = []; const tickCoverageOk = (options.executionMode ?? DEFAULT_EXECUTION_MODE) !== "fanout_fanin" || (ledgerPassesAngleCoverage(preloadedFindingsLedger, config, options.gate) && (!structuredFindings || (anglesPassCoverage(rawFindingsInput, config, options.gate) && anglesPassCoverage(structuredFindings, config, options.gate)))); if (options.gate === "pre_approval_gate" && preloadedFindingsLedger?.verifiedItems?.length > 0 && ledgerVerifiesChecklist(preloadedFindingsLedger) && tickCoverageOk) { + // Dry run: the composition and clean guards read the would-be ticked artifact; the edits run after the last refusal (pendingTickWrites). + // An open act item names a row by its label text or by a spec id (`ac:` / `dod:`); see tickReviewerVerifiedItems. + // An evidence_only act item also holds its row: its remedy is evidence, but the judge still says the row is unproven. + const namedCriteria = [...(preloadedFindingsLedger.findings ?? []), ...enrichedActItems, ...jsonUnactedFindings] + .filter((f) => f?.judgeDisposition === "act") + .flatMap((f) => [f.defectKey, f.judgeCriterion, ...(Array.isArray(f.checkedCriteria) ? f.checkedCriteria : [])]) + .filter((c) => typeof c === "string").map((c) => c.trim()).filter(Boolean); coordinationContext.refinementArtifact = await tickReviewerVerifiedItems({ repo: options.repo, pr: options.pr, verifiedItems: preloadedFindingsLedger.verifiedItems, coordinationContext, + pendingWrites: pendingTickWrites, + namedCriteria, + pinnedSpecDigest: specAuthorityIdentity?.specDigest, }, gh); } // Set only when the posted checkpoint verdict differs from the ledger's review @@ -3543,15 +3644,6 @@ export async function upsertCheckpointVerdict(options, { env = process.env, ghCo // gate review) and the legacy issue-comment surface (updateComment below) are // covered, in addition to the low-level guards in the write helpers. guardCommentBodyNoIssuePrIds(desiredBody, { ref: "gate verdict comment body", allowedRefs: options.allowedRefs }); - const findingSurfaceFields = findingSurface - ? { - round: findingSurface.round, - inlineComments: mergeSameDefectFindings(findingSurface.locatable).length, - bodyFiled: findingSurface.nonLocatable.length, - folded: findingSurface.folded.length, - suppressed: findingSurface.suppressedCount, - } - : {}; // SUBMITTED-REVIEW RESOLUTION (review gate): a same-head own SUBMITTED review // is this round's existing surface. Without this, a same-head rerun POSTs a // duplicate submitted review (findOwnPendingReview above only sees the @@ -3559,13 +3651,43 @@ export async function upsertCheckpointVerdict(options, { env = process.env, ghCo // deliberate escape hatch that bypasses this dedup to force a fresh review. // Runs AFTER resolveFindingSurface so findingSurface.locatable already tells // us whether a correction needs a new inline comment GitHub cannot add to an - // already-submitted review. - if (isReviewGate && !options.newRound) { - const ownSubmitted = await findOwnSubmittedReview( + // already-submitted review. The lookup and its refusal run before the queued ticks. + const ownSubmitted = isReviewGate && !options.newRound + ? await findOwnSubmittedReview( { repo: options.repo, pr: options.pr, headSha: canonicalHeadSha }, gh, + ) + : null; + // A still-unposted locatable finding would need an inline comment, and + // GitHub exposes no endpoint to add one to an already-submitted review. + // Fail closed rather than silently drop it to body text; --new-round + // posts a fresh review that CAN carry the inline comment. + if (ownSubmitted && findingSurface && findingSurface.locatable.length > 0) { + throw new Error( + `Cannot correct the already-submitted review ${ownSubmitted.id} on ${options.repo}#${options.pr}: ` + + `this round adds ${findingSurface.locatable.length} inline finding(s), and GitHub has no endpoint to add inline ` + + `comments to a submitted review. Re-run with --new-round to post a fresh review round that carries the new inline comment(s).`, ); - if (ownSubmitted) { + } + // createGateReview refuses an inline finding comment that carries a raw issue/PR id; run that guard here so the refusal precedes the ticks. + for (const finding of findingSurface ? mergeSameDefectFindings(findingSurface.locatable) : []) { + guardCommentBodyNoIssuePrIds(renderInlineCommentBody(finding, { round: findingSurface.round }), { ref: "gate review inline finding comment", allowedRefs: options.allowedRefs }); + } + // The last refusal has passed: apply the queued reviewer-verified ticks now, before the first review or comment write, so a refused post leaves no ticked box. + // A failed write throws before any post. A failed post (HTTP error) after the write keeps the ticks; a rerun is idempotent and re-ticks only the rows still unticked. + // Validate every live checklist against its dry-run first, so a concurrent row change refuses before any body write. + for (const write of pendingTickWrites) await write(true); + for (const write of pendingTickWrites) await write(); + const findingSurfaceFields = findingSurface + ? { + round: findingSurface.round, + inlineComments: mergeSameDefectFindings(findingSurface.locatable).length, + bodyFiled: findingSurface.nonLocatable.length, + folded: findingSurface.folded.length, + suppressed: findingSurface.suppressedCount, + } + : {}; + if (ownSubmitted) { const reviewBaseResult = { ok: true, repo: options.repo, @@ -3581,17 +3703,6 @@ export async function upsertCheckpointVerdict(options, { env = process.env, ghCo ...(findingsLedgerWarning ? { findingsLedgerWarning } : {}), ...(specAuthority ? { specAuthority } : {}), }; - // A still-unposted locatable finding would need an inline comment, and - // GitHub exposes no endpoint to add one to an already-submitted review. - // Fail closed rather than silently drop it to body text; --new-round - // posts a fresh review that CAN carry the inline comment. - if (findingSurface && findingSurface.locatable.length > 0) { - throw new Error( - `Cannot correct the already-submitted review ${ownSubmitted.id} on ${options.repo}#${options.pr}: ` - + `this round adds ${findingSurface.locatable.length} inline finding(s), and GitHub has no endpoint to add inline ` - + `comments to a submitted review. Re-run with --new-round to post a fresh review round that carries the new inline comment(s).`, - ); - } // Byte-identical body: nothing to correct, suppress the duplicate repost. if (desiredBody === ownSubmitted.body) { return { @@ -3629,7 +3740,6 @@ export async function upsertCheckpointVerdict(options, { env = process.env, ghCo ...((updatedReview.reviewUrl ?? ownSubmitted.commentUrl) ? { commentUrl: updatedReview.reviewUrl ?? ownSubmitted.commentUrl } : {}), ...(verificationWarning ? { verificationWarning } : {}), }; - } } const desiredExecutionMode = options.executionMode ?? DEFAULT_EXECUTION_MODE; const compositionResultFields = checkpointComposition diff --git a/skills/docs/acceptance-criteria-verification.md b/skills/docs/acceptance-criteria-verification.md index 9f93e5e26..3ff3f9aa3 100644 --- a/skills/docs/acceptance-criteria-verification.md +++ b/skills/docs/acceptance-criteria-verification.md @@ -25,7 +25,10 @@ Canonical owner for the acceptance-criteria verification procedure run during th 4. **Verify each AC item** against the proposed changes on the current PR head. -5. **Automatic tick before composing:** the reviewers of the `acceptance-criteria` and `pr-checklist` angles list the exact label of each checklist item they verified at this head in their findings artifact's `verifiedItems` field (defined in the review agent's structured findings artifact). Fan-in carries the deduplicated union into the durable findings ledger. For a `pre_approval_gate` post with `--findings-ledger`, `upsert-checkpoint-verdict.mjs` ticks exactly those labels before it composes the verdict. In `fanout_fanin` mode the ledger must first pass the fan-out angle-coverage checks. In every mode its provenance must be consistent and record a fresh (not carried) `acceptance-criteria` or `pr-checklist` review. Otherwise nothing is ticked. The poster flips each matching `- [ ]` to `- [x]` in the PR body's unchecked Acceptance criteria and Definition of done items. When the spec-of-record still has unticked AC items (`uncheckedAcItems`), it also ticks each linked issue body that the coordination load read and that carries unticked AC items. A label outside those unchecked lists is not ticked. A linked issue whose body the coordination load could not fetch is skipped and stays spec-of-record unavailable. The match is exact-label, and the tick never blanket-checks and never unchecks. Each changed body gets one edit. The poster then reloads the refinement artifact, so the verdict composition and the `clean` guards read the ticked bodies. A failed fetch or edit of a body being ticked fails closed, and no verdict is posted. The tick does not run for `draft_gate`. On a matrix-representation issue the issue-side tick is a no-op, and the PR body's derived checklist is the ticked surface. +5. **Automatic tick before composing:** the reviewers of the `acceptance-criteria` and `pr-checklist` angles list the exact label of each checklist item they verified at this head in their findings artifact's `verifiedItems` field (defined in the review agent's structured findings artifact). Fan-in carries the deduplicated union into the durable findings ledger. For a `pre_approval_gate` post with `--findings-ledger`, `upsert-checkpoint-verdict.mjs` computes the ticked bodies as a dry run for exactly those labels before it composes the verdict. In `fanout_fanin` mode the ledger must first pass the fan-out angle-coverage checks. In every mode its provenance must be consistent and record a fresh (not carried) `acceptance-criteria` or `pr-checklist` review. Otherwise nothing is ticked. The poster flips each matching `- [ ]` to `- [x]` in the PR body's unchecked Acceptance criteria and Definition of done items. When the spec-of-record still has unticked AC items (`uncheckedAcItems`), it also ticks each linked issue body that the coordination load read and that carries unticked AC items. A label outside those unchecked lists is not ticked. A verified label that an open judge act item (including one with an evidence-only remedy) names (by label text or by an `ac:` / `dod:` id) stays unticked. An id that does not resolve in the pinned spec, or whose text matches no unchecked PR row, skips every PR-body row of its category; the skip applies to PR-body rows only, not to linked issue bodies. Do not hand-tick a row left unticked for an open act item with `tick-verified-checkboxes`. A linked issue whose body the coordination load could not fetch is skipped and stays spec-of-record unavailable. The match is exact-label, and the tick never blanket-checks and never unchecks. Each changed body gets one edit, written only after every refusal check and directly before the verdict is posted. The verdict composition and the `clean` guards read the dry-run ticked bodies. A failed fetch or edit of a body being ticked fails closed, and no verdict is posted. A refusal changes no body. A failed post (HTTP error) after the write keeps the ticks, and a rerun is idempotent. The tick does not run for `draft_gate`. On a matrix-representation issue the issue-side tick is a no-op, and the PR body's derived checklist is the ticked surface. + + + **CHECKLIST-SCOPE-ONE-MODEL:** every PR-body tick-integrity check reads checklist scope from `checklistLineScopes` in `packages/core/src/loop/issue-refinement-artifact.mjs`: the fence-aware scope of a line is `ac`, `dod`, or none, set by its nearest Acceptance criteria or Definition of done heading, so a Definition of done section nested under Acceptance criteria is `dod`. The category-scoped tick, the live checklist preview and the `edit-pr` untick guard key a checkbox row by that scope plus its label. A heading rename inside one scope keeps a checked row. A heading edit that moves a checked row into or out of a scope counts as losing it and needs `--allow-untick`. A second fence scanner or section classifier MUST NOT decide that scope for the PR body. The linked-issue tick keeps its own `acOnly` section filter. **Manual tick paths:** the automatic tick needs `verifiedItems`, and only the fan-out path writes them (per-angle artifacts through `consolidate-fanin`). It also reads the unchecked PR-body lists, which the coordination state fills only for a tracker-backed PR with a linked issue (step 7). The tick reads `verifiedItems` only from a trusted ledger: consistent provenance that records a fresh `acceptance-criteria` or `pr-checklist` review, plus fan-out angle coverage in `fanout_fanin` mode. Two paths therefore tick nothing automatically. The first is a post that carries no trusted `verifiedItems`: a post without `--findings-ledger`, a light-mode `inline_single_agent` round (with or without a ledger), or a ledger that lacks a fresh `acceptance-criteria` or `pr-checklist` review or fails the `fanout_fanin` angle coverage. The second is a lightweight or issue-less PR. On those paths the operator ticks each verified item before posting with `node scripts/github/tick-verified-checkboxes.mjs --repo --issue --pr --verified ...`. Without a tracker-backed issue, the operator passes `--pr` alone. The CLI applies the same exact-label tick. On a fan-out post for a tracker-backed PR with a linked issue, the CLI is a manual correction tool only. diff --git a/skills/docs/required-rules.json b/skills/docs/required-rules.json index 699bb0fe1..2a285a695 100644 --- a/skills/docs/required-rules.json +++ b/skills/docs/required-rules.json @@ -80,6 +80,10 @@ "enforcement": "agent", "enforcementNote": "Path assumptions are agent behavior." }, + { + "id": "CHECKLIST-SCOPE-ONE-MODEL", + "enforcement": "runtime" + }, { "id": "CLOSING-REF-BRANCH-MISMATCH", "enforcement": "runtime" diff --git a/test/github/edit-pr.test.mjs b/test/github/edit-pr.test.mjs index ff6fe386f..8b24a9e26 100644 --- a/test/github/edit-pr.test.mjs +++ b/test/github/edit-pr.test.mjs @@ -7,6 +7,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { parseEditPrCliArgs, editPr as editPrRaw, runCli } from "../../scripts/github/edit-pr.mjs"; +import { checklistLineScopes } from "@dev-loops/core/loop/issue-refinement-artifact"; import { captureStream, makeGhStub } from "../_helpers.mjs"; // A body edit reads the PR's current body (waiver-line baseline) through @@ -454,3 +455,102 @@ test("editPr: the closing-reference refusal still fails closed when currentBody /CLOSING-REF-BRANCH-MISMATCH.*fail closed/s, ); }); + +test("editPr (#2898): refuses a body that unticks checked rows, naming them; --allow-untick lets it through", async () => { + const rows = Array.from({ length: 15 }, (_, i) => `row ${i + 1}`); + const live = rows.map((r) => `- [x] ${r}`).join("\n"); + const next = rows.map((r) => `- [ ] ${r}`).join("\n"); + const fetchPrContext = async () => ({ headRefName: "x", closingIssuesReferences: [], body: live }); + const refused = stubGh(); + await assert.rejects( + () => editPr({ repo: "o/n", pr: 5, body: next, addAssignees: [], removeAssignees: [] }, { run: refused.run, fetchPrContext }), + /untick or remove 15 checked row.*"row 1".*"row 15"/s, + ); + assert.equal(refused.calls.length, 0); + const allowed = stubGh(); + await editPr({ repo: "o/n", pr: 5, body: next, addAssignees: [], removeAssignees: [], allowUntick: true }, { run: allowed.run, fetchPrContext }); + assert.equal(allowed.calls.length, 1); + assert.equal(parseEditPrCliArgs(["--repo", "o/n", "--pr", "5", "--body", "x", "--allow-untick"]).allowUntick, true); +}); + +test("editPr (#2898): the untick guard counts duplicate labels and reports a removed or reworded checked row", async () => { + const live = ["## AC", "- [x] Tests pass", "## DoD", "- [x] Tests pass", "- [x] Docs updated"].join("\n"); + const fetchPrContext = async () => ({ headRefName: "x", closingIssuesReferences: [], body: live }); + const attempt = (body) => editPr({ repo: "o/n", pr: 5, body, addAssignees: [], removeAssignees: [] }, { run: stubGh().run, fetchPrContext }); + // both duplicates kept checked: accepted + await attempt(live); + // one of two identical checked labels unticked: refused + await assert.rejects(() => attempt(live.replace("## DoD\n- [x] Tests pass", "## DoD\n- [ ] Tests pass")), /untick or remove 1 checked row.*"Tests pass"/s); + // a checked label removed: refused + await assert.rejects(() => attempt(live.replace("\n- [x] Docs updated", "")), /untick or remove 1 checked row.*"Docs updated"/s); +}); + +test("editPr (#2898): the untick guard ignores fenced rows and scopes labels to their section", async () => { + const attempt = (live, body) => editPr( + { repo: "o/n", pr: 5, body, addAssignees: [], removeAssignees: [] }, + { run: stubGh().run, fetchPrContext: async () => ({ headRefName: "x", closingIssuesReferences: [], body: live }) }, + ); + const fenced = "```\n- [x] Tests pass\n```"; + // unticking a real row while adding a fenced checked example: refused + await assert.rejects(() => attempt("## AC\n- [x] Tests pass", `## AC\n- [ ] Tests pass\n${fenced}`), /untick or remove 1 checked row.*"Tests pass"/s); + // editing a fenced checked example: accepted + await attempt(`## AC\n- [x] Tests pass\n${fenced}`, `## AC\n- [x] Tests pass\n${fenced.replace("[x]", "[ ]")}`); + // swapping the tick between sections with the same label: refused + await assert.rejects(() => attempt("## AC\n- [x] Tests pass\n## Validation\n- [ ] Tests pass", "## AC\n- [ ] Tests pass\n## Validation\n- [x] Tests pass"), /untick or remove 1 checked row.*"Tests pass"/s); +}); + +const attemptEdit = (live, body) => editPr( + { repo: "o/n", pr: 5, body, addAssignees: [], removeAssignees: [] }, + { run: stubGh().run, fetchPrContext: async () => ({ headRefName: "x", closingIssuesReferences: [], body: live }) }, +); + +test("editPr (#2898): a heading rename inside one scope is accepted; a heading-only edit that leaves the scope is refused", async () => { + // rename inside the AC scope: accepted + await attemptEdit("## AC\n- [x] Tests pass", "## Acceptance criteria\n- [x] Tests pass"); + // heading-only edit: the AC heading becomes a plain heading, the checked row leaves the AC scope + await assert.rejects(() => attemptEdit("## AC\n- [x] Tests pass", "## Criteria\n- [x] Tests pass"), /untick or remove 1 checked row.*"Tests pass"/s); + // a plain heading nested into the AC scope moves its checked row into AC: its scope key changes, so it is refused + await assert.rejects(() => attemptEdit("## AC\n- [x] A\n## Notes\n- [x] B", "## AC\n- [x] A\n### Notes\n- [x] B"), /untick or remove 1 checked row.*"B"/s); +}); + +test("editPr (#2898): a four-backtick fence holding a three-backtick line hides its checked rows", async () => { + const fenced = "````\n```\n- [x] Hidden\n````"; + await attemptEdit(`## AC\n- [x] Tests pass\n${fenced}`, `## AC\n- [x] Tests pass\n${fenced.replace("[x]", "[ ]")}`); + await assert.rejects(() => attemptEdit(`## AC\n- [x] Tests pass\n${fenced}`, `## AC\n- [ ] Tests pass\n${fenced}`), /untick or remove 1 checked row.*"Tests pass"/s); + // the row after the closed four-backtick fence is a real row + await assert.rejects(() => attemptEdit(`${fenced}\n- [x] Real`, `${fenced}\n- [ ] Real`), /"Real"/); +}); + +test("editPr (#2898): a Definition of done nested under Acceptance criteria is its own scope", async () => { + const body = "## Acceptance criteria\n- [x] A\n### Definition of done\n- [x] A\n### Notes\n- [x] A"; + const scopes = checklistLineScopes(body); + assert.deepEqual([scopes[1], scopes[3], scopes[5]], ["ac", "dod", "ac"]); + // unticking the nested DoD copy is a loss of a dod row, not an ac row + await assert.rejects(() => attemptEdit(body, body.replace("### Definition of done\n- [x] A", "### Definition of done\n- [ ] A")), /untick or remove 1 checked row.*"A"/s); +}); + +test("editPr (#2898): a heading rename plus a real untick is refused naming the unticked row", async () => { + await assert.rejects(() => attemptEdit("## AC\n- [x] Tests pass\n- [x] Docs", "## Acceptance criteria\n- [x] Tests pass\n- [ ] Docs"), /untick or remove 1 checked row.*"Docs"/s); +}); + +test("editPr (#2898): renaming both AC and DoD headings with a swap moves every checked row out of its scope", async () => { + const live = "## AC\n- [x] A\n## DoD\n- [x] D"; + // the headings swap names, so each row changes scope + await assert.rejects(() => attemptEdit(live, "## DoD\n- [x] A\n## AC\n- [x] D"), /untick or remove 2 checked row/s); + // both renamed within their own scope: accepted + await attemptEdit(live, "## Acceptance criteria\n- [x] A\n## Definition of done\n- [x] D"); +}); + +test("editPr (#2898): the untick guard sees every checkbox row form the gate parser reads", async () => { + const attempt = (live, body) => editPr( + { repo: "o/n", pr: 5, body, addAssignees: [], removeAssignees: [] }, + { run: stubGh().run, fetchPrContext: async () => ({ headRefName: "x", closingIssuesReferences: [], body: live }) }, + ); + for (const marker of ["-", "*", "+", "1.", "1)", "> -", "> 1."]) { + await assert.rejects(() => attempt(`${marker} [x] Docs updated`, `${marker} [ ] Docs updated`), /untick or remove 1 checked row.*"Docs updated"/s, marker); + } + // Not checkbox rows: unticking them is no refusal. + for (const line of ["plain text line without a checkbox", "[x] Docs updated"]) { + await attempt(line, line.replace("[x]", "[ ]")); + } +}); diff --git a/test/github/upsert-checkpoint-verdict.test.mjs b/test/github/upsert-checkpoint-verdict.test.mjs index e1bc580ae..eb79d2356 100644 --- a/test/github/upsert-checkpoint-verdict.test.mjs +++ b/test/github/upsert-checkpoint-verdict.test.mjs @@ -31,6 +31,7 @@ import { buildValidationResultsPath } from "../../scripts/github/write-gate-cont import { loadDevLoopConfig } from "@dev-loops/core/config"; import { evaluatePrGateCoordination, PR_CHECKPOINT_ACTION } from "@dev-loops/core/loop/pr-gate-coordination"; import { renderFallbackGateReviewCommentBody } from "../../skills/dev-loop/scripts/post-gate-verdict-fallback.mjs"; +import { computeSpecDigest, extractSpecFromBody } from "@dev-loops/core/loop/spec-authority"; // The legacy input shapes in this file predate the round cap. A fixture repo root that does not set the cap gets // maxReviewRounds: 0 (the cap has its own tests in upsert-checkpoint-verdict-round-cap.test.mjs). @@ -9815,11 +9816,13 @@ function makeAcRunChild({ issueComments = [], ciSuccess = true, prBody = DEFAULT_TEST_PR_BODY, + prFiles = [], // true fails every body edit; "pr" or "issue" fails only that kind. Mutable // through the returned `control` so a second run can succeed. failBodyEdit = false, // Fail every `gh issue view` after this many successful ones (null: never). failIssueViewAfter = null, + failIssueViewCount = Infinity, } = {}) { const calls = []; const control = { failBodyEdit }; @@ -9849,7 +9852,12 @@ function makeAcRunChild({ // gh issue view --json body — the linked-issue spec-of-record fetch. if (args[0] === "issue" && args[1] === "view") { const issue = args[2]; - if (failIssueViewAfter !== null && issueViews++ >= failIssueViewAfter) return { code: 1, stdout: "", stderr: "HTTP 502: issue view failed\n" }; + // A PR is also an issue: the live PR body, after any concurrent edit. + if (issue === "17") { + if (control.concurrentPrBody !== undefined) currentPrBody = control.concurrentPrBody; + return { code: 0, stdout: JSON.stringify({ body: currentPrBody }) + "\n", stderr: "" }; + } + if (failIssueViewAfter !== null && issueViews++ >= failIssueViewAfter && issueViews <= failIssueViewAfter + failIssueViewCount) return { code: 1, stdout: "", stderr: "HTTP 502: issue view failed\n" }; const body = issueBodies[issue]; if (body === undefined) return { code: 1, stdout: "", stderr: "not found\n" }; return { code: 0, stdout: JSON.stringify({ body }) + "\n", stderr: "" }; @@ -9867,7 +9875,7 @@ function makeAcRunChild({ if (a.includes("graphql")) return { code: 0, stdout: '{"data":{"repository":{"pullRequest":{"reviewThreads":{"nodes":[]}}}}}\n', stderr: "" }; if (a.includes("issues/17/comments")) return { code: 0, stdout: JSON.stringify(issueComments) + "\n", stderr: "" }; if (a.includes("pulls/17/reviews")) return { code: 0, stdout: JSON.stringify(reviews) + "\n", stderr: "" }; - if (a.includes("pulls/17/files")) return { code: 0, stdout: "[]\n", stderr: "" }; + if (a.includes("pulls/17/files")) return { code: 0, stdout: JSON.stringify(prFiles) + "\n", stderr: "" }; // Dispatch pr view --json on the exact --json field list, so the // full pr view (which lists `files` among many fields) is not mistaken for // the dedicated `--json files` files-list call. @@ -10697,15 +10705,17 @@ const COMPOSITION_UNCHECKED_AC_PR_BODY = [ "- none", "", ].join("\n"); -function makeCompositionRunChild(prBody, extraComments = [], { issueBody = TICKED_AC_ISSUE_BODY, failBodyEdit = false, failIssueViewAfter = null, closingIssues = [900] } = {}) { +function makeCompositionRunChild(prBody, extraComments = [], { prFiles, issueBody = TICKED_AC_ISSUE_BODY, failBodyEdit = false, failIssueViewAfter = null, failIssueViewCount, closingIssues = [900], extraIssueBodies = {} } = {}) { return makeAcRunChild({ isDraft: false, // Only issue 900 has a body; any other linked issue fails every fetch. closingIssues: closingIssues.map((number) => ({ number })), - issueBodyByNumber: { 900: issueBody }, + issueBodyByNumber: { 900: issueBody, ...extraIssueBodies }, prBody, + prFiles, failBodyEdit, failIssueViewAfter, + failIssueViewCount, reviews: [1, 2, 3, 4, 5].map((i) => ({ author: { login: "copilot-pull-request-reviewer[bot]" }, state: "COMMENTED", submittedAt: `2026-06-01T20:0${i}:00Z`, commit: { oid: `${i}`.repeat(40) }, @@ -10727,14 +10737,14 @@ function makeCompositionRunChild(prBody, extraComments = [], { issueBody = TICKE // fresh acceptance-criteria (or pr-checklist) review. const TICK_PROVENANCE = { distinctReviewers: 1, perAngle: [{ angle: "acceptance-criteria", reviewer: "reviewer-ac" }] }; -async function withCompositionRound({ overallVerdict, prBody, gate = "pre_approval_gate", requireFanoutEvidence = false, extraComments = [], verifiedItems, provenance = verifiedItems === undefined ? undefined : TICK_PROVENANCE, issueBody, failBodyEdit, failIssueViewAfter, closingIssues }, fn) { +async function withCompositionRound({ overallVerdict, prBody, gate = "pre_approval_gate", requireFanoutEvidence = false, extraComments = [], verifiedItems, provenance = verifiedItems === undefined ? undefined : TICK_PROVENANCE, issueBody, failBodyEdit, failIssueViewAfter, failIssueViewCount, closingIssues, extraIssueBodies, findings, prFiles }, fn) { const repoRoot = await stageConfigRepoRoot(requireFanoutEvidence); try { - const ledgerPath = await writeVerdictLedger(repoRoot, { overallVerdict, gate, verifiedItems, provenance }); + const ledgerPath = await writeVerdictLedger(repoRoot, { overallVerdict, gate, verifiedItems, provenance, findings }); const ledgerBefore = await readFile(ledgerPath, "utf8"); const { runChild, calls, editedBodies, control } = gate === "draft_gate" ? makeAcRunChild({ isDraft: true, prBody }) - : makeCompositionRunChild(prBody, extraComments, { issueBody, failBodyEdit, failIssueViewAfter, closingIssues }); + : makeCompositionRunChild(prBody, extraComments, { prFiles, issueBody, failBodyEdit, failIssueViewAfter, failIssueViewCount, closingIssues, extraIssueBodies }); const post = (overrides = {}) => upsertCheckpointVerdict({ repo: "owner/repo", pr: 17, gate, headSha: GATE_FULL_HEAD, findingsSummary: "review round summary", @@ -11107,8 +11117,8 @@ const TICK_ISSUE_BODY_WITH_UNVERIFIED = [ const reviewPosted = (calls) => calls.some((c) => c.args.some((x) => x.includes("pulls/17/reviews")) && c.args.includes("POST")); test("tick: a failed post-tick issue re-fetch cannot drop the unverified issue AC item, so no clean is posted", async () => { - // Views 1-3 (coordination load, tick fetch, edit-issue guard body read) succeed; the reload re-fetch fails. - const round = { overallVerdict: "clean", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, issueBody: TICK_ISSUE_BODY_WITH_UNVERIFIED, failIssueViewAfter: 3 }; + // Views 1-2 (coordination load, tick fetch) succeed; the dry-run reload re-fetch fails. + const round = { overallVerdict: "clean", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, issueBody: TICK_ISSUE_BODY_WITH_UNVERIFIED, failIssueViewAfter: 2, failIssueViewCount: 1 }; await withCompositionRound(round, async ({ post, calls }) => { await assert.rejects( () => post({ verdict: "clean", findingsSeverityCounts: CLEAN_COUNTS }), @@ -12352,3 +12362,266 @@ test("buildCoordinationEvaluatorInput threads draftCapRoundFixed so pre_approval assert.ok(!evaluate(false).allowedNextActions.includes(PR_CHECKPOINT_ACTION.RUN_PRE_APPROVAL_GATE), "no proof: entry refused"); assert.ok(!evaluate(undefined).allowedNextActions.includes(PR_CHECKPOINT_ACTION.RUN_PRE_APPROVAL_GATE), "an unresolved proof (resolver threw) fails closed"); }); + +test("tick (#2898): the dry-run artifact keeps a label a reload-only issue carries unticked even when another issue's row is flipped", async () => { + const extraIssueBodies = { 901: ["## Acceptance criteria", "", "- [ ] criterion number 1", ""].join("\n") }; + const round = { overallVerdict: "clean", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, issueBody: UNTICKED_TICK_ISSUE_BODY, closingIssues: [900, 901], extraIssueBodies, failIssueViewAfter: 1, failIssueViewCount: 1 }; + await withCompositionRound(round, async ({ post }) => { + const result = await post({ findingsSeverityCounts: CLEAN_COUNTS }); + assert.deepEqual(result.gateBlockers, [{ kind: "unticked spec-of-record Acceptance criteria", item: "criterion number 1" }]); + }); +}); + +const bodyEdits = (calls) => calls.filter((c) => c.args[1] === "edit" && (c.args.includes("--body") || c.args.includes("--body-file"))); +const openAct = (extra) => ({ angle: "acceptance-criteria", severity: "medium", summary: "no evidence for that row", judgeDisposition: "act", ...extra }); +const UNTICKED_TICK_ISSUE_BODY = ["## Acceptance criteria", "", "- [ ] criterion number 1", "- [ ] criterion number 2", ""].join("\n"); + +test("tick (#2898): a refused verdict post changes no PR body and no issue body", async () => { + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, issueBody: UNTICKED_TICK_ISSUE_BODY, verifiedItems: TICK_ITEMS }, async ({ post, calls, editedBodies }) => { + await assert.rejects(() => post({ verdict: "clean", findingsSeverityCounts: CLEAN_COUNTS }), /contradicts the consolidated ledger/); + assert.equal(editedBodies.length, 0); + assert.equal(bodyEdits(calls).length, 0); + }); +}); + +test("tick (#2898): a refusal raised after the deferred tick point (the desiredBody issue/PR id guard) changes no PR body and no issue body", async () => { + await withCompositionRound({ overallVerdict: "clean", prBody: TICK_PR_BODY, issueBody: UNTICKED_TICK_ISSUE_BODY, verifiedItems: TICK_ITEMS }, async ({ post, calls, editedBodies }) => { + await assert.rejects(() => post({ findingsSeverityCounts: CLEAN_COUNTS, findingsSummary: "tracked as 1234 and #1234" }), /comment-id-guard/); + assert.equal(editedBodies.length, 0); + assert.equal(bodyEdits(calls).length, 0); + }); +}); + +test("tick (#2898): a row an open act item names by label stays unticked; a criterion with a prefix or a surrounding clause names it too", async () => { + for (const judgeCriterion of [TICK_ITEMS[12], `DoD: ${TICK_ITEMS[12]}`, `the row ${TICK_ITEMS[12]} has no evidence`]) { + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [openAct({ judgeCriterion })] }, async ({ post, editedBodies }) => { + await post(); + assert.equal(editedBodies.length, 1); + assert.match(editedBodies[0].body, new RegExp(`^- \\[ \\] ${TICK_ITEMS[12]}$`, "m")); + assert.match(editedBodies[0].body, new RegExp(`^- \\[x\\] ${TICK_ITEMS[1]}$`, "m")); + // "criterion number 1" is a prefix of the named "criterion number 13" but names a different row. + assert.match(editedBodies[0].body, new RegExp(`^- \\[x\\] ${TICK_ITEMS[0]}$`, "m")); + }); + } + // Accepted forms: case, internal whitespace and dropped backticks still name the row; a quoted label followed by a period too. + const tickedRow = "The `edit-pr.mjs` USAGE text documents `--allow-untick`."; + const rowBody = ["## Acceptance criteria", "", `- [ ] ${tickedRow}`, "- [ ] Tests pass", "", "## Definition of done", "", "- [x] done", ""].join("\n"); + for (const judgeCriterion of ["the EDIT-PR.MJS usage text documents --allow-untick", "The edit-pr.mjs USAGE text documents --allow-untick", `"${tickedRow}"`]) { + await withCompositionRound({ overallVerdict: "findings_present", prBody: rowBody, verifiedItems: [tickedRow, "Tests pass"], findings: [openAct({ judgeCriterion })] }, async ({ post, editedBodies }) => { + await post(); + assert.match(editedBodies[0].body, /^- \[ \] The `edit-pr\.mjs` USAGE/m); + assert.match(editedBodies[0].body, /^- \[x\] Tests pass$/m); + }); + } + // Rejected forms: a label only inside a longer word, and a criterion naming a different row, name nothing. + for (const judgeCriterion of ["Tests passed", "an unrelated row"]) { + await withCompositionRound({ overallVerdict: "findings_present", prBody: rowBody, verifiedItems: ["Tests pass"], findings: [openAct({ judgeCriterion })] }, async ({ post, editedBodies }) => { + await post(); + assert.match(editedBodies[0].body, /^- \[x\] Tests pass$/m); + }); + } + // A criterion that does not contain the label names nothing: every verified row is ticked. + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [openAct({ judgeCriterion: "an unrelated clause" })] }, async ({ post, editedBodies }) => { + await post(); + assert.doesNotMatch(editedBodies[0].body, /- \[ \]/); + }); +}); + +// A --spec-authority file pinning `specDigest`, in a scratch dir the OS cleans up. +async function writeTickPin(name, specDigest) { + const file = path.join(await mkdtemp(path.join(os.tmpdir(), "dev-loops-tick-pin-")), name); + await writeFile(file, JSON.stringify({ specDigest, headSha: "b".repeat(40), contentDigest: `sha256:${"c".repeat(64)}`, checkedCriteria: [] })); + return file; +} + +test("tick (#2898): a spec id (dod: / ac:) resolves against the pinned spec and skips only its row, whatever order the PR body lists them", async () => { + const unticked = (body, item) => assert.match(body, new RegExp(`^- \\[ \\] ${item}$`, "m")); + const ticked = (body, item) => assert.match(body, new RegExp(`^- \\[x\\] ${item}$`, "m")); + const pin = await writeTickPin("pinned.json", computeSpecDigest(extractSpecFromBody(TICK_PR_BODY))); + // The pinned spec is not the PR body's spec (the linked issue): the id resolves nothing, so every DoD row stays unticked. + // With no pin at all the same category skip applies. + for (const specAuthority of [await writeTickPin("other.json", `sha256:${"d".repeat(64)}`), undefined]) { + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [openAct({ judgeCriterion: "dod:1" })] }, async ({ post, editedBodies }) => { + await post({ specAuthority }); + for (const item of TICK_ITEMS) (TICK_ITEMS.slice(12).includes(item) ? unticked : ticked)(editedBodies[0].body, item); + }); + } + // DoD row 1 is "criterion number 14": only that row stays unticked. + for (const judgeCriterion of ["dod:1", "(dod:1)", "dod:1 The tick never flips a row that an open act item names."]) { + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [openAct({ judgeCriterion })] }, async ({ post, editedBodies }) => { + await post({ specAuthority: pin }); + unticked(editedBodies[0].body, TICK_ITEMS[13]); + for (const item of TICK_ITEMS.filter((_, i) => i !== 13)) ticked(editedBodies[0].body, item); + }); + } + // An evidence_only act item holds its row too. + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [openAct({ judgeCriterion: "dod:1", remedyKind: "evidence_only" })] }, async ({ post, editedBodies }) => { + await post({ specAuthority: pin }); + unticked(editedBodies[0].body, TICK_ITEMS[13]); + for (const item of TICK_ITEMS.filter((_, i) => i !== 13)) ticked(editedBodies[0].body, item); + }); + // AC row 0 is "criterion number 1". + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [openAct({ judgeCriterion: "ac:0" })] }, async ({ post, editedBodies }) => { + await post({ specAuthority: pin }); + unticked(editedBodies[0].body, TICK_ITEMS[0]); + for (const item of TICK_ITEMS.slice(1)) ticked(editedBodies[0].body, item); + }); + // Two ids across categories skip their two rows. + for (const judgeCriterion of ["ac:0/dod:1", "ac:0,dod:1", "(ac:0) (dod:1)"]) { + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [openAct({ judgeCriterion })] }, async ({ post, editedBodies }) => { + await post({ specAuthority: pin }); + unticked(editedBodies[0].body, TICK_ITEMS[0]); + unticked(editedBodies[0].body, TICK_ITEMS[13]); + for (const item of TICK_ITEMS.filter((_, i) => i !== 0 && i !== 13)) ticked(editedBodies[0].body, item); + }); + } + // An id the pinned spec cannot resolve (no such row) skips every PR row of its category. + for (const [judgeCriterion, skipped] of [["dod:99", TICK_ITEMS.slice(12)], ["ac:99", TICK_ITEMS.slice(0, 12)]]) { + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [openAct({ judgeCriterion })] }, async ({ post, editedBodies }) => { + await post({ specAuthority: pin }); + for (const item of TICK_ITEMS) (skipped.includes(item) ? unticked : ticked)(editedBodies[0].body, item); + }); + } + // The pinned spec resolves the id, but no unchecked PR row carries that text (the row is already checked): the whole category is skipped. + const preChecked = TICK_PR_BODY.replace(`- [ ] ${TICK_ITEMS[12]}`, `- [x] ${TICK_ITEMS[12]}`); + await withCompositionRound({ overallVerdict: "findings_present", prBody: preChecked, verifiedItems: TICK_ITEMS.slice(13), findings: [openAct({ judgeCriterion: "dod:0" })] }, async ({ post, editedBodies, calls }) => { + await post({ specAuthority: pin }); + assert.equal(editedBodies.length, 0); + assert.equal(bodyEdits(calls).length, 0); + }); +}); + +test("tick (#2898): a pinned spec and a PR body without an AC checklist still tick the verified issue rows", async () => { + const issueBody = ["## Acceptance criteria", "", "- [ ] criterion number 1", "- [ ] criterion number 2", ""].join("\n"); + const pin = await writeTickPin("pinned.json", `sha256:${"d".repeat(64)}`); + await withCompositionRound({ overallVerdict: "clean", prBody: "## Scope\n\nNo checklist here.\n", verifiedItems: TICK_ITEMS, issueBody }, async ({ post, editedBodies }) => { + await post({ specAuthority: pin, findingsSeverityCounts: CLEAN_COUNTS }); + assert.equal(editedBodies.length, 1); + assert.match(editedBodies[0].body, /^- \[x\] criterion number 1$/m); + assert.match(editedBodies[0].body, /^- \[x\] criterion number 2$/m); + }); +}); + +test("tick (#2898): a review-gate rerun is refused with the already-submitted refusal message", async () => { + await withTempDir(async (tempDir) => { + const { body } = await createReviewGateAndCaptureBody(tempDir, { nextAction: "none — informational review, no re-gate required" }); + const submitted = seededSubmittedReview({ id: 982, body }); + const ledgerPath = await writeReviewGateLedger(tempDir, [LOCATABLE_FINDING], { overallVerdict: "findings_present", verifiedItems: TICK_ITEMS }); + const { runChild, calls } = makeGhMock( + reviewGateFindingSurfaceEntries({ + files: [{ filename: "src/db.mjs", patch: SINGLE_SURFACE_PATCH }], + pendingReviews: [submitted], + reviews: [submitted], + submittedReviews: [submitted], + }), + ); + await assert.rejects( + upsertCheckpointVerdict({ + repo: "owner/repo", pr: 17, gate: "review", headSha: SINGLE_SURFACE_HEAD, verdict: "findings_present", + findingsSummary: "one locatable finding", nextAction: "stay draft and fix", + findingsLedger: ledgerPath, judgeSpecSource: "none", executionMode: "fanout_fanin", submit: "comment", + }, { env: runIdFreeEnv({ DEVLOOPS_RUN_ID: "" }), ghCommand: "gh", runChild, repoRoot: tempDir }), + /already-submitted review 982/, + ); + assert.equal(bodyEdits(calls).length, 0); + }, { prefix: "dev-loops-upsert-review-refused-no-tick-" }); +}); + +test("tick (#2898): an issue id in a locatable finding summary is refused before any PR or issue body edit", async () => { + const finding = { angle: "correctness", severity: "medium", summary: "see 1234 and #1234", files: ["src/db.mjs"], line: 2, recommendation: "fix it" }; + await withCompositionRound({ + overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [finding], + prFiles: [{ filename: "src/db.mjs", patch: SINGLE_SURFACE_PATCH }], + }, async ({ post, calls }) => { + await assert.rejects(post({ verdict: "findings_present" }), /issue\/PR|#1234/i); + assert.equal(bodyEdits(calls).length, 0); + }); +}); + +test("tick (#2898): an open act item carrying checkedCriteria and no judgeCriterion keeps the named category unticked", async () => { + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, findings: [openAct({ checkedCriteria: ["dod:1"] })] }, async ({ post, editedBodies }) => { + await post({ specAuthority: await writeTickPin("pinned.json", computeSpecDigest(extractSpecFromBody(TICK_PR_BODY))) }); + assert.match(editedBodies[0].body, new RegExp(`^- \\[ \\] ${TICK_ITEMS[13]}$`, "m")); + assert.match(editedBodies[0].body, new RegExp(`^- \\[x\\] ${TICK_ITEMS[0]}$`, "m")); + assert.match(editedBodies[0].body, new RegExp(`^- \\[x\\] ${TICK_ITEMS[12]}$`, "m")); + }); +}); + +test("tick (#2898): the deferred write re-applies the tick to the live body and keeps a concurrent edit", async () => { + await withCompositionRound({ overallVerdict: "clean", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS }, async ({ post, editedBodies, control }) => { + control.concurrentPrBody = `${TICK_PR_BODY}\nConcurrent note added during the round.\n`; + await post({ findingsSeverityCounts: CLEAN_COUNTS }); + assert.equal(editedBodies.length, 1); + assert.match(editedBodies[0].body, /^Concurrent note added during the round\.$/m); + for (const item of TICK_ITEMS) assert.match(editedBodies[0].body, new RegExp(`^- \\[x\\] ${item}$`, "m")); + }); +}); + +test("tick (#2898): a concurrent unchecked row added to the live checklist refuses before any body write", async () => { + await withCompositionRound({ overallVerdict: "clean", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS }, async ({ post, editedBodies, control }) => { + control.concurrentPrBody = TICK_PR_BODY.replace("## Definition of done\n", "## Definition of done\n\n- [ ] a row added concurrently\n"); + await assert.rejects(() => post({ findingsSeverityCounts: CLEAN_COUNTS }), /checklist of PR 17 changed after the gate read it/); + assert.equal(editedBodies.length, 0); + }); +}); + +test("tick (#2898): an ac id skips only the AC row of a label the DoD repeats; the DoD row still ticks", async () => { + const body = ["## Acceptance criteria", "", "- [ ] Shared label", "", "## Definition of done", "", "- [ ] Shared label", ""].join("\n"); + const pin = await writeTickPin("pinned.json", computeSpecDigest(extractSpecFromBody(body))); + for (const [judgeCriterion, acTicked, dodTicked] of [["ac:0", false, true], ["dod:0", true, false]]) { + await withCompositionRound({ overallVerdict: "findings_present", prBody: body, verifiedItems: ["Shared label"], findings: [openAct({ judgeCriterion })] }, async ({ post, editedBodies }) => { + await post({ specAuthority: pin }); + const [acPart, dodPart] = editedBodies[0].body.split("## Definition of done"); + assert.equal(/- \[x\] Shared label/.test(acPart), acTicked, `${judgeCriterion} AC`); + assert.equal(/- \[x\] Shared label/.test(dodPart), dodTicked, `${judgeCriterion} DoD`); + }); + } + // An unresolved id (no pin) skips only its own category too. + await withCompositionRound({ overallVerdict: "findings_present", prBody: body, verifiedItems: ["Shared label"], findings: [openAct({ judgeCriterion: "ac:0" })] }, async ({ post, editedBodies }) => { + await post(); + const [acPart, dodPart] = editedBodies[0].body.split("## Definition of done"); + assert.doesNotMatch(acPart, /- \[x\]/); + assert.match(dodPart, /- \[x\] Shared label/); + }); +}); + +test("tick (#2898): an open dod id keeps a Definition of done nested under Acceptance criteria unticked", async () => { + const body = ["## Acceptance criteria", "", "- [ ] Shared label", "", "### Definition of done", "", "- [ ] Shared label", ""].join("\n"); + const pin = await writeTickPin("pinned.json", computeSpecDigest(extractSpecFromBody(body))); + await withCompositionRound({ overallVerdict: "findings_present", prBody: body, verifiedItems: ["Shared label"], findings: [openAct({ judgeCriterion: "dod:0" })] }, async ({ post, editedBodies }) => { + await post({ specAuthority: pin }); + const [acPart, dodPart] = editedBodies[0].body.split("### Definition of done"); + assert.match(acPart, /- \[x\] Shared label/); + assert.doesNotMatch(dodPart, /- \[x\]/); + }); +}); + +test("tick (#2898): a concurrent heading edit that moves a row into the AC scope refuses before any body write", async () => { + const withValidation = (heading) => TICK_PR_BODY.replace("## Definition of done\n", `${heading}\n\n- [ ] an unverified row\n\n## Definition of done\n`); + await withCompositionRound({ overallVerdict: "clean", prBody: withValidation("## Validation"), verifiedItems: TICK_ITEMS }, async ({ post, editedBodies, control }) => { + control.concurrentPrBody = withValidation("### Validation"); + await assert.rejects(() => post({ findingsSeverityCounts: CLEAN_COUNTS }), /checklist of PR 17 changed after the gate read it/); + assert.equal(editedBodies.length, 0); + }); +}); + +test("tick (#2898): a criterion supplied only through --findings-json keeps its row unticked next to a trusted ledger", async () => { + await withCompositionRound({ overallVerdict: "findings_present", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS }, async ({ post, editedBodies }) => { + const dir = await mkdtemp(path.join(os.tmpdir(), "dev-loops-tick-findings-")); + const findingsJson = path.join(dir, "findings.json"); + await writeFile(findingsJson, JSON.stringify([openAct({ judgeCriterion: TICK_ITEMS[12] })])); + await post({ findingsJson }); + assert.match(editedBodies[0].body, new RegExp(`^- \\[ \\] ${TICK_ITEMS[12]}$`, "m")); + assert.match(editedBodies[0].body, new RegExp(`^- \\[x\\] ${TICK_ITEMS[1]}$`, "m")); + }); +}); + +test("tick (#2898): the dry-run artifact keeps an issue's unticked AC row that only the reload reads", async () => { + const extraIssueBodies = { 901: ["## Acceptance criteria", "", "- [ ] only the reload sees this row", ""].join("\n") }; + // View 1 (coordination load of issue 901) fails once; the tick fetch and the reload then read issue 900 and 901. + const round = { overallVerdict: "clean", prBody: TICK_PR_BODY, verifiedItems: TICK_ITEMS, issueBody: UNTICKED_TICK_ISSUE_BODY, closingIssues: [900, 901], extraIssueBodies, failIssueViewAfter: 1, failIssueViewCount: 1 }; + await withCompositionRound(round, async ({ post }) => { + const result = await post({ findingsSeverityCounts: CLEAN_COUNTS }); + assert.deepEqual(result.gateBlockers, [{ kind: "unticked spec-of-record Acceptance criteria", item: "only the reload sees this row" }]); + }); +});