From 14845e210e45b6014dce4c401748c45b3e19c8f3 Mon Sep 17 00:00:00 2001 From: probe Date: Fri, 23 Oct 2026 06:21:40 +0800 Subject: [PATCH] fix(verify): put the browser acceptance records back when the run is over MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A verification run rewrites the tracked acceptance records — every record carries the time and the machine it ran on — so the working tree is dirty once the suites are over. The release packaging runs in a separate process afterwards and refuses to run with a dirty tree, so the CLI release build failed with "Packaging requires a clean committed working tree". The suites that write a record restore what they wrote, but one of them did not: the isolated-chromium suite in browser-use-real-input.test.ts. It only runs where the system Chrome sits on a Linux path, so the gap only ever showed up on the Linux CI job. Two things now hold the tree: - the suite restores its own record, the way the other four do; - the verification run restores every tracked record from HEAD when it is over, so a suite that forgets cannot break the packaging gate again. The restore is git-based on purpose: it restores the committed content of every record file, and it does not depend on which process wrote it. Locally the records stay — they are what the run just produced, and the published matrix is written from them. --- .../browser-use-real-input.test.ts | 5 +- release/public-source.json | 1 + scripts/lib/acceptance-records.mjs | 112 ++++++++++++++++++ scripts/verify.mjs | 18 +++ test/source-sync.test.mjs | 65 +++++++++- 5 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 scripts/lib/acceptance-records.mjs diff --git a/packages/webui/test/integration/browser-use-real-input.test.ts b/packages/webui/test/integration/browser-use-real-input.test.ts index e37f12989..624832078 100644 --- a/packages/webui/test/integration/browser-use-real-input.test.ts +++ b/packages/webui/test/integration/browser-use-real-input.test.ts @@ -23,7 +23,7 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { WebuiIsolatedChromiumProvider } from "../../src/runtime/browser/isolated-chromium/provider.js"; import type { WebuiBrowserTargetId } from "../../src/shared/contracts/browser.js"; -import { beginAcceptanceSuite, observeAcceptance } from "../browser/acceptance-record.js"; +import { beginAcceptanceSuite, observeAcceptance, restoreAcceptanceRecords } from "../browser/acceptance-record.js"; /** The record file this suite writes. */ const SUITE = "isolated-chromium"; @@ -105,6 +105,9 @@ describe.skipIf(!available)("the browser input path, against a real page", () => }, 60_000); afterAll(async () => { + // This suite rewrites the record file, and the file is tracked, so on CI + // the run puts it back the way the checkout had it. + restoreAcceptanceRecords(); await provider?.close(); if (dataDir) await rm(dataDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); }); diff --git a/release/public-source.json b/release/public-source.json index 428a90bd9..9b40bb4bf 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -4473,6 +4473,7 @@ "scripts/ci-changes.mjs", "scripts/export-source-preview.mjs", "scripts/gen-tsconfig-paths.mjs", + "scripts/lib/acceptance-records.mjs", "scripts/lib/builtin-skills.mjs", "scripts/lib/cli-release.mjs", "scripts/lib/local-runtime-assets.mjs", diff --git a/scripts/lib/acceptance-records.mjs b/scripts/lib/acceptance-records.mjs new file mode 100644 index 000000000..9a517b103 --- /dev/null +++ b/scripts/lib/acceptance-records.mjs @@ -0,0 +1,112 @@ +// The browser acceptance records, and putting them back. +// +// A record file is tracked: the plan's `complete/` directory is where the +// accepted evidence lives, and the file in the checkout is the record of the +// run that was accepted. A run rewrites it — every record carries the time and +// the machine it ran on — so the working tree is dirty once the suite is over. +// +// That dirt is not a source change, and the release packaging refuses to run +// with a dirty tree ("Packaging requires a clean committed working tree"), so +// a CI run has to put the records back before it is over. +// +// The restore is git-based, and it restores the committed content of every +// record file. The suites that write a record live in their own processes and +// each of them restores what it wrote from the copy it kept in memory +// (`restoreAcceptanceRecords` in the acceptance-record module); this one does +// not depend on which process wrote what, so a suite that forgot to restore +// its own cannot leave the tree dirty for the packaging step. +// +// Locally the records stay: they are what the run just produced, and the +// published matrix is written from them. + +import { spawnSync } from "node:child_process"; +import { realpathSync } from "node:fs"; +import path from "node:path"; + +// The tracked acceptance record files, wherever the plans put them. A plan that +// adds a suite gets its record file covered without this file knowing the +// suite's name. +const RECORD_PATHSPEC = ":(glob).plan/*/complete/*.acceptance.json"; + +/** + * Whether this is a CI run. + * + * `CI` is the variable the suites already test, and `GITHUB_ACTIONS` is the + * one the runner always sets; either is enough to say the run's own records + * are throwaway and the committed ones are the evidence. + */ +function isCi() { + return process.env.CI === "true" || process.env.GITHUB_ACTIONS === "true"; +} + +/** + * The path as the filesystem sees it. + * + * Git answers with the real path, and a checkout reached through a symbolic + * link — a temporary directory on macOS, for one — is not the same string as + * the one the caller passed. Comparing the two as written would decide that + * the checkout is somewhere else, and nothing would be restored. + */ +function realPath(target) { + try { + return realpathSync.native(target); + } catch { + return path.resolve(target); + } +} + +/** + * Whether `root` is the top of a git checkout. + * + * An exported source archive has no repository at all, and there is nothing to + * restore there. A directory that merely happens to sit inside some other + * repository is worse than that: git would answer for the wrong tree, so the + * top level has to be this tree before anything is restored. + */ +function isCheckoutRoot(root) { + const top = spawnSync("git", ["rev-parse", "--show-toplevel"], { + cwd: root, + encoding: "utf8", + }); + if (top.status !== 0) return false; + return realPath(top.stdout.trim()) === realPath(root); +} + +/** + * The acceptance record files this checkout tracks. + * + * Read from the index rather than from a list of suite names, so a plan that + * adds a suite is covered without this file being told about it. + */ +export function acceptanceRecordFiles(root) { + if (!isCheckoutRoot(root)) return []; + const listed = spawnSync("git", ["ls-files", "-z", "--", RECORD_PATHSPEC], { + cwd: root, + encoding: "utf8", + }); + if (listed.status !== 0) return []; + return listed.stdout.split("\0").filter((file) => file !== ""); +} + +/** + * Puts the acceptance records back the way the checkout had them. + * + * On CI only: locally the records are what the run just produced and they are + * kept. Returns the files it restored. + */ +export function restoreAcceptanceRecords(root) { + if (!isCi()) return []; + const files = acceptanceRecordFiles(root); + if (files.length === 0) return []; + const restored = spawnSync( + "git", + ["restore", "--source=HEAD", "--worktree", "--", ...files], + { cwd: root, encoding: "utf8" }, + ); + if (restored.status !== 0) { + throw new Error( + `Could not restore the acceptance records: ${restored.stderr.trim()}`, + ); + } + return files; +} diff --git a/scripts/verify.mjs b/scripts/verify.mjs index 97f2bfa19..9bee87314 100644 --- a/scripts/verify.mjs +++ b/scripts/verify.mjs @@ -12,6 +12,8 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { parseArgs } from "node:util"; +import { restoreAcceptanceRecords } from "./lib/acceptance-records.mjs"; + // Single definition of the full verification pipeline. GitHub CI runs `pnpm verify` // so a local run executes the same gates in the same order; previously the order // only existed inside the workflow file and could not be reproduced locally. @@ -246,6 +248,22 @@ try { } report.status = failed ? "FAIL" : "PASS"; } finally { + // The browser acceptance records are tracked files that a test run rewrites, + // and the release packaging runs in a separate process after this one and + // refuses to run with a dirty tree. Put them back before the run is over, so + // a verification run leaves the checkout the way it found it. A failure to + // restore is reported rather than thrown: the gate that failed is the + // interesting one, and the packaging step says so plainly if the tree is + // still dirty. + try { + const restored = restoreAcceptanceRecords(root); + if (restored.length > 0) + console.log(`acceptance records restored: ${restored.length} file(s)`); + } catch (error) { + console.error( + `could not restore the acceptance records: ${error instanceof Error ? error.message : String(error)}`, + ); + } rmSync(temporary, { recursive: true, force: true }); for (const gate of report.gates) if (gate.status === "NOT_RUN") diff --git a/test/source-sync.test.mjs b/test/source-sync.test.mjs index eb6cdfa88..e4ad2a71c 100644 --- a/test/source-sync.test.mjs +++ b/test/source-sync.test.mjs @@ -23,6 +23,7 @@ import { copyMcodeToolsArtifact, downloadMcodeToolsArtifact, MCODE_TOOLS_ARTIFAC import { checkWindowsSourceLocation, runWindowsSourceLocationCheck } from '../scripts/check-windows-source-location.mjs'; import { recordBuildIntegrity, verifyBuildIntegrity } from '../scripts/lib/webui-build-integrity.mjs'; import { validateExceptionLedger, sourceFingerprint } from '../scripts/lib/webui-dependency-rules.mjs'; +import { acceptanceRecordFiles, restoreAcceptanceRecords } from '../scripts/lib/acceptance-records.mjs'; test('permanent loader approval follows tokens rather than line numbers', () => { const loader = 'const entry = new URL("./embedded/cli.mjs", import.meta.url); await import(entry.href);'; @@ -657,9 +658,16 @@ function verificationFixture(t) { const directory = mkdtempSync(path.join(tmpdir(), 'verify-fixture-')); t.after(() => rmSync(directory, { recursive: true, force: true })); const root = path.join(directory, 'source'); - mkdirSync(path.join(root, 'scripts'), { recursive: true }); + mkdirSync(path.join(root, 'scripts/lib'), { recursive: true }); const verifier = path.join(root, 'scripts/verify.mjs'); copyFileSync(new URL('../scripts/verify.mjs', import.meta.url), verifier); + // The verifier imports the module that puts the acceptance records back, so + // the fixture carries it the same way it carries a stub for the export step. + // The fixture is not a checkout, so the restore finds nothing to do there. + copyFileSync( + new URL('../scripts/lib/acceptance-records.mjs', import.meta.url), + path.join(root, 'scripts/lib/acceptance-records.mjs'), + ); const manager = path.join(directory, 'manager.cjs'); writeFileSync(manager, ` const fs = require('node:fs'); @@ -700,6 +708,55 @@ function verificationFixture(t) { }; } +test('a verification run puts the browser acceptance records back the way the checkout had them', t => { + const directory = mkdtempSync(path.join(tmpdir(), 'acceptance-records-')); + t.after(() => rmSync(directory, { recursive: true, force: true })); + const git = (...args) => execFileSync('git', ['-C', directory, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + const commit = (...args) => git('-c', 'user.name=fixture', '-c', 'user.email=fixture@local', ...args); + git('init'); + // The repository's own attributes, so the checkout writes the records the + // way the repository writes them. Without them a Windows machine would + // check the file out with CRLF, and git would keep calling it modified + // after it had been restored. + copyFileSync(new URL('../.gitattributes', import.meta.url), path.join(directory, '.gitattributes')); + commit('add', '--', '.gitattributes'); + commit('commit', '-m', 'the attributes'); + commit('commit', '--allow-empty', '-m', 'base'); + const record = '.plan/browser-use/complete/isolated-chromium.acceptance.json'; + mkdirSync(path.join(directory, path.dirname(record)), { recursive: true }); + writeFileSync(path.join(directory, record), '[]\n'); + commit('add', '--', record); + commit('commit', '-m', 'the accepted record'); + const dirty = () => git('status', '--porcelain', '--untracked-files=no').trim(); + + assert.deepEqual(acceptanceRecordFiles(directory), [record]); + + // A run rewrites the record: every record carries the time and the machine + // it ran on, so the working tree is dirty once the suite is over. + writeFileSync(path.join(directory, record), '[{"at":"2026-01-01T00:00:00.000Z"}]\n'); + assert.equal(dirty(), `M ${record}`); + + const previous = { ci: process.env.CI, actions: process.env.GITHUB_ACTIONS }; + delete process.env.CI; + delete process.env.GITHUB_ACTIONS; + try { + // Locally the records stay: they are what the run just produced, and the + // published matrix is written from them. + assert.deepEqual(restoreAcceptanceRecords(directory), []); + assert.equal(dirty(), `M ${record}`); + + // On CI they go back, so the release packaging finds a clean tree. + process.env.CI = 'true'; + assert.deepEqual(restoreAcceptanceRecords(directory), [record]); + assert.equal(dirty(), ''); + } finally { + for (const [name, value] of [['CI', previous.ci], ['GITHUB_ACTIONS', previous.actions]]) { + if (value === undefined) delete process.env[name]; + else process.env[name] = value; + } + } +}); + test('platform verification omits only the compiler gates and invalid profiles fail closed', t => { const f = verificationFixture(t); const full = f.run(['--list']); @@ -991,6 +1048,12 @@ test('Windows contract profile selects focused gates', () => { try { const fixture = path.join(root, 'verify.mjs'); copyFileSync(new URL('../scripts/verify.mjs', import.meta.url), fixture); + // The verifier imports the module that puts the acceptance records back. + mkdirSync(path.join(root, 'lib'), { recursive: true }); + copyFileSync( + new URL('../scripts/lib/acceptance-records.mjs', import.meta.url), + path.join(root, 'lib/acceptance-records.mjs'), + ); const preload = path.join(root, 'platform.cjs'); writeFileSync(preload, "Object.defineProperty(process, 'platform', { value: 'win32' });\n"); const result = spawnSync(process.execPath, ['--require', preload, fixture, '--profile', 'windows', '--list'], {