diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 36efc1ecc245..1388aaf98563 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -237,6 +237,30 @@ jobs: - name: Test general-server shard partition run: node --test ./scripts/__tests__/run-vitest-stable-shard.test.mjs + # BLO-24241: unlike the step above (a hard-required, generous-floor + # sanity check), this one asserts strict 100% manifest coverage and is + # allowed to fail. A missing suite shows up as a real red X on this + # step, naming the exact suites and the one-line fix, without failing + # `policy` and skipping build/typecheck/e2e for the whole PR the way + # the old bare >=90% assertion did. + - name: Check shard duration manifest freshness (non-blocking) + continue-on-error: true + run: node --test ./scripts/check-shard-manifest-freshness.test.mjs + + # BLO-24241: the weekly refresh path. These are required (not + # continue-on-error) because a break here only surfaces on a Monday + # schedule that nobody is watching -- the merge step used to clobber the + # manifest's durable "$notes" prose, and the measurement runner used to + # buffer vitest's stdout into an ENOBUFS kill. + - name: Test shard duration measurement runner + run: node --test ./scripts/__tests__/measure-general-server-shard-durations.test.mjs + + - name: Test shard duration manifest merge + run: node --test ./scripts/__tests__/merge-shard-duration-manifest.test.mjs + + - name: Test shard duration refresh workflow + run: node --test ./scripts/__tests__/refresh-shard-manifest-workflow.test.mjs + - name: Test vitest project coverage run: node --test ./scripts/__tests__/vitest-project-coverage.test.mjs diff --git a/.github/workflows/refresh-shard-manifest.yml b/.github/workflows/refresh-shard-manifest.yml new file mode 100644 index 000000000000..f5ab6c64c252 --- /dev/null +++ b/.github/workflows/refresh-shard-manifest.yml @@ -0,0 +1,240 @@ +name: Refresh Shard Manifest + +# Keeps scripts/general-server-shard-durations.json from silently going stale +# (BLO-24241). The manifest's own $comment says it should be refreshed "when +# shard timing drifts materially", but nothing measured that drift before +# this workflow existed -- refresh depended entirely on a human noticing +# (which is how #1117 hit the old 90% coverage cliff, and how the 138s+ +# heartbeat-queued-backlog-convergence suite spent a stretch silently +# absorbing the median weight instead of its own). This runs on a schedule +# instead, mirroring one real ARC PR run: a 4-way general-server shard +# matrix, merged into a single manifest update, opened as a PR the same way +# refresh-lockfile.yml opens lockfile refreshes. +# +# Deliberately opens a PR rather than pushing straight to master: unlike a +# lockfile refresh (mechanically regenerated, always safe), a duration +# manifest update is worth a diff a human can skim for a suite that +# regressed or improved by an order of magnitude before it starts steering +# the shard matrix. + +on: + schedule: + - cron: "23 5 * * 1" # Monday 05:23 UTC -- clear of the hourly/half-hourly pile-up + workflow_dispatch: + +concurrency: + group: refresh-shard-manifest-master + cancel-in-progress: false + +jobs: + measure: + name: Measure shard ${{ matrix.shard_index }} + runs-on: default + timeout-minutes: 90 + strategy: + fail-fast: false + matrix: + shard_index: [0, 1, 2, 3] + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + # Via the wrapper, not pnpm/action-setup directly: it adds the one + # jittered retry around the registry fetch that v6's self-update needs + # (BLO-28813). No `version:` -- the wrapper resolves the pin from + # package.json "packageManager", which is why the checkout above has to + # come first. + - name: Setup pnpm + uses: ./.github/actions/setup-pnpm + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version: 24 + cache: pnpm + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Measure this shard's suite durations + run: | + node scripts/measure-general-server-shard-durations.mjs \ + --all \ + --shard-index ${{ matrix.shard_index }} \ + --shard-count 4 \ + --output /tmp/shard-${{ matrix.shard_index }}.json + + - name: Upload measured durations + uses: actions/upload-artifact@v4 + with: + name: shard-durations-${{ matrix.shard_index }} + path: /tmp/shard-${{ matrix.shard_index }}.json + retention-days: 1 + if-no-files-found: error + + refresh: + needs: [measure] + # Not a bare `needs:`. With fail-fast: false across four shards, a single + # infra failure in one shard would otherwise skip the merge and leave the + # manifest un-refreshed for the whole week -- discarding three good + # quarters to punish one bad one. The merge is safe against partial input + # by construction: it spreads `measured` over the existing `durations`, so + # an absent quarter keeps its previous values, and the prune is keyed on + # the on-disk suite set rather than the measured set so it cannot delete + # them. If all four shards fail there is nothing to download and this job + # fails loudly, which is the right outcome for a total measurement loss. + if: ${{ !cancelled() }} + runs-on: default + timeout-minutes: 10 + permissions: + contents: write + pull-requests: write + # For the stall alarm's actions/runs read at the end of this job. + actions: read + env: + # BLO-25515/BLO-24150: a GITHUB_TOKEN-authored push/PR gets its own + # workflow runs quarantined behind `action_required` -- or, per + # BLO-24150's later occurrence, never queued at all -- once the merge + # queue is the only merge path. A weekly manifest PR that can never pass + # checks never merges, so the manifest never refreshes and this + # workflow's entire reason to exist quietly stops holding. This is an + # internal workflow, so fail closed when the App credential is absent + # instead of creating a PR that cannot receive CI. + COMMITPERCLIP_ENABLED: ${{ secrets.COMMITPERCLIP_KEY != '' }} + + steps: + - name: Checkout repository + uses: actions/checkout@v6 + + - name: Require commitperclip App credential + if: env.COMMITPERCLIP_ENABLED != 'true' + run: | + echo "::error::COMMITPERCLIP_KEY is required for unattended shard-manifest refreshes; refusing to create a GITHUB_TOKEN-authored PR" + exit 1 + + - name: Download measured durations + uses: actions/download-artifact@v4 + with: + pattern: shard-durations-* + path: /tmp/shard-durations + merge-multiple: true + + - name: Setup Node.js + uses: actions/setup-node@v6 + with: + node-version: 24 + + # No `pnpm install` needed ahead of this: get-bot-token.mjs imports only + # node: builtins, same as the merge script below. + - name: Generate commitperclip token + id: bot-token + if: env.COMMITPERCLIP_ENABLED == 'true' + run: | + TOKEN=$(node .github/scripts/get-bot-token.mjs) + echo "::add-mask::$TOKEN" + echo "value=$TOKEN" >> "$GITHUB_OUTPUT" + env: + COMMITPERCLIP_KEY: ${{ secrets.COMMITPERCLIP_KEY }} + COMMITPERCLIP_APP_ID: ${{ vars.COMMITPERCLIP_APP_ID }} + + - name: Merge shards into the manifest + env: + GITHUB_RUN_ID: ${{ github.run_id }} + run: | + node scripts/merge-shard-duration-manifest.mjs --shard-dir /tmp/shard-durations + + - name: Create or update pull request + id: upsert-pr + env: + # BLO-25515: push and PR creation must use the App token so this + # PR's checks are not attributed to GITHUB_TOKEN. The guard above + # makes the output mandatory before this step can run. + GH_TOKEN: ${{ steps.bot-token.outputs.value }} + REPO_OWNER: ${{ github.repository_owner }} + run: | + if git diff --quiet -- scripts/general-server-shard-durations.json; then + echo "Manifest unchanged, nothing to do." + echo "pr_url=" >> "$GITHUB_OUTPUT" + exit 0 + fi + + BRANCH="chore/refresh-shard-manifest" + # Use a non-App commit identity: the PR policy rejects ordinary + # commits stamped with allyblockcast[bot]. The App token authenticates + # the push, but the local author remains an explicit bot identity. + git config user.name "shard-manifest-bot" + git config user.email "shard-manifest-bot@paperclip.blockcast.net" + + git checkout -B "$BRANCH" + git add scripts/general-server-shard-durations.json + git commit -m "ci: refresh general-server shard durations" + # Push through an explicit x-access-token URL rather than the + # `origin` remote actions/checkout wired up: that remote's stored + # credential is always github.token, which would silently undo the + # point of GH_TOKEN above. + git push --force "https://x-access-token:${GH_TOKEN}@github.com/${{ github.repository }}.git" "$BRANCH" + + pr_url="$( + gh pr list --state open --head "$BRANCH" --json url,headRepositoryOwner \ + --jq ".[] | select(.headRepositoryOwner.login == \"$REPO_OWNER\") | .url" | + head -n 1 + )" + if [ -z "$pr_url" ]; then + pr_url="$(gh pr create \ + --head "$BRANCH" \ + --title "ci: refresh general-server shard durations" \ + --body "Auto-generated weekly refresh of scripts/general-server-shard-durations.json from a live 4-way general-server measurement run (BLO-24241). Review for any suite whose duration moved by an order of magnitude before merging -- everything else is routine drift.")" + echo "Created new PR: $pr_url" + else + echo "PR already exists: $pr_url" + fi + echo "pr_url=$pr_url" >> "$GITHUB_OUTPUT" + + # BLO-25515/BLO-24150, ported from refresh-lockfile.yml:161. Unlike that + # workflow this one deliberately does not merge its own PR -- a duration + # manifest is worth a human skim first. The App-token guard above should + # prevent the known GITHUB_TOKEN quarantine, but the alarm still covers + # both observed shapes: runs parked in `action_required`, and + # BLO-24150's later occurrence (#1237, #1240) where zero runs are queued + # at all. Silence is exactly the failure mode this workflow exists to + # remove, so make it loud. + - name: Alert if the refresh PR got no CI + if: steps.upsert-pr.outputs.pr_url != '' + env: + # This step only reads Actions runs and posts a diagnostic comment. + # Use the workflow token here because the job-level `actions: read` + # and `pull-requests: write` permissions apply to it. The + # commitperclip installation token is intentionally limited to the + # push/PR-creation step above; an installation may not have Actions + # read permission, and an alarm that dies on its first 403 would + # erase the signal it exists to provide. + GH_TOKEN: ${{ github.token }} + run: | + pr_url="${{ steps.upsert-pr.outputs.pr_url }}" + head_sha="$(gh pr view "$pr_url" --json headRefOid --jq .headRefOid)" + + # Runs are queued asynchronously after the push, so a zero count read + # immediately after `gh pr create` means "too early", not "stalled". + # Poll first, otherwise this alarm cries wolf every single Monday and + # becomes the thing people learn to ignore. + action_required_count=0 + total_run_count=0 + for _ in $(seq 1 8); do + runs_json="$(gh api "repos/${{ github.repository }}/actions/runs?head_sha=$head_sha&per_page=50")" + total_run_count="$(printf '%s' "$runs_json" | jq '.workflow_runs | length')" + action_required_count="$(printf '%s' "$runs_json" | jq '[.workflow_runs[] | select(.status == "action_required")] | length')" + if [ "${total_run_count:-0}" -gt 0 ]; then + break + fi + sleep 15 + done + + if [ "${action_required_count:-0}" -gt 0 ]; then + gh pr comment "$pr_url" --body "🚨 **Weekly shard-manifest refresh is stalled on a human approval gate (BLO-24150/BLO-25515).** \`$action_required_count\` workflow run(s) on this commit are sitting in \`action_required\`, so this PR's checks cannot start until someone clicks *Approve and run* (Actions tab → this PR → pending runs). Until then the PR cannot merge and \`scripts/general-server-shard-durations.json\` keeps drifting — which is the exact depends-on-a-human-noticing failure this workflow exists to remove (BLO-24241). The App-token guard should prevent this path; inspect the workflow run's token-generation and repository-permission errors." + echo "Posted stall alert: $action_required_count run(s) awaiting approval." + elif [ "${total_run_count:-0}" -eq 0 ]; then + gh pr comment "$pr_url" --body "🚨 **Weekly shard-manifest refresh opened a PR with zero workflow runs (BLO-24150/BLO-25515).** No workflow runs are recorded for \`$head_sha\` after ~2 minutes of polling, so this PR's required checks will never report and it can never merge. This is the same class of stall as \`action_required\`, just without a run object to click approve on. Check the Actions tab; verify the App installation, token-generation step, and workflow permissions, then re-run *Refresh Shard Manifest* via workflow_dispatch. Until resolved, \`scripts/general-server-shard-durations.json\` keeps drifting — see https://paperclip.blockcast.net/BLO/issues/BLO-24241." + echo "Posted stall alert: zero workflow runs recorded for $head_sha." + else + echo "Refresh PR has CI running normally (action_required=$action_required_count, total_runs=$total_run_count) — no stall signature." + fi diff --git a/scripts/__tests__/measure-general-server-shard-durations.test.mjs b/scripts/__tests__/measure-general-server-shard-durations.test.mjs new file mode 100644 index 000000000000..473eb4178197 --- /dev/null +++ b/scripts/__tests__/measure-general-server-shard-durations.test.mjs @@ -0,0 +1,181 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + mergeDurations, + parseVitestJsonReport, + selectTargetFiles, +} from "../measure-general-server-shard-durations.mjs"; + +test("selectTargetFiles defaults to only the suites missing from the manifest", () => { + const allFiles = ["a.test.ts", "b.test.ts", "c.test.ts"]; + // Mixed fixture on purpose: a.test.ts is already measured, so the default + // mode must skip it. Seeding every key as missing would assert the same + // result as the --all case below and would still pass with the filter + // deleted, proving nothing about the filter. + const durations = { "a.test.ts": 10 }; + + const target = selectTargetFiles({ allFiles, durations, all: false, shardIndex: null, shardCount: null }); + + assert.deepEqual(target, ["b.test.ts", "c.test.ts"]); +}); + +test("selectTargetFiles defaults to an empty list when the manifest already covers every suite", () => { + const allFiles = ["a.test.ts", "b.test.ts"]; + const durations = { "a.test.ts": 10, "b.test.ts": 20 }; + + const target = selectTargetFiles({ allFiles, durations, all: false, shardIndex: null, shardCount: null }); + + assert.deepEqual(target, [], "a fully covered manifest must leave nothing to measure"); +}); + +test("selectTargetFiles treats a zero-millisecond entry as measured, not missing", () => { + const allFiles = ["a.test.ts", "b.test.ts"]; + const durations = { "a.test.ts": 0 }; + + const target = selectTargetFiles({ allFiles, durations, all: false, shardIndex: null, shardCount: null }); + + assert.deepEqual(target, ["b.test.ts"], "0 is a real measurement and must not re-measure"); +}); + +test("selectTargetFiles --all re-measures every suite regardless of manifest state", () => { + const allFiles = ["b.test.ts", "a.test.ts"]; + const durations = { "a.test.ts": 10, "b.test.ts": 20 }; + + const target = selectTargetFiles({ allFiles, durations, all: true, shardIndex: null, shardCount: null }); + + assert.deepEqual(target, ["a.test.ts", "b.test.ts"]); +}); + +test("selectTargetFiles slices the target list across shard-index/shard-count", () => { + const allFiles = ["a.test.ts", "b.test.ts", "c.test.ts", "d.test.ts"]; + const durations = {}; + + const shard0 = selectTargetFiles({ allFiles, durations, all: true, shardIndex: 0, shardCount: 2 }); + const shard1 = selectTargetFiles({ allFiles, durations, all: true, shardIndex: 1, shardCount: 2 }); + + assert.deepEqual([...shard0, ...shard1].sort(), [...allFiles].sort()); + assert.deepEqual(shard0.filter((file) => shard1.includes(file)), [], "shards must not overlap"); +}); + +test("parseVitestJsonReport rebases absolute test file paths onto repo-relative keys", () => { + const report = { + testResults: [ + { + name: "/repo/server/src/__tests__/a.test.ts", + startTime: 1000, + endTime: 1250, + status: "passed", + assertionResults: [{ status: "passed" }], + }, + { + name: "/repo/server/src/__tests__/b.test.ts", + startTime: 2000, + endTime: 2000.7, + status: "passed", + assertionResults: [{ status: "passed" }], + }, + ], + }; + + const measured = parseVitestJsonReport(JSON.stringify(report), "/repo"); + + assert.deepEqual(measured, { + "server/src/__tests__/a.test.ts": 250, + "server/src/__tests__/b.test.ts": 1, + }); +}); + +// A failing suite still ran, so its span is a real cost the partition should +// carry. Only a suite that never executed is excluded below. +test("parseVitestJsonReport keeps a failed suite's duration", () => { + const report = { + testResults: [ + { + name: "/repo/server/src/__tests__/a.test.ts", + startTime: 1000, + endTime: 4000, + status: "failed", + assertionResults: [{ status: "failed" }], + }, + ], + }; + + const measured = parseVitestJsonReport(JSON.stringify(report), "/repo"); + + assert.deepEqual(measured, { "server/src/__tests__/a.test.ts": 3000 }); +}); + +// The regression: a collection error yields no tests and a ~0 span, and +// because vitest folds test results to derive file status it reports `passed`. +// Recorded as-is, the suite would carry a near-zero partition weight for a week. +test("parseVitestJsonReport drops a suite that collected no tests", () => { + const report = { + testResults: [ + { + name: "/repo/server/src/__tests__/broken.test.ts", + startTime: 5000, + endTime: 5000, + status: "passed", + assertionResults: [], + }, + { + name: "/repo/server/src/__tests__/ok.test.ts", + startTime: 6000, + endTime: 6800, + status: "passed", + assertionResults: [{ status: "passed" }], + }, + ], + }; + + const measured = parseVitestJsonReport(JSON.stringify(report), "/repo"); + + assert.deepEqual( + measured, + { "server/src/__tests__/ok.test.ts": 800 }, + "a suite that ran nothing must not be recorded as a 0ms measurement", + ); +}); + +test("parseVitestJsonReport drops a suite whose reported status is neither passed nor failed", () => { + const report = { + testResults: [ + { + name: "/repo/server/src/__tests__/skipped.test.ts", + startTime: 1000, + endTime: 1005, + status: "skipped", + assertionResults: [{ status: "skipped" }], + }, + ], + }; + + const measured = parseVitestJsonReport(JSON.stringify(report), "/repo"); + + assert.deepEqual(measured, {}, "an unrecognised file status is not a measurement"); +}); + +// The guard must key off positive evidence of a non-run. If a reporter upgrade +// drops these fields, recording the span is the correct fallback -- a guard +// that quietly emptied the manifest would be worse than the bug it closes. +test("parseVitestJsonReport still records entries when the reporter omits status metadata", () => { + const report = { + testResults: [{ name: "/repo/server/src/__tests__/a.test.ts", startTime: 1000, endTime: 1250 }], + }; + + const measured = parseVitestJsonReport(JSON.stringify(report), "/repo"); + + assert.deepEqual(measured, { "server/src/__tests__/a.test.ts": 250 }); +}); + +test("mergeDurations overwrites existing entries with fresh measurements and keeps keys sorted", () => { + const existing = { "z.test.ts": 100, "a.test.ts": 50 }; + const measured = { "a.test.ts": 75, "m.test.ts": 10 }; + + const merged = mergeDurations(existing, measured); + + assert.deepEqual(Object.keys(merged), ["a.test.ts", "m.test.ts", "z.test.ts"]); + assert.equal(merged["a.test.ts"], 75, "a fresh measurement must win over the stale entry"); + assert.equal(merged["z.test.ts"], 100, "an un-remeasured suite keeps its prior duration"); +}); diff --git a/scripts/__tests__/merge-shard-duration-manifest.test.mjs b/scripts/__tests__/merge-shard-duration-manifest.test.mjs new file mode 100644 index 000000000000..74cb0fbf5ee6 --- /dev/null +++ b/scripts/__tests__/merge-shard-duration-manifest.test.mjs @@ -0,0 +1,169 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import test from "node:test"; + +import { + REWRITTEN_KEYS, + formatProvenanceComment, + mergeManifest, + readShardMeasurements, +} from "../merge-shard-duration-manifest.mjs"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); +const durationsManifest = path.join(repoRoot, "scripts", "general-server-shard-durations.json"); + +function manifestFixture() { + return { + $comment: "STALE PROVENANCE from run 111 on 2026-01-01.", + $notes: "NOTE ON UNITS: these are test-execution durations; do not 'fix' a small value by hand.", + unit: "ms", + durations: { "a.test.ts": 10, "z.test.ts": 30 }, + }; +} + +// The regression this file exists for: the workflow's original inline merge +// overwrote $comment with a provenance-only template, deleting the units note +// that explains why a trivial suite reads as single-digit ms. First scheduled +// run would have destroyed it, unwatched (BLO-24241). +test("a refresh preserves the durable $notes prose verbatim", () => { + const manifest = manifestFixture(); + + const next = mergeManifest({ + manifest, + measured: { "a.test.ts": 12 }, + runId: "999", + date: "2026-08-19", + }); + + assert.equal(next["$notes"], manifest["$notes"], "$notes must survive a refresh untouched"); +}); + +test("a refresh rewrites only $comment and durations, passing every other key through", () => { + const manifest = manifestFixture(); + + const next = mergeManifest({ manifest, measured: { "a.test.ts": 12 }, runId: "999", date: "2026-08-19" }); + + const changed = Object.keys({ ...manifest, ...next }).filter( + (key) => JSON.stringify(next[key]) !== JSON.stringify(manifest[key]), + ); + assert.deepEqual(changed.sort(), [...REWRITTEN_KEYS].sort()); + assert.equal(next.unit, "ms"); +}); + +test("a refresh replaces the provenance sentence with the current run and date", () => { + const manifest = manifestFixture(); + + const next = mergeManifest({ manifest, measured: { "a.test.ts": 12 }, runId: "999", date: "2026-08-19" }); + + assert.ok(!next["$comment"].includes("run 111"), "stale run id must not survive"); + assert.ok(next["$comment"].includes("999")); + assert.ok(next["$comment"].includes("2026-08-19")); +}); + +test("fresh measurements win over stale entries and un-measured suites keep their duration", () => { + const next = mergeManifest({ + manifest: manifestFixture(), + measured: { "a.test.ts": 12, "m.test.ts": 5 }, + runId: "999", + date: "2026-08-19", + }); + + assert.deepEqual(Object.keys(next.durations), ["a.test.ts", "m.test.ts", "z.test.ts"]); + assert.equal(next.durations["a.test.ts"], 12, "a fresh measurement must win"); + assert.equal(next.durations["z.test.ts"], 30, "an un-remeasured suite keeps its prior duration"); +}); + +// Without a prune the merge is purely additive, so a deleted suite keeps its +// entry forever and totalCount in the provenance sentence overstates coverage. +test("a refresh prunes entries for suites no longer on disk", () => { + const next = mergeManifest({ + manifest: manifestFixture(), + measured: { "a.test.ts": 12 }, + runId: "999", + date: "2026-08-19", + knownSuites: ["a.test.ts"], + }); + + assert.deepEqual(Object.keys(next.durations), ["a.test.ts"], "z.test.ts was deleted from the repo"); + assert.ok(next["$comment"].includes("1 total"), "totalCount must reflect the prune"); +}); + +// The prune is keyed on the suite set ON DISK, never on what this run +// measured. The measure matrix is fail-fast: false across four shards, so a +// measured-keyed prune would let one failed shard delete a quarter of the +// manifest, unattended, on a Monday-morning schedule. +test("a refresh keeps on-disk suites that this run did not measure", () => { + const next = mergeManifest({ + manifest: manifestFixture(), + measured: { "a.test.ts": 12 }, + runId: "999", + date: "2026-08-19", + knownSuites: ["a.test.ts", "z.test.ts"], + }); + + assert.deepEqual(Object.keys(next.durations), ["a.test.ts", "z.test.ts"]); + assert.equal(next.durations["z.test.ts"], 30, "an unmeasured but still-present suite keeps its duration"); +}); + +test("a refresh prunes nothing when the suite set is empty or absent", () => { + for (const knownSuites of [undefined, null, []]) { + const next = mergeManifest({ + manifest: manifestFixture(), + measured: { "a.test.ts": 12 }, + runId: "999", + date: "2026-08-19", + knownSuites, + }); + + assert.deepEqual( + Object.keys(next.durations), + ["a.test.ts", "z.test.ts"], + `knownSuites=${JSON.stringify(knownSuites)} means a broken checkout, not a mass deletion`, + ); + } +}); + +// The prune must not become a way to smuggle a new suite in without a +// measurement: knownSuites filters, it does not seed. +test("a refresh does not invent entries for on-disk suites that have no duration", () => { + const next = mergeManifest({ + manifest: manifestFixture(), + measured: { "a.test.ts": 12 }, + runId: "999", + date: "2026-08-19", + knownSuites: ["a.test.ts", "z.test.ts", "brand-new.test.ts"], + }); + + assert.deepEqual(Object.keys(next.durations), ["a.test.ts", "z.test.ts"]); +}); + +test("readShardMeasurements folds every shard artifact together and ignores non-JSON files", () => { const files = { "shard-0.json": { "a.test.ts": 1 }, "shard-1.json": { "b.test.ts": 2 }, "README.md": null }; + + const measured = readShardMeasurements("/tmp/shards", { + readDir: () => Object.keys(files), + readFile: (filePath) => JSON.stringify(files[path.basename(filePath)]), + }); + + assert.deepEqual(measured, { "a.test.ts": 1, "b.test.ts": 2 }); +}); + +test("formatProvenanceComment points the reader at $notes for durable guidance", () => { + const comment = formatProvenanceComment({ runId: "1", date: "2026-08-19", measuredCount: 4, totalCount: 10 }); + + assert.ok(comment.includes("$notes"), "regenerated prose must point at the durable key"); +}); + +// Guards the split itself: if someone folds the units note back into +// $comment, the weekly refresh silently starts deleting it again. +test("the real manifest keeps its durable guidance in $notes, not in the regenerated $comment", () => { + const manifest = JSON.parse(readFileSync(durationsManifest, "utf8")); + + assert.ok(manifest["$notes"], "manifest must carry a $notes key"); + assert.ok(manifest["$notes"].includes("NOTE ON UNITS"), "the units note belongs in $notes"); + assert.ok( + !manifest["$comment"].includes("NOTE ON UNITS"), + "the units note must not live in $comment -- the weekly refresh regenerates that field", + ); +}); diff --git a/scripts/__tests__/refresh-shard-manifest-workflow.test.mjs b/scripts/__tests__/refresh-shard-manifest-workflow.test.mjs new file mode 100644 index 000000000000..de5c660f426d --- /dev/null +++ b/scripts/__tests__/refresh-shard-manifest-workflow.test.mjs @@ -0,0 +1,59 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import test from "node:test"; + +const workflow = readFileSync( + new URL("../../.github/workflows/refresh-shard-manifest.yml", import.meta.url), + "utf8", +); + +function stepBlock(name, nextName) { + const start = workflow.indexOf(`\n - name: ${name}\n`); + assert.notEqual(start, -1, `refresh workflow must define the ${name} step`); + const end = workflow.indexOf(`\n - name: ${nextName}\n`, start + 1); + assert.notEqual(end, -1, `refresh workflow must define ${nextName} after ${name}`); + return workflow.slice(start, end); +} + +test("refresh refuses to create a GITHUB_TOKEN-authored pull request", () => { + assert.match( + workflow, + /COMMITPERCLIP_ENABLED: \$\{\{ secrets\.COMMITPERCLIP_KEY != '' \}\}/, + ); + + const credentialGuard = stepBlock( + "Require commitperclip App credential", + "Download measured durations", + ); + assert.match(credentialGuard, /if: env\.COMMITPERCLIP_ENABLED != 'true'/); + assert.match(credentialGuard, /exit 1/); + + const upsert = stepBlock("Create or update pull request", "Alert if the refresh PR got no CI"); + assert.match(upsert, /GH_TOKEN: \$\{\{ steps\.bot-token\.outputs\.value \}\}/); + assert.doesNotMatch( + upsert, + /GH_TOKEN:\s*\$\{\{\s*github\.token\s*\}\}|\|\|\s*github\.token/, + ); + assert.match( + upsert, + /git push(?: --force(?:-with-lease)?)? "https:\/\/x-access-token:\$\{GH_TOKEN\}@github\.com\/\$\{\{ github\.repository \}\}\.git" "\$BRANCH"/, + ); + assert.match(upsert, /git config user\.email "shard-manifest-bot@paperclip\.blockcast\.net"/); + assert.doesNotMatch(upsert, /git config user\.email .*allyblockcast\[bot\]/); + assert.doesNotMatch(upsert, /git push(?: --force)? origin/); + assert.doesNotMatch(workflow, /\|\|\s*github\.token/); +}); + +test("refresh keeps partial-shard recovery and uses the workflow token for the alarm", () => { + assert.match(workflow, /needs: \[measure\][\s\S]*?if: \$\{\{ !cancelled\(\) \}\}/); + assert.match(workflow, /permissions:[\s\S]*?actions: read/); + assert.match(workflow, /permissions:[\s\S]*?pull-requests: write/); + + const alarmStart = workflow.indexOf("\n - name: Alert if the refresh PR got no CI\n"); + assert.notEqual(alarmStart, -1, "refresh workflow must retain the no-CI alarm"); + const alarm = workflow.slice(alarmStart); + assert.match(alarm, /GH_TOKEN: \$\{\{ github\.token \}\}/); + assert.doesNotMatch(alarm, /GH_TOKEN: \$\{\{ steps\.bot-token\.outputs\.value \}\}/); + assert.match(alarm, /action_required_count/); + assert.match(alarm, /total_run_count/); +}); diff --git a/scripts/__tests__/run-vitest-stable-shard.test.mjs b/scripts/__tests__/run-vitest-stable-shard.test.mjs index 176d569dc762..ed98c21c9935 100644 --- a/scripts/__tests__/run-vitest-stable-shard.test.mjs +++ b/scripts/__tests__/run-vitest-stable-shard.test.mjs @@ -9,6 +9,11 @@ import { loadShardDurations, partitionGeneralServerSuites, } from "../general-server-shard.mjs"; +import { + HARD_FAIL_COVERAGE_FLOOR, + evaluateManifestFreshness, + formatMissingSuitesDiagnostic, +} from "../check-shard-manifest-freshness.mjs"; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", ".."); const script = path.join(repoRoot, "scripts", "run-vitest-stable.mjs"); @@ -123,16 +128,27 @@ test("a missing or malformed manifest degrades to uniform weights", () => { assert.equal(Math.abs(shards[0].files.length - shards[1].files.length), 0); }); +// BLO-24241: this used to hard-fail below 90% coverage, which is a cliff — +// #1117 crossed it at 359/399=89.97% and failed `policy`, which skips build, +// typecheck, e2e and every test lane over a single missing JSON entry. The +// floor here is now HARD_FAIL_COVERAGE_FLOOR (well below 90%), so a suite or +// two of ordinary drift can no longer cascade. Whether coverage is complete +// is separately, precisely asserted (with the missing suites named) in +// check-shard-manifest-freshness.test.mjs, wired into pr.yml with +// continue-on-error so that assertion stays visible without blocking. test("the checked-in manifest loads and covers most of the current suite set", () => { const durations = loadShardDurations(durationsManifest); assert.ok(Object.keys(durations).length > 0, "manifest must parse to a non-empty duration map"); const shard = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", "1"]); const currentFiles = shard.selectedGeneralServerSuites; - const known = currentFiles.filter((file) => durations[file] !== undefined).length; + const { missing, coverage } = evaluateManifestFreshness({ files: currentFiles, durations }); + if (missing.length > 0) { + console.warn(formatMissingSuitesDiagnostic({ missing, coverage, totalSuites: currentFiles.length })); + } assert.ok( - known / currentFiles.length >= 0.9, - `manifest is stale: only ${known} of ${currentFiles.length} suites have recorded durations — regenerate it from a recent PR run (see the manifest's $comment)`, + coverage >= HARD_FAIL_COVERAGE_FLOOR, + `manifest coverage ${(coverage * 100).toFixed(1)}% has fallen below the abandonment floor of ${HARD_FAIL_COVERAGE_FLOOR * 100}% — regenerate it (see the manifest's $comment)`, ); }); diff --git a/scripts/check-shard-manifest-freshness.mjs b/scripts/check-shard-manifest-freshness.mjs new file mode 100644 index 000000000000..57c3d096baef --- /dev/null +++ b/scripts/check-shard-manifest-freshness.mjs @@ -0,0 +1,49 @@ +/** + * check-shard-manifest-freshness.mjs + * + * Detects drift between the general-server suite set on disk and + * scripts/general-server-shard-durations.json (BLO-24241). + * + * A suite absent from the manifest silently gets the median weight (see + * defaultSuiteWeight in general-server-shard.mjs) instead of a build + * failure -- the right default for a typical new suite, the wrong one for a + * heavyweight (a 138s+ suite ran packed as median-sized before this ticket). + * The old gate (scripts/__tests__/run-vitest-stable-shard.test.mjs asserting + * coverage >=90%) turned that silent drift into a hard cliff: one PR + * crossing 90% failed the `policy` job outright, which skips build, + * typecheck, e2e and every test lane over a single missing JSON entry + * (#1117 hit this at 359/399=89.97%). + * + * These two functions now back two separate, differently-tiered checks: + * - run-vitest-stable-shard.test.mjs keeps a REQUIRED assertion, but at a + * much more generous floor (HARD_FAIL_COVERAGE_FLOOR, well below the old + * 90%) so ordinary one-or-two-suite drift can no longer cascade. It + * always prints formatMissingSuitesDiagnostic() as a warning even when + * it doesn't fail. + * - scripts/check-shard-manifest-freshness.test.mjs asserts + * strict 100% coverage and is wired into .github/workflows/pr.yml with + * continue-on-error, so it stays visible (a real red X on its own step, + * naming every missing suite and the fix) without failing `policy`. + */ + +export const HARD_FAIL_COVERAGE_FLOOR = 0.75; + +export const ONE_LINE_FIX = + "Run `node scripts/measure-general-server-shard-durations.mjs --update` to backfill real durations for the suites below (or wait for the next .github/workflows/refresh-shard-manifest.yml run)."; + +export function evaluateManifestFreshness({ files, durations }) { + const missing = files.filter((file) => durations[file] === undefined).sort(); + const coverage = files.length === 0 ? 1 : (files.length - missing.length) / files.length; + return { missing, coverage, totalSuites: files.length }; +} + +export function formatMissingSuitesDiagnostic({ missing, coverage, totalSuites }) { + const pct = (coverage * 100).toFixed(1); + const lines = [ + `Shard duration manifest coverage is ${pct}% (${missing.length} of ${totalSuites} general-server suite(s) missing a recorded duration).`, + ...missing.map((file) => ` - ${file}`), + "", + ONE_LINE_FIX, + ]; + return lines.join("\n"); +} diff --git a/scripts/check-shard-manifest-freshness.test.mjs b/scripts/check-shard-manifest-freshness.test.mjs new file mode 100644 index 000000000000..1580b1af8c17 --- /dev/null +++ b/scripts/check-shard-manifest-freshness.test.mjs @@ -0,0 +1,75 @@ +import assert from "node:assert/strict"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import test from "node:test"; + +import { loadShardDurations } from "./general-server-shard.mjs"; +import { + HARD_FAIL_COVERAGE_FLOOR, + ONE_LINE_FIX, + evaluateManifestFreshness, + formatMissingSuitesDiagnostic, +} from "./check-shard-manifest-freshness.mjs"; +import { collectGeneralServerSuiteFiles } from "./run-vitest-stable-suites.mjs"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const durationsManifest = path.join(repoRoot, "scripts", "general-server-shard-durations.json"); + +test("evaluateManifestFreshness flags a suite present in the file list but absent from the manifest", () => { + const files = ["a.test.ts", "b.test.ts", "c.test.ts"]; + const durations = { "a.test.ts": 10, "c.test.ts": 30 }; + + const result = evaluateManifestFreshness({ files, durations }); + + assert.deepEqual(result.missing, ["b.test.ts"]); + assert.equal(result.totalSuites, 3); + assert.ok(Math.abs(result.coverage - 2 / 3) < 1e-9); +}); + +test("evaluateManifestFreshness reports full coverage when nothing is missing", () => { + const files = ["a.test.ts", "b.test.ts"]; + const durations = { "a.test.ts": 10, "b.test.ts": 20, "unrelated.test.ts": 5 }; + + const result = evaluateManifestFreshness({ files, durations }); + + assert.deepEqual(result.missing, []); + assert.equal(result.coverage, 1); +}); + +test("formatMissingSuitesDiagnostic names every missing suite and the one-line fix", () => { + const diagnostic = formatMissingSuitesDiagnostic({ + missing: ["server/src/__tests__/heavy-suite.test.ts"], + coverage: 0.99, + totalSuites: 100, + }); + + assert.ok(diagnostic.includes("server/src/__tests__/heavy-suite.test.ts")); + assert.ok(diagnostic.includes("99.0%")); + assert.ok(diagnostic.includes(ONE_LINE_FIX)); +}); + +// The direct verifying signal for BLO-24241: no suite present on disk may be +// absent from the manifest. Before the manifest fix landed in the same PR +// this failed against the real tree (3 suites were missing); it now proves +// the fix stuck. Unlike the old >=90% assertion in +// run-vitest-stable-shard.test.mjs, this step is wired into +// .github/workflows/pr.yml with continue-on-error, so a future regression +// here shows as a visible red X on its own step instead of failing the +// `policy` job and skipping build/typecheck/e2e for the whole PR. +test("no general-server suite present on disk is absent from the manifest", () => { + const files = collectGeneralServerSuiteFiles(repoRoot); + const durations = loadShardDurations(durationsManifest); + + const result = evaluateManifestFreshness({ files, durations }); + + assert.deepEqual( + result.missing, + [], + `manifest is missing entries for suites present on disk:\n${formatMissingSuitesDiagnostic(result)}`, + ); +}); + +test("the hard-fail floor sits well below the old 90% cliff so a suite or two of drift cannot cascade", () => { + assert.ok(HARD_FAIL_COVERAGE_FLOOR < 0.9); + assert.ok(HARD_FAIL_COVERAGE_FLOOR > 0, "floor must still catch genuine abandonment"); +}); diff --git a/scripts/general-server-shard-durations.json b/scripts/general-server-shard-durations.json index 2bad760ebc9b..632f84f5f7b3 100644 --- a/scripts/general-server-shard-durations.json +++ b/scripts/general-server-shard-durations.json @@ -1,5 +1,6 @@ { - "$comment": "Per-suite Vitest elapsed durations (ms) for the general-server lane, used by scripts/general-server-shard.mjs to balance suites across the PR shard matrix. Sampled from the four successful ARC PR jobs in run 31248977534 on 2026-08-08. Suites absent from that run are intentionally omitted and use the median fallback. EXCEPTION (BLO-21092): server/src/__tests__/plugin-status-metrics.test.ts was added after that sampling run and is NOT ARC-sampled -- it is a local measurement (median 52ms of 3 runs) scaled by 0.64, the ARC/local ratio measured on metrics-service.test.ts (269ms manifest / 418ms local) as a co-located control. Both values sit far below the 123ms median, so shard balance is insensitive to the estimate; the entry exists because the >=90% coverage assertion in scripts/__tests__/run-vitest-stable-shard.test.mjs had zero headroom (397/441 = 0.9002), so ANY new suite failed CI. That headroom problem is master drift, not this suite's: 44 other suites were already missing. Regenerate wholesale.", + "$comment": "Per-suite Vitest durations (ms) for the general-server lane, used by scripts/general-server-shard.mjs to balance suites across the PR shard matrix. Sampled from four successful ARC PR jobs in run 31248977534 on 2026-08-08, then topped up as master added suites, most recently on 2026-08-24 via scripts/measure-general-server-shard-durations.mjs; coverage is 443/443. This sentence is regenerated on every refresh; see \"$notes\" for guidance that is meant to persist.", + "$notes": "DURABLE NOTES -- unlike \"$comment\" (regenerated on every refresh), this key is never rewritten by .github/workflows/refresh-shard-manifest.yml. Put guidance that must outlive a re-sample here. NOTE ON UNITS: these are per-file TEST-EXECUTION durations (the Vitest JSON reporter's testResults[].startTime->endTime), not full wall-clock -- they exclude each file's transform/setup/import cost, which is why a trivial suite can read as single-digit ms. That fixed per-file cost is roughly uniform and the LPT partition also lands near-equal file counts per shard, so it does not skew balance; do not 'fix' a small value here by hand. Suites absent from this manifest use the median fallback (defaultSuiteWeight in scripts/general-server-shard.mjs); the refresh workflow re-samples weekly so refresh does not depend on a human noticing drift.", "unit": "ms", "durations": { "server/src/__tests__/ac-policy-assignee-routing.test.ts": 19, @@ -18,6 +19,7 @@ "server/src/__tests__/agent-auth-middleware.test.ts": 96, "server/src/__tests__/agent-hires-instructions-materialize.test.ts": 7694, "server/src/__tests__/agent-image-bump.test.ts": 57393, + "server/src/__tests__/agent-inbox-lite-status-contract.test.ts": 11, "server/src/__tests__/agent-instructions-service.test.ts": 595, "server/src/__tests__/agent-invokability.test.ts": 14, "server/src/__tests__/agent-permissions-service.test.ts": 18, @@ -39,7 +41,10 @@ "server/src/__tests__/app-private-hostname-gate.test.ts": 8, "server/src/__tests__/app-vite-dev-routing.test.ts": 10, "server/src/__tests__/append-public-url.test.ts": 75, + "server/src/__tests__/approval-gate-reconciler.test.ts": 3220, + "server/src/__tests__/approval-insert.test.ts": 10, "server/src/__tests__/approval-linked-agent-migration.test.ts": 15122, + "server/src/__tests__/approval-payload-title-guard.test.ts": 4684, "server/src/__tests__/approval-withdraw-plugin-event.test.ts": 9363, "server/src/__tests__/approvals-service.test.ts": 4562, "server/src/__tests__/attachment-types.test.ts": 12, @@ -55,9 +60,11 @@ "server/src/__tests__/board-mutation-guard.test.ts": 120, "server/src/__tests__/body-limits.test.ts": 4, "server/src/__tests__/body-parsers.test.ts": 93, + "server/src/__tests__/branch-run-claims.test.ts": 17110, "server/src/__tests__/budgets-service.test.ts": 4803, "server/src/__tests__/build-commit.test.ts": 6, "server/src/__tests__/built-in-agents.test.ts": 29548, + "server/src/__tests__/ccrotate-capacity-retry.test.ts": 29, "server/src/__tests__/ccrotate-plugin-retirement.test.ts": 14, "server/src/__tests__/ccrotate-target.test.ts": 9, "server/src/__tests__/change-consent-gate.test.ts": 5402, @@ -86,6 +93,7 @@ "server/src/__tests__/company-skills-service.test.ts": 48058, "server/src/__tests__/company-skills.test.ts": 106, "server/src/__tests__/config-pr-reviewer-pool.test.ts": 40, + "server/src/__tests__/config-recovery-action-bounds.test.ts": 21, "server/src/__tests__/cursor-local-adapter-environment.test.ts": 530, "server/src/__tests__/cursor-local-adapter.test.ts": 29, "server/src/__tests__/cursor-local-execute.test.ts": 2579, @@ -127,6 +135,7 @@ "server/src/__tests__/evidence-gate-wiring.test.ts": 65, "server/src/__tests__/evidence-gate.test.ts": 79, "server/src/__tests__/execution-lock-orphan-cleanup.test.ts": 5464, + "server/src/__tests__/execution-workspace-per-run-isolation.test.ts": 1980, "server/src/__tests__/execution-workspace-policy.test.ts": 20, "server/src/__tests__/execution-workspaces-derive-agent-cwd.test.ts": 4, "server/src/__tests__/execution-workspaces-service.test.ts": 49219, @@ -155,6 +164,7 @@ "server/src/__tests__/heartbeat-active-run-output-watchdog.test.ts": 80985, "server/src/__tests__/heartbeat-adapter-resolution-guard.test.ts": 5157, "server/src/__tests__/heartbeat-agent-error-reason.test.ts": 8409, + "server/src/__tests__/heartbeat-agent-liveness-gauge.test.ts": 2738, "server/src/__tests__/heartbeat-api-tier-dispatch-fence.test.ts": 4606, "server/src/__tests__/heartbeat-archived-company-guard.test.ts": 7802, "server/src/__tests__/heartbeat-auto-checkout.test.ts": 7, @@ -184,12 +194,14 @@ "server/src/__tests__/heartbeat-pr-review-gate-replay.test.ts": 6650, "server/src/__tests__/heartbeat-pr-review-queue-fairness.test.ts": 11150, "server/src/__tests__/heartbeat-pr-review-request-coalescing.test.ts": 7783, + "server/src/__tests__/heartbeat-pr-review-task-key-casing.test.ts": 1328, "server/src/__tests__/heartbeat-preferred-workspace-fail-loud.test.ts": 9673, "server/src/__tests__/heartbeat-project-env.test.ts": 109, "server/src/__tests__/heartbeat-provider-capacity-horizon.test.ts": 10247, "server/src/__tests__/heartbeat-queued-backlog-convergence.test.ts": 184313, "server/src/__tests__/heartbeat-rate-limit-exhausted.test.ts": 8401, "server/src/__tests__/heartbeat-rate-limit-retry-schedule.test.ts": 13, + "server/src/__tests__/heartbeat-recoverable-error-family.test.ts": 8, "server/src/__tests__/heartbeat-responsible-user-invariant.test.ts": 24618, "server/src/__tests__/heartbeat-retry-scheduling.test.ts": 47427, "server/src/__tests__/heartbeat-run-log.test.ts": 12, @@ -203,6 +215,8 @@ "server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts": 32461, "server/src/__tests__/heartbeat-stale-run-dispatch-deadlock.test.ts": 10502, "server/src/__tests__/heartbeat-start-lock.test.ts": 63, + "server/src/__tests__/heartbeat-timeout-outcome-persistence.test.ts": 2660, + "server/src/__tests__/heartbeat-timeout-outcome.test.ts": 6, "server/src/__tests__/heartbeat-timer-wake-coalescing.test.ts": 4980, "server/src/__tests__/heartbeat-timer-wake-session-reset-pf4.test.ts": 18, "server/src/__tests__/heartbeat-wake-dispatch-retry.test.ts": 29126, @@ -214,8 +228,10 @@ "server/src/__tests__/heartbeat-zombie-guard.test.ts": 7, "server/src/__tests__/helm-runtime-config.test.ts": 7, "server/src/__tests__/helpers/cleanup-heartbeat-test-state.test.ts": 7738, + "server/src/__tests__/helpers/truncate-company-scoped-test-state.test.ts": 2250, "server/src/__tests__/hire-hook.test.ts": 21, "server/src/__tests__/http-log-policy.test.ts": 11, + "server/src/__tests__/human-gated-ageing.test.ts": 85, "server/src/__tests__/in-review-gate.test.ts": 10, "server/src/__tests__/inbox-dismissals.test.ts": 8466, "server/src/__tests__/instance-settings-service.test.ts": 45, @@ -225,11 +241,14 @@ "server/src/__tests__/invite-token-entropy.test.ts": 24, "server/src/__tests__/issue-approvals-service.test.ts": 17, "server/src/__tests__/issue-blocker-attention.test.ts": 16723, + "server/src/__tests__/issue-checkout-routine-lock-conflict.test.ts": 2384, "server/src/__tests__/issue-comment-redaction.test.ts": 8819, "server/src/__tests__/issue-continuation-summary.test.ts": 14, + "server/src/__tests__/issue-coordination-metadata-refusal-persistence.test.ts": 4616, "server/src/__tests__/issue-denied-write-recovery-persistence.test.ts": 6499, "server/src/__tests__/issue-duplicate-backfill.test.ts": 13, "server/src/__tests__/issue-efficiency.test.ts": 9903, + "server/src/__tests__/issue-execution-lock.test.ts": 12, "server/src/__tests__/issue-execution-policy.test.ts": 50, "server/src/__tests__/issue-force-release.test.ts": 182, "server/src/__tests__/issue-goal-fallback.test.ts": 10, @@ -238,9 +257,12 @@ "server/src/__tests__/issue-monitor-convergence-message.test.ts": 9, "server/src/__tests__/issue-monitor-scheduler.test.ts": 17626, "server/src/__tests__/issue-pull-requests-identity-guard.test.ts": 9, + "server/src/__tests__/issue-pull-requests-ownership-selection.test.ts": 9, "server/src/__tests__/issue-recovery-actions.test.ts": 18445, "server/src/__tests__/issue-references-service.test.ts": 5364, + "server/src/__tests__/issue-repo-binding-guard.test.ts": 956, "server/src/__tests__/issue-rewake-throttle.test.ts": 20, + "server/src/__tests__/issue-routine-lock-conflict-matcher.test.ts": 15, "server/src/__tests__/issue-run-holding.test.ts": 13, "server/src/__tests__/issue-thread-interactions-service.test.ts": 10249, "server/src/__tests__/issue-thread-interactions-telemetry.test.ts": 5130, @@ -251,12 +273,14 @@ "server/src/__tests__/issues-identifier-provider.test.ts": 5175, "server/src/__tests__/issues-last-activity-at.test.ts": 20223, "server/src/__tests__/issues-list-query-parsing.test.ts": 62, + "server/src/__tests__/issues-open-assignment-census.test.ts": 39821, "server/src/__tests__/issues-patch-evidence.test.ts": 7008, "server/src/__tests__/issues-user-context.test.ts": 11, "server/src/__tests__/join-request-dedupe.test.ts": 10, "server/src/__tests__/json-schema-secret-refs.test.ts": 6, "server/src/__tests__/k8s-job-liveness-run-scoped.test.ts": 630, "server/src/__tests__/k8s-job-liveness.test.ts": 37, + "server/src/__tests__/lifecycle-hook-command-audit.test.ts": 535, "server/src/__tests__/linear-webhook-fixture-replay.test.ts": 4757, "server/src/__tests__/linear-webhook-fixtures.test.ts": 10, "server/src/__tests__/linear-webhook.test.ts": 138, @@ -273,6 +297,7 @@ "server/src/__tests__/opencode-k8s-seed-transport.test.ts": 17886, "server/src/__tests__/opencode-local-adapter.test.ts": 20, "server/src/__tests__/opencode-local-skill-sync.test.ts": 55, + "server/src/__tests__/overdue-scheduled-retry-metrics.test.ts": 2730, "server/src/__tests__/paperclip-env.test.ts": 9, "server/src/__tests__/paperclip-node-role.test.ts": 126, "server/src/__tests__/paperclip-skill-utils.test.ts": 66, @@ -283,14 +308,21 @@ "server/src/__tests__/pi-local-skill-sync.test.ts": 61, "server/src/__tests__/pipelines-service.test.ts": 10822, "server/src/__tests__/plugin-access-authorization-host-services.test.ts": 10370, + "server/src/__tests__/plugin-activation-retry-provenance.test.ts": 23, "server/src/__tests__/plugin-activation-retry.test.ts": 71, "server/src/__tests__/plugin-agent-invoke-wake-fanout.test.ts": 12026, + "server/src/__tests__/plugin-config-masking.test.ts": 38, + "server/src/__tests__/plugin-config-validator.test.ts": 88, + "server/src/__tests__/plugin-config-write-race.test.ts": 4521, "server/src/__tests__/plugin-database.test.ts": 11018, "server/src/__tests__/plugin-dev-watcher.test.ts": 84, "server/src/__tests__/plugin-environment-driver-seam.test.ts": 46, "server/src/__tests__/plugin-event-outbox.test.ts": 7681, "server/src/__tests__/plugin-execution-workspace-bridge.test.ts": 17, "server/src/__tests__/plugin-install-autobuild.test.ts": 13114, + "server/src/__tests__/plugin-isolated-store-migration.test.ts": 235, + "server/src/__tests__/plugin-job-scheduler.test.ts": 32, + "server/src/__tests__/plugin-job-store-failure-streak.test.ts": 1379, "server/src/__tests__/plugin-lifecycle-restart.test.ts": 16, "server/src/__tests__/plugin-local-folders.test.ts": 13662, "server/src/__tests__/plugin-managed-routines.test.ts": 5321, @@ -308,8 +340,11 @@ "server/src/__tests__/plugin-ui-static.test.ts": 2735, "server/src/__tests__/plugin-webhook-verification.test.ts": 2775, "server/src/__tests__/plugin-worker-manager.test.ts": 1094, + "server/src/__tests__/pr-comment-review-gate-check.test.ts": 271, + "server/src/__tests__/pr-comment-review-gate.test.ts": 8, "server/src/__tests__/pr-reconciler-sweep.test.ts": 18, "server/src/__tests__/pr-review-gate-status-target.test.ts": 17, + "server/src/__tests__/pr-review-request-ageing.test.ts": 34, "server/src/__tests__/private-hostname-guard.test.ts": 81, "server/src/__tests__/process-crash-guard-exit.test.ts": 2859, "server/src/__tests__/process-crash-guard.test.ts": 409, @@ -317,12 +352,16 @@ "server/src/__tests__/productivity-review-service.test.ts": 58751, "server/src/__tests__/project-icon-persistence.test.ts": 6613, "server/src/__tests__/project-list-metrics.test.ts": 8, + "server/src/__tests__/project-primary-workspace-provenance.test.ts": 4371, "server/src/__tests__/project-shortname-resolution.test.ts": 7, + "server/src/__tests__/pull-request-work-products.test.ts": 12, "server/src/__tests__/qa-routine-secrets-e2e.test.ts": 5503, + "server/src/__tests__/queued-run-age-metrics.test.ts": 4199, "server/src/__tests__/quota-exhausted-hook.test.ts": 67, "server/src/__tests__/quota-windows-service.test.ts": 15, "server/src/__tests__/quota-windows.test.ts": 121, "server/src/__tests__/recovery-classifiers.test.ts": 25, + "server/src/__tests__/recovery-expired-wake-horizon.test.ts": 1035, "server/src/__tests__/recovery-observability.test.ts": 5273, "server/src/__tests__/recovery-stale-issue-lock-sweep.test.ts": 8734, "server/src/__tests__/redact-sensitive.test.ts": 11, @@ -334,6 +373,7 @@ "server/src/__tests__/runtime-api.test.ts": 11, "server/src/__tests__/sandbox-provider-runtime.test.ts": 11, "server/src/__tests__/secret-provider-registry.test.ts": 11, + "server/src/__tests__/secret-value-digest.test.ts": 3, "server/src/__tests__/secrets-service.test.ts": 8171, "server/src/__tests__/security-audit-overrides.test.ts": 10, "server/src/__tests__/server-info.test.ts": 2357, @@ -344,8 +384,12 @@ "server/src/__tests__/smoke-lab.test.ts": 7846, "server/src/__tests__/source-trust.test.ts": 10584, "server/src/__tests__/sse-backpressure.test.ts": 11, + "server/src/__tests__/startup-filesystem-io.test.ts": 154, "server/src/__tests__/static-index-html.test.ts": 92, "server/src/__tests__/storage-local-provider.test.ts": 74, + "server/src/__tests__/stranded-blocked-issue-reconciler.test.ts": 3747, + "server/src/__tests__/stranded-run-recovery.test.ts": 8, + "server/src/__tests__/successful-run-handoff-liveness.test.ts": 817, "server/src/__tests__/summary-slots.test.ts": 7447, "server/src/__tests__/tar-security-override.test.ts": 7, "server/src/__tests__/task-watchdogs-classifier.test.ts": 22, @@ -391,6 +435,7 @@ "server/src/services/issue-thread-interactions.test.ts": 2107, "server/src/services/recovery/model-profile-hint.test.ts": 11, "server/src/services/recovery/provider-failure-classification.test.ts": 17, + "server/src/services/recovery/service.infra-class-continuation.test.ts": 7, "server/src/services/recovery/service.pause-durability.test.ts": 6, "server/src/services/recovery/strand-comment-provider-capacity.test.ts": 47, "server/src/services/recovery/successful-run-handoff.test.ts": 16, diff --git a/scripts/measure-general-server-shard-durations.mjs b/scripts/measure-general-server-shard-durations.mjs new file mode 100644 index 000000000000..896eedaed853 --- /dev/null +++ b/scripts/measure-general-server-shard-durations.mjs @@ -0,0 +1,218 @@ +#!/usr/bin/env node +/** + * measure-general-server-shard-durations.mjs + * + * Measures real per-suite Vitest test-execution durations for the + * general-server lane and folds them into + * scripts/general-server-shard-durations.json. + * + * These are the JSON reporter's per-file testResults[].startTime->endTime + * spans, NOT full wall-clock: they exclude each file's transform/setup/import + * cost, which is why a trivial suite can read as single-digit ms. See the + * manifest's own "$notes" key for why that does not skew the partition. + * + * This is the "one-line fix" scripts/check-shard-manifest-freshness.mjs + * points at, and the executor .github/workflows/refresh-shard-manifest.yml + * runs on a schedule so refreshing the manifest does not depend on a human + * noticing drift (BLO-24241). + * + * Usage: + * node scripts/measure-general-server-shard-durations.mjs [--update] [--all] + * [--shard-index N --shard-count M] [--output ] + * + * --update Write the measured durations into the manifest on disk + * (default: print the measured-only JSON to stdout so a + * caller — e.g. a CI job merging several shards — can + * combine results before writing). + * --all Re-measure every general-server suite instead of only + * the ones currently missing from the manifest. + * --shard-index/--shard-count + * Measure only this slice of the target file list, so a + * full-manifest refresh can be split across parallel CI + * jobs the same way the real test lane is. + * --output Also write the measured-only durations to this path as + * JSON (used to hand results between CI jobs). + */ + +import { spawnSync } from "node:child_process"; +import { readFileSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import process from "node:process"; +import { fileURLToPath } from "node:url"; + +import { loadShardDurations } from "./general-server-shard.mjs"; +import { collectGeneralServerSuiteFiles } from "./run-vitest-stable-suites.mjs"; + +const MEASURE_VITEST_ARGS = ["--no-file-parallelism", "--maxWorkers=1"]; + +export function selectTargetFiles({ allFiles, durations, all, shardIndex, shardCount }) { + const base = all ? allFiles : allFiles.filter((file) => durations[file] === undefined); + const target = [...base].sort((a, b) => a.localeCompare(b)); + if (shardIndex === null || shardCount === null) { + return target; + } + return target.filter((_, index) => index % shardCount === shardIndex); +} + +// Parses a Vitest --reporter=json report into { repoPath: ms }. Vitest test +// file `name` fields are absolute paths, so they are rebased onto repoRoot to +// match the manifest's repo-relative keys. +// +// An entry only counts as a measurement if the file actually ran tests. +// Vitest derives a test FILE's status by folding its tests' results, so a file +// that threw during collection -- import error, bad top-level await -- has no +// tests to fold: it comes back with an empty `assertionResults` and an +// endTime - startTime of roughly 0, and (because nothing failed) often a +// `passed` status. measureFiles deliberately tolerates a non-zero vitest exit +// so that ordinary test failures still yield real timings, which means without +// this guard that 0 is indistinguishable from a genuinely fast suite and is +// written to the manifest as one -- handing what may be the heaviest suite in +// the lane a near-zero partition weight until the next weekly --all pass. +// +// Skipping is the safe direction rather than recording a 0: mergeManifest +// keeps the suite's previous duration, and a suite that never had one stays +// absent and keeps the median default, which check-shard-manifest-freshness +// then reports by name. Note the conditions are written to skip only on +// positive evidence of a non-run -- an absent `assertionResults` or `status` +// (reporter shape drift) keeps the entry, because a guard that could silently +// empty the manifest would be worse than the bug it closes. +// +// Known cost of that direction: a suite that legitimately registers no tests +// -- everything behind a `describe.skip`, or a runtime condition never met in +// CI -- is indistinguishable from a collection error here and is skipped the +// same way. It therefore never acquires a manifest entry and stays on +// check-shard-manifest-freshness's missing list permanently, which no refresh +// run can clear. That is intentional (a zero-test suite costs ~0s, so the +// median default it falls back to is harmless), but it means a name that +// never leaves the missing list is a signal to go look at the suite, not a +// sign the refresh workflow is broken. +export function parseVitestJsonReport(reportText, repoRoot) { + const report = JSON.parse(reportText); + const measured = {}; + for (const testFile of report.testResults ?? []) { + const repoPath = path.relative(repoRoot, testFile.name).split(path.sep).join("/"); + const durationMs = Math.round(testFile.endTime - testFile.startTime); + + const collectedNothing = + Array.isArray(testFile.assertionResults) && testFile.assertionResults.length === 0; + const unknownStatus = + typeof testFile.status === "string" && !["passed", "failed"].includes(testFile.status); + if (collectedNothing || unknownStatus) { + continue; + } + + if (Number.isFinite(durationMs) && durationMs >= 0) { + measured[repoPath] = durationMs; + } + } + return measured; +} + +export function mergeDurations(existing, measured) { + const merged = { ...existing, ...measured }; + const sorted = {}; + for (const key of Object.keys(merged).sort()) { + sorted[key] = merged[key]; + } + return sorted; +} + +function measureFiles(repoRoot, files) { + if (files.length === 0) { + return {}; + } + + const tempDir = mkdtempSync(path.join(os.tmpdir(), "shard-measure-")); + const reportPath = path.join(tempDir, "report.json"); + try { + // Mirror run-vitest-stable.mjs's per-invocation isolation: a dedicated + // PAPERCLIP_HOME/TMPDIR keeps this measurement run from colliding with + // any other Paperclip process (dev server, another test run) using the + // caller's ambient environment, and keeps fixture socket paths short. + const env = { + ...process.env, + NODE_ENV: "test", + PAPERCLIP_HOME: path.join(tempDir, "h"), + PAPERCLIP_INSTANCE_ID: `measure-shard-${process.pid}`, + TMPDIR: path.join(tempDir, "t"), + }; + mkdirSync(env.PAPERCLIP_HOME, { recursive: true }); + mkdirSync(env.TMPDIR, { recursive: true }); + + // stdout is inherited, not piped, on purpose. The timing data comes from + // --outputFile, so nothing here ever reads the child's stdout -- and + // piping it would cap it at Node's default 1 MiB maxBuffer. A real + // refresh shard runs ~108 server suites for 25-37 min; once their + // combined stdout crossed that cap, spawnSync would kill vitest and set + // `error` to ENOBUFS, failing the shard and (via `needs: [measure]`) + // silently skipping the whole manifest refresh -- exactly the + // depends-on-a-human-noticing failure this workflow exists to remove. + // Inheriting also puts suite progress in the job log. + const result = spawnSync( + "pnpm", + ["exec", "vitest", "run", "--project", "@paperclipai/server", ...files, ...MEASURE_VITEST_ARGS, "--reporter=json", `--outputFile=${reportPath}`], + { cwd: repoRoot, env, stdio: ["ignore", "inherit", "inherit"] }, + ); + if (result.error) { + throw result.error; + } + // Vitest exits non-zero on test failures, but the JSON report (and the + // durations in it) is still written and still real. Only a missing + // report means the run never actually produced timing data. + const reportText = readFileSync(reportPath, "utf8"); + return parseVitestJsonReport(reportText, repoRoot); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } +} + +function parseCliOptions(argv) { + const options = { update: false, all: false, shardIndex: null, shardCount: null, output: null }; + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + if (arg === "--update") { + options.update = true; + } else if (arg === "--all") { + options.all = true; + } else if (arg === "--shard-index") { + options.shardIndex = Number(argv[(index += 1)]); + } else if (arg === "--shard-count") { + options.shardCount = Number(argv[(index += 1)]); + } else if (arg === "--output") { + options.output = argv[(index += 1)]; + } + } + return options; +} + +function isMainModule() { + return process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url); +} + +if (isMainModule()) { + const repoRoot = process.cwd(); + const manifestPath = path.join(repoRoot, "scripts", "general-server-shard-durations.json"); + const options = parseCliOptions(process.argv.slice(2)); + + const allFiles = collectGeneralServerSuiteFiles(repoRoot); + const durations = loadShardDurations(manifestPath); + const targetFiles = selectTargetFiles({ allFiles, durations, ...options }); + + console.log(`[measure-shard-durations] measuring ${targetFiles.length} suite(s)...`); + const measured = measureFiles(repoRoot, targetFiles); + + if (options.output) { + writeFileSync(options.output, JSON.stringify(measured, null, 2) + "\n"); + } + + if (options.update) { + const manifest = JSON.parse(readFileSync(manifestPath, "utf8")); + manifest.durations = mergeDurations(manifest.durations, measured); + writeFileSync(manifestPath, JSON.stringify(manifest, null, 2) + "\n"); + console.log(`[measure-shard-durations] updated ${Object.keys(measured).length} entries in ${manifestPath}`); + } else { + console.log(JSON.stringify(measured, null, 2)); + } +} diff --git a/scripts/merge-shard-duration-manifest.mjs b/scripts/merge-shard-duration-manifest.mjs new file mode 100644 index 000000000000..c86a55d46159 --- /dev/null +++ b/scripts/merge-shard-duration-manifest.mjs @@ -0,0 +1,181 @@ +#!/usr/bin/env node +/** + * merge-shard-duration-manifest.mjs + * + * Merges the per-shard measurement artifacts produced by the `measure` matrix + * in .github/workflows/refresh-shard-manifest.yml into + * scripts/general-server-shard-durations.json. + * + * Rewrites exactly two things: `durations` (merged, fresh measurements win, + * keys sorted) and `$comment` (the sampling provenance sentence). Every other + * manifest key is passed through untouched -- notably `$notes`, which holds + * the durable guidance about what these numbers are and why a trivial suite + * reads as single-digit ms. + * + * This lives in a script rather than inline in the workflow so it can be unit + * tested. The first version was a YAML heredoc that unconditionally + * overwrote `$comment` and so destroyed the units note on its first run -- + * and because that run is a Monday-morning schedule, nobody would have been + * watching when it happened (BLO-24241). + * + * Usage: + * node scripts/merge-shard-duration-manifest.mjs --shard-dir + * [--manifest ] [--run-id ] [--date YYYY-MM-DD] + */ + +import { readFileSync, readdirSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import process from "node:process"; +import { fileURLToPath } from "node:url"; + +import { collectGeneralServerSuiteFiles } from "./run-vitest-stable-suites.mjs"; + +// Manifest keys this merge is allowed to rewrite. Anything else -- `$notes`, +// `unit`, and whatever a future reader adds -- survives a refresh verbatim. +export const REWRITTEN_KEYS = ["$comment", "durations"]; + +// Reads every per-shard artifact in shardDir and folds them into one map. +// Shard outputs are disjoint by construction (selectTargetFiles slices by +// index % count), so collision order does not matter. +export function readShardMeasurements(shardDir, { readDir = readdirSync, readFile = readFileSync } = {}) { + let measured = {}; + for (const file of readDir(shardDir).sort()) { + if (!file.endsWith(".json")) { + continue; + } + const shard = JSON.parse(readFile(path.join(shardDir, file), "utf8")); + measured = { ...measured, ...shard }; + } + return measured; +} + +export function formatProvenanceComment({ runId, date, measuredCount, totalCount }) { + return ( + `Per-suite Vitest durations (ms) for the general-server lane, used by ` + + `scripts/general-server-shard.mjs to balance suites across the PR shard matrix. ` + + `Re-sampled from the four .github/workflows/refresh-shard-manifest.yml shard jobs ` + + `in run ${runId} on ${date}: ${measuredCount} suite(s) measured, ${totalCount} total ` + + `entries. This sentence is regenerated on every refresh; see "$notes" for guidance ` + + `that is meant to persist.` + ); +} + +// Returns a new manifest object. Key order follows the input manifest so a +// refresh produces a minimal diff. +// +// `knownSuites` is the suite set that exists ON DISK, and entries outside it +// are pruned. Without that the merge is purely additive: a deleted suite keeps +// its entry forever, and the totalCount in the provenance sentence slowly +// overstates coverage (the freshness check only looks for *missing* suites, so +// nothing else notices). +// +// It is deliberately the on-disk set and NOT the set measured by this run. +// The measure matrix is `fail-fast: false` across four shards, so keying the +// prune on what came back would let one failed shard silently delete a quarter +// of the manifest -- trading a slow overstatement for a fast, unattended data +// loss on a Monday-morning schedule nobody is watching. An empty or absent +// knownSuites prunes nothing, for the same reason. +export function mergeManifest({ manifest, measured, runId, date, knownSuites = null }) { + const merged = { ...manifest.durations, ...measured }; + + const known = knownSuites instanceof Set ? knownSuites : knownSuites ? new Set(knownSuites) : null; + const keep = known && known.size > 0 ? (key) => known.has(key) : () => true; + + const durations = {}; + for (const key of Object.keys(merged).sort()) { + if (keep(key)) { + durations[key] = merged[key]; + } + } + + const next = { ...manifest }; + next.durations = durations; + next["$comment"] = formatProvenanceComment({ + runId, + date, + measuredCount: Object.keys(measured).length, + totalCount: Object.keys(durations).length, + }); + return next; +} + +function parseCliOptions(argv) { + const options = { shardDir: null, manifest: null, runId: null, date: null }; + for (let index = 0; index < argv.length; index += 1) { + const arg = argv[index]; + if (arg === "--shard-dir") { + options.shardDir = argv[(index += 1)]; + } else if (arg === "--manifest") { + options.manifest = argv[(index += 1)]; + } else if (arg === "--run-id") { + options.runId = argv[(index += 1)]; + } else if (arg === "--date") { + options.date = argv[(index += 1)]; + } + } + return options; +} + +function isMainModule() { + return process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url); +} + +if (isMainModule()) { + const options = parseCliOptions(process.argv.slice(2)); + if (!options.shardDir) { + console.error("[merge-shard-manifest] --shard-dir is required"); + process.exit(2); + } + + const manifestPath = + options.manifest ?? path.join(process.cwd(), "scripts", "general-server-shard-durations.json"); + const manifest = JSON.parse(readFileSync(manifestPath, "utf8")); + const measured = readShardMeasurements(options.shardDir); + + if (Object.keys(measured).length === 0) { + // Every shard job produced an empty artifact. Writing the manifest here + // would replace real durations with nothing but a fresh provenance line, + // so fail loudly instead -- an empty refresh is a broken measure matrix. + console.error("[merge-shard-manifest] no measurements found in " + options.shardDir); + process.exit(1); + } + + // The suite set on disk, used to prune entries for deleted suites. An empty + // result means the checkout is broken or the cwd is wrong, not that every + // suite was deleted -- prune nothing rather than empty the manifest. + const knownSuites = collectGeneralServerSuiteFiles(process.cwd()); + if (knownSuites.length === 0) { + console.warn( + "[merge-shard-manifest] collected 0 general-server suites from " + + `${process.cwd()}; skipping the prune of deleted suites`, + ); + } + + const next = mergeManifest({ + manifest, + measured, + runId: options.runId ?? process.env.GITHUB_RUN_ID ?? "unknown", + date: options.date ?? new Date().toISOString().slice(0, 10), + knownSuites, + }); + + const pruned = Object.keys(manifest.durations ?? {}).filter( + (key) => next.durations[key] === undefined, + ); + + writeFileSync(manifestPath, JSON.stringify(next, null, 2) + "\n"); + console.log( + `[merge-shard-manifest] merged ${Object.keys(measured).length} measured suite(s) into ` + + `${Object.keys(next.durations).length} total manifest entries.`, + ); + if (pruned.length > 0) { + // Named, not just counted: this lands in a PR a human skims, and "why did + // 3 entries vanish" should be answerable from the job log alone. + console.log( + `[merge-shard-manifest] pruned ${pruned.length} entr(ies) for suites no longer on disk:`, + ); + for (const key of pruned) { + console.log(` - ${key}`); + } + } +} diff --git a/scripts/run-vitest-stable-suites.mjs b/scripts/run-vitest-stable-suites.mjs new file mode 100644 index 000000000000..d7923e8bac78 --- /dev/null +++ b/scripts/run-vitest-stable-suites.mjs @@ -0,0 +1,80 @@ +// Suite enumeration shared between scripts/run-vitest-stable.mjs (the actual +// test runner) and scripts/check-shard-manifest-freshness.mjs (the manifest +// drift diagnostic, BLO-24241). Previously each file walked the tree and +// applied the route/authz exclusion independently; a diagnostic computed +// from a second, drifted copy of this logic would tell you nothing about +// the manifest that actually feeds the real runner. + +import { readdirSync, statSync } from "node:fs"; +import path from "node:path"; + +const routeTestPattern = /[^/]*(?:route|routes|authz)[^/]*\.test\.ts$/; + +// Server suites that are not route/authz-named but must still run in the +// serialized lane (shared DB state, ordering sensitivity, etc.) rather than +// the parallel general-server shards. +export const additionalSerializedServerTests = new Set([ + "server/src/__tests__/approval-routes-idempotency.test.ts", + "server/src/__tests__/assets.test.ts", + "server/src/__tests__/authz-company-access.test.ts", + "server/src/__tests__/companies-route-path-guard.test.ts", + "server/src/__tests__/company-portability.test.ts", + "server/src/__tests__/costs-service.test.ts", + "server/src/__tests__/express5-auth-wildcard.test.ts", + "server/src/__tests__/health-dev-server-token.test.ts", + "server/src/__tests__/health.test.ts", + "server/src/__tests__/heartbeat-dependency-scheduling.test.ts", + "server/src/__tests__/heartbeat-issue-liveness-escalation.test.ts", + "server/src/__tests__/heartbeat-process-recovery.test.ts", + "server/src/__tests__/invite-accept-existing-member.test.ts", + "server/src/__tests__/invite-accept-gateway-defaults.test.ts", + "server/src/__tests__/invite-accept-replay.test.ts", + "server/src/__tests__/invite-expiry.test.ts", + "server/src/__tests__/invite-join-manager.test.ts", + "server/src/__tests__/invite-onboarding-text.test.ts", + "server/src/__tests__/issues-checkout-wakeup.test.ts", + "server/src/__tests__/issues-service.test.ts", + "server/src/__tests__/opencode-local-adapter-environment.test.ts", + "server/src/__tests__/project-routes-env.test.ts", + "server/src/__tests__/redaction.test.ts", + "server/src/__tests__/routines-e2e.test.ts", +]); + +export function walk(dir) { + const entries = readdirSync(dir); + const files = []; + for (const entry of entries) { + const absolute = path.join(dir, entry); + const stats = statSync(absolute); + if (stats.isDirectory()) { + files.push(...walk(absolute)); + } else if (stats.isFile()) { + files.push(absolute); + } + } + return files; +} + +export function toRepoPath(repoRoot, file) { + return path.relative(repoRoot, file).split(path.sep).join("/"); +} + +export function isRouteOrAuthzTest(repoPath) { + if (routeTestPattern.test(repoPath)) { + return true; + } + return additionalSerializedServerTests.has(repoPath); +} + +// Every server test file the general-server group is responsible for, i.e. +// the whole server project minus the route/authz suites that run in the +// dedicated serialized shards. This is the same set scripts/general-server- +// shard-durations.json is expected to have an entry for. +export function collectGeneralServerSuiteFiles(repoRoot) { + const serverSrcDir = path.join(repoRoot, "server", "src"); + return walk(serverSrcDir) + .map((file) => toRepoPath(repoRoot, file)) + .filter((repoPath) => repoPath.endsWith(".test.ts")) + .filter((repoPath) => !isRouteOrAuthzTest(repoPath)) + .sort((a, b) => a.localeCompare(b)); +} diff --git a/scripts/run-vitest-stable.mjs b/scripts/run-vitest-stable.mjs index 84b7b32ec0b3..dfcc9420d3e9 100644 --- a/scripts/run-vitest-stable.mjs +++ b/scripts/run-vitest-stable.mjs @@ -1,10 +1,16 @@ #!/usr/bin/env node import { spawnSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, readdirSync, statSync } from "node:fs"; +import { mkdirSync, mkdtempSync, writeSync } from "node:fs"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; import { loadShardDurations, selectGeneralServerShard } from "./general-server-shard.mjs"; +import { + collectGeneralServerSuiteFiles, + isRouteOrAuthzTest, + toRepoPath as toRepoPathFromRoot, + walk, +} from "./run-vitest-stable-suites.mjs"; const repoRoot = process.cwd(); const scriptsDir = path.dirname(fileURLToPath(import.meta.url)); @@ -12,7 +18,6 @@ const generalServerShardDurations = loadShardDurations( path.join(scriptsDir, "general-server-shard-durations.json"), ); const serverRoot = path.join(repoRoot, "server"); -const serverSrcDir = path.join(repoRoot, "server", "src"); const serverTestsDir = path.join(repoRoot, "server", "src", "__tests__"); // Every non-server project that CI must execute. This list is NOT derived from // the root vitest.config.ts `projects` array -- it is a second, independent @@ -43,33 +48,6 @@ const nonServerProjects = [ "@paperclipai/ui", "paperclipai", ]; -const routeTestPattern = /[^/]*(?:route|routes|authz)[^/]*\.test\.ts$/; -const additionalSerializedServerTests = new Set([ - "server/src/__tests__/approval-routes-idempotency.test.ts", - "server/src/__tests__/assets.test.ts", - "server/src/__tests__/authz-company-access.test.ts", - "server/src/__tests__/companies-route-path-guard.test.ts", - "server/src/__tests__/company-portability.test.ts", - "server/src/__tests__/costs-service.test.ts", - "server/src/__tests__/express5-auth-wildcard.test.ts", - "server/src/__tests__/health-dev-server-token.test.ts", - "server/src/__tests__/health.test.ts", - "server/src/__tests__/heartbeat-dependency-scheduling.test.ts", - "server/src/__tests__/heartbeat-issue-liveness-escalation.test.ts", - "server/src/__tests__/heartbeat-process-recovery.test.ts", - "server/src/__tests__/invite-accept-existing-member.test.ts", - "server/src/__tests__/invite-accept-gateway-defaults.test.ts", - "server/src/__tests__/invite-accept-replay.test.ts", - "server/src/__tests__/invite-expiry.test.ts", - "server/src/__tests__/invite-join-manager.test.ts", - "server/src/__tests__/invite-onboarding-text.test.ts", - "server/src/__tests__/issues-checkout-wakeup.test.ts", - "server/src/__tests__/issues-service.test.ts", - "server/src/__tests__/opencode-local-adapter-environment.test.ts", - "server/src/__tests__/project-routes-env.test.ts", - "server/src/__tests__/redaction.test.ts", - "server/src/__tests__/routines-e2e.test.ts", -]); let invocationIndex = 0; const serializedModeName = "serialized"; const generalModeName = "general"; @@ -92,37 +70,14 @@ const arcWorkspaceVitestArgs = [ "--hookTimeout=60000", ]; -function walk(dir) { - const entries = readdirSync(dir); - const files = []; - for (const entry of entries) { - const absolute = path.join(dir, entry); - const stats = statSync(absolute); - if (stats.isDirectory()) { - files.push(...walk(absolute)); - } else if (stats.isFile()) { - files.push(absolute); - } - } - return files; -} - function toRepoPath(file) { - return path.relative(repoRoot, file).split(path.sep).join("/"); + return toRepoPathFromRoot(repoRoot, file); } function toServerPath(file) { return path.relative(serverRoot, file).split(path.sep).join("/"); } -function isRouteOrAuthzTest(file) { - if (routeTestPattern.test(file)) { - return true; - } - - return additionalSerializedServerTests.has(file); -} - function fail(message) { console.error(`[test:run] ${message}`); process.exit(1); @@ -430,11 +385,7 @@ const routeTests = walk(serverTestsDir) // config pins maxWorkers to 1, so the only way to parallelize is across jobs. // Suites are partitioned by recorded duration (scripts/general-server-shard.mjs) // rather than round-robin, so one slow suite cluster can't stretch a single shard. -const generalServerTestFiles = walk(serverSrcDir) - .map((file) => toRepoPath(file)) - .filter((repoPath) => repoPath.endsWith(".test.ts")) - .filter((repoPath) => !isRouteOrAuthzTest(repoPath)) - .sort((a, b) => a.localeCompare(b)); +const generalServerTestFiles = collectGeneralServerSuiteFiles(repoRoot); const options = parseCliOptions(process.argv.slice(2)); if (options.dryRun) { @@ -442,37 +393,40 @@ if (options.dryRun) { options.mode === serializedModeName ? selectSerializedSuites(routeTests, options.shardIndex, options.shardCount) : routeTests; - console.log( - JSON.stringify( - { - mode: options.mode, - shardIndex: options.shardIndex, - shardCount: options.shardCount, - group: options.group, - availableGeneralGroups: generalGroupNames, - nonServerProjects, - generalWorkspacesAProjects, - generalWorkspacesBProjects, - generalWorkspacesBVitestArgs: arcWorkspaceVitestArgs, - serializedSuiteCount: routeTests.length, - selectedSerializedSuites: serializedSuites.map((routeTest) => routeTest.repoPath), - generalServerSuiteCount: generalServerTestFiles.length, - selectedGeneralServerSuites: - options.mode === generalModeName && - options.group === generalServerGroupName && - options.shardCount !== null - ? selectGeneralServerShard( - generalServerTestFiles, - options.shardIndex, - options.shardCount, - generalServerShardDurations, - ) - : null, - }, - null, - 2, - ), + const dryRunPayload = JSON.stringify( + { + mode: options.mode, + shardIndex: options.shardIndex, + shardCount: options.shardCount, + group: options.group, + availableGeneralGroups: generalGroupNames, + nonServerProjects, + generalWorkspacesAProjects, + generalWorkspacesBProjects, + generalWorkspacesBVitestArgs: arcWorkspaceVitestArgs, + serializedSuiteCount: routeTests.length, + selectedSerializedSuites: serializedSuites.map((routeTest) => routeTest.repoPath), + generalServerSuiteCount: generalServerTestFiles.length, + selectedGeneralServerSuites: + options.mode === generalModeName && + options.group === generalServerGroupName && + options.shardCount !== null + ? selectGeneralServerShard( + generalServerTestFiles, + options.shardIndex, + options.shardCount, + generalServerShardDurations, + ) + : null, + }, + null, + 2, ); + // This payload is consumed through a pipe by policy tests and by other + // tooling. `console.log` followed by `process.exit` can leave the pipe's + // final chunk unwritten once the suite list grows beyond the stream buffer. + // A synchronous write makes the machine-readable dry-run contract complete. + writeSync(1, `${dryRunPayload}\n`); process.exit(0); }