Mechanical rename produced by scripts/msd-rename.cjs: gsd/Gsd/GSD -> msd/Msd/MSD
across contents and paths, upstream package/repo coordinates -> @golem15/msd-core
and golem15com/msd-core. Deep links into upstream history, sibling upstream
packages, the GSD-2 import feature, CHANGELOG.md and .changeset/ are kept as-is.
Hand edits on top: MSD block-letter banner and logos, LICENSE copyright line,
package/plugin identity, regenerated lockfile, install-tree fixtures, derived
registries and benchmark baseline; migration checksum baseline re-locked
(MSD keeps its own install state, so no install had applied the old sums);
sort-order and regex-escaped expectations in tests adjusted.
The Windows node-24 scoped-test job hit its 15-minute ceiling twice on this
branch and GitHub reported both as cancelled, which is how a job timeout
surfaces. The same job finishes in about two minutes twenty on next, across
four consecutive runs. cleanup() is the only hot-path change here.
It was doing up to seven filesystem calls per invocation: three probes of the
temp root, two more for each conventional temp dir, then an existence check and
a realpath of the target. On Windows fs.realpathSync.native opens a file handle
and Defender charges for each one, and this runs in the teardown of effectively
every test.
The root candidates are now memoized on the live os.tmpdir() value. The key
matters: two files in the suite override TMPDIR mid-run and restore it, so a
plain module-level hoist would go stale for them, while re-reading os.tmpdir()
costs an env lookup. Measured over 20,000 calls: 546ms unmemoized, 10ms
memoized.
The symlink-escape check is removed rather than optimized, because it was
guarding something that cannot happen. Verified directly on node v26.5.1:
fs.rmSync(link, {recursive: true, force: true})
link exists false
victim exists true
file exists true
rmSync unlinks a top-level symlink and leaves its target alone, and a symlink
nested inside a tree being recursively removed is also unlinked rather than
followed. The check cost two filesystem calls per teardown on the slowest
platform in the matrix and bought nothing. Its test asserted the victim
survived, which was true with or without the guard.
What closed the original defect is untouched: a target outside the known temp
roots is still refused, before the chdir and before the rmSync, with the roots
named in the message.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A fourth failure of the same guard, found by review before it reached CI: the
config-schema property suite builds fixtures through a getWritableTmp() helper
that returns the first writable of /private/tmp, /tmp, os.tmpdir(). On macOS
that is /private/tmp while os.tmpdir() is /var/folders/.../T, so five cleanup()
calls in that file were refused outright.
The three earlier breakages were all spellings of one root. This one is not: the
suite legitimately uses more than one temp root, so the premise was wrong rather
than the encoding. tmpRootCandidates() now probes the conventional system temp
dirs alongside os.tmpdir(), each included only if it exists on the host, so the
accepted set stays a bounded explicit list instead of growing a patch per
platform.
Two corrections that follow from the same review:
A root that is itself a filesystem root already ends in a separator, and
appending another built `//`, which only the literal `/` satisfies — TMPDIR=/
would have refused every descendant. The separator is only appended when it is
not already there.
The refusal messages named os.tmpdir(), which stopped being the boundary. They
now name the roots actually compared against. Every failure of this guard so far
was diagnosed from that message in a CI log, so it should show what was checked
rather than a stale approximation of it.
The symlink-escape check keeps its refusal but drops its claim. fs.rmSync does
not follow a top-level symlink — it unlinks the link and leaves the target
alone — so that check was never closing a live escape, and the test asserting
the victim survived would have passed with the guard removed. Both now say what
is true: a defense-in-depth boundary against a future change to the deletion
mechanism, with only the refusal itself load-bearing.
The QA path helpers were evaluated for reuse rather than keeping a third copy of
path containment. They resolve against one project directory and have no
multi-root or Windows short-name handling, so they are not a drop-in; noted
rather than forced.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An isolated review found the safety precondition was not safe. Test 4 uses the
repo's own tests/ directory as a path that must never be deleted, and asserted
beforehand that it sits outside the temp root — but it computed that root as
path.resolve(os.tmpdir()) alone, while cleanup() accepts a target under any of
several root spellings. For a checkout under the realpath'd temp root the
precondition reads "outside, safe to proceed" while the guard reads "inside,
delete it", and rmSync runs on tests/ before the assertion fails. The commit
that added it claimed it would fail loudly instead of destructively; it would
have done the opposite.
The fix is not a second root in the test. tmpRootCandidates() is exported and
the precondition calls it, so there is one source of truth and nothing left to
drift. A mirror was the defect, not its contents.
The same review bounded what the refusal actually guarantees: the check is a
string prefix test, so a symlink living under tmpdir but pointing outside it
passes while rmSync follows the link and deletes the real directory. When the
target exists its real path is now checked too, against the same predicate —
factored into one function so the two comparisons cannot diverge the way the
test's copy did. A realpath failure refuses rather than proceeds; a safety
check that cannot verify must not report safe, which is the whole subject of
this branch. Missing targets are skipped, since rmSync with force no-ops on
them and realpathSync would only throw ENOENT.
`const isTmpPath = true` is gone. It survived the previous commit as a way to
keep the catch's `&& isTmpPath` reading as a real condition, but a constant
dressed as a test states nothing; the guard clauses above throw, so the catch
comment now says the invariant in words instead.
The new coverage does not depend on the platform the bug lives on. The root
list is asserted directly — non-empty, absolute, deduped, and containing a
freshly created temp dir — and the symlink refusal runs everywhere, skipping
only where symlink creation is unavailable. The previous realpath test was
coverage-identical to the control on Linux, so the only lanes the matrix runs
could not have verified the fix it was written for.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Windows CI refused legitimate temp directories for the same reason macOS did,
one commit earlier:
cleanup() refused to remove a path outside os.tmpdir():
C:\Users\runneradmin\AppData\Local\Temp\bug-3491-7zisup
GitHub's windows runners report os.tmpdir() in the 8.3 SHORT form
(C:\Users\RUNNER~1\AppData\Local\Temp) while callers hold the expanded LONG
form. fs.realpathSync() does not reliably expand 8.3 names; only
fs.realpathSync.native() does. Casing can differ independently (C:\ vs c:\).
Two platform-specific breakages from the same check is a sign the check was
written against one spelling rather than the concept, so this stops patching
symptoms. tmpRootCandidates() collects every variant derivable from
os.tmpdir() — resolved, realpath'd, and native-realpath'd — each probe isolated
in its own try/catch so an unavailable variant contributes nothing instead of
crashing teardown, then deduped. A target is accepted under any of them, and
the comparison folds case on win32 only, where casing genuinely varies. The
error message still prints the original-case path.
On this machine three variants collapse to two: /var/folders/.../T and
/private/var/folders/.../T. Both spellings of a real temp dir are accepted and
removed.
The Linux matrix passed every one of these broken states — 30544 and then
30545, both lanes green — because /tmp has neither symlink indirection nor
short names. The platform CI shards are the only thing that has caught any of
it, which is worth stating plainly given what this branch is about.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The guard added in the previous commit refused legitimate temp directories on
macOS. os.tmpdir() returns a path under /var/folders/..., /var is a symlink to
/private/var, and path.resolve() does not resolve symlinks. So a caller that
passed the realpath'd form — via fs.realpathSync(), or via process.cwd() after
chdir-ing into a temp dir, which returns the resolved path — produced
/private/var/folders/... and failed a check written against /var/folders/...
Measured on this machine before the fix:
mkdtemp path /var/folders/.../probe-XXX allowed
fs.realpathSync of the same dir /private/var/folders/.../probe-XXX REFUSED
process.cwd() after chdir to it /private/var/folders/.../probe-XXX REFUSED
The accepted-roots set is now built from both the resolved and realpath'd forms
of os.tmpdir(), deduped — on Linux they are identical, so the set collapses to
one entry and nothing changes there. realpathSync is wrapped because it throws
if the root is momentarily missing, and a safety check must not become a new
crash. The target itself is deliberately NOT realpath'd: cleanup() is called on
already-deleted directories, where realpathSync raises ENOENT.
The roots are computed per call rather than hoisted to module scope, because two
test files override TMPDIR and a hoisted value would go stale for them.
Worth naming: the remote matrix runs linux-node22 and linux-node24 only, and it
passed 30544/30544 on the broken commit. os.tmpdir() is /tmp on Linux with no
symlink indirection, so that matrix could not have caught this at any sample
size. The regression test added here branches on whether realpath differs from
the original path, so it exercises the real case on macOS and stays meaningful
rather than vacuous on Linux.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cleanup() computed `isTmpPath` — the exact predicate for "is this path safe to
delete" — and consulted it only inside the catch block, to classify a transient
Windows error. The rmSync above it ran unconditionally against whatever path it
was handed, with `recursive: true, force: true`. The guard existed and the
destructive call never asked it.
The chdir at the top of the function makes the failure mode worse rather than
better: a wrong target first moves the process out of the tree, then deletes it.
So the predicate moves above both, and an out-of-tmpdir path is refused before
either can run. The refusal throws and names the path; returning quietly would
reproduce the fail-open shape this wave exists to remove.
The catch keeps its `&& isTmpPath` term. It is now always true, but it states
the condition the swallow depends on rather than inheriting it from a check
twenty lines up, and it stays correct if the guard is ever relaxed.
All 300+ call sites resolve under os.tmpdir() today, including the two files
that override TMPDIR — both create their override root through the real
os.tmpdir() first — so nothing legitimate is refused.
The regression test targets tests/ itself: a real directory that must never be
deleted, so nothing is created and nothing needs tearing down. It asserts the
throw names the path, that cwd is unchanged (the chdir hazard), and that a known
file inside still exists — proving the directory was not emptied rather than
merely still present.
Whether tests/ is outside os.tmpdir() is environment-dependent: os.tmpdir() is
/tmp on Linux, so a checkout under /tmp would put it inside, and there a correct
guard would delete this directory rather than refuse it. The test asserts that
precondition before calling cleanup, so that environment fails loudly instead of
destructively.
Refs #3057
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pay down pre-existing error→warn lint debt. Removes dead imports/vars, unused functions, redundant regex/string escapes, and stale eslint-disable directives; converts unused `catch (_e)` to optional catch binding (src/*.cts).
No behavior change. Lint 345→125 warnings (0 errors); deferred categories (n/no-process-exit, test-sleeps, control-regex) tracked in #732 for follow-up. Full test suite green (0 failures); code-review verified all removals unused and all escape fixes semantics-preserving.
Closes#732
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The windows-test-parity ratchet greps test source for fs.rmSync-without-
maxRetries (and six other Windows-portability anti-patterns), failing when an
integer offender COUNT exceeds a frozen baseline (rmSync: 95). A count ratchet
is a Goodhart metric: fixing one offender and adding another keeps the count
constant, so a new defect slips through green. Replace it — and every other
count ratchet in the repo — with a layered, masking-proof design.
Behavioral seam test
- tests/helpers-cleanup.test.cjs proves helpers.cleanup() carries the Windows
EBUSY retry budget. cleanup() delegates retries to Node's fs.rmSync via
maxRetries (it owns no loop), so the test asserts the option contract
(recursive/force/maxRetries>0/retryDelay>0) + real-FS removal + the cwd-guard,
rather than a loop that does not exist. The EBUSY risk is now tested ONCE at
the helper, not approximated textually at every call site.
Write-time ESLint rule (AST-accurate, replaces the grep)
- eslint-rules/no-raw-rmsync-in-tests.cjs (error in tests/**/*.test.cjs) bans
raw fs.rmSync, steering to cleanup(). Catches member, computed (fs['rmSync']),
destructured and aliased forms; escape hatch is inline
`// eslint-disable-next-line local/no-raw-rmsync-in-tests -- <reason>` only.
- Migrated 336 raw fs.rmSync teardown calls across ~116 test files to cleanup().
~18 genuinely load-bearing sites (mid-test SUT/fault-injection removals,
error-swallowing or name-colliding local teardown helpers) keep the raw call
with an inline eslint-disable + reason.
Shared anti-ratchet primitive
- scripts/lib/allowlist-ratchet.cjs:
- assertWithinAllowlist: fails on NOVEL ids (new offender introduced) AND on
STALE ids (a known offender was fixed but not pruned) — identity, not count,
and a ratchet DOWN toward zero.
- assertTightCeiling: a size/length budget whose ceiling must stay within a
grace band of the high-water mark, so budgets may only tighten, never creep.
Ratchets converted onto the primitive
- windows-test-parity-guard.test.cjs: rmSync rule deleted (now ESLint-enforced);
the remaining six patterns moved from integer baselines to named-set
allowlists with ratchet-down.
- scripts/lint-test-file-count.{cjs,allowlist.json}: per-module integer counts →
named filename sets (closes the swap-a-file-keep-the-count blind spot); a
module dropping under cap now FAILS to force pruning its allowlist entry.
- enh-2790 skill-count `<= 63` → named skill allowlist (ratchets toward ~58).
Size budgets hardened (tighten-only)
- agent-size / workflow-size / feat-3039 help-tiered: ceilings lowered to the
current high-water mark and an assertTightCeiling anti-creep check added per
tier. Fixed external-contract limits (description ≤100 chars, agent ≤100 KB)
are intentionally left as-is — they are not grandfathered creeping budgets.
No user-facing behavior change (tests + tooling only); no USER_FACING_PREFIXES
touched, so no changeset fragment is required.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
