9263fa1e46ca7b0fc2645cf56fb59884120a83cd
4 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Tom Boucher
|
a28dcec981 |
chore(#597): replace count-based ratchet guards with AST lint + named-set allowlists (#603)
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>
|
||
|
Tom Boucher
|
3a98ce83e5 |
fix(3597): windows parity batch — sdk/install/hook/worktree clusters (21 files)
Three parallel diagnostic agents (G: sdk/install path-sep, H: hook scripts, I: worktree/workspace) characterised the remaining windows-22 failures on |
||
|
Tom Boucher
|
bf3c736029 |
feat(shell-projection): centralize managed hook command policy (#3450)
* feat(shell-projection): route persistent PATH hints through projection seam * refactor(shell-projection): unify managed hook command policy * fix(install): guard malformed settings hooks during uninstall * chore(changeset): add fragment for pr 3450 * fix(install): guard malformed settings hook entries |
||
|
Tom Boucher
|
807db75d55 |
fix(#2620): detect HOME-relative PATH entries before suggesting absolute export (#2625)
* fix(#2620): detect HOME-relative PATH entries before suggesting absolute export When the installer reported `gsd-sdk` not on PATH and suggested appending an absolute `export PATH="/home/user/.npm-global/bin:$PATH"` line to the user's rc file, a user who had the equivalent `export PATH="$HOME/.npm-global/bin:$PATH"` already in their shell profile would get a duplicate entry — the installer only compared the absolute form. Add `homePathCoveredByRc(globalBin, homeDir, rcFileNames?)` to `bin/install.js` and export it for test-mode callers. The helper scans `~/.zshrc`, `~/.bashrc`, `~/.bash_profile`, `~/.profile`, grepping each file for `export PATH=` / bare `PATH=` lines and substituting the common HOME forms (\$HOME, \${HOME}, leading ~/) with the real home directory before comparing each resolved PATH segment against globalBin. Trailing slashes are normalised so `.npm-global/bin/` matches `.npm-global/bin`. Missing / unreadable / malformed rc files are swallowed — the caller falls back to the existing absolute suggestion. Tests cover $HOME, \${HOME}, and ~/ forms, absolute match, trailing-slash match, commented-out lines, missing rc files, and unreadable rc files (directory where a file is expected). Closes #2620 * fix(#2620): skip relative PATH segments in homePathCoveredByRc CodeRabbit flagged that the helper unconditionally resolved every non-$-containing segment against homeAbs via path.resolve(homeAbs, …), which silently turns a bare relative segment like `bin` or `node_modules/.bin` into `$HOME/bin` / `$HOME/node_modules/.bin`. That is wrong: bare PATH segments depend on the shell's cwd at lookup time, not on $HOME — so the helper was returning true for rc files that do not actually cover globalBin. Guard the compare with path.isAbsolute(expanded) after HOME expansion. Only segments that are absolute on their own (or that became absolute via $HOME / \${HOME} / ~ substitution) are compared against targetAbs. Relative segments are skipped. Add two regression tests covering a bare `bin` segment and a nested `node_modules/.bin` segment; both previously returned true when home happened to contain a matching subdirectory and now correctly return false. Closes #2620 (CodeRabbit follow-up) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(#2620): wire homePathCoveredByRc into installer suggestion path CodeRabbit flagged that homePathCoveredByRc was added in the previous commit but never called from the installer, so the user-facing PATH warning stayed unchanged — users with `export PATH="$HOME/.npm-global/bin:$PATH"` in their rc would still get a duplicate absolute-path suggestion. Add `maybeSuggestPathExport(globalBin, homeDir)` that: - skips silently when globalBin is already on process.env.PATH; - prints a "try reopening your shell" diagnostic when homePathCoveredByRc returns true (the directory IS on PATH via an rc entry — just not in the current shell); - otherwise falls through to the absolute-path `echo 'export PATH="…:$PATH"' >> ~/.zshrc` suggestion. Call it from installSdkIfNeeded after the sdk/dist check succeeds, resolving globalBin via `npm prefix -g` (plus `/bin` on POSIX). Swallow any exec failure so the installer keeps working when npm is weird. Export maybeSuggestPathExport for tests. Add three new regression tests (installer-flow coverage per CodeRabbit nitpick): - rc covers globalBin via $HOME form → no absolute suggestion emitted - rc covers only an unrelated directory → absolute suggestion emitted - globalBin already on process.env.PATH → no output at all Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |