From 3b84c89885e6d8282f2950db14d1372816e84c72 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 30 Oct 2026 14:29:39 -0500 Subject: [PATCH] fix(ci): use complete merge diffs and allow audited documentation (#18362) ## Thinking Path > - Paperclip manages AI agents and needs reliable checks for each contribution. > - CI selects affected suites and then affected Vitest files before allocating runners. > - Recent runs compared a tested merge against a stale event base. > - That included unrelated master changes and sometimes reached the compare API file cap. > - Ordinary documentation also forced every suite and invalidated test caches. > - This PR reads complete merge trees and exempts a small set of audited prose inputs. > - Tests retain conservative coverage when the diff or an input is uncertain. ## Linked Issues or Issue Description Refs #18348 and #18214. This is a follow-up to the merged affected-test selector. Related CI performance work: #16513 and #16500 address setup and suite execution. This PR changes diff collection and input classification. ## What Changed - Fetch the exact tested merge and its parents into a temporary bare repository. Read the complete tree diff against the actual first parent. Validate the second parent against the event PR head. Never execute PR code in the routing job. - Carry the immutable base, merge and PR-head identities into the plan. Validate the same complete diff again before file selection. - Use one prose policy across the planner, Vitest selector and task cache adapter. Exempt only `doc/DEVELOPING.md`, `doc/DATABASE.md`, and Markdown under `doc/plans/`. - Keep the uncached typecheck and CLI guidance guard for the first two documents. Keep policy checks for historical plans. Keep existing coverage for all other docs, runtime skills, fixtures and unknown inputs. - Extend candidate-workflow qualification to exercise mixed source and prose changes. ## Verification - Passed: 15 planner and Git routing tests, including a 353-file diff, stale base, rename, deletion, wrong parent and fetch-failure cases. - Passed: 83 routing, Vitest graph, exact selection, workflow and real Turbo cache checks. - Passed: `pnpm -r typecheck` and `pnpm build`. - Full local `pnpm test:run` was attempted and stopped after repeated embedded PostgreSQL startup failures in native-session and Muse tests. It did not pass or complete. The exact cause remains unverified. The changed files do not include those tests or their database setup. Linux CI results are recorded below. - Passed: real mixed Inbox/source/prose graph qualification selected 699/911 workspace test files, preserving the source-only selection. - Passed: both GitHub CI qualification jobs, including real mixed source/prose Vitest selection. Greptile reviewed commit `10ac6f5ec9` at 5/5 with no findings. - First full CI attempt: the [chat shard](https://github.com/paperclipai/paperclip/actions/runs/41111003419/job/124253342684) received a runner shutdown signal. The [Runner shard](https://github.com/paperclipai/paperclip/actions/runs/41111003419/job/124253341271) failed the unchanged Antigravity authentication success assertion. All 11 cases in that file passed when run directly. That Runner lane does not use the changed selector/cache path. The exact assertion-failure cause remains unverified. The chat shard passed on the one retry. The authentication assertion also passed on retry, but the same Runner shard failed a different unchanged test: `native-distribution-integrity.test.ts:242`, "retains spawned native bytes until the exact child exits", exceeded 5000ms. The [retry log](https://github.com/paperclipai/paperclip/actions/runs/41111003419/job/124259174201) preserves the failure. The Runner shard and required verify aggregate are still red; all other checks passed. All 33 native-distribution tests then passed in a direct local run. This shows recovery in isolation, not the cause of the Linux timeout. No further blind reruns were issued. - Replayed 15 real recent PR merge commits through a blob-filtered bare fetch. Nine needed a corrected base. The four API comparisons that returned 300 filenames became complete diffs of 3, 57, 66 and 54 files. The 3-file case (#18326) becomes eligible for graph selection. Most other samples still correctly use full CI because they include inputs outside this narrow prose policy. - Cache proof: prose edits preserve a product-test cache hit; changing runtime API documentation invalidates it. ## Risks - Prose exceptions must be removed or revised if product code starts reading those files. The allowlist is deliberately small. Runtime docs and skills remain covered. - A network or Git failure keeps the full-suite fallback. Unexpected merge topology also keeps full CI. Stack and force-full policies remain in place. - This does not claim an end-to-end speedup yet. Many mixed-content PRs still require full CI. The routing fetch reads only shallow commit/tree objects; it does not check out or execute PR files. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository tools and code execution. The exact served model ID and context-window size are not exposed to this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip --- .github/scripts/ci-diff.mjs | 22 +++++ .github/scripts/ci-inputs.mjs | 10 +++ .github/scripts/ci-plan.mjs | 24 ++++-- .github/scripts/tests/ci-plan.test.mjs | 83 ++++++++++++++++++- .../ci-incremental-qualification.yml | 4 +- .github/workflows/ci-routing.yml | 15 ++-- .github/workflows/pr-trusted.yml | 1 - doc/DEVELOPING.md | 25 ++++-- scripts/ci/cache.integration.test.mjs | 16 +++- scripts/ci/execution-plan.mjs | 3 +- scripts/ci/run-task.mjs | 3 +- scripts/ci/select-tests.mjs | 21 +++-- .../ci/test-selection.integration.test.mjs | 17 +++- 13 files changed, 207 insertions(+), 37 deletions(-) create mode 100644 .github/scripts/ci-diff.mjs create mode 100644 .github/scripts/ci-inputs.mjs diff --git a/.github/scripts/ci-diff.mjs b/.github/scripts/ci-diff.mjs new file mode 100644 index 00000000000..324b87a18c5 --- /dev/null +++ b/.github/scripts/ci-diff.mjs @@ -0,0 +1,22 @@ +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; + +// Read trees only. The routing job fetches Git objects into a bare repository; +// no pull request code, hooks, filters or package scripts run here. +export function mergeDiff(merge, prHead, cwd = process.cwd()) { + assert.match(merge ?? "", /^[a-f0-9]{40}$/); + assert.match(prHead ?? "", /^[a-f0-9]{40}$/); + const git = (...args) => execFileSync("git", args, { + cwd, encoding: "utf8", maxBuffer: 16 * 1024 * 1024, + }).trim(); + const header = git("--no-replace-objects", "cat-file", "-p", merge).split("\n\n", 1)[0]; + const parents = [...header.matchAll(/^parent ([a-f0-9]{40})$/gm)].map((match) => match[1]); + assert.equal(parents.length, 2, "Expected a GitHub pull request merge commit"); + assert.equal(parents[1], prHead, "Merge commit does not contain the expected PR head"); + const base = parents[0]; + const changed = execFileSync("git", ["diff", "--no-ext-diff", "--no-textconv", "--no-renames", + "--name-only", "-z", base, merge, "--"], { + cwd, encoding: "utf8", maxBuffer: 16 * 1024 * 1024, + }).split("\0").filter(Boolean).sort(); + return { diff: { source: "git-merge-parent", base, head: merge, prHead }, changed }; +} diff --git a/.github/scripts/ci-inputs.mjs b/.github/scripts/ci-inputs.mjs new file mode 100644 index 00000000000..ad17a8acdc5 --- /dev/null +++ b/.github/scripts/ci-inputs.mjs @@ -0,0 +1,10 @@ +// Audited prose inputs, shared by routing, Vitest selection and task hashing. +// Do not broaden this to all Markdown: docs/api and bundled skills are runtime inputs. +export function documentationInput(file) { + if (file === "doc/DEVELOPING.md" || file === "doc/DATABASE.md") return "guidance"; + if (/^doc\/plans\/(?:[^/]+\/)*[^/]+\.md$/.test(file) + && !file.split("/").includes("..")) return "historical"; + return null; +} + +export const productInputs = (files) => files.filter((file) => !documentationInput(file)); diff --git a/.github/scripts/ci-plan.mjs b/.github/scripts/ci-plan.mjs index 3019b936ecf..f58665218a2 100644 --- a/.github/scripts/ci-plan.mjs +++ b/.github/scripts/ci-plan.mjs @@ -2,6 +2,8 @@ import { appendFileSync, readFileSync, realpathSync } from "node:fs"; import { execFileSync } from "node:child_process"; import { matchesGlob } from "node:path"; import { fileURLToPath } from "node:url"; +import { documentationInput, productInputs } from "./ci-inputs.mjs"; +import { mergeDiff } from "./ci-diff.mjs"; const releaseNote = /^releases\/(beta\/)?v[^/]+\.md$/; const validPath = (file) => typeof file === "string" && file.length > 0 @@ -52,28 +54,33 @@ export function planTasks({ config, lanes, paths, mode = "enabled", forceReason const changed = [...new Set(paths)]; if (!changed.length || changed.some((file) => !validPath(file))) forceReason ??= "Missing or invalid diff"; const matches = (file, globs) => globs.some((glob) => matchesGlob(file, glob)); - const globalChanges = changed.filter((file) => matches(file, config.globalDependencies)); + const sourceChanges = productInputs(changed); + const globalChanges = sourceChanges.filter((file) => matches(file, config.globalDependencies)); if (globalChanges.length) forceReason ??= `Global CI input changed: ${globalChanges[0]}`; - const unknown = changed.filter((file) => !releaseNote.test(file) + const unknown = sourceChanges.filter((file) => !releaseNote.test(file) && !matches(file, config.globalDependencies) && ![...inputs.values()].some((globs) => matches(file, globs))); if (unknown.length) forceReason ??= `Unclassified path: ${unknown[0]}`; if (mode === "full") forceReason ??= "Full CI requested"; const reasons = Object.fromEntries([...inputs].map(([task, globs]) => [task, - forceReason ? [forceReason] : [...new Set([...globalChanges, ...changed.filter((file) => matches(file, globs))])], + forceReason ? [forceReason] : [...new Set([...globalChanges, ...sourceChanges.filter((file) => matches(file, globs))])], ])); + // This uncached lane owns the repository-wide CLI guidance check. + if (reasons["ci:typecheck"]) reasons["ci:typecheck"].push(...changed.filter((file) => documentationInput(file) === "guidance")); const jobs = Object.fromEntries(Object.entries(lanes).map(([job, entries]) => { const affected = entries.filter((entry) => reasons[entry.task].length > 0); const selected = mode === "shadow" ? entries : affected; return [job, { run: selected.length > 0, matrix: { include: selected.length ? selected : entries }, selected: selected.length, proposed: affected.length, total: entries.length }]; })); - return { version: 1, mode, forceReason, changed, reasons, jobs }; + return { version: 1, mode, forceReason, changed, documentation: changed.filter(documentationInput), reasons, jobs }; } export function planSummary(plan) { const rows = Object.entries(plan.jobs).map(([job, lane]) => `| ${job} | ${lane.proposed}/${lane.total} | ${lane.selected}/${lane.total} |`); return ["### Incremental CI", "", `Mode: **${plan.mode}**${plan.forceReason ? ` — ${plan.forceReason}` : ""}`, + ...(plan.diff ? [`Diff: \`${plan.diff.base}\` → \`${plan.diff.head}\` (complete Git tree comparison)`, ""] : []), + `Documentation inputs: ${(plan.documentation ?? []).join(", ") || "none"}`, "", "| Suite | Affected jobs | Scheduled jobs |", "| --- | ---: | ---: |", ...rows, "", "
Selection reasons", "", "```json", JSON.stringify(plan.reasons, null, 2), "```", "
", ""].join("\n"); @@ -83,9 +90,13 @@ if (process.argv[1] && realpathSync(process.argv[1]) === fileURLToPath(import.me try { const config = JSON.parse(readFileSync(process.env.CI_TASK_CONFIG ?? "turbo.json", "utf8")); const lanes = JSON.parse(readFileSync(process.env.CI_TASK_MATRIX ?? ".github/ci-task-matrix.json", "utf8")); - let paths = [], forceReason = process.env.CI_FORCE_REASON || null; + let paths = [], diff, forceReason = process.env.CI_FORCE_REASON || null; try { - if (process.env.CI_COMPARISON) { + if (process.env.CI_DIFF_REPOSITORY) { + const result = mergeDiff(process.env.EVENT_MERGE_SHA, process.env.EVENT_HEAD_SHA, process.env.CI_DIFF_REPOSITORY); + paths = result.changed; + diff = result.diff; + } else if (process.env.CI_COMPARISON) { paths = comparisonPaths(JSON.parse(readFileSync(process.env.CI_COMPARISON, "utf8"))); } else { const [base, head = "HEAD"] = process.argv.slice(2); @@ -95,6 +106,7 @@ if (process.argv[1] && realpathSync(process.argv[1]) === fileURLToPath(import.me } } catch (error) { forceReason ??= `Diff unavailable: ${error.message}`; } const plan = planTasks({ config, lanes, paths, mode: process.env.CI_INCREMENTAL_MODE || "enabled", forceReason }); + if (diff) plan.diff = diff; if (process.env.GITHUB_OUTPUT) appendFileSync(process.env.GITHUB_OUTPUT, `plan=${JSON.stringify(plan)}\n`); else console.log(JSON.stringify(plan, null, 2)); if (process.env.GITHUB_STEP_SUMMARY) appendFileSync(process.env.GITHUB_STEP_SUMMARY, planSummary(plan)); diff --git a/.github/scripts/tests/ci-plan.test.mjs b/.github/scripts/tests/ci-plan.test.mjs index da6dc6955fa..81fcc16475a 100644 --- a/.github/scripts/tests/ci-plan.test.mjs +++ b/.github/scripts/tests/ci-plan.test.mjs @@ -5,6 +5,7 @@ import { tmpdir } from "node:os"; import path from "node:path"; import test from "node:test"; import { comparisonPaths, planTasks } from "../ci-plan.mjs"; +import { mergeDiff } from "../ci-diff.mjs"; const root = new URL("../../../", import.meta.url); const config = JSON.parse(readFileSync(new URL("turbo.json", root), "utf8")); @@ -189,6 +190,30 @@ test("the real routing step uses pinned trusted definitions and defaults to full const shell = block.split(" run: |\n")[1].split("\n").map((line) => line.slice(10)).join("\n"); const directory = mkdtempSync(path.join(tmpdir(), "ci-routing-plan-")); try { + const repository = path.join(directory, "source"); + mkdirSync(repository); + const gitBin = execFileSync("which", ["git"], { encoding: "utf8" }).trim(); + const git = (...args) => execFileSync(gitBin, args, { cwd: repository, encoding: "utf8" }).trim(); + git("init", "-q", "-b", "main"); git("config", "user.name", "CI"); git("config", "user.email", "ci@example.invalid"); + writeFileSync(path.join(repository, "base"), "base"); git("add", "."); git("commit", "-qm", "base"); + git("checkout", "-qb", "pr"); + mkdirSync(path.join(repository, "ui/src/components"), { recursive: true }); + writeFileSync(path.join(repository, "ui/src/components/Sidebar.tsx"), "change"); + git("add", "."); git("commit", "-qm", "PR"); + const prHead = git("rev-parse", "HEAD"); + git("checkout", "-q", "main"); + writeFileSync(path.join(repository, "unrelated-master"), "already merged"); + git("add", "."); git("commit", "-qm", "advance base"); + const base = git("rev-parse", "HEAD"); + git("merge", "--no-ff", "pr", "-m", "tested merge"); + const merge = git("rev-parse", "HEAD"); + writeFileSync(path.join(directory, "git"), `#!/usr/bin/env node +const {spawnSync}=require('node:child_process'); +if(process.env.TEST_GIT_FAILURE==='true')process.exit(1); +const args=process.argv.slice(2).map(a=>a==='https://github.com/paperclipai/paperclip.git'?process.env.TEST_GIT_REPOSITORY:a); +const r=spawnSync(${JSON.stringify(gitBin)},args,{stdio:'inherit'});process.exit(r.status??1); +`); + chmodSync(path.join(directory, "git"), 0o755); const gh = path.join(directory, "gh"); writeFileSync(gh, `#!/usr/bin/env node const fs = require('node:fs'), path = require('node:path'); @@ -199,27 +224,77 @@ else if (endpoint?.includes('/contents/')) { const [file, query] = endpoint.split('/contents/')[1].split('?'); if (query !== 'ref=' + 'a'.repeat(40)) process.exit(2); process.stdout.write(fs.readFileSync(path.join(process.env.TEST_TRUSTED_ROOT, file))); -} else if (endpoint?.includes('/compare/')) console.log(JSON.stringify({ files: [{filename:'ui/src/components/Sidebar.tsx'}] })); +} else process.exit(3); `); chmodSync(gh, 0o755); - for (const overrides of [{}, { TEST_FETCH_FAILURE: "true" }, { PROFILE_CI: "true" }, { STACK_JSON: "{}" }, { FULL_CI: "false" }]) { + for (const overrides of [{}, { TEST_FETCH_FAILURE: "true" }, { TEST_GIT_FAILURE: "true" }, { PROFILE_CI: "true" }, { STACK_JSON: "{}" }, { FULL_CI: "false" }]) { const output = path.join(directory, "output"); writeFileSync(output, ""); const result = spawnSync("bash", ["-c", shell], { encoding: "utf8", env: { ...process.env, PATH: `${directory}:${process.env.PATH}`, TEST_TRUSTED_ROOT: root.pathname, RUNNER_TEMP: directory, FULL_CI: "true", PROFILE_CI: "false", FORCE_FULL_CI: "false", - CI_INCREMENTAL_MODE: "enabled", STACK_JSON: "null", EVENT_BASE_SHA: "b".repeat(40), EVENT_MERGE_SHA: "c".repeat(40), + CI_INCREMENTAL_MODE: "enabled", STACK_JSON: "null", EVENT_BASE_SHA: "b".repeat(40), EVENT_HEAD_SHA: prHead, EVENT_MERGE_SHA: merge, + TEST_GIT_REPOSITORY: repository, GITHUB_OUTPUT: output, GITHUB_STEP_SUMMARY: "", ...overrides, } }); assert.equal(result.status, 0, result.stderr); const value = readFileSync(output, "utf8").trim(); - if (overrides.TEST_FETCH_FAILURE || overrides.FULL_CI === "false") assert.equal(value, "plan="); + if (overrides.TEST_FETCH_FAILURE || overrides.TEST_GIT_FAILURE || overrides.FULL_CI === "false") assert.equal(value, "plan="); else { assert.notEqual(value, "plan=", `${JSON.stringify(overrides)}: ${result.stderr}`); const resultPlan = JSON.parse(value.slice("plan=".length)); assert.equal(resultPlan.jobs.general_tests.selected, overrides.PROFILE_CI || overrides.STACK_JSON ? 19 : 2); + assert.equal(resultPlan.diff.base, base); + assert.deepEqual(resultPlan.changed, ["ui/src/components/Sidebar.tsx"]); } } } finally { rmSync(directory, { recursive: true, force: true }); } }); + + +test("audited prose does not widen source selection, but guidance still runs its uncached guard", () => { + const source = "ui/src/components/Sidebar.tsx"; + const sourcePlan = plan([source]); + for (const doc of ["doc/DEVELOPING.md", "doc/DATABASE.md", "doc/plans/2026-10-30-example.md"]) { + const mixed = plan([source, doc]); + assert.equal(mixed.forceReason, null); + assert.deepEqual(mixed.jobs, sourcePlan.jobs); + assert.deepEqual(mixed.documentation, [doc]); + } + const guidance = plan(["doc/DEVELOPING.md"]); + assert.deepEqual([...selectedTasks(guidance)], ["ci:typecheck"]); + assert.equal(config.tasks["//#ci:typecheck"].cache, false); + assert.deepEqual([...selectedTasks(plan(["doc/plans/2026-10-30-example.md"]))], []); + for (const file of ["docs/api/issues.md", "doc/unknown.md", "doc/plans/tool.sh", "skills/paperclip/SKILL.md", + "server/src/onboarding-assets/default/AGENTS.md", "tests/fixtures/example.md"]) { + assert.ok(selectedTasks(plan([source, file])).has("ci:server"), file); + } +}); + +test("complete merge trees handle more than 300 files, base advancement, renames and deletions", (t) => { + const directory = mkdtempSync(path.join(tmpdir(), "ci-merge-diff-")); + t.after(() => rmSync(directory, { recursive: true, force: true })); + const git = (...args) => execFileSync("git", args, { cwd: directory, encoding: "utf8" }).trim(); + const write = (file, value) => { mkdirSync(path.dirname(path.join(directory, file)), { recursive: true }); writeFileSync(path.join(directory, file), value); }; + const commit = () => { git("add", "."); git("commit", "-qm", "fixture"); return git("rev-parse", "HEAD"); }; + git("init", "-q", "-b", "main"); git("config", "user.name", "CI"); git("config", "user.email", "ci@example.invalid"); + write("ui/src/deleted.ts", "old"); write("ui/src/renamed.ts", "rename"); const staleBase = commit(); + git("checkout", "-qb", "pr"); + for (let index = 0; index < 350; index++) write(`ui/src/component-${index}.ts`, "new"); + git("rm", "ui/src/deleted.ts"); + mkdirSync(path.join(directory, "doc/plans"), { recursive: true }); + git("mv", "ui/src/renamed.ts", "doc/plans/2026-10-30-moved.md"); + const prHead = commit(); + git("checkout", "-q", "main"); write(".github/workflows/unrelated.yml", "already merged CI change"); const base = commit(); + git("merge", "--no-ff", "pr", "-m", "tested merge"); const head = git("rev-parse", "HEAD"); + const result = mergeDiff(head, prHead, directory); + assert.equal(result.diff.base, base); assert.notEqual(result.diff.base, staleBase); + assert.equal(result.changed.length, 353); + assert.ok(result.changed.includes("ui/src/deleted.ts")); assert.ok(result.changed.includes("ui/src/renamed.ts")); + assert.ok(!result.changed.includes(".github/workflows/unrelated.yml")); + assert.equal(plan(result.changed).forceReason, null); + assert.throws(() => mergeDiff(head, base, directory), /expected PR head/); + assert.throws(() => mergeDiff(prHead, prHead, directory), /merge commit/); + assert.throws(() => mergeDiff("f".repeat(40), prHead, directory)); +}); diff --git a/.github/workflows/ci-incremental-qualification.yml b/.github/workflows/ci-incremental-qualification.yml index bce43fc09ab..3b4d608dff8 100644 --- a/.github/workflows/ci-incremental-qualification.yml +++ b/.github/workflows/ci-incremental-qualification.yml @@ -7,6 +7,8 @@ on: - turbo.json - .github/ci-task-matrix.json - .github/scripts/ci-plan.mjs + - .github/scripts/ci-diff.mjs + - .github/scripts/ci-inputs.mjs - .github/scripts/tests/ci-plan.test.mjs - .github/actions/run-ci-task/** - .github/workflows/ci-routing.yml @@ -57,7 +59,7 @@ jobs: ['ui/src/pages/Inbox.tsx', '@paperclipai/ui', 'ui/src/pages/Inbox.test.tsx'], ['server/src/middleware/auth.ts', '@paperclipai/server', 'server/src/__tests__/agent-auth-middleware.test.ts'], ]) { - const selected = await createTestSelection(planTasks({ config, lanes, paths: [file] }), head); + const selected = await createTestSelection(planTasks({ config, lanes, paths: [file, "doc/DEVELOPING.md", "doc/plans/2026-10-30-ci-proof.md"] }), head); const graph = selected.graphs.find(graph => graph.project === project); assert.ok(graph.related.includes(related)); assert.ok(graph.alwaysRun.length > 0, 'Filesystem contracts must remain covered'); diff --git a/.github/workflows/ci-routing.yml b/.github/workflows/ci-routing.yml index 122282b5a0a..1b156165c5a 100644 --- a/.github/workflows/ci-routing.yml +++ b/.github/workflows/ci-routing.yml @@ -350,24 +350,27 @@ jobs: FORCE_FULL_CI: ${{ contains(github.event.pull_request.labels.*.name, 'full-ci') }} CI_INCREMENTAL_MODE: ${{ vars.CI_INCREMENTAL_MODE || 'enabled' }} STACK_JSON: ${{ toJSON(github.event.pull_request.stack) }} - EVENT_BASE_SHA: ${{ github.event.pull_request.base.sha }} + EVENT_HEAD_SHA: ${{ github.event.pull_request.head.sha }} EVENT_MERGE_SHA: ${{ github.sha }} run: | set -euo pipefail fallback() { echo 'plan=' >> "$GITHUB_OUTPUT"; exit 0; } [[ "$FULL_CI" == 'true' ]] || fallback - [[ "$EVENT_BASE_SHA" =~ ^[0-9a-f]{40}$ && "$EVENT_MERGE_SHA" =~ ^[0-9a-f]{40}$ ]] || fallback + [[ "$EVENT_HEAD_SHA" =~ ^[0-9a-f]{40}$ && "$EVENT_MERGE_SHA" =~ ^[0-9a-f]{40}$ ]] || fallback trusted="$(gh api /repos/paperclipai/paperclip/git/ref/heads/master --jq .object.sha)" || fallback [[ "$trusted" =~ ^[0-9a-f]{40}$ ]] || fallback directory="$(mktemp -d)" trap 'rm -rf "$directory"' EXIT - for file in .github/scripts/ci-plan.mjs .github/ci-task-matrix.json turbo.json; do + for file in .github/scripts/ci-plan.mjs .github/scripts/ci-inputs.mjs .github/scripts/ci-diff.mjs .github/ci-task-matrix.json turbo.json; do gh api -H 'Accept: application/vnd.github.raw+json' \ "/repos/paperclipai/paperclip/contents/$file?ref=$trusted" \ > "$directory/${file##*/}" || fallback done - gh api "/repos/paperclipai/paperclip/compare/$EVENT_BASE_SHA...$EVENT_MERGE_SHA" \ - > "$directory/comparison.json" || fallback + # Read the exact tested merge and both parents. The event's base SHA can + # lag behind its actual first parent. A bare tree diff has no API file cap. + git init --bare "$directory/repository.git" >/dev/null || fallback + git -C "$directory/repository.git" fetch --quiet --no-tags --filter=blob:none --depth=2 \ + https://github.com/paperclipai/paperclip.git "$EVENT_MERGE_SHA" || fallback if [[ "$PROFILE_CI" == 'true' || "$FORCE_FULL_CI" == 'true' ]]; then export CI_FORCE_REASON='Full suite requested by label' elif [[ "$STACK_JSON" != 'null' && -n "$STACK_JSON" ]]; then @@ -375,5 +378,5 @@ jobs: fi export CI_TASK_CONFIG="$directory/turbo.json" export CI_TASK_MATRIX="$directory/ci-task-matrix.json" - export CI_COMPARISON="$directory/comparison.json" + export CI_DIFF_REPOSITORY="$directory/repository.git" node "$directory/ci-plan.mjs" || fallback diff --git a/.github/workflows/pr-trusted.yml b/.github/workflows/pr-trusted.yml index 7c46153bb48..b1b73c1f085 100644 --- a/.github/workflows/pr-trusted.yml +++ b/.github/workflows/pr-trusted.yml @@ -40,7 +40,6 @@ jobs: plan: ${{ steps.publish.outputs.plan }} env: CI_PLAN: ${{ needs.gate.outputs.plan }} - BASE_SHA: ${{ github.event.pull_request.base.sha }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 with: diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index 04b519620da..3ab1e2dea46 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -171,10 +171,12 @@ does not include the lower PRs' code. For ordinary full-scope PRs, the routing job further selects affected suites using `turbo.json` and `.github/ci-task-matrix.json`. It downloads the planner -and both definitions from one canonical master revision, without executing PR -code. The comparison covers the tested merge commit. Both rename paths count; -failed, empty, malformed, or potentially truncated comparisons select the full -suite. Unclassified files also select full CI. Stack verification retains the +and its helpers and definitions from one canonical master revision, without executing PR +code. It fetches the exact tested merge commit and its parents into a temporary +bare Git repository, validates the PR head, and compares the merge tree against +its first parent. The event's base SHA can be stale. Git tree comparisons have +no GitHub compare-API 300-file cap. Renames count as deletion plus addition; +failed, empty, or malformed comparisons select the full suite. Unclassified files also select full CI. Stack verification retains the existing scope policy and runs all suites whenever that policy requires them. Changes to global inputs, including the task graph, package scripts, workflows, and execution helpers, force full execution and disable result-cache restore and @@ -193,7 +195,16 @@ canary dry run. The uncached typecheck job also runs the repository-wide CLI guidance safety guard. It still scans UI changes when unrelated server shards are omitted. -For ordinary source-only PRs, a separate `Select Vitest files` job refines the +The shared policy in `.github/scripts/ci-inputs.mjs` exempts only audited prose: +`doc/DEVELOPING.md`, `doc/DATABASE.md`, and Markdown under `doc/plans/`. +The first two still select the uncached typecheck/guidance lane; historical plans +need only the always-running policy checks. These inputs do not widen product +test selection or enter product task cache hashes. All other docs, bundled +skills, fixtures and unknown paths keep their existing conservative coverage. +A source change plus exempt prose remains graph eligible. When adding a reader +of exempt prose, update this policy as well as the task inputs. + +For ordinary source PRs (including the audited prose inputs above), a separate `Select Vitest files` job refines the general and serialized test lanes before their runners are allocated. It runs on the gate-approved test fleet, loads the real Vitest projects, and follows imports across workspace packages. Only projects with their own source changes @@ -226,11 +237,11 @@ selected per lane, and the `vitest-selection` artifact records the manifest and dependency audit. Selected jobs execute exact Vitest test specifications, so similarly named `.test.ts` and `.test.tsx` files cannot overlap between shards. -Missing/deleted/renamed files, non-source inputs, unrecognized graph inputs, +Missing/deleted/renamed source files, non-exempt non-source inputs, unrecognized graph inputs, setup/config changes, failed dependency installs, graph errors and timeouts use the complete original task plan. Source mutations during dependency setup also disable narrowing. A published manifest must match the checked-out -merge SHA, its plan hash and the expected shard/project ownership. Missing or +merge SHA and parent diff, its plan hash and the expected shard/project ownership. Missing or invalid manifests fail selected jobs and the required aggregate; they cannot turn reduced matrices green. Failed-job retries download the original artifact ID saved with the plan, even when GitHub increments the run attempt. Turbo hashes the selected file assignment through diff --git a/scripts/ci/cache.integration.test.mjs b/scripts/ci/cache.integration.test.mjs index 90c94bf6c10..8aebc6d92e3 100644 --- a/scripts/ci/cache.integration.test.mjs +++ b/scripts/ci/cache.integration.test.mjs @@ -21,13 +21,19 @@ test("real Turbo keeps full runs and different selected test sets in separate ca for (const file of ["run-task.mjs", "execution-plan.mjs", "test-selection-manifest.mjs"]) { copyFileSync(new URL(`scripts/ci/${file}`, repositoryRoot), path.join(directory, "scripts/ci", file)); } + mkdirSync(path.join(directory, ".github/scripts"), { recursive: true }); + copyFileSync(new URL(".github/scripts/ci-inputs.mjs", repositoryRoot), path.join(directory, ".github/scripts/ci-inputs.mjs")); write(".gitignore", ".turbo/\n.ci-test-selection/\nresult.txt\ncalls.txt\n"); json("package.json", { name: "selection-cache-proof", private: true, packageManager: "npm@10.0.0", workspaces: ["packages/*"], scripts: { "test:run:general": "node check.cjs" } }); json("packages/fixture/package.json", { name: "fixture", private: true }); json("package-lock.json", { name: "selection-cache-proof", lockfileVersion: 3, packages: { "": { name: "selection-cache-proof" } } }); - json("turbo.json", { globalDependencies: [], globalEnv: ["CI_TASK_CACHE_SCOPE", "PAPERCLIP_*"], tasks: { + json("turbo.json", { globalDependencies: ["doc/**", "docs/**"], globalEnv: ["CI_TASK_CACHE_SCOPE", "PAPERCLIP_*"], tasks: { "//#ci:server": { inputs: ["check.cjs", "server/**"], outputs: ["result.txt"], cache: true }, } }); + mkdirSync(path.join(directory, "doc/plans"), { recursive: true }); + mkdirSync(path.join(directory, "docs/api"), { recursive: true }); + write("doc/DEVELOPING.md", "before"); write("doc/plans/example.md", "before"); + write("docs/api/issues.md", "before"); write("server/src/a.test.ts", "// a"); write("server/src/b.test.ts", "// b"); write("check.cjs", `const fs=require('node:fs');fs.appendFileSync('calls.txt','call\\n');const file=process.env.PAPERCLIP_CI_TEST_SELECTION;fs.writeFileSync('result.txt',file?JSON.parse(fs.readFileSync(file)).lanes['general-server-without-chat'].shards[0]['@paperclipai/server'].join(','):'full');`); write("bin/pnpm", `#!${process.execPath}\nconst {spawnSync}=require('node:child_process');const args=process.argv.slice(2);if(args[0]==='--version')console.log('9.15.4');else{const r=spawnSync(${JSON.stringify(turbo)},args.slice(2),{stdio:'inherit'});process.exit(r.status??1);}`); @@ -51,9 +57,13 @@ test("real Turbo keeps full runs and different selected test sets in separate ca assert.equal(readFileSync(path.join(directory, "result.txt"), "utf8"), testFile ?? "full"); }; run(); run(); assert.equal(calls(), 1); + write("doc/DEVELOPING.md", "after"); write("doc/plans/example.md", "after"); + run(); assert.equal(calls(), 1, "audited prose does not invalidate product test results"); run("server/src/a.test.ts"); run("server/src/a.test.ts"); assert.equal(calls(), 2); run("server/src/b.test.ts"); assert.equal(calls(), 3); run(); assert.equal(calls(), 3, "the original full result is reusable without accepting a subset result"); + write("docs/api/issues.md", "after"); + run(); assert.equal(calls(), 4, "runtime documentation must invalidate results"); } finally { rmSync(directory, { recursive: true, force: true }); } }); @@ -82,6 +92,8 @@ test("real Turbo executes existing suite scripts and invalidates source, argumen const json = (name, value) => write(name, JSON.stringify(value)); try { execFileSync("git", ["init", "-q"], { cwd: directory }); + mkdirSync(path.join(directory, ".github/scripts"), { recursive: true }); + copyFileSync(new URL(".github/scripts/ci-inputs.mjs", repositoryRoot), path.join(directory, ".github/scripts/ci-inputs.mjs")); write(".gitignore", ".turbo/\ngenerated/\nresult.txt\ncalls.txt\n"); mkdirSync(path.join(directory, "packages/fixture"), { recursive: true }); json("packages/fixture/package.json", { name: "fixture", private: true }); @@ -133,6 +145,8 @@ test("the task launcher bypasses cache reads and writes when execution is forced for (const name of ["run-task.mjs", "execution-plan.mjs", "test-selection-manifest.mjs"]) { copyFileSync(new URL(`scripts/ci/${name}`, repositoryRoot), path.join(directory, "scripts/ci", name)); } + mkdirSync(path.join(directory, ".github/scripts"), { recursive: true }); + copyFileSync(new URL(".github/scripts/ci-inputs.mjs", repositoryRoot), path.join(directory, ".github/scripts/ci-inputs.mjs")); write(".gitignore", ".turbo/\ncalls.txt\nresult.txt\nbin/\n"); json("packages/fixture/package.json", { name: "fixture", private: true }); json("package.json", { name: "forced-cache-proof", private: true, packageManager: "npm@10.0.0", diff --git a/scripts/ci/execution-plan.mjs b/scripts/ci/execution-plan.mjs index 4d1357d8c8a..1e764294c87 100644 --- a/scripts/ci/execution-plan.mjs +++ b/scripts/ci/execution-plan.mjs @@ -1,5 +1,6 @@ import { execFileSync } from "node:child_process"; import { matchesGlob } from "node:path"; +import { productInputs } from "../../.github/scripts/ci-inputs.mjs"; export function sourceFiles(cwd = process.cwd()) { return [...new Set(execFileSync("git", ["ls-files", "--cached", "--others", "--exclude-standard", "-z"], @@ -30,7 +31,7 @@ export function executionPlan(config, task, args, files = sourceFiles()) { target = "@paperclipai/paperclip-runner#check:runner"; commandArgs = args; } else throw new Error(`Missing task command: ${task}`); - const select = (globs) => files.filter((file) => globs.some((glob) => matchesGlob(file, glob))).map(literalGlob); + const select = (globs) => productInputs(files).filter((file) => globs.some((glob) => matchesGlob(file, glob))).map(literalGlob); const selectedInputs = select(definition.inputs); const inputs = target.startsWith("//#") ? selectedInputs : selectedInputs.map((input) => `$TURBO_ROOT$/${input}`); diff --git a/scripts/ci/run-task.mjs b/scripts/ci/run-task.mjs index cc5ea03f666..b6a683f688d 100644 --- a/scripts/ci/run-task.mjs +++ b/scripts/ci/run-task.mjs @@ -4,6 +4,7 @@ import { createHash } from "node:crypto"; import { matchesGlob } from "node:path"; import { executionPlan, sourceFiles } from "./execution-plan.mjs"; import { loadSelection, selectionHash } from "./test-selection-manifest.mjs"; +import { productInputs } from "../../.github/scripts/ci-inputs.mjs"; const task = process.env.CI_TASK; if (!/^ci:[a-z-]+$/.test(task ?? "")) throw new Error("Invalid CI task"); @@ -27,7 +28,7 @@ writeFileSync(runtimeConfig, JSON.stringify(execution.config)); const classified = [...config.globalDependencies, ...Object.values(config.tasks).flatMap((entry) => entry.inputs)]; const unknownInputs = createHash("sha256"); let uncacheable = false; -for (const file of files) { +for (const file of productInputs(files)) { if (classified.some((glob) => matchesGlob(file, glob))) continue; unknownInputs.update(JSON.stringify(file)); try { diff --git a/scripts/ci/select-tests.mjs b/scripts/ci/select-tests.mjs index d6a913e7356..5ad76a10e24 100644 --- a/scripts/ci/select-tests.mjs +++ b/scripts/ci/select-tests.mjs @@ -4,11 +4,15 @@ import { fileURLToPath } from "node:url"; import path from "node:path"; import assert from "node:assert/strict"; import { refineTestPlan, selectionDurations } from "./test-selection-plan.mjs"; +import { productInputs } from "../../.github/scripts/ci-inputs.mjs"; +import { mergeDiff } from "../../.github/scripts/ci-diff.mjs"; export function selectionEligibility(plan) { if (plan?.version !== 1 || plan.mode !== "enabled" || plan.forceReason) return "Full/shadow/forced task plan"; - if (!Array.isArray(plan.changed) || !plan.changed.length || plan.changed.length >= 300) return "Incomplete comparison"; - for (const file of plan.changed) { + if (!Array.isArray(plan.changed) || !plan.changed.length) return "Incomplete comparison"; + const changed = productInputs(plan.changed); + if (!changed.length) return "Documentation-only diff"; + for (const file of changed) { if (!/^(?:ui\/src|server\/src|cli\/src|packages\/.+\/src)\/.+\.[cm]?[jt]sx?$/.test(file) || /(?:config|setup)|\.d\.[cm]?ts$/i.test(file) || file.split("/").includes("..")) return `Broad input: ${file}`; try { @@ -50,14 +54,15 @@ export async function createTestSelection(plan, head) { } const { selectProjectTests } = await import("./vitest-selection-graph.mjs"); const graphs = []; + const changed = productInputs(plan.changed); for (const project of projects) { - const graph = await selectProjectTests({ project, changed: plan.changed, onlyOwnChanges: true }); + const graph = await selectProjectTests({ project, changed, onlyOwnChanges: true }); graphs.push(graph); console.log(`${project}: ${graph.selected.length}/${graph.inventory.length} files, ${graph.alwaysRun.length} retained conservatively, ${graph.seconds.toFixed(1)}s`); for (const owner of Object.values(owners)) if (owner[project] === null) owner[project] = graph.inventory; } const covered = new Set(graphs.flatMap((graph) => graph.coveredChanges)); - for (const file of plan.changed) assert.ok(covered.has(file), `Change outside the resolved test graphs: ${file}`); + for (const file of changed) assert.ok(covered.has(file), `Change outside the resolved test graphs: ${file}`); const server = graphs.find((graph) => graph.project === "@paperclipai/server"); if (server) { const general = runnerInventory("general-server-without-chat"); @@ -87,10 +92,10 @@ async function main() { try { const head = git("rev-parse", "HEAD"); assert.equal(head, process.env.GITHUB_SHA, "Selection must describe the tested merge commit"); - assert.match(process.env.BASE_SHA ?? "", /^[a-f0-9]{40}$/); - const changed = execFileSync("git", ["diff", "--name-only", "--no-renames", "-z", process.env.BASE_SHA, head, "--"], - { encoding: "utf8", maxBuffer: 16 * 1024 * 1024 }).split("\0").filter(Boolean).sort(); - assert.deepEqual(changed, [...new Set(plan.changed)].sort(), "Local diff disagrees with the trusted routing diff"); + assert.ok(plan.diff, "Missing trusted merge diff"); + const local = mergeDiff(head, plan.diff.prHead); + assert.deepEqual(local.diff, plan.diff, "Selection must use the trusted merge parents"); + assert.deepEqual(local.changed, [...new Set(plan.changed)].sort(), "Local diff disagrees with the trusted routing diff"); assert.equal(git("status", "--porcelain", "--untracked-files=normal"), "", "Source inputs changed during dependency setup"); process.env.NODE_ENV = "test"; process.env.VITEST = "true"; diff --git a/scripts/ci/test-selection.integration.test.mjs b/scripts/ci/test-selection.integration.test.mjs index 9f74d860448..15442fcd097 100644 --- a/scripts/ci/test-selection.integration.test.mjs +++ b/scripts/ci/test-selection.integration.test.mjs @@ -8,6 +8,7 @@ import { selectProjectTests } from "./vitest-selection-graph.mjs"; import { partitionSelection, refineTestPlan } from "./test-selection-plan.mjs"; import { selectionHash, selectedShard, validateSelection } from "./test-selection-manifest.mjs"; import { selectionEligibility } from "./select-tests.mjs"; +import { mergeDiff } from "../../.github/scripts/ci-diff.mjs"; const root = process.cwd(); const head = execFileSync("git", ["rev-parse", "HEAD"], { encoding: "utf8" }).trim(); @@ -166,10 +167,13 @@ test("the CLI publishes the complete coarse plan if setup mutates tracked inputs const commit = () => git("-c", "user.name=CI", "-c", "user.email=ci@example.invalid", "commit", "-qam", "fixture"); git("init", "-q"); writeFileSync(file, "export const value = 1;"); git("add", "."); commit(); const base = git("rev-parse", "HEAD"); + git("checkout", "-qb", "pr"); writeFileSync(file, "export const value = 2;"); commit(); + const prHead = git("rev-parse", "HEAD"); + git("checkout", "--detach", base); git("-c", "user.name=CI", "-c", "user.email=ci@example.invalid", "merge", "--no-ff", "pr", "-m", "merge"); const merge = git("rev-parse", "HEAD"); writeFileSync(file, "export const value = 3; // generated during setup"); - const plan = { version: 1, mode: "enabled", forceReason: null, changed: ["ui/src/input.ts"], + const plan = { version: 1, mode: "enabled", forceReason: null, ...mergeDiff(merge, prHead, directory), jobs: { general_tests: { run: true, selected: 2, matrix: { include: [{ shard_index: 0 }, { shard_index: 1 }] } } } }; const result = spawnSync(process.execPath, [path.join(root, "scripts/ci/select-tests.mjs")], { cwd: directory, encoding: "utf8", env: { ...process.env, CI_PLAN: JSON.stringify(plan), BASE_SHA: base, @@ -181,3 +185,14 @@ test("the CLI publishes the complete coarse plan if setup mutates tracked inputs assert.deepEqual(fallback.jobs, plan.jobs); assert.equal(fallback.testSelection, undefined); }); + + +test("mixed audited documentation stays graph eligible and large complete source diffs are allowed", () => { + const plan = fixturePlan().plan; + for (const doc of ["doc/DEVELOPING.md", "doc/DATABASE.md", "doc/plans/2026-10-30-example.md"]) { + assert.equal(selectionEligibility({ ...plan, changed: [...plan.changed, doc] }), null); + } + assert.equal(selectionEligibility({ ...plan, changed: Array(350).fill(plan.changed[0]) }), null); + assert.ok(selectionEligibility({ ...plan, changed: ["doc/DEVELOPING.md"] })); + assert.ok(selectionEligibility({ ...plan, changed: [...plan.changed, "docs/api/issues.md"] })); +});