fix(bench): close two provenance-gate bypasses and guard the no-op run

git status --porcelain quotes any path with a space and prints renames as
'old -> new', so slicing the status prefix and prefix-testing the result missed
both: a daemon source renamed IN, or one with a space in its name, read as a
clean tree. Let git match instead -- -z (never quotes) with a pathspec -- and
cover config/scripts/build-orcad.mjs, which link 5 executes but link 1 did not
watch.

--cycles 0 and --output-bytes 0 are the siblings of the --sample-every 0 fix:
each leaves the churn loop doing no work while the summary still reports a
provenance-stamped zero-growth series. Reject both.

Also realpath repoRoot before the containment check (a symlinked worktree path
failed it closed) and add the bench:daemon-pty-churn-rss script entry its
siblings all have.
This commit is contained in:
Brennan Benson
2026-08-28 06:40:57 -07:00
parent d07744c539
commit 78f661fcac
3 changed files with 34 additions and 9 deletions
+1
View File
@@ -128,6 +128,7 @@
"bench:macos-computer-helper-owner-loss": "node config/scripts/macos-computer-helper-owner-loss-benchmark.mjs",
"bench:startup": "pnpm run ensure:electron-runtime && node tests/tools/benchmarks/startup-time-bench.mjs",
"bench:daemon-coldstart": "pnpm run ensure:electron-runtime && node tests/tools/benchmarks/daemon-coldstart-bench.mjs",
"bench:daemon-pty-churn-rss": "node tests/tools/benchmarks/daemon-pty-churn-rss-bench.mjs",
"bench:wsl-hook-relay-reattach": "pnpm run build:relay && node config/scripts/wsl-hook-relay-reattach-benchmark.mjs",
"bench:wsl-git-shell": "node config/scripts/wsl-git-shell-benchmark.mjs",
"bench:hang-watchdog-memory": "pnpm run ensure:electron-runtime && node config/scripts/hang-watchdog-memory-benchmark.mjs",
@@ -31,14 +31,28 @@ export function assertDaemonBuildProvenance(repoRoot) {
const headSha = git('rev-parse', 'HEAD')
const branch = git('rev-parse', '--abbrev-ref', 'HEAD')
const relevant = ['src/main/', 'src/shared/', 'config/patches/', 'pnpm-lock.yaml', 'package.json']
// NOTE: no .trim() on the whole output — porcelain lines start with a
// two-character status whose first column may be a meaningful space.
const dirty = execFileSync('git', ['status', '--porcelain'], { cwd: repoRoot, encoding: 'utf8' })
.split('\n')
.filter((line) => line.length > 3)
.map((line) => line.slice(3))
.filter((file) => relevant.some((prefix) => file.startsWith(prefix)))
const relevant = [
'src/main',
'src/shared',
'config/patches',
'config/scripts/build-orcad.mjs',
'pnpm-lock.yaml',
'package.json'
]
// Why -z and a pathspec rather than parsing paths out of the default format: the
// default quotes any path with a space or non-ASCII byte (core.quotePath), and a
// rename prints `old -> new`, so a prefix test on the sliced line misses both — a
// daemon source moved in, or one with a space in its name, would read as clean.
// With -z git never quotes, and letting git do the matching removes the parse.
const dirty = execFileSync('git', ['status', '--porcelain', '-z', '--', ...relevant], {
cwd: repoRoot,
encoding: 'utf8'
})
.split('\0')
// -z emits a bare `new\0old\0` pair for R/C; the status record is the one with
// the two-column prefix, and the trailing source path carries none.
.filter((record) => record.length > 3 && record[2] === ' ')
.map((record) => record.slice(3))
if (dirty.length > 0) {
throw new Error(
`PROVENANCE FAILURE: daemon-relevant paths are dirty; results would not be attributable to HEAD ${headSha}:\n ${dirty.join('\n ')}`
@@ -63,7 +77,7 @@ export function assertDaemonBuildProvenance(repoRoot) {
`PROVENANCE FAILURE: installed node-pty at ${installedRealRoot} was not built from the checked-out patch (expected patch_hash=${patchSha256}). Run pnpm install.`
)
}
if (!installedRealRoot.startsWith(repoRoot + sep)) {
if (!installedRealRoot.startsWith(realpathSync(repoRoot) + sep)) {
throw new Error(
`PROVENANCE FAILURE: node_modules/node-pty resolves outside this worktree (${installedRealRoot}); a sibling worktree's install cannot prove this HEAD`
)
@@ -72,6 +72,11 @@ function parseArgs(argv) {
break
case '--cycles':
args.cycles = num('--cycles')
// Zero cycles churns nothing: the loop never runs, `samples` holds only the
// baseline, and the summary reports a flat zero-growth series from no work.
if (args.cycles === 0) {
throw new Error('--cycles requires a positive integer')
}
break
case '--sample-every':
args.sampleEvery = num('--sample-every')
@@ -83,6 +88,11 @@ function parseArgs(argv) {
break
case '--output-bytes':
args.outputBytes = num('--output-bytes')
// Zero bytes leaves `command` null, so every cycle degenerates to bare
// create+kill and the RSS series flattens for lack of churn, not for lack of a leak.
if (args.outputBytes === 0) {
throw new Error('--output-bytes requires a positive integer')
}
break
case '--settle-ms':
args.settleMs = num('--settle-ms')