diff --git a/scripts/github-mutate.mjs b/scripts/github-mutate.mjs index d9a9f2fa..d562836b 100755 --- a/scripts/github-mutate.mjs +++ b/scripts/github-mutate.mjs @@ -1,5 +1,5 @@ #!/usr/bin/env node -import { appendFileSync, readFileSync } from "node:fs"; +import { appendFileSync, existsSync, readFileSync } from "node:fs"; import { runGitHubCommandWithRetry } from "./lib/github-retry.mjs"; import { executeMutationDocument } from "./lib/mutation-document-execution.mjs"; @@ -38,6 +38,26 @@ export function mutationRunner(command, argv, options) { }); } +function completedKeysFromAudit(auditPath) { + if (!auditPath || !existsSync(auditPath)) return []; + const keys = []; + for (const line of readFileSync(auditPath, "utf8").split(/\r?\n/)) { + if (!line.trim()) continue; + try { + const receipt = JSON.parse(line); + if ( + receipt?.operationKey && + (receipt.status === "succeeded" || receipt.status === "already_applied") + ) { + keys.push(String(receipt.operationKey)); + } + } catch { + // Older audit lines may be whole-batch JSON; skip unreadable receipts. + } + } + return keys; +} + try { const args = parseArgs(process.argv.slice(2)); const document = JSON.parse(readFileSync(args.requestPath, "utf8")); @@ -45,11 +65,16 @@ try { document, execute: args.execute, runner: mutationRunner, + dependencies: { + completedOperationKeys: completedKeysFromAudit(args.auditPath), + onReceipt(receipt) { + if (!args.auditPath) return; + appendFileSync(args.auditPath, `${JSON.stringify(receipt)}\n`, "utf8"); + }, + }, }); - if (args.auditPath) { - appendFileSync(args.auditPath, `${JSON.stringify(output)}\n`, "utf8"); - } process.stdout.write(`${JSON.stringify(output, null, 2)}\n`); + if (output?.partialFailure) process.exitCode = 2; } catch (error) { console.error(String(error?.message || error)); process.exit(2); diff --git a/scripts/lib/authority-head-refresh.mjs b/scripts/lib/authority-head-refresh.mjs index f00a7b33..e5ed02f2 100644 --- a/scripts/lib/authority-head-refresh.mjs +++ b/scripts/lib/authority-head-refresh.mjs @@ -80,11 +80,11 @@ function fetchLiveHead({ request, runner }) { * Refresh the `expectedHead` and bind `authorityBranch` for every PR-scoped * operation against live GitHub state before the authorization prompt. * Operations without a PR head binding are returned unchanged. A failed read - * fails closed before any approval prompt. + * fails closed before any approval prompt. A supplied expected head is verified + * and never replaced; head movement requires regenerating the operation. * - * Returns `{ requests, refreshed }` where `refreshed` contains only operations - * whose expected head moved. Branch identity is nevertheless bound to every - * PR-scoped output request. + * Returns `{ requests, refreshed }` where `refreshed` is empty on success. + * Branch identity is bound to every PR-scoped output request. */ export function refreshExpectedHeads({ requests = [], @@ -98,21 +98,16 @@ export function refreshExpectedHeads({ const request = requests[index]; if (!headRefreshCandidate(request)) continue; const observed = fetchLiveHead({ request, runner }); + if (String(observed.head).toLowerCase() !== String(request.expectedHead).toLowerCase()) { + throw new Error( + `expected_head_mismatch: expected ${request.expectedHead}, observed ${observed.head}`, + ); + } output[index] = { ...output[index], - expectedHead: observed.head, + expectedHead: request.expectedHead, authorityBranch: observed.branch, }; - if (String(observed.head).toLowerCase() !== String(request.expectedHead).toLowerCase()) { - refreshed.push({ - index, - pr: request.pr, - repo: request.repo, - from: String(request.expectedHead), - to: observed.head, - branch: observed.branch, - }); - } } return { requests: output, refreshed }; } diff --git a/scripts/lib/base-health-live.mjs b/scripts/lib/base-health-live.mjs index dcf21e86..a41edc1f 100644 --- a/scripts/lib/base-health-live.mjs +++ b/scripts/lib/base-health-live.mjs @@ -1,9 +1,9 @@ -import { spawnSync } from "node:child_process"; +import { boundedSpawnSync } from "./subprocess-policy.mjs"; import { collectPaginated } from "./github-pagination.mjs"; import { snapshotIntegritySha256 } from "./snapshot-schema.mjs"; function defaultRunGh(args) { - const result = spawnSync("gh", args, { + const result = boundedSpawnSync("gh", args, { encoding: "utf8", maxBuffer: 50 * 1024 * 1024, }); diff --git a/scripts/lib/delivery-workflow-controller.mjs b/scripts/lib/delivery-workflow-controller.mjs index 550697fe..4e9b79ce 100644 --- a/scripts/lib/delivery-workflow-controller.mjs +++ b/scripts/lib/delivery-workflow-controller.mjs @@ -279,8 +279,14 @@ export function createDeliveryWorkflowController(options = {}) { headSha = next.headSha || null; changed = true; } - if (Object.hasOwn(next, "issue")) issue = next.issue; - if (Object.hasOwn(next, "pr")) pr = next.pr; + if (Object.hasOwn(next, "issue") && next.issue !== issue) { + issue = next.issue; + changed = true; + } + if (Object.hasOwn(next, "pr") && next.pr !== pr) { + pr = next.pr; + changed = true; + } if (changed) { stateGeneration += 1; attempts.noProgressSteps = 0; diff --git a/scripts/lib/eval-contracts.mjs b/scripts/lib/eval-contracts.mjs index 0194fe73..c354564e 100644 --- a/scripts/lib/eval-contracts.mjs +++ b/scripts/lib/eval-contracts.mjs @@ -2,7 +2,7 @@ import { createHash } from "node:crypto"; import { existsSync, readFileSync, readdirSync, statSync } from "node:fs"; import { basename, join, relative, sep } from "node:path"; -import { routeShippingGithubPrompt } from "./skill-router.mjs"; +import { ROUTABLE_WORKFLOWS, routeShippingGithubPrompt } from "./skill-router.mjs"; import { KNOWN_LENS_IDS, KNOWN_SECURITY_SURFACE_IDS, planReviewScope } from "./review-scope.mjs"; import { ASSERTION_TO_PROBE, @@ -88,9 +88,7 @@ function inferredWorkflow(item) { if (item.expected_workflow !== undefined) return item.expected_workflow; if (!["must-trigger", "routing"].includes(item.category)) return undefined; const workflows = (item.expected_resources || []).filter((resource) => - /^references\/(fix-pr-bots|watch-pr|re-review-pr|research-issue|create-pr-for-issue|full-review-pr|security-review|status|merge-pr|supersede-pr|overtake-pr)\.md$/.test( - resource, - ), + ROUTABLE_WORKFLOWS.includes(resource), ); return workflows.length === 1 ? workflows[0] : undefined; } diff --git a/scripts/lib/github-lifecycle-mutation-broker.mjs b/scripts/lib/github-lifecycle-mutation-broker.mjs index 6abb8fbc..425f9b2b 100644 --- a/scripts/lib/github-lifecycle-mutation-broker.mjs +++ b/scripts/lib/github-lifecycle-mutation-broker.mjs @@ -1,4 +1,3 @@ -import { spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; import { classifyAuthority } from "./authority-grant.mjs"; @@ -14,6 +13,7 @@ import { validateLifecycleMutation, verifyLifecycleMutation, } from "./lifecycle-mutations.mjs"; +import { boundedSpawnSync } from "./subprocess-policy.mjs"; const ACTIONS = new Set([ "push_code", @@ -207,7 +207,7 @@ export function planLifecycleMutationRequest( export function executeLifecycleMutationRequest({ request, execute = false, - runner = (command, args, options) => spawnSync(command, args, options), + runner = boundedSpawnSync, authorityPublicKey = null, requireTrustedAuthority = false, authorityNow, diff --git a/scripts/lib/github-mutation-broker.mjs b/scripts/lib/github-mutation-broker.mjs index e37396be..4dbcf155 100644 --- a/scripts/lib/github-mutation-broker.mjs +++ b/scripts/lib/github-mutation-broker.mjs @@ -1,4 +1,3 @@ -import { spawnSync } from "node:child_process"; import { createHash } from "node:crypto"; import { classifyAuthority } from "./authority-grant.mjs"; @@ -9,6 +8,7 @@ import { import { authorizeMutation } from "./mutation-policy.mjs"; import { evaluateHeadBranchCleanup } from "./merge-branch-cleanup.mjs"; import { classifyMergeOutcome, readMergeState } from "./merge-outcome.mjs"; +import { boundedSpawnSync } from "./subprocess-policy.mjs"; const PR_ACTIONS = new Set([ "post_review", @@ -732,7 +732,7 @@ export function planMutationRequest( export function executeMutationRequest({ request, execute = false, - runner = (command, args, options) => spawnSync(command, args, options), + runner = boundedSpawnSync, authorityPublicKey = null, requireTrustedAuthority = false, authorityNow, @@ -756,12 +756,28 @@ export function executeMutationRequest({ const commentEditTarget = verifyOwnCommentTarget({ request: plan.request, runner }); const mergeState = readMergeState({ request: plan.request, runner }); const preMergeOutcome = classifyMergeOutcome(mergeState); - if (preMergeOutcome) { + if (preMergeOutcome === "merged") { return { ...plan, executed: false, status: "already_applied", - outcome: preMergeOutcome === "merged" ? "already_merged" : preMergeOutcome, + outcome: "already_merged", + observedHead, + observedBase: retargetState?.observedBase ?? null, + threadTarget, + commentEditTarget, + existingMutation: null, + idempotencyClaim: null, + stdout: "", + verification: mergeState, + }; + } + if (preMergeOutcome === "queued" || preMergeOutcome === "auto_merge_enabled") { + return { + ...plan, + executed: false, + status: "not_merged", + outcome: preMergeOutcome, observedHead, observedBase: retargetState?.observedBase ?? null, threadTarget, @@ -861,7 +877,9 @@ export function executeMutationRequest({ if (plan.request.action === "merge_pr") { verification = readMergeState({ request: plan.request, runner }); outcome = classifyMergeOutcome(verification); - if (!outcome) throw new Error("merge_outcome_unverified"); + if (outcome !== "merged") { + throw new Error(`merge_outcome_unverified:${outcome || "unknown"}`); + } } else if (REVIEW_THREAD_ACTIONS.has(plan.request.action)) { verification = verifyReviewThreadTarget({ request: plan.request, runner }); if (verification.isResolved !== true) { diff --git a/scripts/lib/live-snapshot.mjs b/scripts/lib/live-snapshot.mjs index 95eb017e..88c89192 100644 --- a/scripts/lib/live-snapshot.mjs +++ b/scripts/lib/live-snapshot.mjs @@ -152,6 +152,7 @@ export function captureLiveSnapshot({ repo, pr, maxAgeSeconds = 300, + expectedHead = null, runner = boundedSpawnSync, } = {}) { const result = runner( @@ -177,6 +178,7 @@ export function captureLiveSnapshot({ snapshot, repo, pr, + expectedHead, maxAgeSeconds, requireComplete: false, }); diff --git a/scripts/lib/mutation-document-execution.mjs b/scripts/lib/mutation-document-execution.mjs index 838839b6..02d66c8b 100644 --- a/scripts/lib/mutation-document-execution.mjs +++ b/scripts/lib/mutation-document-execution.mjs @@ -145,18 +145,61 @@ export function executeMutationDocument({ } } + const completedKeys = new Set( + Array.isArray(deps.completedOperationKeys) ? deps.completedOperationKeys : [], + ); const results = []; + let partialFailure = false; + for (const request of requests) { - results.push( - deps.executeMutationWithAuthority({ + const operationKey = + request.idempotencyKey || + [request.action, request.repo, request.pr ?? request.issue ?? ""].join(":"); + if (completedKeys.has(operationKey)) { + const skipped = { + action: request.action, + status: "already_applied", + outcome: "already_completed", + skipped: true, + operationKey, + }; + results.push(skipped); + deps.onReceipt?.(skipped); + continue; + } + + try { + const result = deps.executeMutationWithAuthority({ request, execute, runner, env: effectiveEnv, readFile, - }), - ); + }); + const receipt = { ...result, operationKey }; + results.push(receipt); + deps.onReceipt?.(receipt); + if (receipt?.status === "succeeded" || receipt?.status === "already_applied") { + completedKeys.add(operationKey); + } + } catch (error) { + partialFailure = true; + const failed = { + action: request.action, + status: "failed", + error: String(error?.message || error), + operationKey, + }; + results.push(failed); + deps.onReceipt?.(failed); + break; + } + } + + if (normalized.singular) { + const only = results[0]; + if (only?.status === "failed") throw new Error(only.error); + return only; } - if (normalized.singular) return results[0]; - return { batch: true, results }; + return { batch: true, results, partialFailure }; } diff --git a/scripts/lib/skill-router.mjs b/scripts/lib/skill-router.mjs index aef7b7c3..773f52ab 100644 --- a/scripts/lib/skill-router.mjs +++ b/scripts/lib/skill-router.mjs @@ -34,6 +34,48 @@ const MULTI_BASE_REQUEST = /\b(?:backport|back-port|port)\b[\s\S]{0,180}\b(?:pr| const DELIVERY_NAME = /\bgithub[- ]?delivery\b/; const DELIVERY_UPDATE = /\b(update|upgrade)\b[\s\S]*\bgithub[- ]?delivery\b|\bgithub[- ]?delivery\b[\s\S]*\b(update|upgrade|latest stable release)\b/; const DELIVERY_CONFIG = /\b(set ?up|install|configure|configuration|settings?|protection mode|windows hello)\b[\s\S]*\bgithub[- ]?delivery\b|\bgithub[- ]?delivery\b[\s\S]*\b(set ?up|install|configure|configuration|settings?|protection mode|windows hello)\b/; +const SPEC_STANDARDS_REQUEST = /\b(?:spec(?:ification)? and standards review|standards review|spec(?:ification)? review)\b/; +const STACKED_PR_REQUEST = /\b(?:stacked prs?|pr stack|restack|open pr stack|bottom pr in (?:my|the) stack|retarget (?:and rebase )?the children|manage[- ]stacked[- ]prs)\b/; +const AGENT_BRIEF_REQUEST = /\b(?:ready[- ]for[- ]agent|agent brief|issue contract)\b/; +const ISSUE_TRIAGE_REQUEST = /\btriage\b[\s\S]{0,80}\b(?:issue|issues|ticket|tickets)\b|\b(?:issue|issues|ticket|tickets)\b[\s\S]{0,80}\btriage\b/; +const QA_INTAKE_REQUEST = /\bqa intake\b|\bfile\b[\s\S]{0,80}\breproducible\b[\s\S]{0,80}\bbug report/; +const CONFLICT_REQUEST = /\b(?:merge conflicts?|git conflicts?|resolve(?:\s+the)?(?:\s+merge)?\s+conflicts?)\b/; +const OUT_OF_SCOPE_REQUEST = /\b(?:out of scope|rejected enhancement|not now)\b/; + +export const PUBLIC_ROUTE_HANDOFFS = Object.freeze([ + "split-to-prs", + "finishing-a-development-branch", + "git-workflow-and-versioning", +]); + +export const ROUTABLE_WORKFLOWS = Object.freeze([ + "references/update.md", + "references/configuration.md", + "references/open-work-status.md", + "references/consolidate-prs.md", + "references/multi-base-delivery.md", + "references/work-item-delivery.md", + "references/stacked-prs.md", + "references/prepare-and-merge-pr.md", + "references/merge-pr.md", + "references/status.md", + "references/supersede-pr.md", + "references/overtake-pr.md", + "references/spec-standards-review.md", + "references/full-review-pr.md", + "references/simplify-pr.md", + "references/security-review.md", + "references/re-review-pr.md", + "references/watch-pr.md", + "references/create-pr-for-issue.md", + "references/create-pr-from-local-work.md", + "references/research-issue.md", + "references/issue-workflows.md", + "references/agent-brief.md", + "references/out-of-scope.md", + "references/resolve-conflicts.md", + "references/fix-pr-bots.md", +]); function prepareAndMergeActions(text) { const actions = ["merge_pr", "post_comment", "post_issue_comment", "close_linked_issue"]; @@ -83,7 +125,12 @@ function isPrepareAndMergeRequest(text) { } function isMergeDiscussion(text) { - return PR_REFERENCE.test(text) && MERGE_INTENT.test(text.replace(MERGE_READY_PHRASE, "")) && !hasExplicitMergeIntent(text); + return ( + PR_REFERENCE.test(text) && + MERGE_INTENT.test(text.replace(MERGE_READY_PHRASE, "")) && + !hasExplicitMergeIntent(text) && + !CONFLICT_REQUEST.test(text) + ); } function isOpenWorkRequest(text) { @@ -124,6 +171,16 @@ export function routeShippingGithubPrompt(prompt) { readOnly ? [] : workItemDeliveryActions(text), ); } + if (CONFLICT_REQUEST.test(text)) { + return result("references/resolve-conflicts.md", "maintainer", ["push_code"]); + } + if (STACKED_PR_REQUEST.test(text)) { + return result( + "references/stacked-prs.md", + hasExplicitMergeIntent(text) ? "maintainer" : "read-only", + hasExplicitMergeIntent(text) ? ["merge_pr", "post_comment"] : [], + ); + } if (isPrepareAndMergeRequest(text)) return result("references/prepare-and-merge-pr.md", "maintainer", prepareAndMergeActions(text)); if ((hasExplicitMergeIntent(text) && PR_REFERENCE.test(text)) || /^merge it\b/.test(text) || /^ship it\b/.test(text)) { @@ -137,6 +194,9 @@ export function routeShippingGithubPrompt(prompt) { if (/\b(overtake|take over|maintainer overtake|take it over)\b[\s\S]*\b(?:pr|pull request)\b/.test(text)) { return result("references/overtake-pr.md", "maintainer", ["push_code", "post_comment", "close_pr"]); } + if (SPEC_STANDARDS_REQUEST.test(text) && PR_WORD.test(text)) { + return result("references/spec-standards-review.md", "review"); + } if (FULL_REVIEW_REQUEST.test(text)) { const simplifyRequested = SIMPLIFY_REQUEST.test(text); return result("references/full-review-pr.md", /\bfix\b/.test(text) || simplifyRequested ? "maintainer" : "review", simplifyRequested ? ["push_code"] : []); @@ -156,6 +216,18 @@ export function routeShippingGithubPrompt(prompt) { const issueCreationAction = issueCreationActionForPrompt(text); if (issueCreationAction) return result("references/issue-workflows.md", "maintainer", [issueCreationAction]); + if (ISSUE_TRIAGE_REQUEST.test(text) && !CONSOLIDATE_PR_REQUEST.test(text)) { + return result("references/issue-workflows.md", "maintainer"); + } + if (QA_INTAKE_REQUEST.test(text)) { + return result("references/issue-workflows.md", "maintainer"); + } + if (AGENT_BRIEF_REQUEST.test(text)) { + return result("references/agent-brief.md", "maintainer"); + } + if (OUT_OF_SCOPE_REQUEST.test(text)) { + return result("references/out-of-scope.md", "read-only"); + } if (FIX_REVIEW_REQUEST.test(text) || /\bmake\b[\s\S]*\b(?:pr|pull request)\b[\s\S]*\bmerge[- ]?ready\b/.test(text)) { return result("references/fix-pr-bots.md", "maintainer", ["push_code"]); diff --git a/scripts/merge-pr-driver.mjs b/scripts/merge-pr-driver.mjs index 9bb31b07..32d26f42 100644 --- a/scripts/merge-pr-driver.mjs +++ b/scripts/merge-pr-driver.mjs @@ -5,7 +5,7 @@ * Chains the existing gates, broker, review evidence, and cleanup evaluators so * the agent reviews one plan instead of hand-rolling each step. */ -import { spawnSync } from "node:child_process"; +import { boundedSpawnSync } from "./lib/subprocess-policy.mjs"; import { appendFileSync, realpathSync } from "node:fs"; import { pathToFileURL } from "node:url"; @@ -206,7 +206,7 @@ export function detectMergeMethod(capabilities = null) { export function readRepositoryMergeCapabilities( repo, - runner = (command, args, options) => spawnSync(command, args, options), + runner = boundedSpawnSync, ) { const result = runner("gh", ["api", `repos/${repo}`], { encoding: "utf8", @@ -480,6 +480,11 @@ async function main() { }); const merged = receipts.find((item) => item.name === "merge")?.receipt; + if (!isFinalMergeOutcome(merged)) { + throw new Error( + `merge_not_final:${merged?.outcome || merged?.status || "unknown"}`, + ); + } const cleanup = evaluateHeadBranchCleanup({ actorLogin: process.env.GH_ACTOR_LOGIN || null, headOwnerLogin: null, diff --git a/scripts/ship-gate-snapshot.mjs b/scripts/ship-gate-snapshot.mjs index a5f6fa80..f9a1e51c 100755 --- a/scripts/ship-gate-snapshot.mjs +++ b/scripts/ship-gate-snapshot.mjs @@ -5,7 +5,6 @@ * Requires: gh auth */ import { Buffer } from "node:buffer"; -import { spawnSync } from "node:child_process"; import { writeFileSync } from "node:fs"; import { evaluateRequiredCheckWorkflowMapping, @@ -26,6 +25,7 @@ import { attachRepositoryPermissions, feedbackPermissionLogins, } from "./lib/feedback-authority.mjs"; +import { boundedSpawnSync } from "./lib/subprocess-policy.mjs"; function parseArgs(argv) { const positionals = []; @@ -57,7 +57,7 @@ function parseArgs(argv) { } function ghOk(args) { - const result = spawnSync("gh", args, { + const result = boundedSpawnSync("gh", args, { encoding: "utf8", maxBuffer: 50 * 1024 * 1024, }); diff --git a/scripts/ship-gate.mjs b/scripts/ship-gate.mjs index 7aafe73b..504ed7b2 100755 --- a/scripts/ship-gate.mjs +++ b/scripts/ship-gate.mjs @@ -56,6 +56,7 @@ try { : captureLiveSnapshot({ repo: args.repo, pr: args.pr, + expectedHead: args.expectedHead, maxAgeSeconds: args.maxAgeSeconds, }); diff --git a/tests/unit/authority-head-refresh.test.mjs b/tests/unit/authority-head-refresh.test.mjs index 4cb12a5e..1873080c 100644 --- a/tests/unit/authority-head-refresh.test.mjs +++ b/tests/unit/authority-head-refresh.test.mjs @@ -53,39 +53,33 @@ test("refreshExpectedHeads binds the live PR branch even when the head already m test("refreshExpectedHeads updates a stale head, binds branch, and reports the delta", () => { const runner = () => liveHead("b".repeat(40), "feature/review"); - const result = refreshExpectedHeads({ - requests: [request()], - runner, - }); - assert.equal(result.refreshed.length, 1); - assert.deepEqual(result.refreshed[0], { - index: 0, - pr: 32, - repo: "acme/widgets", - from: "a".repeat(40), - to: "b".repeat(40), - branch: "feature/review", - }); - assert.equal(result.requests[0].expectedHead, "b".repeat(40)); - assert.equal(result.requests[0].authorityBranch, "feature/review"); - assert.equal(result.requests[0].body, "Status update"); + assert.throws( + () => refreshExpectedHeads({ requests: [request()], runner }), + /expected_head_mismatch: expected a{40}, observed b{40}/, + ); }); -test("refreshExpectedHeads refreshes only stale operations while binding every PR branch", () => { +test("refreshExpectedHeads binds matching heads and fails closed before authorizing a moved head", () => { const runner = (args) => liveHead("c".repeat(40), `feature/pr-${args[3]}`); const stale = request({ pr: 1, expectedHead: "a".repeat(40), idempotencyKey: "k1" }); const fresh = request({ pr: 2, expectedHead: "c".repeat(40), idempotencyKey: "k2" }); + assert.throws( + () => + refreshExpectedHeads({ + requests: [stale, fresh, request({ action: "post_issue_comment", pr: 3, expectedHead: undefined })], + runner, + }), + /expected_head_mismatch/, + ); const result = refreshExpectedHeads({ - requests: [stale, fresh, request({ action: "post_issue_comment", pr: 3, expectedHead: undefined })], + requests: [fresh, request({ action: "post_issue_comment", pr: 3, expectedHead: undefined })], runner, }); - assert.deepEqual(result.refreshed.map((entry) => entry.pr), [1]); + assert.equal(result.refreshed.length, 0); assert.equal(result.requests[0].expectedHead, "c".repeat(40)); - assert.equal(result.requests[0].authorityBranch, "feature/pr-1"); - assert.equal(result.requests[1].expectedHead, "c".repeat(40)); - assert.equal(result.requests[1].authorityBranch, "feature/pr-2"); - assert.equal(result.requests[2].action, "post_issue_comment"); - assert.equal(result.requests[2].authorityBranch, undefined); + assert.equal(result.requests[0].authorityBranch, "feature/pr-2"); + assert.equal(result.requests[1].action, "post_issue_comment"); + assert.equal(result.requests[1].authorityBranch, undefined); }); test("refreshExpectedHeads fails closed when live head or branch evidence is invalid", () => { diff --git a/tests/unit/authority-mode-enforcement.test.mjs b/tests/unit/authority-mode-enforcement.test.mjs index cdd09afb..4db75441 100644 --- a/tests/unit/authority-mode-enforcement.test.mjs +++ b/tests/unit/authority-mode-enforcement.test.mjs @@ -133,7 +133,9 @@ test("mutation document does not prompt in off mode even when the action is intr }); assert.equal(authorized, false); assert.equal(executed, true); - assert.deepEqual(result, { action: "post_comment", executed: true }); + assert.equal(result.action, "post_comment"); + assert.equal(result.executed, true); + assert.equal(result.operationKey, "test-key"); }); test("mutation document still batches authority in high-assurance mode", () => { diff --git a/tests/unit/github-mutate-entrypoint.test.mjs b/tests/unit/github-mutate-entrypoint.test.mjs index a7c3f80d..bcd0a0e6 100644 --- a/tests/unit/github-mutate-entrypoint.test.mjs +++ b/tests/unit/github-mutate-entrypoint.test.mjs @@ -11,4 +11,6 @@ test("github-mutate delegates routine orchestration to the mutation document exe assert.match(source, /executeMutationDocument/); assert.doesNotMatch(source, /executeMutationWithAuthority/); assert.match(source, /--request FILE \[--execute\] \[--audit FILE\]/); + assert.match(source, /completedKeysFromAudit/); + assert.match(source, /onReceipt/); }); diff --git a/tests/unit/head-integrity-gates.test.mjs b/tests/unit/head-integrity-gates.test.mjs new file mode 100644 index 00000000..2cd6de89 --- /dev/null +++ b/tests/unit/head-integrity-gates.test.mjs @@ -0,0 +1,105 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import test from "node:test"; + +import { refreshExpectedHeads } from "../../scripts/lib/authority-head-refresh.mjs"; +import { createDeliveryWorkflowController } from "../../scripts/lib/delivery-workflow-controller.mjs"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "../.."); +const SHA_A = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; +const SHA_B = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + +function readySnapshot({ headOid }) { + return { + schemaVersion: 1, + capturedAt: new Date().toISOString(), + repo: "acme/widget", + pr: 42, + headOid, + baseOid: "cccccccccccccccccccccccccccccccccccccccc", + mergeStateStatus: "CLEAN", + reviewDecision: "APPROVED", + statusCheckRollup: { state: "SUCCESS" }, + evidence: { + checks: { authoritative: true, sha: headOid, reason: "fixture" }, + }, + }; +} + +function runShipGate(args) { + return spawnSync(process.execPath, [join(ROOT, "scripts/ship-gate.mjs"), ...args], { + encoding: "utf8", + cwd: ROOT, + }); +} + +test("live ship gate rejects a moved expected head", () => { + const dir = mkdtempSync(join(tmpdir(), "gd-live-gate-")); + const snapshotPath = join(dir, "ready.json"); + writeFileSync(snapshotPath, JSON.stringify(readySnapshot({ headOid: SHA_B })), "utf8"); + + const result = runShipGate([ + "acme/widget", + "42", + "--expected-head", + SHA_A, + "--snapshot", + snapshotPath, + ]); + + assert.equal(result.status, 2); + assert.match(String(result.stderr || result.stdout || ""), /expected_head_mismatch/); +}); + +test("authority acquisition never retargets a stale reviewed request", () => { + const request = { + action: "post_review", + repo: "acme/widget", + pr: 42, + expectedHead: SHA_A, + body: "review of SHA A", + }; + + assert.throws( + () => + refreshExpectedHeads({ + requests: [request], + runner: () => + JSON.stringify({ + headRefOid: SHA_B, + headRefName: "feature", + }), + }), + /expected_head_mismatch/, + ); +}); + +test("changing the PR target invalidates controller evidence generation", () => { + const controller = createDeliveryWorkflowController({ + workflow: "status", + repo: "o/r", + pr: 1, + headSha: "h", + graph: { A: [] }, + startPhase: "A", + }); + controller.recordEvidence({ + key: "pr-ship-gate:o/r:1", + covers: ["checks"], + authoritative: true, + }); + + const update = controller.updateRefs({ pr: 2 }); + assert.equal(update.changed, true); + assert.ok(update.stateGeneration > 0); + const decision = controller.decideEvidence({ + key: "pr-ship-gate:o/r:1", + requires: ["checks"], + }); + assert.equal(decision.action, "allow"); + assert.notEqual(decision.reason, "evidence_already_covered"); +}); diff --git a/tests/unit/merge-outcome.test.mjs b/tests/unit/merge-outcome.test.mjs index 65c066b2..07e84a65 100644 --- a/tests/unit/merge-outcome.test.mjs +++ b/tests/unit/merge-outcome.test.mjs @@ -78,13 +78,15 @@ test("successful gh merge exit reports queued when GitHub placed the PR in merge inQueue: true, queueEntry: { state: "AWAITING_CHECKS" }, }); - const result = executeMutationRequest({ - request: mergeRequest(), - execute: true, - runner: harness.runner, - }); - assert.equal(result.status, "succeeded"); - assert.equal(result.outcome, "queued"); + assert.throws( + () => + executeMutationRequest({ + request: mergeRequest(), + execute: true, + runner: harness.runner, + }), + /merge_outcome_unverified:queued/, + ); assert.equal(harness.wasMergeCalled(), true); }); @@ -92,13 +94,16 @@ test("successful gh merge exit reports auto_merge_enabled when GitHub enabled au const harness = mergeRunner({ autoMergeRequest: { enabledAt: "2026-08-09T09:00:00Z", mergeMethod: "MERGE" }, }); - const result = executeMutationRequest({ - request: mergeRequest(), - execute: true, - runner: harness.runner, - }); - assert.equal(result.status, "succeeded"); - assert.equal(result.outcome, "auto_merge_enabled"); + assert.throws( + () => + executeMutationRequest({ + request: mergeRequest(), + execute: true, + runner: harness.runner, + }), + /merge_outcome_unverified:auto_merge_enabled/, + ); + assert.equal(harness.wasMergeCalled(), true); }); test("merge broker reports merged only when GitHub exposes merged state", () => { @@ -129,6 +134,36 @@ test("already merged PR is an idempotent outcome and does not invoke gh pr merge assert.equal(harness.wasMergeCalled(), false); }); +test("already queued PR is not merged and does not invoke gh pr merge", () => { + const harness = mergeRunner({}, { + inQueue: true, + queueEntry: { state: "AWAITING_CHECKS" }, + }); + const result = executeMutationRequest({ + request: mergeRequest(), + execute: true, + runner: harness.runner, + }); + assert.equal(result.status, "not_merged"); + assert.equal(result.outcome, "queued"); + assert.equal(result.executed, false); + assert.equal(harness.wasMergeCalled(), false); +}); + +test("already enabled auto-merge is not a final merge and does not invoke gh pr merge", () => { + const harness = mergeRunner({}, { + autoMergeRequest: { enabledAt: "2026-08-09T09:00:00Z", mergeMethod: "MERGE" }, + }); + const result = executeMutationRequest({ + request: mergeRequest(), + execute: true, + runner: harness.runner, + }); + assert.equal(result.status, "not_merged"); + assert.equal(result.outcome, "auto_merge_enabled"); + assert.equal(harness.wasMergeCalled(), false); +}); + test("merge transaction does not post merged thanks for a queued outcome", () => { const calls = []; const receipts = executeMergeTransaction({ diff --git a/tests/unit/mutation-document-execution.test.mjs b/tests/unit/mutation-document-execution.test.mjs index 75899729..85f56444 100644 --- a/tests/unit/mutation-document-execution.test.mjs +++ b/tests/unit/mutation-document-execution.test.mjs @@ -56,7 +56,9 @@ test("dry-run never requests trusted authority", () => { }); assert.equal(authorized, false); - assert.deepEqual(result, { action: "push_code", executed: false }); + assert.equal(result.action, "push_code"); + assert.equal(result.executed, false); + assert.equal(result.operationKey, "push_code:acme/widgets:"); }); test("execution batches only missing trusted grants and executes refreshed requests in order", () => { @@ -137,33 +139,73 @@ test("execution batches only missing trusted grants and executes refreshed reque ]); assert.deepEqual(result, { batch: true, + partialFailure: false, results: [ - { action: "post_comment", grant: "gd1.generated0.signature" }, - { action: "assign_issue", grant: "gd1.existing.signature" }, - { action: "create_pr", grant: "gd1.generated1.signature" }, + { + action: "post_comment", + grant: "gd1.generated0.signature", + operationKey: "post_comment:acme/widgets:4", + }, + { + action: "assign_issue", + grant: "gd1.existing.signature", + operationKey: "assign_issue:acme/widgets:7", + }, + { + action: "create_pr", + grant: "gd1.generated1.signature", + operationKey: "create_pr:acme/widgets:", + }, ], }); }); test("multi-request execution stops at the first failed operation", () => { const executed = []; - assert.throws( - () => - executeMutationDocument({ - document: [request("one"), request("two"), request("three")], - execute: false, - dependencies: { - mutationRequiresTrustedAuthority: () => false, - executeMutationWithAuthority({ request: current }) { - executed.push(current.action); - if (current.action === "two") throw new Error("second failed"); - return { action: current.action }; - }, - }, - }), - /second failed/, - ); + const persisted = []; + const result = executeMutationDocument({ + document: [request("one"), request("two"), request("three")], + execute: false, + dependencies: { + mutationRequiresTrustedAuthority: () => false, + onReceipt(receipt) { + persisted.push(receipt.action); + }, + executeMutationWithAuthority({ request: current }) { + executed.push(current.action); + if (current.action === "two") throw new Error("second failed"); + return { action: current.action, status: "succeeded" }; + }, + }, + }); assert.deepEqual(executed, ["one", "two"]); + assert.deepEqual(persisted, ["one", "two"]); + assert.equal(result.partialFailure, true); + assert.equal(result.results[0].status, "succeeded"); + assert.equal(result.results[1].status, "failed"); + assert.equal(result.results[1].error, "second failed"); + assert.equal(result.results.length, 2); +}); + +test("retries skip operations that already completed", () => { + const executed = []; + const result = executeMutationDocument({ + document: [request("one", { idempotencyKey: "k-one" }), request("two", { idempotencyKey: "k-two" })], + execute: false, + dependencies: { + completedOperationKeys: ["k-one"], + mutationRequiresTrustedAuthority: () => false, + executeMutationWithAuthority({ request: current }) { + executed.push(current.action); + return { action: current.action, status: "succeeded" }; + }, + }, + }); + assert.deepEqual(executed, ["two"]); + assert.equal(result.results[0].status, "already_applied"); + assert.equal(result.results[0].skipped, true); + assert.equal(result.results[1].action, "two"); + assert.equal(result.partialFailure, false); }); test("execution validates every request before prompting for authority", () => { diff --git a/tests/unit/relational-invariants.test.mjs b/tests/unit/relational-invariants.test.mjs index b7ab3eaa..b160cc0f 100644 --- a/tests/unit/relational-invariants.test.mjs +++ b/tests/unit/relational-invariants.test.mjs @@ -26,19 +26,17 @@ function commentRequest(overrides = {}) { test("refreshing a stale PR head changes the authority scope hash", () => { const request = commentRequest(); - const before = authorityScopeSha256(request); - - const refreshed = refreshExpectedHeads({ - requests: [request], - runner() { - return JSON.stringify({ headRefOid: B, headRefName: "feature/new-head" }); - }, - }).requests[0]; - - const after = authorityScopeSha256(refreshed); - assert.notEqual(after, before); - assert.equal(refreshed.expectedHead, B); - assert.equal(refreshed.authorityBranch, "feature/new-head"); + assert.throws( + () => + refreshExpectedHeads({ + requests: [request], + runner() { + return JSON.stringify({ headRefOid: B, headRefName: "feature/new-head" }); + }, + }), + /expected_head_mismatch/, + ); + assert.equal(request.expectedHead, A); }); test("binding a live branch also changes authority scope when the head is unchanged", () => { diff --git a/tests/unit/skill-router.test.mjs b/tests/unit/skill-router.test.mjs index f3116849..27a26c15 100644 --- a/tests/unit/skill-router.test.mjs +++ b/tests/unit/skill-router.test.mjs @@ -224,3 +224,54 @@ test("does not trigger for local pre-PR debugging", () => { null, ); }); + +test("routes stacked PR work to the stacked workflow", () => { + assert.equal( + routeShippingGithubPrompt("restack my GitHub PR stack after the bottom PR got review commits").workflow, + "references/stacked-prs.md", + ); + assert.equal( + routeShippingGithubPrompt("Show me the current open PR stack for this repo").mutationMode, + "read-only", + ); + assert.equal( + routeShippingGithubPrompt("Merge the bottom PR in my stack first").workflow, + "references/stacked-prs.md", + ); +}); + +test("routes spec and standards review separately from full review", () => { + const route = routeShippingGithubPrompt("spec and standards review on PR #32"); + assert.equal(route.workflow, "references/spec-standards-review.md"); + assert.equal(route.mutationMode, "review"); +}); + +test("routes agent brief, out-of-scope, conflict, and issue-lifecycle requests", () => { + assert.equal( + routeShippingGithubPrompt("write a ready-for-agent issue contract").workflow, + "references/agent-brief.md", + ); + assert.equal( + routeShippingGithubPrompt("record this as a rejected enhancement / out of scope").workflow, + "references/out-of-scope.md", + ); + assert.equal( + routeShippingGithubPrompt("resolve merge conflicts on this branch").workflow, + "references/resolve-conflicts.md", + ); + assert.equal( + routeShippingGithubPrompt("triage issues #12 and #15").workflow, + "references/issue-workflows.md", + ); + assert.equal( + routeShippingGithubPrompt("run QA intake and file a reproducible bug report").workflow, + "references/issue-workflows.md", + ); +}); + +test("stacked merge requests stay on stacked-prs with merge authority", () => { + const mergeStack = routeShippingGithubPrompt("merge the bottom PR in my stack first"); + assert.equal(mergeStack.workflow, "references/stacked-prs.md"); + assert.equal(mergeStack.mutationMode, "maintainer"); + assert.ok(mergeStack.explicitActions.includes("merge_pr")); +});