* fix(3582): stop the cold-tree fixture racing concurrent hook builds
Two tests in tests/gsd-check-update-worker-platform-gate.test.cjs failed a verification run
with `ENOENT: no such file or directory, lstat '/work/hooks/.dist-staging-20836'`. This is
a race I introduced in #3582, not a flake, and it passed when #3582 merged because it only
fires when the timing lines up.
buildColdInstallTree() copied the LIVE repo hooks/ directory with a filter that excluded
only the basename 'dist'. scripts/build-hooks.js writes atomically through a per-PID
staging dir, hooks/.dist-staging-<pid>, and removes it when finished — and the archived
build-hooks-atomic-write changeset records that NINE test files invoke build-hooks.js from
their before() hooks. So several test processes create and delete staging directories
inside hooks/ while other tests are reading it. cpSync enumerated one, and the owning
process removed it before cpSync got to it.
The helper's own header already states the rule it needed: hooks/dist is excluded because
it "is not present in a raw marketplace checkout either". hooks/.dist-staging-* is
gitignored (.gitignore:21) and equally absent from a raw checkout — it was simply missed.
Fixed by enumerating hooks/ explicitly and skipping 'dist' and any '.dist-staging' prefix
BY NAME, before anything stats or copies the entry, then copying each surviving entry
individually. A name-first skip means a vanishing staging dir is never touched at all.
Worth recording because it corrects the assumption this fix was written under: cpSync's
filter IS invoked before the entry is lstat'd, and returning false leaves it untouched
(verified by deleting inside the callback and returning false — no throw). So merely adding
'.dist-staging' to the old filter would also have closed the race. The explicit enumeration
was kept anyway so correctness does not depend on that Node implementation detail.
Proven by execution both ways: with a staging dir planted in hooks/, the OLD
cpSync-with-filter form copied it straight through into the fixture, while the new form
succeeds and produces no .dist-staging entry with the real hook set intact.
Regression test added beside the existing cold-tree tests: it plants a real
hooks/.dist-staging-test-<random>, asserts the fixture builds clean without it, and removes
only the directory it created.
Repo swept for the same exposure: this helper is the only place doing a bulk enumeration of
the whole live hooks/ tree. The other hooks/-touching tests reference specific named files
or hooks/dist/ and are not exposed. scripts/build-hooks.js is deliberately untouched — its
per-PID staging is what makes its own writes atomic and is correct.
Refs #3582
* fix(3582): make the race regression test hermetic instead of mutating the live tree
The regression test added in the previous commit failed the runner with "failed running
after hook", and it was wrong in two ways — the second one worse than the first.
cleanup() (tests/helpers.cjs:452-487) deliberately THROWS for any path outside the known
temp roots. The test planted hooks/.dist-staging-test-<random> inside the repo and then
asked cleanup() to remove it, so the after-hook threw. That guard is correct and is left
alone.
The real problem is that the test mutated the LIVE hooks/ directory while other test files
concurrently read it — the exact shared-state hazard this change exists to remove. A
regression test for a race must not introduce one.
buildColdInstallTree now takes an optional opts.repoRoot (defaulting to the real REPO_ROOT
and used for both copies it performs), so the test builds a fake repo root under the temp
dir, plants representative hooks plus dist/ and .dist-staging-99999/ THERE, and asserts the
fixture excludes both. All six pre-existing callers pass no arguments and are unaffected.
The test also asserts the real hooks/ listing is identical before and after, so a future
edit that reintroduces live-tree mutation fails loudly.
The name rule is now pinned directly rather than only through the copy. shouldCopyHookEntry
is exported and asserted, including the two cases a sloppier implementation would get
wrong: 'dist-staging-no-dot' and 'distant.js' must both be KEPT. Anything matching on a
loose 'dist' substring or startsWith passes every other case and fails those two.
Also corrected the issue number on the tests introduced here: they were labelled #3631,
which is the unrelated capability-consent bytecode work. This is #3582.
Verified by execution: the predicate rule holds on all nine cases; a fake-root fixture
yields exactly the representative hooks with dist and .dist-staging excluded; the no-arg
default still copies the real tree (29 entries); and the real hooks/ listing is byte-identical
before and after.
Refs #3582
* chore(3582): re-trigger CI after an orphaned Validate Branch Name run
The Validate Branch Name run for this branch (32211622051) sat queued from 03:17 and was
never picked up — updatedAt never advanced past createdAt while the same workflow completed
normally for other branches. `gh run rerun` refused it ("already running") and
`gh run cancel` returned HTTP 500, so the run is orphaned on the GitHub side.
Closing and reopening the PR re-fired the other pull_request workflows but not that one,
whose triggers evidently do not include reopened. An empty commit is the remaining way to
get a fresh run.
No file changes: the tree is identical to dc71534b6, whose remote-runner pass carries
forward unchanged.
Recording this rather than admin-merging past the pending check. Everything else was green
(24 pass, 0 fail), but admin merge is sanctioned only for the missing-secondary-reviewer
case, never to skip a gate that has not actually run.
Refs #3582
---------
Co-authored-by: sim <sim@local>
4e70b245e8