diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 772a2b8f..ab28f2d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,7 +20,6 @@ jobs: timeout-minutes: 5 outputs: node_compat: ${{ steps.scope.outputs.node_compat }} - windows_authority: ${{ steps.scope.outputs.windows_authority }} steps: - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -34,7 +33,7 @@ jobs: node-version: 24 package-manager-cache: false - - name: Detect compatibility and Windows Authority scope + - name: Detect Node compatibility scope id: scope shell: bash env: @@ -44,12 +43,13 @@ jobs: set -euo pipefail if [[ "${EVENT_NAME}" != "pull_request" ]]; then echo "node_compat=true" >> "${GITHUB_OUTPUT}" - echo "windows_authority=true" >> "${GITHUB_OUTPUT}" exit 0 fi + TRUSTED_SCOPE="${RUNNER_TEMP}/github-delivery-ci-scope-base.mjs" + git show "${BASE_SHA}:scripts/ci-scope.mjs" > "${TRUSTED_SCOPE}" git diff --name-only -z "${BASE_SHA}"...HEAD | - node scripts/ci-scope.mjs --mode ci >> "${GITHUB_OUTPUT}" + node "${TRUSTED_SCOPE}" --mode ci >> "${GITHUB_OUTPUT}" core: name: Node 24 / ubuntu-latest @@ -111,18 +111,9 @@ jobs: windows-authority: name: Node 24 / windows-latest - needs: scope - if: always() && (needs.scope.result != 'success' || github.event_name != 'pull_request' || needs.scope.outputs.windows_authority == 'true') runs-on: windows-latest timeout-minutes: 25 steps: - - name: Fail closed when scope detection failed - if: needs.scope.result != 'success' - shell: pwsh - run: | - Write-Error 'Detect CI scope did not complete successfully.' - exit 1 - - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index d6f6def1..dff76578 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -16,41 +16,6 @@ concurrency: cancel-in-progress: true jobs: - csharp_scope: - name: Detect C# analysis scope - runs-on: ubuntu-latest - timeout-minutes: 5 - outputs: - required: ${{ steps.scope.outputs.required }} - steps: - - name: Check out repository - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - fetch-depth: 0 - persist-credentials: false - - - name: Set up Node.js 24 - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 - with: - node-version: 24 - package-manager-cache: false - - - name: Detect C#-relevant pull request changes - id: scope - shell: bash - env: - EVENT_NAME: ${{ github.event_name }} - BASE_SHA: ${{ github.event.pull_request.base.sha }} - run: | - set -euo pipefail - if [[ "${EVENT_NAME}" != "pull_request" ]]; then - echo "required=true" >> "${GITHUB_OUTPUT}" - exit 0 - fi - - git diff --name-only -z "${BASE_SHA}"...HEAD | - node scripts/ci-scope.mjs --mode csharp >> "${GITHUB_OUTPUT}" - analyze: name: CodeQL / Analyze (javascript-typescript) runs-on: ubuntu-latest @@ -80,8 +45,6 @@ jobs: analyze-csharp: name: CodeQL / Analyze (csharp) - needs: csharp_scope - if: always() && (needs.csharp_scope.result != 'success' || github.event_name != 'pull_request' || needs.csharp_scope.outputs.required == 'true') runs-on: windows-latest timeout-minutes: 30 permissions: @@ -89,13 +52,6 @@ jobs: packages: read security-events: write steps: - - name: Fail closed when C# scope detection failed - if: needs.csharp_scope.result != 'success' - shell: pwsh - run: | - Write-Error 'Detect C# analysis scope did not complete successfully.' - exit 1 - - name: Check out repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index 50527e22..6dac4aed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ All notable changes to `github-delivery` are documented here. - Bumped the package version from `0.8.6` to `0.8.7`. - Restricted temporary Windows Authority branch leases to repeated `push_code` batches and tightened classic branch-protection, routing, mutation-boundary, security-policy, and stack portability contracts (PR #297). +- Windows Authority CI and C# CodeQL now run on every pull request so candidate code cannot scope out its own security-critical lanes; the remaining Node 22 compatibility selector is executed from the pull request base version (PR #299). +- Review briefs now apply a global model-facing diff-hunk budget while preserving complete structured review scope for deterministic tooling and on-demand inspection (PR #299). +- Made quiet execution an entrypoint-visible contract: routine rule/skill/workflow loading, file reads, Git/GitHub snapshots, remote fetches, obvious retries, and shell-quoting corrections run without per-tool user-facing narration unless they materially change the plan or expose a blocker (PR #299). ### Fixed @@ -19,6 +22,10 @@ All notable changes to `github-delivery` are documented here. - Bound GitHub, Git, PowerShell, and registry subprocess helpers now copy argv and force a direct spawn (`shell: false`), so library-provided arguments cannot be reconstructed as a shell command (PR #294). - Prevented unusual valid Git filenames from hiding sensitive path changes from scoped CI/CodeQL detection, and made required scoped lanes fail closed when their scope producer fails (PR #296). - Fail closed when classic branch protection may apply but cannot be proved absent, added GitHub-style classic branch-pattern coverage, restored contextual `make this green` routing, and strengthened dynamic mutation-command boundary checks (PR #297). +- Redeem trusted authority before the first mutating GitHub command, including autonomous idempotency tag/ref coordination, so a rejected grant cannot leave coordination state behind before the requested mutation (PR #299). +- Preserve rename/copy source and destination paths with NUL-delimited local branch diff parsing, keep both path generations in review classification, and make every deterministic required probe a first-class pre-open blocker until its canonical structured probe evidence validates against the deterministic trigger files (PR #299). +- Enforce open stack-parent ordering at the mutation execution boundary before merge authority, and abort orphan-workflow cleanup before deletion when the default-branch generation changed during preflight (PR #299). +- Protected Codex streaming now bounds repetitive interleaved tool micro-narration separately from tool-emission stalls: three future-action narration intents without execution/state/workflow progress trigger an interrupt, and evidence/read tool starts do not reset that budget (PR #299). ## [0.8.6] - 2026-08-18 @@ -52,7 +59,7 @@ All notable changes to `github-delivery` are documented here. - Moved the remaining workflow-level `actions: write` permission down to the cleanup job, kept top-level workflow permissions read-only, and added validation that rejects future top-level write scopes while still permitting explicitly allowlisted job-level writes (PR #282). - Hardened PR publication identity and retries: exact duplicate detection now binds target repository, head repository/ref, and base; qualified REST head filters prevent same-repository misses; explicit cross-repository `head_repo` identity is supported; and exact owned idempotent retries converge before the broader duplicate preflight (PR #283). -- Protected existing PR-body screenshots, videos, GitHub uploads, reference-style Markdown images, and other recognized media from accidental body rewrites. Intentional removal requires an exact approved identity list that is included in trusted `update_pr_body` authority scope (PR #283). +- Protected existing PR-body screenshots, videos, GitHub uploads, reference-style Markdown images, and other recognized media from accidental body rewrites. Intentional media removal requires an exact approved identity list that is included in trusted `update_pr_body` authority scope (PR #283). - Prevented cross-repository closing issues and unsafe display URLs from masquerading as trustworthy same-repository work-item evidence, and tightened open-work fixtures so PR-number normalization and repository boundaries are actually exercised (PR #283). ## [0.8.2] - 2026-08-17 @@ -562,4 +569,4 @@ All notable changes to `github-delivery` are documented here. - Executable offline routing and retained-regression evaluations. - Deterministic versioned skill bundles with checksums, installation planning, backups, and restore. - Tag-bound GitHub Releases with checksum verification, SPDX SBOMs, and artifact attestations. -- Dependabot, Dependency Review, CodeQL, Scorecard, and executable repository workflow policy checks. +- Dependabot, Dependency Review, CodeQL, Scorecard, and executable repository workflow policy checks. \ No newline at end of file diff --git a/README.md b/README.md index d2fd2ff3..d550c43b 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ The 0.8.6 line adds the major workflow and safety work developed after 0.8.2, pl - competing-PR consolidation analysis; - conditional head-bound visual review evidence; - multi-base backport/port delivery; -- a substantially leaner GitHub Actions topology with stale-run cancellation and scoped platform lanes; +- a substantially leaner GitHub Actions topology with stale-run cancellation and unconditional security-critical Windows Authority/C# lanes; - fail-closed delivery integrity for moved PR heads, queued/auto-merge outcomes, mutation receipts, and remaining public workflow routing; - bounded GitHub and Git subprocesses on review, verdict, CI forensics, ship-gate, runtime, live-fixture, release, and npm helper paths. @@ -162,7 +162,9 @@ Status, open-work, and competing-PR analysis remain read-only. Implementation-on Routine network-visible issue/PR writes pass through the typed GitHub mutation boundary. Stale-sensitive requests bind expected head state; branch pushes bind repository/remote/branch plus old/new tips; history rewrites use exact force-with-lease semantics rather than bare force. -**Merge is deliberately stricter.** `scripts/merge-pr-driver.mjs` owns settle, final current-head/base/rules/feedback/review-evidence recapture, trusted destructive authority, head-pinned merge execution, and post-merge reconciliation. Generic hand-built merge mutation documents are rejected. +For trusted high-assurance operations, authority redemption happens before the first mutating GitHub command, including autonomous idempotency coordination refs/tags. A rejected grant therefore cannot leave a coordination write behind before the requested mutation. + +**Merge is deliberately stricter.** `scripts/merge-pr-driver.mjs` owns settle, final current-head/base/rules/feedback/review-evidence recapture, trusted destructive authority, head-pinned merge execution, and post-merge reconciliation. The lower mutation execution boundary also rechecks open-PR stack topology and rejects a child merge while its parent PR is still open. Generic hand-built merge mutation documents are rejected. ### Exact-effect trusted authority @@ -207,6 +209,8 @@ A full review can combine: - proactive contract verification appropriate to the changed behavior; - conditional **visual evidence** for rendered/UI surfaces. +The pre-open gate treats those deterministic probes as first-class obligations alongside required bug lenses and security surfaces. A probe detected from the branch diff remains blocking until its canonical structured probe-evidence record validates against the deterministic trigger files. Local branch review uses NUL-delimited Git records so renames and unusual valid paths retain both source and destination identity. + ### Safe simplification Simplification is **explicit-only**. Its goal is lower cognitive load and safer maintenance. **Line count is never the goal**; fewer lines are acceptable only when behavior and clarity improve. @@ -226,6 +230,7 @@ The final ship decision is one authoritative `ready`, `blocked`, or `unknown` re - review decision, stale approvals, last-push requirements, unresolved threads; - conflicts, behind state, merge queue / auto-merge state; - unknown ruleset/state values failing closed; +- open stack-parent topology before destructive merge execution; - exact-head merge execution and read-only reconciliation after ambiguous write results; - partial success when merge succeeded but non-destructive post-merge ceremony did not. @@ -237,7 +242,7 @@ These are intentionally three different concepts. ### Stacked PRs -A stack is a dependency chain where a child PR targets a parent PR branch. Stack operations discover repository-qualified topology, restack bottom-up, preserve layer ownership, and revalidate every surviving child after an upstream head changes. +A stack is a dependency chain where a child PR targets a parent PR branch. Stack operations discover repository-qualified topology, restack bottom-up, preserve layer ownership, and revalidate every surviving child after an upstream head changes. The mutation execution boundary independently rejects a merge while the target PR still points at another open PR's head, so merge-order safety does not depend only on workflow prose. ### Competing PRs @@ -473,24 +478,24 @@ npm run reliability:gate ### Lean required CI topology -The pull-request CI topology is deliberately asymmetric to avoid repeating the full repository suite across every OS/runtime combination: +The pull-request CI topology is deliberately asymmetric to avoid repeating the full repository suite across every OS/runtime combination while keeping the security-critical platform lanes unskippable by PR scope logic: | Required context | PR behavior | |---|---| | **Node 24 / ubuntu-latest** | Canonical full `npm run check`; then bounded Node 26 syntax/package/unit compatibility on the same workspace | -| **Node 22 / ubuntu-latest** | Bounded compatibility lane only when runtime-relevant paths change; forced for `main`/live-fixture acceptance | -| **Node 24 / windows-latest** | Windows Authority restore/build/self-test/publish/install smoke only when Authority/platform-relevant paths change; forced for `main`/live-fixture acceptance | +| **Node 22 / ubuntu-latest** | Bounded compatibility lane only when runtime-relevant paths change; its path classifier is executed from the PR base version | +| **Node 24 / windows-latest** | Always runs Windows Authority restore/build/self-test/publish/install smoke on pull requests | | **Dependency Review** | Runs on pull requests | | **CodeQL / Analyze (javascript-typescript)** | Runs on pull requests | -| **CodeQL / Analyze (csharp)** | Scoped to Windows Authority/C#-relevant PRs; still runs on `main` and schedules | +| **CodeQL / Analyze (csharp)** | Always runs on pull requests, plus `main` and schedules | -There are no macOS PR compatibility lanes and no duplicate Architecture Contracts workflow. Superseded CI, CodeQL, and Dependency Review runs are cancelled when a newer commit arrives. Repository-policy verification is daily and orphan-workflow cleanup is weekly. +There are no macOS PR compatibility lanes and no duplicate Architecture Contracts workflow. Superseded CI, CodeQL, and Dependency Review runs are cancelled when a newer commit arrives. Repository-policy verification is daily and orphan-workflow cleanup is weekly; cleanup pins the default-branch generation before deleting stale workflow histories. -For ordinary runtime-relevant PRs this reduces full `npm run check` executions from **9 to 1**, full unit-suite runtime executions from **9 to 3**, Windows Authority lanes from **2 to 1 when relevant**, and macOS PR jobs from **2 to 0** while retaining Node 22/24/26 compatibility coverage. +For ordinary runtime-relevant PRs this keeps full `npm run check` executions at **1**, full unit-suite runtime executions at **3**, one Windows Authority lane on every PR, and **0** macOS PR jobs while retaining Node 22/24/26 compatibility coverage and unconditional Windows/C# security coverage. ### Live lifecycle fixture -The unit/eval suite proves deterministic contracts. An explicitly opted-in fixture repository exercises the real GitHub lifecycle with immutable repository-identity binding before the first mutation. Fixture runs force the scoped Node 22 and Windows compatibility lanes even when normal PR path filtering would skip them. +The unit/eval suite proves deterministic contracts. An explicitly opted-in fixture repository exercises the real GitHub lifecycle with immutable repository-identity binding before the first mutation. Fixture diffs force the scoped Node 22 compatibility lane; the Windows Authority lane already runs unconditionally. See [`docs/live-integration.md`](docs/live-integration.md) and [`docs/live-github-integration.md`](docs/live-github-integration.md). @@ -554,4 +559,4 @@ The project intentionally fails closed rather than claiming unsupported coverage Some workflow directions were informed by public/open-source agent skills and GitHub automation patterns, including concepts from `OutThisLife/brooklyn-skills`. Adapted ideas are rewritten around GitHub Delivery's own evidence, authority, routing, and lifecycle contracts; relevant workflow files include provenance notes where appropriate. -Licensed under the [MIT License](LICENSE). +Licensed under the [MIT License](LICENSE). \ No newline at end of file diff --git a/SKILL.md b/SKILL.md index 51bf3b59..9c28dce1 100644 --- a/SKILL.md +++ b/SKILL.md @@ -104,7 +104,7 @@ persistent `delivery-controller.mjs` checkpoint. Route/phase graph stay locked; the controller owns transitions, evidence/retry/resource/no-progress accounting and resume. Only phase/state/blocker/required-evidence/execution change is progress. Conditional policy extends unchanged context. The controller grants -no GitHub write authority. +no GitHub write authority. Run routine deterministic tooling quietly; narrate only material progress or blockers (GD-CORE-009). ## Mandatory entrypoint behavior diff --git a/references/gate-helpers.md b/references/gate-helpers.md index 49c1c7ae..7c90ea65 100644 --- a/references/gate-helpers.md +++ b/references/gate-helpers.md @@ -185,4 +185,4 @@ For an **unopened** branch, run the pre-open gate instead of `bug-scope.mjs`/`se node "/scripts/pre-open-gate.mjs" OWNER/REPO BASE_REF HEAD_REF ``` -Exit `0` (`decision: "ready"`) means the branch diff has no required bug/security scope. Exit `1` (`decision: "blocked"`) lists the required bug lenses and security surfaces that must be reviewed (and Confirmed High/Critical findings fixed) before opening. Exit `2` (`decision: "unknown"`) means the branch diff could not be collected completely — never open a PR on unknown. See `references/create-pr-for-issue.md` step C2. +Exit `0` (`decision: "ready"`) means every deterministic pre-open obligation is cleared. Exit `1` (`decision: "blocked"`) lists remaining bug lenses, security surfaces, and required probes. Lens/surface evidence uses the pre-open `done` / justified `n/a` records; probe evidence uses the canonical structured `clean` / `findings` / `n-a` records and is validated against the deterministic trigger files. Confirmed High/Critical findings must be fixed before opening. Exit `2` (`decision: "unknown"`) means the branch diff could not be collected completely — never open a PR on unknown. See `references/create-pr-for-issue.md` step D. diff --git a/references/policy-kernel.md b/references/policy-kernel.md index 922e316d..245c5a38 100644 --- a/references/policy-kernel.md +++ b/references/policy-kernel.md @@ -42,6 +42,8 @@ On unchanged relevant code and state, reuse valid passing evidence. Do not run a Execute deterministic tool calls without narrating each one. User-facing progress updates are for phase changes, material new evidence or plan changes, blockers, or needed user input; do not precede each test/read/write with micro-narration such as “let me run” or “I’ll run”. +Rule/skill/workflow loading, file reads, Git/GitHub snapshots, remote fetches, obvious command retries, and shell-quoting corrections are not user-facing progress by themselves. Perform them directly. Do not emit status messages whose only new information is “loaded X”, “next I’ll verify Y”, “I’ll lock/fetch/recapture Z”, or that an obvious retry will now be attempted. If a failed tool call has a deterministic correction and does not change scope or create a blocker, correct it and retry quietly; surface the failure only when it materially changes the plan, evidence, authority, or user action required. + ### GD-CORE-010 — Minimise evidence acquisition and context Prefer the highest-level authoritative helper or aggregate read that can decide the current step. Reuse one valid state snapshot while relevant state is unchanged. Do not delegate deterministic script or gate interpretation to a subagent. Escalate evidence progressively: decision/status, failing component, focused excerpt, then full raw output only when required. diff --git a/scripts/ci-scope.mjs b/scripts/ci-scope.mjs index 7215c7bf..c73fe73e 100644 --- a/scripts/ci-scope.mjs +++ b/scripts/ci-scope.mjs @@ -16,6 +16,7 @@ const WINDOWS_AUTHORITY_PATTERNS = [ /^\.github-delivery-fixtures\//, /^global\.json$/, /^package\.json$/, + /^scripts\/ci-scope\.mjs$/, /^scripts\/prepare-authority-host-runtime-smoke\.mjs$/, /^scripts\/lib\/authority-host-(install|release)\.mjs$/, /^\.github\/workflows\/ci\.yml$/, @@ -24,6 +25,7 @@ const WINDOWS_AUTHORITY_PATTERNS = [ const CSHARP_PATTERNS = [ /^authority-host\/windows\//, /^global\.json$/, + /^scripts\/ci-scope\.mjs$/, /^\.github\/workflows\/codeql\.yml$/, ]; diff --git a/scripts/cleanup-orphaned-workflows.mjs b/scripts/cleanup-orphaned-workflows.mjs index 44c851be..99c32e21 100644 --- a/scripts/cleanup-orphaned-workflows.mjs +++ b/scripts/cleanup-orphaned-workflows.mjs @@ -4,6 +4,7 @@ import { pathToFileURL } from "node:url"; const API_ROOT = "https://api.github.com"; const LOCAL_WORKFLOW_PREFIX = ".github/workflows/"; const DEFAULT_MAX_DELETIONS = 500; +const COMMIT_SHA_RE = /^[0-9a-f]{40}$/i; function encodePath(path) { return path.split("/").map(encodeURIComponent).join("/"); @@ -79,6 +80,17 @@ export function createGitHubClient({ token, fetchImpl = fetch, apiRoot = API_ROO return { request, exists, paginate }; } +async function readBranchGeneration({ client, owner, repo, branch }) { + const payload = await client.request( + `/repos/${owner}/${repo}/git/ref/heads/${encodePath(branch)}`, + ); + const sha = String(payload?.object?.sha || "").toLowerCase(); + if (!COMMIT_SHA_RE.test(sha)) { + throw new Error(`default_branch_generation_invalid:${branch}`); + } + return sha; +} + async function workflowStillExistsOnRunHead({ client, owner, repo, workflow, runs, log }) { const heads = new Map(); for (const run of runs) { @@ -133,6 +145,12 @@ export async function cleanupOrphanedWorkflowRuns({ if (!defaultBranch) { throw new Error("Repository response did not include default_branch"); } + const defaultBranchGeneration = await readBranchGeneration({ + client, + owner, + repo, + branch: defaultBranch, + }); const workflowEntries = await client.request( `/repos/${owner}/${repo}/contents/.github/workflows?ref=${encodeURIComponent(defaultBranch)}`, @@ -196,6 +214,20 @@ export async function cleanupOrphanedWorkflowRuns({ `Preflight approved ${cleanupPlans.length} orphan workflow(s) containing ${plannedRuns} run(s).`, ); + // The active-path snapshot is valid only for the exact default-branch generation + // that produced it. Abort before the first DELETE if main moved during preflight. + const currentDefaultBranchGeneration = await readBranchGeneration({ + client, + owner, + repo, + branch: defaultBranch, + }); + if (currentDefaultBranchGeneration !== defaultBranchGeneration) { + throw new Error( + `default_branch_moved_during_cleanup:${defaultBranchGeneration}:${currentDefaultBranchGeneration}`, + ); + } + let deletedRuns = 0; let capped = false; const failures = []; @@ -240,6 +272,7 @@ export async function cleanupOrphanedWorkflowRuns({ plannedRuns, deletedRuns, capped, + defaultBranchGeneration, }; log(`Cleanup complete: ${JSON.stringify(summary)}`); return summary; diff --git a/scripts/lib/authority-execution.mjs b/scripts/lib/authority-execution.mjs index 7c652393..159d55b3 100644 --- a/scripts/lib/authority-execution.mjs +++ b/scripts/lib/authority-execution.mjs @@ -1,4 +1,5 @@ import { redeemAuthorityBeforeMutation } from "./authority-redemption.mjs"; +import { isReadOnlyGitHubCommand } from "./github-retry.mjs"; function sameCommand(command, args, plannedCommand) { if (!Array.isArray(plannedCommand) || plannedCommand.length === 0) return false; @@ -7,6 +8,11 @@ function sameCommand(command, args, plannedCommand) { return args.length === plannedArgs.length && args.every((value, index) => value === plannedArgs[index]); } +function isPreWriteCoordinationMutation(command, args, plannedCommand) { + if (sameCommand(command, args, plannedCommand)) return true; + return command === "gh" && !isReadOnlyGitHubCommand(command, args); +} + export function makeRedemptionRunner({ plannedCommand, authority, @@ -23,20 +29,26 @@ export function makeRedemptionRunner({ let redemptionAttempted = false; let writeAttempted = false; + function redeem() { + if (!redemptionAttempted) { + redemptionAttempted = true; + redemptionReceipt = redeemAuthorityBeforeMutation({ + authority, + authorityGrant, + redeemer, + }); + } + return redemptionReceipt ? structuredClone(redemptionReceipt) : null; + } + return { runner(command, args, options) { const isPlannedWrite = sameCommand(command, args, plannedCommand); - if (isPlannedWrite && !redemptionAttempted) { - redemptionAttempted = true; - redemptionReceipt = redeemAuthorityBeforeMutation({ - authority, - authorityGrant, - redeemer, - }); - } + if (isPreWriteCoordinationMutation(command, args, plannedCommand)) redeem(); if (isPlannedWrite) writeAttempted = true; return runner(command, args, options); }, + redeem, redemption() { return redemptionReceipt ? structuredClone(redemptionReceipt) : null; }, diff --git a/scripts/lib/branch-review-input.mjs b/scripts/lib/branch-review-input.mjs new file mode 100644 index 00000000..01836f1c --- /dev/null +++ b/scripts/lib/branch-review-input.mjs @@ -0,0 +1,107 @@ +import { boundedSpawnSync } from "./subprocess-policy.mjs"; + +function run(command, args, { trim = true } = {}) { + const result = boundedSpawnSync(command, args, { + encoding: "utf8", + maxBuffer: 50 * 1024 * 1024, + }); + if (result.status !== 0) { + throw new Error(String(result.stderr || result.stdout || `${command} failed`).trim()); + } + const output = String(result.stdout || ""); + return trim ? output.trim() : output; +} + +function maybe(command, args) { + const result = boundedSpawnSync(command, args, { + encoding: "utf8", + maxBuffer: 50 * 1024 * 1024, + }); + if (result.status !== 0) return null; + return String(result.stdout || "").trim(); +} + +export function parseNullDelimitedNameStatus(output) { + const fields = String(output || "").split("\0"); + if (fields.at(-1) === "") fields.pop(); + else if (fields.length > 1 || fields[0]) throw new Error("branch_diff_name_status_not_nul_terminated"); + + const rows = []; + for (let index = 0; index < fields.length;) { + const status = fields[index++]; + if (!status) throw new Error("branch_diff_status_missing"); + const kind = status[0]; + if (kind === "R" || kind === "C") { + const previousPath = fields[index++]; + const path = fields[index++]; + if (previousPath === undefined || path === undefined) { + throw new Error("branch_diff_rename_record_incomplete"); + } + rows.push({ status, previousPath, path }); + continue; + } + const path = fields[index++]; + if (path === undefined) throw new Error("branch_diff_path_missing"); + rows.push({ status, previousPath: null, path }); + } + return rows; +} + +function resolveRepoForBranch() { + try { + const name = JSON.parse(run("gh", ["repo", "view", "--json", "nameWithOwner"])).nameWithOwner; + if (typeof name === "string" && name.includes("/")) return name; + } catch { + // Fall through to the configured git remote. + } + const remote = maybe("git", ["remote", "get-url", "origin"]); + if (!remote) return null; + const match = String(remote).match(/(?:[:/])([^/:]+)\/([^/]+?)(?:\.git)?$/); + return match ? `${match[1]}/${match[2]}` : null; +} + +function patchForRecord(baseRef, headRef, record) { + const paths = record.previousPath + ? [record.previousPath, record.path] + : [record.path]; + return run( + "git", + [ + "diff", + "--no-ext-diff", + "--unified=3", + `${baseRef}...${headRef}`, + "--", + ...paths, + ], + { trim: false }, + ); +} + +export function collectBranchReviewInput(baseRef, headRef) { + const nameStatus = run( + "git", + ["diff", "--name-status", "-z", `${baseRef}...${headRef}`], + { trim: false }, + ); + const records = parseNullDelimitedNameStatus(nameStatus); + const files = records.map((record) => { + const patch = patchForRecord(baseRef, headRef, record); + const additions = patch + .split(/\r?\n/) + .filter((line) => line.startsWith("+") && !line.startsWith("+++")).length; + const deletions = patch + .split(/\r?\n/) + .filter((line) => line.startsWith("-") && !line.startsWith("---")).length; + return { + path: record.path, + previousPath: record.previousPath, + status: record.status, + patch, + additions, + deletions, + }; + }); + const headRefOid = maybe("git", ["rev-parse", "--verify", headRef]) || null; + return { repo: resolveRepoForBranch(), pr: null, headRefOid, files }; +} diff --git a/scripts/lib/codex-progress-watchdog.mjs b/scripts/lib/codex-progress-watchdog.mjs index d1647f23..2f18b691 100644 --- a/scripts/lib/codex-progress-watchdog.mjs +++ b/scripts/lib/codex-progress-watchdog.mjs @@ -28,6 +28,9 @@ const FINALIZATION_WATCHDOG_OPTIONS = Object.freeze({ noProgressTokenHardLimit: 16_000, }); +const MICRO_NARRATION_INTENT_THRESHOLD = 3; +const MICRO_NARRATION_INTENT = /(?:^|\b(?:so|and|then|next)[,:]?\s+)(?:next\s+)?(?:let me|i(?:'|’)ll|i will|i need to|i(?:'|’)m going to|i am going to)\s+(?:(?:start(?:ing)?(?:\s+by)?|now|next|then|first|just)\s+)*(?:load(?:ing)?|read(?:ing)?|verif(?:y|ying)|check(?:ing)?|inspect(?:ing)?|fetch(?:ing)?|recaptur(?:e|ing)|lock(?:ing)?|run(?:ning)?|execut(?:e|ing)|invok(?:e|ing)|call(?:ing)?|search(?:ing)?|open(?:ing)?|us(?:e|ing)|apply(?:ing)?|patch(?:ing)?|edit(?:ing)?|writ(?:e|ing)|updat(?:e|ing)|fix(?:ing)?|chang(?:e|ing))\b/i; + export function isCodexGeneratedTextMethod(method) { return GENERATED_TEXT_METHODS.has(String(method || "")); } @@ -86,6 +89,46 @@ function activeTextWatchdog(watchdog, context) { return context.finalizing ? finalizationWatchdog(context) : watchdog; } +function resetMicroNarration(context) { + context.microNarrationBuffer = ""; + context.microNarrationIntentCount = 0; +} + +function observeMicroNarration(delta, context) { + if (typeof delta !== "string" || delta.length === 0) return { action: "allow" }; + const current = `${context.microNarrationBuffer || ""}${delta}`; + let lastBoundary = -1; + for (let index = 0; index < current.length; index += 1) { + if (/[\n.!?]/.test(current[index])) lastBoundary = index; + } + if (lastBoundary < 0) { + context.microNarrationBuffer = current.length > 2_000 ? current.slice(-2_000) : current; + return { action: "allow" }; + } + + const complete = current.slice(0, lastBoundary + 1); + context.microNarrationBuffer = current.slice(lastBoundary + 1); + for (const clause of complete + .split(/\n+/) + .flatMap((line) => line.split(/(?<=[.!?])\s+/)) + .map((line) => line.trim()) + .filter(Boolean)) { + if (clause.length > 320 || !MICRO_NARRATION_INTENT.test(clause)) continue; + context.microNarrationIntentCount = Number(context.microNarrationIntentCount || 0) + 1; + if (context.microNarrationIntentCount >= MICRO_NARRATION_INTENT_THRESHOLD) { + return { + action: "interrupt", + reason: "micro_narration_budget_exhausted", + details: { + microNarrationIntentCount: context.microNarrationIntentCount, + hardLimit: MICRO_NARRATION_INTENT_THRESHOLD, + }, + }; + } + } + return { action: "allow" }; +} + export function observeCodexAppServerMessage(watchdog, message, context = {}) { if (!watchdog || typeof watchdog.observeAssistantDelta !== "function") { throw new Error("watchdog is required"); @@ -95,6 +138,12 @@ export function observeCodexAppServerMessage(watchdog, message, context = {}) { const { method, params = {} } = message; if (isCodexGeneratedTextMethod(method)) { + if (!context.finalizing) { + const narrationDecision = observeMicroNarration(params.delta || "", context); + if (narrationDecision.action === "interrupt") { + return maybeInterrupt(narrationDecision, params, context); + } + } const decision = activeTextWatchdog(watchdog, context).observeAssistantDelta(params.delta || ""); return maybeInterrupt(decision, params, context); } @@ -107,16 +156,19 @@ export function observeCodexAppServerMessage(watchdog, message, context = {}) { } if (method === "turn/diff/updated") { - watchdog.observeDiffProgress(params.diff || ""); + const progress = watchdog.observeDiffProgress(params.diff || ""); + if (progress?.progressed) resetMicroNarration(context); return { decision: { action: "allow" } }; } if (method === "turn/plan/updated") { - watchdog.observePlanProgress(params.plan || []); + const progress = watchdog.observePlanProgress(params.plan || []); + if (progress?.progressed) resetMicroNarration(context); const complete = planIsComplete(params.plan); if (complete && !context.finalizing) { context.finalizing = true; context.finalizationWatchdog = createProgressWatchdog(FINALIZATION_WATCHDOG_OPTIONS); + resetMicroNarration(context); } else if (!complete && context.finalizing) { context.finalizing = false; context.finalizationWatchdog = null; @@ -127,7 +179,11 @@ export function observeCodexAppServerMessage(watchdog, message, context = {}) { if (method === "item/started") { const item = params.item; if (RUNTIME_WORK_ITEM_TYPES.has(String(item?.type || ""))) { + const pendingToolEmissionSignal = + typeof watchdog.snapshot === "function" && + Number(watchdog.snapshot().toolEmissionIntentCount || 0) > 0; watchdog.recordToolStart({ type: item.type, id: item.id || null }); + if (pendingToolEmissionSignal) resetMicroNarration(context); context.finalizing = false; context.finalizationWatchdog = null; } @@ -152,8 +208,10 @@ export function observeCodexAppServerMessage(watchdog, message, context = {}) { if (isSuccessfulAppServerItem(item)) { if (classification.kind === "state-change" && item?.type !== "fileChange") { watchdog.recordStateProgress("codex_state_change_completed"); + resetMicroNarration(context); } else if (classification.kind === "execution") { watchdog.recordExecutionProgress({ kind: "codex_execution_completed" }); + resetMicroNarration(context); } } return { decision: { action: "allow" } }; @@ -163,6 +221,7 @@ export function observeCodexAppServerMessage(watchdog, message, context = {}) { context.interruptedTurns.delete(params.turn.id); context.finalizing = false; context.finalizationWatchdog = null; + resetMicroNarration(context); } return { decision: { action: "allow" } }; diff --git a/scripts/lib/merge-stack-policy.mjs b/scripts/lib/merge-stack-policy.mjs new file mode 100644 index 00000000..5691390e --- /dev/null +++ b/scripts/lib/merge-stack-policy.mjs @@ -0,0 +1,110 @@ +import { buildGraph, normalizePullPages, stackRefKey } from "../inspect-stack.mjs"; + +function required(value, name) { + if (value === undefined || value === null || value === "") { + throw new Error(`${name}_required`); + } + return value; +} + +function positiveInteger(value, name) { + const number = Number(value); + if (!Number.isInteger(number) || number <= 0) throw new Error(`${name}_invalid`); + return number; +} + +function parseJson(output, code) { + try { + return JSON.parse(String(output || "null")); + } catch { + throw new Error(code); + } +} + +function runOrThrow(runner, args, code) { + const result = runner("gh", args, { + encoding: "utf8", + maxBuffer: 50 * 1024 * 1024, + }); + if (result?.status !== 0) { + const detail = String(result?.stderr || result?.stdout || "").trim(); + throw new Error(`${code}${detail ? `:${detail}` : ""}`); + } + return String(result?.stdout || ""); +} + +function parseOpenPulls(output) { + return normalizePullPages(parseJson(output || "[]", "merge_stack_pr_pages_invalid_json")); +} + +export function evaluateMergeStackEligibility({ prs = [], targetPr } = {}) { + const number = positiveInteger(targetPr, "pr"); + const target = prs.find((pr) => pr.number === number); + if (!target) { + return { + eligible: false, + reason: "stack_target_pr_missing", + pr: number, + parentPr: null, + }; + } + + const { byHead } = buildGraph(prs); + const parent = byHead.get(stackRefKey(target.baseRepoFullName, target.baseRefName)) || null; + if (parent) { + return { + eligible: false, + reason: "stack_parent_unlanded", + pr: number, + parentPr: parent.number, + parentHeadRepo: parent.headRepoFullName, + parentHeadRef: parent.headRefName, + baseRepo: target.baseRepoFullName, + baseRef: target.baseRefName, + }; + } + + return { + eligible: true, + reason: null, + pr: number, + parentPr: null, + baseRepo: target.baseRepoFullName, + baseRef: target.baseRefName, + }; +} + +export function verifyMergeStackEligibility({ request, runner } = {}) { + if (request?.action !== "merge_pr") return null; + if (typeof runner !== "function") throw new Error("merge_stack_runner_required"); + const repo = required(request.repo, "repo"); + const pr = positiveInteger(request.pr, "pr"); + const openPulls = parseOpenPulls( + runOrThrow( + runner, + ["api", `repos/${repo}/pulls?state=open&per_page=100`, "--paginate", "--slurp"], + "merge_stack_evidence_unreadable", + ), + ); + const decision = evaluateMergeStackEligibility({ prs: openPulls, targetPr: pr }); + if (decision.eligible) return decision; + + if (decision.reason === "stack_target_pr_missing") { + const target = parseJson( + runOrThrow(runner, ["api", `repos/${repo}/pulls/${pr}`], "merge_stack_target_unreadable"), + "merge_stack_target_invalid_json", + ); + if (target?.merged_at || target?.merged === true) { + return { + eligible: true, + reason: null, + pr, + parentPr: null, + alreadyMerged: true, + }; + } + } + + const parent = decision.parentPr ? `:parent_pr=${decision.parentPr}` : ""; + throw new Error(`${decision.reason}${parent}`); +} diff --git a/scripts/lib/mutation-execution-context.mjs b/scripts/lib/mutation-execution-context.mjs index 71179113..0140af76 100644 --- a/scripts/lib/mutation-execution-context.mjs +++ b/scripts/lib/mutation-execution-context.mjs @@ -11,6 +11,7 @@ import { makeAuthorityRedeemer, } from "./authority-host-client.mjs"; import { classifyMergeOutcome, readMergeState } from "./merge-outcome.mjs"; +import { verifyMergeStackEligibility } from "./merge-stack-policy.mjs"; import { actionDefinition } from "./mutation-action-registry.mjs"; import { boundedSpawnSync } from "./subprocess-policy.mjs"; import { readUserConfig, resolveAuthorityMode } from "./user-config.mjs"; @@ -226,6 +227,13 @@ export function executeMutationWithAuthority({ config, }); const planned = planWithAuthorityOptions(request, options); + + // Merge topology is an execution invariant, not only a workflow instruction. + // A child whose base is another open PR head cannot reach destructive authority. + const stackEligibility = execute === true + ? verifyMergeStackEligibility({ request: planned.request, runner }) + : null; + const pipeName = runtimeEnv.GITHUB_DELIVERY_AUTHORITY_PIPE || undefined; const resolvedRedeemer = redeemer === undefined @@ -250,6 +258,7 @@ export function executeMutationWithAuthority({ }); return { ...receipt, + stackEligibility, redemption: execution.redemption(), }; } catch (error) { @@ -262,6 +271,7 @@ export function executeMutationWithAuthority({ if (reconciled) { return { ...reconciled, + stackEligibility, redemption: execution.redemption(), }; } diff --git a/scripts/lib/pre-open-evidence.mjs b/scripts/lib/pre-open-evidence.mjs index 88abf0c0..1a326984 100644 --- a/scripts/lib/pre-open-evidence.mjs +++ b/scripts/lib/pre-open-evidence.mjs @@ -1,19 +1,13 @@ /** * Validate review-completion evidence for the pre-open gate. * - * The gate derives required scope from the diff shape. Evidence is the - * machine-checkable record that each required lens/surface was actually - * reviewed: `done` means the pass ran and found nothing needing a fix (or the - * findings were fixed), `n/a (why)` means the boundary is untouched and the - * reason is recorded. This module validates the evidence payload so the gate - * can clear a `blocked` scope only for lenses/surfaces that carry valid - * evidence, and never clear on malformed or self-asserted input. + * Lenses/surfaces use the compact `done` / `n/a ` contract. Deterministic + * probes retain their existing structured machine evidence and are validated + * against the actual diff scope by `probe-evidence.mjs` inside the gate. */ export const PRE_OPEN_EVIDENCE_SCHEMA_VERSION = 1; -const VALID_STATUSES = new Set(["done", "n/a"]); - /** * @param {unknown} value * @returns {value is Record} @@ -22,18 +16,10 @@ function isRecord(value) { return value !== null && typeof value === "object" && !Array.isArray(value); } -/** - * @param {unknown} value - * @returns {value is string} - */ -function isNonEmptyString(value) { - return typeof value === "string" && value.trim().length > 0; -} - /** * @param {unknown} status * @param {string} id - * @returns {string | null} the normalized status when valid, else an error message + * @returns {string} the normalized status when valid, else an error message */ function normalizeStatus(status, id) { if (typeof status !== "string") return `status for ${id} must be a string`; @@ -44,11 +30,22 @@ function normalizeStatus(status, id) { return `invalid status for ${id}: ${JSON.stringify(trimmed)} (expected "done" or "n/a ")`; } +function normalizeEvidenceBlock(block, prefix, errors) { + const normalized = {}; + for (const [id, status] of Object.entries(block)) { + const value = normalizeStatus(status, `${prefix}:${id}`); + if (value === "done" || /^n\/a\s+\S/.test(value)) normalized[id] = value; + else errors.push(value); + } + return normalized; +} + /** * Validate a pre-open evidence payload. * - * @param {unknown} input - * @returns {{ ok: true, evidence: PreOpenEvidence } | { ok: false, errors: string[] }} + * `probes` is optional for schema-version compatibility with older evidence. + * When present it is passed through as a probe-id -> structured record map; + * scope-aware validation happens in the pre-open gate. */ export function validatePreOpenEvidence(input) { const errors = []; @@ -56,33 +53,34 @@ export function validatePreOpenEvidence(input) { if (input.schemaVersion !== undefined && input.schemaVersion !== PRE_OPEN_EVIDENCE_SCHEMA_VERSION) { errors.push(`evidence schemaVersion must be ${PRE_OPEN_EVIDENCE_SCHEMA_VERSION}`); } - const lenses = {}; - const surfaces = {}; + const lensBlock = isRecord(input.lenses) ? input.lenses : {}; const surfaceBlock = isRecord(input.surfaces) ? input.surfaces : {}; + const probeBlock = isRecord(input.probes) ? input.probes : {}; if (!isRecord(input.lenses) || !isRecord(input.surfaces)) { errors.push("evidence must have object fields lenses and surfaces"); } - for (const [id, status] of Object.entries(lensBlock)) { - const normalized = normalizeStatus(status, `lens:${id}`); - if (normalized === "done" || /^n\/a\s+\S/.test(normalized)) lenses[id] = normalized; - else errors.push(normalized); - } - for (const [id, status] of Object.entries(surfaceBlock)) { - const normalized = normalizeStatus(status, `surface:${id}`); - if (normalized === "done" || /^n\/a\s+\S/.test(normalized)) surfaces[id] = normalized; - else errors.push(normalized); + if (input.probes !== undefined && !isRecord(input.probes)) { + errors.push("evidence probes must be an object when provided"); } + + const lenses = normalizeEvidenceBlock(lensBlock, "lens", errors); + const surfaces = normalizeEvidenceBlock(surfaceBlock, "surface", errors); if (errors.length) return { ok: false, errors }; - return { ok: true, evidence: { schemaVersion: PRE_OPEN_EVIDENCE_SCHEMA_VERSION, lenses, surfaces } }; + + return { + ok: true, + evidence: { + schemaVersion: PRE_OPEN_EVIDENCE_SCHEMA_VERSION, + lenses, + surfaces, + probes: structuredClone(probeBlock), + }, + }; } /** - * Whether evidence clears a required id. - * - * @param {Record} map - * @param {string} id - * @returns {boolean} + * Whether compact lens/surface evidence clears a required id. */ export function evidenceClears(map, id) { const status = map[id]; diff --git a/scripts/lib/review-scope.mjs b/scripts/lib/review-scope.mjs index 1e98f836..243ca19d 100644 --- a/scripts/lib/review-scope.mjs +++ b/scripts/lib/review-scope.mjs @@ -3,6 +3,8 @@ import { boundedSpawnSync } from "./subprocess-policy.mjs"; import { PROBE_REGISTRY, validateProbeRegistry } from "./probe-registry.mjs"; import { planVisualEvidence } from "./visual-evidence.mjs"; +export { collectBranchReviewInput } from "./branch-review-input.mjs"; + const CODE_RE = /\.(?:[cm]?[jt]sx?|mjs|cjs|py|go|rs|java|kt|rb|php|cs|swift|c|cc|cpp|h|hpp|vue|svelte)$/i; const DOC_RE = /\.(?:md|txt|rst|adoc)$/i; const OPERATIONAL_POLICY_RE = /(^|\/)(?:SKILL\.md|references\/.*\.md|overrides\/)/i; @@ -202,7 +204,9 @@ export function planReviewScope(input = {}) { const { added, removed } = patchLines(file.patch); const changedText = [...added, ...removed].join("\n"); const symbols = extractSymbols(file.path, changedText); - const isLogic = CODE_RE.test(file.path) || OPERATIONAL_POLICY_RE.test(file.path) || /^\.github\//.test(file.path); + const isLogic = paths.some( + (path) => CODE_RE.test(path) || OPERATIONAL_POLICY_RE.test(path) || /^\.github\//.test(path), + ); if (isLogic) logicFiles.push(file.path); if (isLogic && !file.patch && file.status !== "removed") missingPatches.push(file.path); @@ -226,8 +230,10 @@ export function planReviewScope(input = {}) { workflowPermissionChanges.push(...workflowSignals(file.path, added, removed, evidence)); - if (LOCK_RE.test(file.path) || MANIFEST_RE.test(file.path)) { - const kind = LOCK_RE.test(file.path) ? "lockfile" : "manifest"; + const lockChanged = paths.some((path) => LOCK_RE.test(path)); + const manifestChanged = paths.some((path) => MANIFEST_RE.test(path)); + if (lockChanged || manifestChanged) { + const kind = lockChanged ? "lockfile" : "manifest"; dependencyChanges.push({ file: file.path, kind, additions: file.additions, deletions: file.deletions }); addEvidence(evidence, "supply_chain", "security", 3, `${kind} changed`, file.path); } @@ -272,7 +278,12 @@ export function planReviewScope(input = {}) { const bugLenses = finalize(lensEvidence); const requiredSecurity = domains.filter((item) => item.category === "security" && item.required); const requiredBug = bugLenses.filter((item) => item.required); - const docsOnly = files.length > 0 && files.every((file) => DOC_RE.test(file.path) && !OPERATIONAL_POLICY_RE.test(file.path)); + const docsOnly = files.length > 0 && files.every((file) => { + const paths = [file.path, file.previousPath].filter(Boolean); + return paths.every( + (path) => DOC_RE.test(path) && !OPERATIONAL_POLICY_RE.test(path), + ); + }); const criticalSecurity = requiredSecurity.some((item) => item.confidence === "high") || removedControlLeads.length > 0; const criticalBug = requiredBug.some((item) => item.confidence === "high"); const securityDepth = docsOnly ? "skip" : criticalSecurity ? "full" : requiredSecurity.length ? "targeted" : logicFiles.length ? "baseline" : "skip"; @@ -335,44 +346,3 @@ export function collectPrReviewInput(repo, pr) { assertCompletePrFileEnumeration(meta.changedFiles, files.length); return { repo, pr, ...meta, files }; } - -function git(args) { - const result = boundedSpawnSync("git", args, { encoding: "utf8", maxBuffer: 50 * 1024 * 1024 }); - if (result.status !== 0) throw new Error(String(result.stderr || result.stdout || "git failed").trim()); - return String(result.stdout || "").trim(); -} - -function gitMaybe(args) { - const result = boundedSpawnSync("git", args, { encoding: "utf8", maxBuffer: 50 * 1024 * 1024 }); - if (result.status !== 0) return null; - return String(result.stdout || "").trim(); -} - -function resolveRepoForBranch() { - try { - const name = JSON.parse(gh(["repo", "view", "--json", "nameWithOwner"])).nameWithOwner; - if (typeof name === "string" && name.includes("/")) return name; - } catch { - // fall through to git remote - } - const remote = gitMaybe(["remote", "get-url", "origin"]); - if (!remote) return null; - const match = String(remote).match(/(?:[:/])([^/:]+)\/([^/]+?)(?:\.git)?$/); - return match ? `${match[1]}/${match[2]}` : null; -} - -export function collectBranchReviewInput(baseRef, headRef) { - const nameStatus = git(["diff", "--name-status", `${baseRef}...${headRef}`]); - const paths = nameStatus.split(/\r?\n/).filter(Boolean).map((line) => { - const [status, ...rest] = line.split(/\s+/); - return { status, path: rest.join(" ").replace(/^"|"$/g, "") }; - }); - const files = paths.map(({ status, path }) => { - const patch = git(["diff", "--no-ext-diff", "--unified=3", `${baseRef}...${headRef}`, "--", path]); - const added = patch.split(/\r?\n/).filter((line) => line.startsWith("+") && !line.startsWith("+++")).length; - const deleted = patch.split(/\r?\n/).filter((line) => line.startsWith("-") && !line.startsWith("---")).length; - return { path, status, patch, additions: added, deletions: deleted }; - }); - const headRefOid = gitMaybe(["rev-parse", "--verify", headRef]) || null; - return { repo: resolveRepoForBranch(), pr: null, headRefOid, files }; -} diff --git a/scripts/pre-open-gate.mjs b/scripts/pre-open-gate.mjs index 8f6ef895..2476fffe 100644 --- a/scripts/pre-open-gate.mjs +++ b/scripts/pre-open-gate.mjs @@ -1,46 +1,81 @@ #!/usr/bin/env node import { isDirectInvocation } from "./lib/direct-invocation.mjs"; -import { collectBranchReviewInput, planReviewScope } from "./lib/review-scope.mjs"; +import { collectBranchReviewInput } from "./lib/branch-review-input.mjs"; +import { planReviewScope } from "./lib/review-scope.mjs"; import { projectBugScope, projectSecurityScope } from "./lib/review-scope-compat.mjs"; import { evidenceClears, validatePreOpenEvidence } from "./lib/pre-open-evidence.mjs"; +import { validateProbeEvidence } from "./lib/probe-evidence.mjs"; function usageError() { throw new Error("Usage: node scripts/pre-open-gate.mjs OWNER/REPO BASE_REF HEAD_REF [--output FILE] [--evidence-file FILE] | --self-test"); } +function probeCoverage(plan, evidence) { + const requiredProbes = plan.requiredProbes || []; + const errors = validateProbeEvidence(evidence?.probes ?? {}, { + requiredProbes, + probeEvidence: plan.probeEvidence || {}, + }); + const errorsByProbe = new Map(); + for (const error of errors) { + const probeId = error?.probeId || "unknown"; + if (!errorsByProbe.has(probeId)) errorsByProbe.set(probeId, []); + errorsByProbe.get(probeId).push(error); + } + return { requiredProbes, errors, errorsByProbe }; +} + export function evaluate(plan, evidence = null) { const bugScope = projectBugScope(plan); const securityScope = projectSecurityScope(plan); const implementationDiffPresent = Number(plan?.fileCount || 0) > 0; - const scopeBlockers = [ - ...bugScope.requiredLenses.map((id) => `bug:requiredLenses:${id}`), - ...securityScope.requiredSurfaces.map((id) => `security:requiredSurfaces:${id}`), - ]; - const complete = implementationDiffPresent && plan.complete && bugScope.complete && securityScope.complete; const lensMap = evidence?.lenses ?? {}; const surfaceMap = evidence?.surfaces ?? {}; const clearedByEvidence = []; - const remainingScopeBlockers = scopeBlockers.filter((blocker) => { - const [axis, , id] = blocker.split(":"); - const cleared = axis === "bug" ? evidenceClears(lensMap, id) : evidenceClears(surfaceMap, id); - if (cleared) clearedByEvidence.push(blocker); - return !cleared; - }); - const blockers = implementationDiffPresent - ? remainingScopeBlockers + const blockers = []; + + for (const id of bugScope.requiredLenses) { + const blocker = `bug:requiredLenses:${id}`; + if (evidenceClears(lensMap, id)) clearedByEvidence.push(blocker); + else blockers.push(blocker); + } + for (const id of securityScope.requiredSurfaces) { + const blocker = `security:requiredSurfaces:${id}`; + if (evidenceClears(surfaceMap, id)) clearedByEvidence.push(blocker); + else blockers.push(blocker); + } + + const probes = probeCoverage(plan, evidence); + for (const id of probes.requiredProbes) { + const blocker = `probe:requiredProbes:${id}`; + if (probes.errorsByProbe.has(id)) blockers.push(blocker); + else clearedByEvidence.push(blocker); + } + for (const [probeId, errors] of probes.errorsByProbe) { + if (probes.requiredProbes.includes(probeId)) continue; + for (const error of errors) { + blockers.push(`probe:evidence:${error.code}:${probeId}`); + } + } + + const complete = implementationDiffPresent && plan.complete && bugScope.complete && securityScope.complete; + const finalBlockers = implementationDiffPresent + ? blockers : ["workflow:implementation_missing"]; const decision = !implementationDiffPresent ? "blocked" : !complete ? "unknown" - : blockers.length + : finalBlockers.length ? "blocked" : "ready"; return { bugScope, securityScope, - blockers, + requiredProbes: probes.requiredProbes, + probeEvidenceErrors: probes.errors, + blockers: finalBlockers, clearedByEvidence, decision, complete, @@ -49,7 +84,7 @@ export function evaluate(plan, evidence = null) { }; } -function report({ repo, baseRef, headRef, headRefOid, bugScope, securityScope, blockers, clearedByEvidence, decision, complete, implementationDiffPresent, evidenceApplied }) { +function report({ repo, baseRef, headRef, headRefOid, bugScope, securityScope, requiredProbes, probeEvidenceErrors, blockers, clearedByEvidence, decision, complete, implementationDiffPresent, evidenceApplied }) { return { schemaVersion: 1, kind: "github-delivery/pre-open-gate", @@ -63,13 +98,15 @@ function report({ repo, baseRef, headRef, headRefOid, bugScope, securityScope, b evidenceApplied, bugScope, securityScope, + requiredProbes, + probeEvidenceErrors, blockers, clearedByEvidence, instructions: [ "workflow:implementation_missing: this pre-open gate requires a non-empty candidate implementation diff; implement first, then rerun the gate before publication.", - "decision=blocked: complete every remaining required bug lens and security surface on this branch diff (with --evidence-file), fix Confirmed High/Critical findings, then rerun before opening the PR.", + "decision=blocked: complete every remaining required bug lens, security surface, and deterministic probe on this branch diff (with --evidence-file), fix Confirmed High/Critical findings, then rerun before opening the PR.", "decision=unknown: restore complete branch evidence (fetch base, checkout head) and rerun; never open a PR from an incomplete diff.", - "decision=ready: the non-empty candidate branch diff has no required bug/security scope, or every required lens/surface carries valid done/n-a evidence; you may proceed to open the PR.", + "decision=ready: the non-empty candidate branch diff has no remaining review obligations, or every required lens/surface plus every canonical structured probe-evidence record validates; you may proceed to open the PR.", ], }; } diff --git a/scripts/review-brief.mjs b/scripts/review-brief.mjs index 8ede95d8..b9dee133 100644 --- a/scripts/review-brief.mjs +++ b/scripts/review-brief.mjs @@ -4,7 +4,7 @@ * so the review agent starts from facts instead of re-reading sources. * * Usage: - * node scripts/review-brief.mjs OWNER/REPO PR_NUMBER [--max-hunk-lines N] [--json] + * node scripts/review-brief.mjs OWNER/REPO PR_NUMBER [--max-hunk-lines N] [--max-total-hunk-lines N] [--json] * * Default output is a compact text brief (fast to read). --json emits the full * structured plan for tooling. @@ -18,19 +18,36 @@ import { planReviewDepthExecution } from "./lib/review-depth-execution.mjs"; import { extractRequiredProbeBlocks } from "./lib/probe-blocks.mjs"; import { ownedHelperEffect } from "./lib/watchdog-evidence-registry.mjs"; +const DEFAULT_MAX_HUNK_LINES = 24; +const DEFAULT_MAX_TOTAL_HUNK_LINES = 1_200; const USAGE = - "Usage: node scripts/review-brief.mjs OWNER/REPO PR_NUMBER [--max-hunk-lines N] [--no-reference-map] [--json]"; + "Usage: node scripts/review-brief.mjs OWNER/REPO PR_NUMBER [--max-hunk-lines N] [--max-total-hunk-lines N] [--no-reference-map] [--json]"; + +function positiveInteger(value, option) { + const number = Number(value); + if (!Number.isInteger(number) || number < 1) { + throw new Error(`${option} requires a positive integer`); + } + return number; +} function parseArgs(argv) { const positional = []; - const options = { maxHunkLines: 24, referenceMap: true, json: false }; + const options = { + maxHunkLines: DEFAULT_MAX_HUNK_LINES, + maxTotalHunkLines: DEFAULT_MAX_TOTAL_HUNK_LINES, + referenceMap: true, + json: false, + }; for (let index = 0; index < argv.length; index += 1) { const value = argv[index]; if (value === "--max-hunk-lines") { - options.maxHunkLines = Number(argv[++index]); - if (!Number.isInteger(options.maxHunkLines) || options.maxHunkLines < 1) { - throw new Error("--max-hunk-lines requires a positive integer"); - } + options.maxHunkLines = positiveInteger(argv[++index], "--max-hunk-lines"); + } else if (value === "--max-total-hunk-lines") { + options.maxTotalHunkLines = positiveInteger( + argv[++index], + "--max-total-hunk-lines", + ); } else if (value === "--no-reference-map") { options.referenceMap = false; } else if (value === "--json") { @@ -50,7 +67,12 @@ export function hunkLines(patch = "", maxLines) { const lines = String(patch).split(/\r?\n/); const shown = lines.slice(0, maxLines); const truncated = lines.length > maxLines; - return { text: shown.join("\n"), truncated, totalLines: lines.length }; + return { + text: shown.join("\n"), + truncated, + totalLines: lines.length, + shownLines: shown.length, + }; } export function normalizeFile(file) { @@ -64,7 +86,17 @@ export function normalizeFile(file) { }; } -export function briefText({ meta, plan, files, bugScope, securityScope, executionPlan = null, maxHunkLines, probeBlocks = [] }) { +export function briefText({ + meta, + plan, + files, + bugScope, + securityScope, + executionPlan = null, + maxHunkLines = DEFAULT_MAX_HUNK_LINES, + maxTotalHunkLines = DEFAULT_MAX_TOTAL_HUNK_LINES, + probeBlocks = [], +}) { const out = []; out.push(`# Review brief: ${meta.repo}#${meta.pr}`); out.push(`Head: ${plan.headRefOid || "unknown"}`); @@ -127,17 +159,38 @@ export function briefText({ meta, plan, files, bugScope, securityScope, executio } out.push("## Changed files (diff hunks)"); + let remainingHunkLines = maxTotalHunkLines; for (const raw of files) { const file = normalizeFile(raw); - const { text, truncated, totalLines } = hunkLines(file.patch, maxHunkLines); out.push(`### ${file.path} (+${file.additions}/-${file.deletions})`); - if (text.trim()) { - out.push("```diff"); - out.push(text); - out.push("```"); - if (truncated) out.push(`_(${totalLines} hunk lines; ${maxHunkLines} shown — open the file only if a lens needs more)_`); - } else { + + if (!file.patch.trim()) { out.push("_(no patch text available)_"); + out.push(""); + continue; + } + + if (remainingHunkLines <= 0) { + out.push( + `_(diff hunk omitted: global hunk budget of ${maxTotalHunkLines} lines exhausted; open this file only if a lens needs more)_`, + ); + out.push(""); + continue; + } + + const perFileLimit = Math.min(maxHunkLines, remainingHunkLines); + const { text, truncated, totalLines, shownLines } = hunkLines(file.patch, perFileLimit); + remainingHunkLines -= shownLines; + out.push("```diff"); + out.push(text); + out.push("```"); + if (truncated) { + const globalLimitReached = remainingHunkLines <= 0; + out.push( + globalLimitReached + ? `_(${totalLines} hunk lines; ${shownLines} shown before the global hunk budget of ${maxTotalHunkLines} lines was exhausted; open this file on demand)_` + : `_(${totalLines} hunk lines; ${shownLines} shown; open the file only if a lens needs more)_`, + ); } out.push(""); } @@ -190,7 +243,17 @@ async function main() { } process.stdout.write( - `${briefText({ meta: { repo: args.repo, pr: args.pr }, plan, files: input.files, bugScope, securityScope, executionPlan, maxHunkLines: args.maxHunkLines, probeBlocks })}\n`, + `${briefText({ + meta: { repo: args.repo, pr: args.pr }, + plan, + files: input.files, + bugScope, + securityScope, + executionPlan, + maxHunkLines: args.maxHunkLines, + maxTotalHunkLines: args.maxTotalHunkLines, + probeBlocks, + })}\n`, ); } diff --git a/tests/unit/actions-usage-contract.test.mjs b/tests/unit/actions-usage-contract.test.mjs index bd8160f8..4da73c00 100644 --- a/tests/unit/actions-usage-contract.test.mjs +++ b/tests/unit/actions-usage-contract.test.mjs @@ -29,19 +29,20 @@ test("pull-request CI keeps one canonical full check and bounded compatibility l assert.match(ci, /cancel-in-progress: true/); }); -test("compatibility and Windows lanes use NUL-safe scope evidence and fail closed if detection fails", () => { +test("only Node compatibility remains scoped; Windows Authority is unconditional", () => { assert.match(ci, /scope:/); assert.match(ci, /node_compat:/); - assert.match(ci, /windows_authority:/); + assert.match(ci, /git show "\$\{BASE_SHA\}:scripts\/ci-scope\.mjs"/); assert.match(ci, /git diff --name-only -z/); - assert.match(ci, /node scripts\/ci-scope\.mjs --mode ci/); - assert.match(ci, /needs: scope/); - assert.match(ci, /needs\.scope\.result != 'success'/); + assert.match(ci, /node "\$\{TRUSTED_SCOPE\}" --mode ci/); assert.match(ci, /needs\.scope\.outputs\.node_compat == 'true'/); - assert.match(ci, /needs\.scope\.outputs\.windows_authority == 'true'/); - assert.ok(occurrences(ci, "Fail closed when scope detection failed") >= 2); - assert.match(ci, /authority-host\/windows\//); - assert.match(ci, /scripts\/prepare-authority-host-runtime-smoke\.mjs/); + + const windowsBlock = ci.slice(ci.indexOf(" windows-authority:")); + assert.doesNotMatch(windowsBlock, /needs: scope/); + assert.doesNotMatch(windowsBlock, /needs\.scope\.outputs\.windows_authority/); + assert.doesNotMatch(windowsBlock, /Fail closed when scope detection failed/); + assert.match(windowsBlock, /authority-host\/windows\//); + assert.match(windowsBlock, /scripts\/prepare-authority-host-runtime-smoke\.mjs/); }); test("live fixture diffs force compatibility lanes for acceptance coverage", () => { @@ -89,15 +90,15 @@ test("superseded expensive PR workflows cancel in progress", () => { } }); -test("C# CodeQL uses NUL-safe scope evidence and fails closed on scope errors", () => { - assert.match(codeql, /csharp_scope:/); - assert.match(codeql, /git diff --name-only -z/); - assert.match(codeql, /node scripts\/ci-scope\.mjs --mode csharp/); - assert.match(codeql, /authority-host\/windows\//); - assert.match(codeql, /needs: csharp_scope/); - assert.match(codeql, /needs\.csharp_scope\.result != 'success'/); - assert.match(codeql, /needs\.csharp_scope\.outputs\.required == 'true'/); - assert.match(codeql, /Fail closed when C# scope detection failed/); +test("C# CodeQL always runs on pull requests", () => { + assert.doesNotMatch(codeql, /csharp_scope:/); + assert.match(codeql, /analyze-csharp:/); + assert.match(codeql, /name: CodeQL \/ Analyze \(csharp\)/); + const csharpBlock = codeql.slice(codeql.indexOf(" analyze-csharp:")); + assert.doesNotMatch(csharpBlock, /needs: csharp_scope/); + assert.doesNotMatch(csharpBlock, /needs\.csharp_scope/); + assert.doesNotMatch(csharpBlock, /Fail closed when C# scope detection failed/); + assert.match(csharpBlock, /authority-host\/windows\//); }); test("maintenance schedules stay bounded", () => { diff --git a/tests/unit/cleanup-orphaned-workflows.test.mjs b/tests/unit/cleanup-orphaned-workflows.test.mjs index 49df819d..9674e9a2 100644 --- a/tests/unit/cleanup-orphaned-workflows.test.mjs +++ b/tests/unit/cleanup-orphaned-workflows.test.mjs @@ -25,8 +25,13 @@ function mockFetch(routes, calls) { } function baseRoutes(extra) { + const generation = "a".repeat(40); return [ { path: "/repos/Wibias/github-delivery", body: { default_branch: "main" } }, + { + path: "/repos/Wibias/github-delivery/git/ref/heads/main", + body: { object: { sha: generation } }, + }, { path: "/repos/Wibias/github-delivery/contents/.github/workflows?ref=main", body: [ @@ -35,6 +40,10 @@ function baseRoutes(extra) { ], }, ...extra, + { + path: "/repos/Wibias/github-delivery/git/ref/heads/main", + body: { object: { sha: generation } }, + }, ]; } diff --git a/tests/unit/codex-watchdog-progress-bounds.test.mjs b/tests/unit/codex-watchdog-progress-bounds.test.mjs index c77c3411..29320d3e 100644 --- a/tests/unit/codex-watchdog-progress-bounds.test.mjs +++ b/tests/unit/codex-watchdog-progress-bounds.test.mjs @@ -55,6 +55,17 @@ function started(r, item) { }); } +function completed(r, item) { + return r.onServerMessage({ + method: "item/completed", + params: { + threadId: "thr-progress", + turnId: "turn-progress", + item, + }, + }); +} + test("novel imminent-execution narration is bounded even when every sentence differs", () => { const r = router({ generatedCharHardLimit: 10_000 }); assert.equal(text(r, "Let me grep the duplicate locale key.\n").internalRequests.length, 0); @@ -78,6 +89,74 @@ test("a real tool start clears the pending tool-emission stall without claiming assert.equal(text(r, "I'll run the focused grep.\n").internalRequests.length, 0); }); +test("interleaved evidence tools do not buy a fresh micro-narration budget", () => { + const r = router({ + generatedCharHardLimit: 10_000, + toolEmissionIntentThreshold: 50, + }); + + assert.equal( + text(r, "I will start by loading the canonical agent rules and rewrite plan.\n").internalRequests.length, + 0, + ); + started(r, { + id: "read-1", + type: "commandExecution", + command: 'Get-Content -LiteralPath "AGENTS.md" -Raw', + status: "inProgress", + }); + + assert.equal( + text(r, "Canonical rules are loaded. Next I'll verify the current git state.\n").internalRequests.length, + 0, + ); + started(r, { + id: "read-2", + type: "commandExecution", + command: "git status --short --branch", + status: "inProgress", + }); + + const tripped = text( + r, + "GitHub Delivery owns this stack, so I'll load the stacked-PR workflow next.\n", + ); + assert.equal(tripped.internalRequests.length, 1); + assert.equal(tripped.internalRequests[0].method, "turn/interrupt"); +}); + +test("real execution progress resets the micro-narration budget", () => { + const r = router({ + generatedCharHardLimit: 10_000, + toolEmissionIntentThreshold: 50, + }); + assert.equal(text(r, "I'll load the first rule file.\n").internalRequests.length, 0); + started(r, { + id: "read-a", + type: "commandExecution", + command: 'Get-Content -LiteralPath "AGENTS.md" -Raw', + status: "inProgress", + }); + assert.equal(text(r, "Next I'll verify the branch state.\n").internalRequests.length, 0); + + completed(r, { + id: "test-1", + type: "commandExecution", + command: "npm test", + status: "completed", + exitCode: 0, + }); + + assert.equal(text(r, "I'll load the selected workflow.\n").internalRequests.length, 0); + started(r, { + id: "read-b", + type: "commandExecution", + command: 'Get-Content -LiteralPath "references/stacked-prs.md" -Raw', + status: "inProgress", + }); + assert.equal(text(r, "Then I'll verify the live PR stack.\n").internalRequests.length, 0); +}); + test("repeated malformed tool protocol output accelerates a tool-emission stall", () => { const r = router({ generatedCharHardLimit: 10_000, toolEmissionIntentThreshold: 50 }); assert.equal(text(r, "\n").internalRequests.length, 0); diff --git a/tests/unit/deep-audit-hardening.test.mjs b/tests/unit/deep-audit-hardening.test.mjs new file mode 100644 index 00000000..63ac7d0b --- /dev/null +++ b/tests/unit/deep-audit-hardening.test.mjs @@ -0,0 +1,317 @@ +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; + +import { classifyCiScope } from "../../scripts/ci-scope.mjs"; +import { cleanupOrphanedWorkflowRuns } from "../../scripts/cleanup-orphaned-workflows.mjs"; +import { makeRedemptionRunner } from "../../scripts/lib/authority-execution.mjs"; +import { collectBranchReviewInput } from "../../scripts/lib/branch-review-input.mjs"; +import { evaluateMergeStackEligibility } from "../../scripts/lib/merge-stack-policy.mjs"; +import { planReviewScope } from "../../scripts/lib/review-scope.mjs"; +import { evaluate as evaluatePreOpen } from "../../scripts/pre-open-gate.mjs"; +import { briefText } from "../../scripts/review-brief.mjs"; + +function source(path) { + return readFileSync(new URL(`../../${path}`, import.meta.url), "utf8"); +} + +function git(cwd, args) { + return execFileSync("git", args, { + cwd, + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }).trim(); +} + +test("CI scope policy changes remain conservative when the classifier itself changes", () => { + const scope = classifyCiScope(["scripts/ci-scope.mjs"]); + assert.equal(scope.nodeCompat, true); + assert.equal(scope.windowsAuthority, true); + assert.equal(scope.csharp, true); +}); + +test("security-critical Windows and C# lanes cannot be scoped out by a pull request", () => { + const ci = source(".github/workflows/ci.yml"); + const codeql = source(".github/workflows/codeql.yml"); + + assert.match(ci, /git show "\$\{BASE_SHA\}:scripts\/ci-scope\.mjs"/); + const windowsBlock = ci.slice(ci.indexOf(" windows-authority:")); + assert.doesNotMatch(windowsBlock, /needs: scope/); + assert.doesNotMatch(windowsBlock, /needs\.scope\.outputs\.windows_authority/); + + assert.doesNotMatch(codeql, /csharp_scope:/); + const csharpBlock = codeql.slice(codeql.indexOf(" analyze-csharp:")); + assert.doesNotMatch(csharpBlock, /needs: csharp_scope/); + assert.doesNotMatch(csharpBlock, /needs\.csharp_scope/); +}); + +test("trusted authority is redeemed before an internal coordination write executes", () => { + const events = []; + const nonce = "audit-nonce"; + const authority = { + verified: true, + claims: { + redemption: "required", + scopeSha256: "a".repeat(64), + nonce, + }, + }; + const execution = makeRedemptionRunner({ + plannedCommand: ["gh", "pr", "comment", "42"], + authority, + authorityGrant: "gd1.audit-fixture", + redeemer() { + events.push("redeem"); + return { status: "consumed", nonce, consumedAt: 1 }; + }, + runner() { + events.push("write"); + return { status: 0, stdout: "", stderr: "" }; + }, + }); + + execution.runner("gh", [ + "api", + "repos/acme/widget/git/refs", + "--method", + "POST", + "-f", + "ref=refs/github-delivery/idempotency/test", + "-f", + `sha=${"b".repeat(40)}`, + ], {}); + + assert.deepEqual(events, ["redeem", "write"]); + assert.equal(execution.redemption()?.status, "consumed"); +}); + +test("branch review input preserves rename paths and keeps renamed code in scope", () => { + const root = mkdtempSync(join(tmpdir(), "github-delivery-rename-")); + const previousCwd = process.cwd(); + try { + git(root, ["init", "-b", "main"]); + git(root, ["config", "user.email", "audit@example.invalid"]); + git(root, ["config", "user.name", "Audit Fixture"]); + mkdirSync(join(root, "src"), { recursive: true }); + writeFileSync(join(root, "src", "auth.mjs"), "export const auth = true;\n", "utf8"); + git(root, ["add", "src/auth.mjs"]); + git(root, ["commit", "-m", "base"]); + const base = git(root, ["rev-parse", "HEAD"]); + + git(root, ["mv", "src/auth.mjs", "src/auth"]); + git(root, ["commit", "-m", "rename auth module"]); + const head = git(root, ["rev-parse", "HEAD"]); + + process.chdir(root); + const input = collectBranchReviewInput(base, head); + assert.equal(input.files.length, 1); + assert.equal(input.files[0].status.startsWith("R"), true); + assert.equal(input.files[0].previousPath, "src/auth.mjs"); + assert.equal(input.files[0].path, "src/auth"); + + const plan = planReviewScope(input); + assert.ok(plan.logicFiles.includes("src/auth")); + assert.notEqual(evaluatePreOpen(plan).decision, "ready"); + } finally { + process.chdir(previousCwd); + rmSync(root, { recursive: true, force: true }); + } +}); + +test("code renamed to a documentation path cannot take the docs-only shortcut", () => { + const plan = planReviewScope({ + repo: "acme/widget", + pr: null, + headRefOid: "abc", + files: [ + { + path: "docs/auth.md", + previousPath: "src/auth.mjs", + status: "R100", + patch: "diff --git a/src/auth.mjs b/docs/auth.md", + additions: 0, + deletions: 0, + }, + ], + }); + + assert.ok(plan.logicFiles.includes("docs/auth.md")); + assert.notEqual(plan.bugReview.depth, "skip"); + assert.notEqual(plan.securityReview.depth, "skip"); +}); + +test("pre-open gate blocks until every deterministic required probe has canonical evidence", () => { + const plan = planReviewScope({ + repo: "acme/widget", + pr: null, + headRefOid: "abc", + files: [ + { + path: "tests/worker.test.mjs", + patch: "+setTimeout(() => {}, 100);\n", + additions: 1, + deletions: 0, + status: "modified", + }, + ], + }); + assert.ok(plan.requiredProbes.includes("test-honesty")); + + const first = evaluatePreOpen(plan); + const evidence = { + schemaVersion: 1, + lenses: Object.fromEntries(first.bugScope.requiredLenses.map((id) => [id, "done"])), + surfaces: Object.fromEntries( + first.securityScope.requiredSurfaces.map((id) => [id, "n/a audit fixture boundary untouched"]), + ), + probes: {}, + }; + const blocked = evaluatePreOpen(plan, evidence); + assert.equal(blocked.decision, "blocked"); + assert.ok(blocked.blockers.includes("probe:requiredProbes:test-honesty")); + + evidence.probes["test-honesty"] = { + status: "clean", + files: ["tests/worker.test.mjs"], + }; + const cleared = evaluatePreOpen(plan, evidence); + assert.equal(cleared.blockers.includes("probe:requiredProbes:test-honesty"), false); + assert.equal(cleared.probeEvidenceErrors.length, 0); + assert.equal(cleared.decision, "ready"); +}); + +test("merge execution rejects a child while its stack parent is still open", () => { + const repo = "acme/widget"; + const prs = [ + { + number: 1, + title: "parent", + headRefName: "feature/parent", + baseRefName: "main", + headRepoFullName: repo, + baseRepoFullName: repo, + url: "https://example.invalid/1", + isDraft: false, + headRefOid: "a".repeat(40), + }, + { + number: 2, + title: "child", + headRefName: "feature/child", + baseRefName: "feature/parent", + headRepoFullName: repo, + baseRepoFullName: repo, + url: "https://example.invalid/2", + isDraft: false, + headRefOid: "b".repeat(40), + }, + ]; + const result = evaluateMergeStackEligibility({ prs, targetPr: 2 }); + assert.equal(result.eligible, false); + assert.equal(result.reason, "stack_parent_unlanded"); + assert.equal(result.parentPr, 1); + + const executionBoundary = source("scripts/lib/mutation-execution-context.mjs"); + assert.match(executionBoundary, /verifyMergeStackEligibility/); +}); + +test("orphan cleanup aborts if the default branch generation moves before deletion", async () => { + const calls = []; + let refReads = 0; + const fetchImpl = async (input, init = {}) => { + const url = new URL(typeof input === "string" ? input : input.url); + const method = init.method ?? "GET"; + const path = `${url.pathname}${url.search}`; + calls.push(`${method} ${path}`); + + if (method === "GET" && path === "/repos/Wibias/github-delivery") { + return Response.json({ default_branch: "main" }); + } + if (method === "GET" && path === "/repos/Wibias/github-delivery/git/ref/heads/main") { + refReads += 1; + return Response.json({ object: { sha: refReads === 1 ? "a".repeat(40) : "b".repeat(40) } }); + } + if ( + method === "GET" && + path === "/repos/Wibias/github-delivery/contents/.github/workflows?ref=main" + ) { + return Response.json([ + { type: "file", path: ".github/workflows/ci.yml" }, + { type: "file", path: ".github/workflows/cleanup-orphaned-workflows.yml" }, + ]); + } + if ( + method === "GET" && + path === "/repos/Wibias/github-delivery/actions/workflows?per_page=100&page=1" + ) { + return Response.json({ + workflows: [ + { id: 2, name: "Old helper", path: ".github/workflows/tmp.yml" }, + ], + }); + } + if ( + method === "GET" && + path === "/repos/Wibias/github-delivery/actions/workflows/2/runs?per_page=100&page=1" + ) { + return Response.json({ workflow_runs: [{ id: 200, status: "completed" }] }); + } + if (method === "DELETE") { + return new Response("delete should not be attempted after generation drift", { status: 500 }); + } + return new Response(`Unexpected request: ${method} ${path}`, { status: 500 }); + }; + + await assert.rejects( + cleanupOrphanedWorkflowRuns({ + token: "test-token", + repository: "Wibias/github-delivery", + fetchImpl, + log: () => {}, + }), + /default_branch_moved_during_cleanup/, + ); + assert.equal(calls.some((call) => call.startsWith("DELETE ")), false); + assert.equal(refReads, 2); +}); + +test("review brief applies a global hunk-line budget across huge diffs", () => { + const files = Array.from({ length: 20 }, (_, index) => ({ + path: `src/file-${index}.mjs`, + additions: 30, + deletions: 0, + patch: Array.from({ length: 30 }, (__, line) => `+line ${index}-${line}`).join("\n"), + })); + const text = briefText({ + meta: { repo: "acme/widget", pr: 1 }, + plan: { + headRefOid: "abc", + fileCount: files.length, + logicFiles: files.map((file) => file.path), + requiredProbes: [], + dependencyChanges: [], + removedControlLeads: [], + uncertainty: [], + }, + files, + bugScope: { requiredLenses: [] }, + securityScope: { requiredSurfaces: [] }, + executionPlan: null, + maxHunkLines: 24, + maxTotalHunkLines: 50, + probeBlocks: [], + }); + + const emittedDiffLines = text.split(/\r?\n/).filter((line) => line.startsWith("+line ")); + assert.ok(emittedDiffLines.length <= 50, `expected <= 50 hunk lines, got ${emittedDiffLines.length}`); + assert.match(text, /global hunk budget/i); +}); diff --git a/tests/unit/forward-progress-policy.test.mjs b/tests/unit/forward-progress-policy.test.mjs index c61f2b12..1eb22f94 100644 --- a/tests/unit/forward-progress-policy.test.mjs +++ b/tests/unit/forward-progress-policy.test.mjs @@ -41,6 +41,10 @@ test("verification economy reuses passing evidence on unchanged code and state", test("deterministic tool calls are executed without micro-narration", () => { assert.match(kernel, /do not narrate.*tool calls|execute.*without.*narrat/i); assert.match(kernel, /phase change|material.*change|user input/i); + assert.match(kernel, /rule\/skill\/workflow loading|file reads|remote fetches/i); + assert.match(kernel, /retry quietly|correct.*quietly/i); + assert.match(skill, /routine deterministic tooling quietly/i); + assert.match(skill, /material progress or blockers/i); }); test("context economy minimises evidence acquisition and model-facing output", () => {