Skip to content

Commit d2efcc8

Browse files
author
probe
committed
fix(verify): put the browser acceptance records back when the run is over
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.
1 parent 798d409 commit d2efcc8

5 files changed

Lines changed: 192 additions & 2 deletions

File tree

‎packages/webui/test/integration/browser-use-real-input.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { afterAll, beforeAll, describe, expect, it } from "vitest";
2323

2424
import { WebuiIsolatedChromiumProvider } from "../../src/runtime/browser/isolated-chromium/provider.js";
2525
import type { WebuiBrowserTargetId } from "../../src/shared/contracts/browser.js";
26-
import { beginAcceptanceSuite, observeAcceptance } from "../browser/acceptance-record.js";
26+
import { beginAcceptanceSuite, observeAcceptance, restoreAcceptanceRecords } from "../browser/acceptance-record.js";
2727

2828
/** The record file this suite writes. */
2929
const SUITE = "isolated-chromium";
@@ -105,6 +105,9 @@ describe.skipIf(!available)("the browser input path, against a real page", () =>
105105
}, 60_000);
106106

107107
afterAll(async () => {
108+
// This suite rewrites the record file, and the file is tracked, so on CI
109+
// the run puts it back the way the checkout had it.
110+
restoreAcceptanceRecords();
108111
await provider?.close();
109112
if (dataDir) await rm(dataDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
110113
});

‎release/public-source.json‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎scripts/lib/acceptance-records.mjs‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
// The browser acceptance records, and putting them back.
2+
//
3+
// A record file is tracked: the plan's `complete/` directory is where the
4+
// accepted evidence lives, and the file in the checkout is the record of the
5+
// run that was accepted. A run rewrites it — every record carries the time and
6+
// the machine it ran on — so the working tree is dirty once the suite is over.
7+
//
8+
// That dirt is not a source change, and the release packaging refuses to run
9+
// with a dirty tree ("Packaging requires a clean committed working tree"), so
10+
// a CI run has to put the records back before it is over.
11+
//
12+
// The restore is git-based, and it restores the committed content of every
13+
// record file. The suites that write a record live in their own processes and
14+
// each of them restores what it wrote from the copy it kept in memory
15+
// (`restoreAcceptanceRecords` in the acceptance-record module); this one does
16+
// not depend on which process wrote what, so a suite that forgot to restore
17+
// its own cannot leave the tree dirty for the packaging step.
18+
//
19+
// Locally the records stay: they are what the run just produced, and the
20+
// published matrix is written from them.
21+
22+
import { spawnSync } from "node:child_process";
23+
import { realpathSync } from "node:fs";
24+
import path from "node:path";
25+
26+
// The tracked acceptance record files, wherever the plans put them. A plan that
27+
// adds a suite gets its record file covered without this file knowing the
28+
// suite's name.
29+
const RECORD_PATHSPEC = ":(glob).plan/*/complete/*.acceptance.json";
30+
31+
/**
32+
* Whether this is a CI run.
33+
*
34+
* `CI` is the variable the suites already test, and `GITHUB_ACTIONS` is the
35+
* one the runner always sets; either is enough to say the run's own records
36+
* are throwaway and the committed ones are the evidence.
37+
*/
38+
function isCi() {
39+
return process.env.CI === "true" || process.env.GITHUB_ACTIONS === "true";
40+
}
41+
42+
/**
43+
* The path as the filesystem sees it.
44+
*
45+
* Git answers with the real path, and a checkout reached through a symbolic
46+
* link — a temporary directory on macOS, for one — is not the same string as
47+
* the one the caller passed. Comparing the two as written would decide that
48+
* the checkout is somewhere else, and nothing would be restored.
49+
*/
50+
function realPath(target) {
51+
try {
52+
return realpathSync.native(target);
53+
} catch {
54+
return path.resolve(target);
55+
}
56+
}
57+
58+
/**
59+
* Whether `root` is the top of a git checkout.
60+
*
61+
* An exported source archive has no repository at all, and there is nothing to
62+
* restore there. A directory that merely happens to sit inside some other
63+
* repository is worse than that: git would answer for the wrong tree, so the
64+
* top level has to be this tree before anything is restored.
65+
*/
66+
function isCheckoutRoot(root) {
67+
const top = spawnSync("git", ["rev-parse", "--show-toplevel"], {
68+
cwd: root,
69+
encoding: "utf8",
70+
});
71+
if (top.status !== 0) return false;
72+
return realPath(top.stdout.trim()) === realPath(root);
73+
}
74+
75+
/**
76+
* The acceptance record files this checkout tracks.
77+
*
78+
* Read from the index rather than from a list of suite names, so a plan that
79+
* adds a suite is covered without this file being told about it.
80+
*/
81+
export function acceptanceRecordFiles(root) {
82+
if (!isCheckoutRoot(root)) return [];
83+
const listed = spawnSync("git", ["ls-files", "-z", "--", RECORD_PATHSPEC], {
84+
cwd: root,
85+
encoding: "utf8",
86+
});
87+
if (listed.status !== 0) return [];
88+
return listed.stdout.split("\0").filter((file) => file !== "");
89+
}
90+
91+
/**
92+
* Puts the acceptance records back the way the checkout had them.
93+
*
94+
* On CI only: locally the records are what the run just produced and they are
95+
* kept. Returns the files it restored.
96+
*/
97+
export function restoreAcceptanceRecords(root) {
98+
if (!isCi()) return [];
99+
const files = acceptanceRecordFiles(root);
100+
if (files.length === 0) return [];
101+
const restored = spawnSync(
102+
"git",
103+
["restore", "--source=HEAD", "--worktree", "--", ...files],
104+
{ cwd: root, encoding: "utf8" },
105+
);
106+
if (restored.status !== 0) {
107+
throw new Error(
108+
`Could not restore the acceptance records: ${restored.stderr.trim()}`,
109+
);
110+
}
111+
return files;
112+
}

‎scripts/verify.mjs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import path from "node:path";
1212
import { fileURLToPath } from "node:url";
1313
import { parseArgs } from "node:util";
1414

15+
import { restoreAcceptanceRecords } from "./lib/acceptance-records.mjs";
16+
1517
// Single definition of the full verification pipeline. GitHub CI runs `pnpm verify`
1618
// so a local run executes the same gates in the same order; previously the order
1719
// only existed inside the workflow file and could not be reproduced locally.
@@ -246,6 +248,22 @@ try {
246248
}
247249
report.status = failed ? "FAIL" : "PASS";
248250
} finally {
251+
// The browser acceptance records are tracked files that a test run rewrites,
252+
// and the release packaging runs in a separate process after this one and
253+
// refuses to run with a dirty tree. Put them back before the run is over, so
254+
// a verification run leaves the checkout the way it found it. A failure to
255+
// restore is reported rather than thrown: the gate that failed is the
256+
// interesting one, and the packaging step says so plainly if the tree is
257+
// still dirty.
258+
try {
259+
const restored = restoreAcceptanceRecords(root);
260+
if (restored.length > 0)
261+
console.log(`acceptance records restored: ${restored.length} file(s)`);
262+
} catch (error) {
263+
console.error(
264+
`could not restore the acceptance records: ${error instanceof Error ? error.message : String(error)}`,
265+
);
266+
}
249267
rmSync(temporary, { recursive: true, force: true });
250268
for (const gate of report.gates)
251269
if (gate.status === "NOT_RUN")

‎test/source-sync.test.mjs‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { copyMcodeToolsArtifact, downloadMcodeToolsArtifact, MCODE_TOOLS_ARTIFAC
2323
import { checkWindowsSourceLocation, runWindowsSourceLocationCheck } from '../scripts/check-windows-source-location.mjs';
2424
import { recordBuildIntegrity, verifyBuildIntegrity } from '../scripts/lib/webui-build-integrity.mjs';
2525
import { validateExceptionLedger, sourceFingerprint } from '../scripts/lib/webui-dependency-rules.mjs';
26+
import { acceptanceRecordFiles, restoreAcceptanceRecords } from '../scripts/lib/acceptance-records.mjs';
2627

2728
test('permanent loader approval follows tokens rather than line numbers', () => {
2829
const loader = 'const entry = new URL("./embedded/cli.mjs", import.meta.url); await import(entry.href);';
@@ -657,9 +658,16 @@ function verificationFixture(t) {
657658
const directory = mkdtempSync(path.join(tmpdir(), 'verify-fixture-'));
658659
t.after(() => rmSync(directory, { recursive: true, force: true }));
659660
const root = path.join(directory, 'source');
660-
mkdirSync(path.join(root, 'scripts'), { recursive: true });
661+
mkdirSync(path.join(root, 'scripts/lib'), { recursive: true });
661662
const verifier = path.join(root, 'scripts/verify.mjs');
662663
copyFileSync(new URL('../scripts/verify.mjs', import.meta.url), verifier);
664+
// The verifier imports the module that puts the acceptance records back, so
665+
// the fixture carries it the same way it carries a stub for the export step.
666+
// The fixture is not a checkout, so the restore finds nothing to do there.
667+
copyFileSync(
668+
new URL('../scripts/lib/acceptance-records.mjs', import.meta.url),
669+
path.join(root, 'scripts/lib/acceptance-records.mjs'),
670+
);
663671
const manager = path.join(directory, 'manager.cjs');
664672
writeFileSync(manager, `
665673
const fs = require('node:fs');
@@ -700,6 +708,48 @@ function verificationFixture(t) {
700708
};
701709
}
702710

711+
test('a verification run puts the browser acceptance records back the way the checkout had them', t => {
712+
const directory = mkdtempSync(path.join(tmpdir(), 'acceptance-records-'));
713+
t.after(() => rmSync(directory, { recursive: true, force: true }));
714+
const git = (...args) => execFileSync('git', ['-C', directory, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] });
715+
const commit = (...args) => git('-c', 'user.name=fixture', '-c', 'user.email=fixture@local', ...args);
716+
git('init');
717+
commit('commit', '--allow-empty', '-m', 'base');
718+
const record = '.plan/browser-use/complete/isolated-chromium.acceptance.json';
719+
mkdirSync(path.join(directory, path.dirname(record)), { recursive: true });
720+
writeFileSync(path.join(directory, record), '[]\n');
721+
commit('add', '--', record);
722+
commit('commit', '-m', 'the accepted record');
723+
const dirty = () => git('status', '--porcelain', '--untracked-files=no').trim();
724+
725+
assert.deepEqual(acceptanceRecordFiles(directory), [record]);
726+
727+
// A run rewrites the record: every record carries the time and the machine
728+
// it ran on, so the working tree is dirty once the suite is over.
729+
writeFileSync(path.join(directory, record), '[{"at":"2026-01-01T00:00:00.000Z"}]\n');
730+
assert.equal(dirty(), `M ${record}`);
731+
732+
const previous = { ci: process.env.CI, actions: process.env.GITHUB_ACTIONS };
733+
delete process.env.CI;
734+
delete process.env.GITHUB_ACTIONS;
735+
try {
736+
// Locally the records stay: they are what the run just produced, and the
737+
// published matrix is written from them.
738+
assert.deepEqual(restoreAcceptanceRecords(directory), []);
739+
assert.equal(dirty(), `M ${record}`);
740+
741+
// On CI they go back, so the release packaging finds a clean tree.
742+
process.env.CI = 'true';
743+
assert.deepEqual(restoreAcceptanceRecords(directory), [record]);
744+
assert.equal(dirty(), '');
745+
} finally {
746+
for (const [name, value] of [['CI', previous.ci], ['GITHUB_ACTIONS', previous.actions]]) {
747+
if (value === undefined) delete process.env[name];
748+
else process.env[name] = value;
749+
}
750+
}
751+
});
752+
703753
test('platform verification omits only the compiler gates and invalid profiles fail closed', t => {
704754
const f = verificationFixture(t);
705755
const full = f.run(['--list']);
@@ -991,6 +1041,12 @@ test('Windows contract profile selects focused gates', () => {
9911041
try {
9921042
const fixture = path.join(root, 'verify.mjs');
9931043
copyFileSync(new URL('../scripts/verify.mjs', import.meta.url), fixture);
1044+
// The verifier imports the module that puts the acceptance records back.
1045+
mkdirSync(path.join(root, 'lib'), { recursive: true });
1046+
copyFileSync(
1047+
new URL('../scripts/lib/acceptance-records.mjs', import.meta.url),
1048+
path.join(root, 'lib/acceptance-records.mjs'),
1049+
);
9941050
const preload = path.join(root, 'platform.cjs');
9951051
writeFileSync(preload, "Object.defineProperty(process, 'platform', { value: 'win32' });\n");
9961052
const result = spawnSync(process.execPath, ['--require', preload, fixture, '--profile', 'windows', '--list'], {

0 commit comments

Comments
 (0)