From a28dcec981e70ded4a1f4ae856af463992e72727 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 1 Jun 2026 22:43:49 -0400 Subject: [PATCH] chore(#597): replace count-based ratchet guards with AST lint + named-set allowlists (#603) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 -- ` 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 --- eslint-rules/no-raw-rmsync-in-tests.cjs | 198 ++++++++++++ eslint.config.mjs | 6 +- scripts/lib/allowlist-ratchet.cjs | 136 ++++++++ scripts/lint-test-file-count.allowlist.json | 159 ++++++++-- scripts/lint-test-file-count.cjs | 111 +++++-- ...05-w006-i001-cjs-drift-regression.test.cjs | 8 +- .../4-phase-complete-cjs-regression.test.cjs | 6 +- .../6-validate-cjs-drift-regression.test.cjs | 8 +- tests/active-workstream-store.test.cjs | 5 +- tests/affected-tests-lib.test.cjs | 26 +- tests/agent-size-budget.test.cjs | 45 ++- tests/agent-skills.test.cjs | 2 +- tests/allowlist-ratchet.test.cjs | 297 ++++++++++++++++++ ...lease-tarball-smoke-explicit-home.test.cjs | 6 +- tests/bug-1834-sh-hooks-installed.test.cjs | 1 + tests/bug-1908-uninstall-manifest.test.cjs | 1 + .../bug-1924-preserve-user-artifacts.test.cjs | 1 + tests/bug-1967-cache-invalidation.test.cjs | 3 +- tests/bug-211-launcher-home-fallback.test.cjs | 9 +- tests/bug-2136-sh-hook-version.test.cjs | 1 + ...ug-2256-model-overrides-transport.test.cjs | 10 +- tests/bug-260-worktree-path-guard.test.cjs | 5 +- .../bug-261-worktree-force-add-guard.test.cjs | 18 +- .../bug-2760-codex-install-defensive.test.cjs | 20 +- tests/bug-2771-user-profile-manifest.test.cjs | 2 +- ...-opencode-model-profile-overrides.test.cjs | 13 +- ...ug-2831-opencode-home-path-prefix.test.cjs | 3 +- ...2836-audit-open-summary-uat-drift.test.cjs | 15 +- ...916-handle-branching-default-base.test.cjs | 8 +- ...config-get-context-window-default.test.cjs | 3 +- .../bug-2969-verify-reapply-patches.test.cjs | 5 +- ...bug-2973-profile-user-skills-path.test.cjs | 12 +- ...ug-2995-post-install-script-paths.test.cjs | 3 +- .../bug-2998-pristine-dir-populated.test.cjs | 15 +- ...3127-state-begin-phase-idempotent.test.cjs | 9 +- ...2-execute-phase-stall-safe-resume.test.cjs | 3 +- tests/bug-3245-codex-toml-floats.test.cjs | 5 +- ...ug-3285-codex-hooks-state-allowed.test.cjs | 3 +- .../bug-3407-pristine-stale-content.test.cjs | 3 +- .../bug-3441-path-action-projection.test.cjs | 5 +- ...-3442-shim-projection-drift-guard.test.cjs | 5 +- tests/bug-3509-path-spaces.test.cjs | 6 +- ...added-id-against-unpadded-roadmap.test.cjs | 18 +- ...42-executor-git-stash-prohibition.test.cjs | 3 +- ...nfiguration-manifest-install-path.test.cjs | 4 +- .../bug-3584-runtime-slash-formatter.test.cjs | 7 +- tests/bug-3631-router-raw-flag.test.cjs | 5 +- ...fy-reapply-patches-pristine-drift.test.cjs | 5 +- ...rsor-local-install-migration-lock.test.cjs | 5 +- ...3683-command-colon-namespace-leak.test.cjs | 5 +- ...683-workflow-colon-namespace-leak.test.cjs | 9 +- ...06-ui-safety-gate-false-positives.test.cjs | 9 +- .../bug-3707-locked-worktree-cleanup.test.cjs | 7 +- ...g-376-claude-js-hook-gsd-rewriter.test.cjs | 9 +- ...-state-preserve-executor-authored.test.cjs | 15 +- ...-install-defaults-test-mode-guard.test.cjs | 3 +- tests/bug-416-archive-dir-null.test.cjs | 8 +- ...444-resolver-local-claude-install.test.cjs | 9 +- ...g-474-clock-seam-date-determinism.test.cjs | 2 +- tests/changeset-cli.test.cjs | 5 +- tests/changeset-new.test.cjs | 3 +- tests/check-update-config-dir.test.cjs | 3 +- tests/ci-rebase-check.test.cjs | 3 +- tests/claude-skills-migration.test.cjs | 9 +- tests/codex-config.test.cjs | 13 +- tests/config-get-default.test.cjs | 3 +- tests/copilot-install.test.cjs | 14 +- tests/core.test.cjs | 12 +- tests/dispatch/trace-correlation.test.cjs | 5 +- .../enh-2538-statusline-last-command.test.cjs | 3 +- tests/enh-2789-description-budget.test.cjs | 3 +- tests/enh-2790-skill-consolidation.test.cjs | 95 +++++- tests/eslint-rules.test.cjs | 174 +++++++++- tests/feat-2795-update-banner.test.cjs | 21 +- tests/feat-3023-model-phase-types.test.cjs | 4 +- tests/feat-3024-dynamic-routing.test.cjs | 4 +- tests/feat-3039-help-tiered.test.cjs | 33 +- tests/feat-3210-fallow-integration.test.cjs | 12 +- ...command-aliases-manifest-coverage.test.cjs | 9 +- tests/feat-3262-scan-phase-plans.test.cjs | 3 +- ...5-fs-fault-injection-atomic-write.test.cjs | 28 +- ...443-effort-install-wiring.install.test.cjs | 13 +- tests/feat-488-effort-sync.test.cjs | 18 +- tests/feat-49-model-policy-presets.test.cjs | 4 +- tests/forensics.test.cjs | 3 +- tests/graphify-auto-update.test.cjs | 9 +- tests/graphify-visualization.test.cjs | 4 +- tests/gsd-statusline.test.cjs | 3 +- tests/helpers-cleanup.test.cjs | 101 ++++++ tests/hermes-skills-migration.test.cjs | 6 +- tests/hooks-opt-in.test.cjs | 5 +- tests/install-minimal-hooks.test.cjs | 36 +-- tests/install-path-detection.test.cjs | 1 + tests/install-update-marker.test.cjs | 22 +- tests/installer-migrations.test.cjs | 1 + ...issue-2517-runtime-aware-profiles.test.cjs | 2 +- ...ue-2639-codex-toml-neutralization.test.cjs | 3 +- tests/issue-498-update-context.test.cjs | 3 +- tests/learnings.test.cjs | 4 +- tests/lint-docs-required.test.cjs | 3 +- tests/lint-pr-check-project-dir.test.cjs | 3 +- tests/lint-skill-deps.test.cjs | 13 +- tests/lint-test-file-count.test.cjs | 168 ++++++++-- tests/milestone-archive.test.cjs | 3 + tests/milestone-summary.test.cjs | 3 +- tests/new-milestone-clear-phases.test.cjs | 1 + tests/next-decimal-roadmap-scan.test.cjs | 1 + .../hub-logger-integration.test.cjs | 3 +- tests/observability/logger.test.cjs | 7 +- .../perf-315-loadconfig-subrepo-scan.test.cjs | 3 +- .../perf-316-state-lock-buffer-alloc.test.cjs | 3 +- tests/perf-317-context-monitor-fs.test.cjs | 5 +- ...rf-407-planning-lock-buffer-alloc.test.cjs | 3 +- tests/phase.test.cjs | 8 +- tests/planning-workspace.test.cjs | 11 +- tests/prune-orphaned-worktrees.test.cjs | 1 + tests/quick-branching.test.cjs | 7 +- tests/qwen-skills-migration.test.cjs | 6 +- tests/reapply-patches.test.cjs | 6 +- ...-artifact-layout-install-profiles.test.cjs | 32 +- .../runtime-artifact-layout-surface.test.cjs | 46 +-- tests/runtime-launcher-parity.test.cjs | 7 +- tests/secret-scan-lint.test.cjs | 9 +- tests/security-prompt-injection.test.cjs | 2 +- tests/security-scan.test.cjs | 4 +- tests/temp-subdir.test.cjs | 1 + tests/verify-health.test.cjs | 2 + tests/windows-test-parity-guard.test.cjs | 188 ++++++----- tests/workflow-size-budget.test.cjs | 46 ++- tests/workstream.test.cjs | 1 + 130 files changed, 2030 insertions(+), 586 deletions(-) create mode 100644 eslint-rules/no-raw-rmsync-in-tests.cjs create mode 100644 scripts/lib/allowlist-ratchet.cjs create mode 100644 tests/allowlist-ratchet.test.cjs create mode 100644 tests/helpers-cleanup.test.cjs diff --git a/eslint-rules/no-raw-rmsync-in-tests.cjs b/eslint-rules/no-raw-rmsync-in-tests.cjs new file mode 100644 index 000000000..ac3f5d7c8 --- /dev/null +++ b/eslint-rules/no-raw-rmsync-in-tests.cjs @@ -0,0 +1,198 @@ +'use strict'; + +/** + * no-raw-rmsync-in-tests + * + * In *.test.cjs files, flag any call that invokes fs.rmSync (directly or via + * destructuring/aliasing). Covers: + * + * (a) MemberExpression with identifier property "rmSync": + * fs.rmSync(d, opts) nodeFs.rmSync(d, opts) + * + * (b) MemberExpression with computed string-literal property "rmSync": + * fs['rmSync'](d, opts) + * + * (c) Bare Identifier whose name is known to be bound to an fs rmSync via: + * const { rmSync } = require('fs'|'node:fs') + * const alias = fs.rmSync (where fs is a require('fs') binding) + * const alias = require('fs').rmSync + * + * The only escape hatch is the native inline disable comment: + * // eslint-disable-next-line local/no-raw-rmsync-in-tests -- + * (ESLint handles that automatically — this rule does not implement it.) + * + * NOTE: The file-level `// allow-test-rule:` annotation does NOT apply to this + * rule. That annotation is for no-source-grep only. + */ + +/** @type {import('eslint').Rule.RuleModule} */ +const rule = { + meta: { + type: 'problem', + docs: { + description: + 'Disallow raw fs.rmSync() calls in test files; use helpers.cleanup() instead', + category: 'Best Practices', + }, + schema: [], + messages: { + noRawRmSync: + 'Raw fs.rmSync() in a test. Use helpers.cleanup(dir) instead — it carries the Windows-EBUSY retry budget (maxRetries/retryDelay). To suppress a rare legit case, use `// eslint-disable-next-line local/no-raw-rmsync-in-tests -- `.', + }, + }, + create(context) { + const filename = context.getFilename(); + + // Only applies in test files + if (!filename.endsWith('.test.cjs')) return {}; + + // --- Track fs-derived bindings --- + // fsBindings: Set of local variable names bound to require('fs'|'node:fs') + // rmSyncBindings: Set of local variable names bound to an fs.rmSync value + const fsBindings = new Set(); + const rmSyncBindings = new Set(); + + /** + * Returns true if `node` is a require('fs') or require('node:fs') call. + */ + function isFsRequire(node) { + return ( + node.type === 'CallExpression' && + node.callee.type === 'Identifier' && + node.callee.name === 'require' && + node.arguments.length === 1 && + node.arguments[0].type === 'Literal' && + (node.arguments[0].value === 'fs' || + node.arguments[0].value === 'node:fs') + ); + } + + /** + * Returns true if `node` is a reference to a known fs binding (Identifier + * whose name is in fsBindings). + */ + function isFsBinding(node) { + return node.type === 'Identifier' && fsBindings.has(node.name); + } + + /** + * Returns true if `node` is an expression that resolves to fs.rmSync: + * - fsBinding.rmSync (MemberExpression, identifier property) + * - require('fs').rmSync + */ + function isFsRmSyncExpression(node) { + if (node.type !== 'MemberExpression') return false; + const prop = node.property; + const isRmSyncProp = + (!node.computed && + prop.type === 'Identifier' && + prop.name === 'rmSync') || + (node.computed && + prop.type === 'Literal' && + prop.value === 'rmSync'); + if (!isRmSyncProp) return false; + return isFsBinding(node.object) || isFsRequire(node.object); + } + + return { + // ── Track `const fs = require('fs')` ────────────────────────────────── + VariableDeclaration(node) { + for (const decl of node.declarations) { + if (!decl.init) continue; + + // const fs = require('fs') + if ( + decl.id.type === 'Identifier' && + isFsRequire(decl.init) + ) { + fsBindings.add(decl.id.name); + continue; + } + + // const { rmSync } = require('fs') + // const { rmSync: del } = require('fs') + if ( + decl.id.type === 'ObjectPattern' && + isFsRequire(decl.init) + ) { + for (const prop of decl.id.properties) { + if ( + prop.type === 'Property' && + prop.key.type === 'Identifier' && + prop.key.name === 'rmSync' && + prop.value.type === 'Identifier' + ) { + rmSyncBindings.add(prop.value.name); + } + } + continue; + } + + // const { rmSync } = fs (where fs is already a known binding) + if ( + decl.id.type === 'ObjectPattern' && + isFsBinding(decl.init) + ) { + for (const prop of decl.id.properties) { + if ( + prop.type === 'Property' && + prop.key.type === 'Identifier' && + prop.key.name === 'rmSync' && + prop.value.type === 'Identifier' + ) { + rmSyncBindings.add(prop.value.name); + } + } + continue; + } + + // const alias = fs.rmSync (or require('fs').rmSync) + if ( + decl.id.type === 'Identifier' && + isFsRmSyncExpression(decl.init) + ) { + rmSyncBindings.add(decl.id.name); + continue; + } + } + }, + + // ── Flag rmSync calls ───────────────────────────────────────────────── + CallExpression(node) { + const callee = node.callee; + + // (a) obj.rmSync(...) — identifier property + if ( + callee.type === 'MemberExpression' && + !callee.computed && + callee.property.type === 'Identifier' && + callee.property.name === 'rmSync' + ) { + context.report({ node, messageId: 'noRawRmSync' }); + return; + } + + // (b) obj['rmSync'](...) — computed string-literal property + if ( + callee.type === 'MemberExpression' && + callee.computed && + callee.property.type === 'Literal' && + callee.property.value === 'rmSync' + ) { + context.report({ node, messageId: 'noRawRmSync' }); + return; + } + + // (c) bare identifier known to be fs.rmSync + if ( + callee.type === 'Identifier' && + rmSyncBindings.has(callee.name) + ) { + context.report({ node, messageId: 'noRawRmSync' }); + } + }, + }; + }, +}; + +module.exports = rule; diff --git a/eslint.config.mjs b/eslint.config.mjs index 1dd078b33..6a30300a5 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -8,16 +8,18 @@ import { fileURLToPath } from 'url'; const __dirname = dirname(fileURLToPath(import.meta.url)); -// Local plugin with three custom AST rules +// Local plugin with custom AST rules import noSourceGrep from './eslint-rules/no-source-grep.cjs'; import noMagicSleepInTests from './eslint-rules/no-magic-sleep-in-tests.cjs'; import noElapsedAssertion from './eslint-rules/no-elapsed-assertion.cjs'; +import noRawRmsyncInTests from './eslint-rules/no-raw-rmsync-in-tests.cjs'; const localPlugin = { rules: { 'no-source-grep': noSourceGrep, 'no-magic-sleep-in-tests': noMagicSleepInTests, 'no-elapsed-assertion': noElapsedAssertion, + 'no-raw-rmsync-in-tests': noRawRmsyncInTests, }, }; @@ -108,6 +110,8 @@ export default tseslint.config( // Timing anti-patterns — warn for now; flip to error after cleanup 'local/no-magic-sleep-in-tests': 'warn', 'local/no-elapsed-assertion': 'warn', + // Ban raw fs.rmSync in tests — use helpers.cleanup() for Windows-EBUSY retry budget + 'local/no-raw-rmsync-in-tests': 'error', // Ban raw setTimeout sync + elapsed/duration-style assertions via no-restricted-syntax 'no-restricted-syntax': [ 'warn', diff --git a/scripts/lib/allowlist-ratchet.cjs b/scripts/lib/allowlist-ratchet.cjs new file mode 100644 index 000000000..d0d686e6d --- /dev/null +++ b/scripts/lib/allowlist-ratchet.cjs @@ -0,0 +1,136 @@ +'use strict'; + +/** + * @file allowlist-ratchet.cjs + * + * Reusable "better than a count ratchet" primitives for CI guards. + * + * ## Motivation (issue #597) + * + * A count ratchet (`assert(offenders.length <= N)`) has a masking blind spot: + * fixing one offender and introducing a new one keeps the count constant, so a + * novel defect slips through green. These helpers enforce on IDENTITY instead, + * making every individual offender visible and requiring monotonic progress + * toward zero. + * + * ## Design + * + * Both functions are pure (no I/O, no global state). The `fail` callback is + * injected by the caller so the same logic can be used with `node:assert.fail`, + * a custom throw, or a message-collector in unit tests. + */ + +/** + * Assert that `current` offenders are all within the known allowlist, and that + * every entry in the allowlist still offends (forcing the allowlist to shrink + * as defects are fixed). + * + * Fails when: + * - Any id in `current` is NOT in `known` → novel offender introduced. + * - Any id in `known` is NOT in `current` → stale allowlist entry must be + * pruned so the guard ratchets toward zero (the ratchet-DOWN direction). + * + * ## Masking blind spot this prevents (issue #597) + * + * A count ratchet (`assert(count <= N)`) allows one offender to be silently + * replaced by another while the count stays at N. By asserting on identity + * instead, every new offender is caught by name, and every fixed offender + * forces the allowlist to shrink. + * + * @param {object} opts + * @param {string} opts.label - Human-readable name for the guard (used + * in failure messages). + * @param {Iterable} opts.current - The offending ids found in the + * current run. + * @param {Iterable} opts.known - The allowlisted ids (baseline). + * @param {function(string): void} opts.fail - Callback invoked with a + * descriptive message on any violation. + * Pass `require('node:assert').fail`, a + * custom thrower, or a collector. The + * function is NOT imported here so callers + * control the failure mode. + * @param {string} [opts.pruneHint] - Optional hint appended to the stale- + * entry failure message (e.g. the name of + * the allowlist file to edit). + * @returns {{ novel: string[], stale: string[] }} Sorted arrays of novel ids + * (in current but not known) and stale ids (in known but not current). + */ +function assertWithinAllowlist({ label, current, known, fail, pruneHint }) { + const currentSet = new Set(current); + const knownSet = new Set(known); + + const novel = [...currentSet].filter((id) => !knownSet.has(id)).sort(); + const stale = [...knownSet].filter((id) => !currentSet.has(id)).sort(); + + if (novel.length > 0) { + const list = novel.map((id) => ` - ${id}`).join('\n'); + fail( + `[${label}] ${novel.length} NEW offender(s) introduced — fix at the source; do not just add to the allowlist.\n${list}` + ); + } + + if (stale.length > 0) { + const list = stale.map((id) => ` - ${id}`).join('\n'); + const hint = pruneHint ? `\n(${pruneHint})` : ''; + fail( + `[${label}] ${stale.length} allowlisted id(s) no longer offend and MUST be pruned so the guard ratchets toward zero.${hint}\n${list}` + ); + } + + return { novel, stale }; +} + +/** + * Assert that an artifact's measured maximum stays within a declared ceiling, + * and that the ceiling itself does not creep above the high-water mark (budgets + * may only decrease, not increase over time). + * + * Fails when: + * - `actualMax > ceiling` → regression: artifact exceeds budget. + * - `ceiling - actualMax > grace` → ceiling sits too far above the measured + * value; tighten it toward `actualMax`. + * + * ## Masking blind spot this prevents (issue #597) + * + * A plain `assert(size <= ceiling)` with a ceiling set generously high allows + * the artifact to grow unchecked as long as it stays under the ceiling. The + * `grace` band forces the ceiling to stay close to the high-water mark, + * ensuring that any upward creep is immediately visible. + * + * @param {object} opts + * @param {string} opts.label - Human-readable name for the guard (used + * in failure messages). + * @param {number} opts.actualMax - The measured value (e.g. bundle size in + * bytes, line count). + * @param {number} opts.ceiling - The declared budget ceiling. + * @param {number} opts.grace - Maximum allowed slack (`ceiling - + * actualMax`) before the ceiling is + * considered too loose. + * @param {function(string): void} opts.fail - Callback invoked with a + * descriptive message on any violation. + * @returns {{ ok: boolean, slack: number }} Whether both checks passed and the + * current slack value. + */ +function assertTightCeiling({ label, actualMax, ceiling, grace, fail }) { + const slack = ceiling - actualMax; + let ok = true; + + if (actualMax > ceiling) { + ok = false; + fail( + `[${label}] Regression: artifact value ${actualMax} exceeds budget ceiling ${ceiling}. ` + + `Raise the ceiling to at most ${actualMax} only if the increase is justified.` + ); + } else if (slack > grace) { + ok = false; + fail( + `[${label}] Ceiling ${ceiling} sits too far above the high-water mark ${actualMax} ` + + `(slack ${slack} > grace ${grace}). Tighten the ceiling toward ${actualMax}. ` + + `Budgets may only decrease.` + ); + } + + return { ok, slack }; +} + +module.exports = { assertWithinAllowlist, assertTightCeiling }; diff --git a/scripts/lint-test-file-count.allowlist.json b/scripts/lint-test-file-count.allowlist.json index 5f3c34492..b5f412484 100644 --- a/scripts/lint-test-file-count.allowlist.json +++ b/scripts/lint-test-file-count.allowlist.json @@ -1,36 +1,131 @@ { - "_doc": "Baseline of modules currently exceeding the 2-test-file limit. Each entry locks in TODAY's count as the ceiling. Reductions are ratcheted automatically — when a cluster drops to ≤ 2, remove its entry. New entries require justification in PR description.", + "_doc": "Baseline of modules currently exceeding the 2-test-file limit. Each entry locks in TODAY's exact test filenames as the allowlisted set (identity ratchet). Adding a NEW test file to a capped module fails (novel). Removing one requires pruning this list (stale, ratchet-down). When a cluster drops to ≤ 2, remove its entry entirely. New entries require justification in PR description.", "modules": { - "phase": { "current": 5, "issue": "3788" }, - "worktree": { "current": 13, "issue": "TBD" }, - "milestone": { "current": 10, "issue": "TBD" }, - "roadmap": { "current": 9, "issue": "TBD" }, - "verify": { "current": 10, "issue": "3767" }, - "install": { "current": 9, "issue": "TBD" }, - "init": { "current": 8, "issue": "TBD" }, - "state": { "current": 11, "issue": "180" }, - "config": { "current": 8, "issue": "TBD" }, - "graphify": { "current": 7, "issue": "TBD" }, - "progress": { "current": 6, "issue": "14" }, - "cli": { "current": 5, "issue": "TBD" }, - "surface": { "current": 5, "issue": "TBD" }, - "commit": { "current": 4, "issue": "TBD" }, - "frontmatter": { "current": 4, "issue": "TBD" }, - "index": { "current": 5, "issue": "180" }, - "intel": { "current": 4, "issue": "TBD" }, - "mvp": { "current": 4, "issue": "TBD" }, - "install-profiles": { "current": 4, "issue": "TBD" }, - "audit": { "current": 4, "issue": "192" }, - "audit-open": { "current": 3, "issue": "TBD" }, - "config-schema": { "current": 3, "issue": "TBD" }, - "profile": { "current": 3, "issue": "TBD" }, - "prompt-budget": { "current": 3, "issue": "TBD" }, - "uat": { "current": 3, "issue": "TBD" }, - "validate": { "current": 3, "issue": "TBD" }, - "workstream": { "current": 3, "issue": "TBD" }, - "gsd-tools": { "current": 3, "issue": "TBD" }, - "runtime-artifact-layout":{ "current": 3, "issue": "TBD" }, - "security": { "current": 3, "issue": "TBD" }, - "gsd-sdk": { "current": 3, "issue": "TBD" } + "audit": { + "files": [ + "audit-fix-command.test.cjs", + "bug-2659-audit-open-crash.test.cjs", + "bug-2836-audit-open-summary-uat-drift.test.cjs", + "bug-2911-audit-open-output-shape.test.cjs" + ], + "issue": "192" + }, + "config": { + "files": [ + "bug-2943-config-get-context-window-default.test.cjs", + "bug-321-config-defaults-clone-strategy.test.cjs", + "bug-3227-config-set-model-overrides.test.cjs", + "bug-442-config-dir-equals-in-path.test.cjs", + "config-field-docs.test.cjs", + "config-get-default.test.cjs", + "config-schema.property.test.cjs", + "config.test.cjs" + ], + "issue": "TBD" + }, + "graphify": { + "files": [ + "graphify-auto-update.test.cjs", + "graphify-query.test.cjs", + "graphify-visualization.test.cjs", + "graphify.test.cjs" + ], + "issue": "TBD" + }, + "intel": { + "files": [ + "bug-2351-intel-kilo-layout.test.cjs", + "bug-3290-intel-updater-layout-block.test.cjs", + "intel.test.cjs" + ], + "issue": "TBD" + }, + "milestone": { + "files": [ + "milestone-archive.test.cjs", + "milestone-helper.test.cjs", + "milestone-prefixed-convention.test.cjs", + "milestone-summary.test.cjs", + "milestone.test.cjs" + ], + "issue": "TBD" + }, + "phase": { + "files": [ + "bug-214-phase-researcher-write-truncation-contract.test.cjs", + "phase-dependency-levels.test.cjs", + "phase.test.cjs" + ], + "issue": "597" + }, + "roadmap": { + "files": [ + "bug-2661-roadmap-sync-parallel.test.cjs", + "bug-3128-roadmap-plan-count-slug-layout.test.cjs", + "bug-3599-roadmap-get-phase-project-code-prefix.test.cjs", + "enh-2447-roadmap-wave-deps.test.cjs", + "roadmap-mode-field.test.cjs", + "roadmap-phase-fallback.test.cjs", + "roadmap.test.cjs" + ], + "issue": "TBD" + }, + "runtime-artifact-layout": { + "files": [ + "runtime-artifact-layout-install-profiles.test.cjs", + "runtime-artifact-layout-surface.test.cjs", + "runtime-artifact-layout.test.cjs" + ], + "issue": "TBD" + }, + "security": { + "files": [ + "security-prompt-injection.test.cjs", + "security-scan.test.cjs", + "security.test.cjs" + ], + "issue": "TBD" + }, + "state": { + "files": [ + "bug-21-state-md-template-frontmatter.test.cjs", + "bug-2630-state-frontmatter-milestone-switch.test.cjs", + "bug-3127-state-begin-phase-idempotent.test.cjs", + "bug-3242-state-update-progress-trample.test.cjs", + "bug-3286-state-write-routing.test.cjs", + "bug-3454-state-dollar-backreference-growth.test.cjs", + "bug-397-state-preserve-executor-authored.test.cjs", + "state-acquirestatelock-non-eexist.test.cjs", + "state-prune.test.cjs", + "state.test.cjs" + ], + "issue": "180" + }, + "verify": { + "files": [ + "bug-2969-verify-reapply-patches.test.cjs", + "bug-2994-verify-reapply-patches-installed-path.test.cjs", + "bug-3381-verify-work-workstream.test.cjs", + "bug-3657-verify-reapply-patches-pristine-drift.test.cjs", + "verify-health.test.cjs", + "verify-mvp-uat.test.cjs", + "verify-test-quality.test.cjs", + "verify-work-auto-transition.test.cjs", + "verify.test.cjs" + ], + "issue": "3767" + }, + "install": { + "files": [ + "bug-410-install-defaults-test-mode-guard.test.cjs", + "install-minimal-hooks.test.cjs", + "install-path-detection.test.cjs", + "install-regressions.test.cjs", + "install-runtime-artifacts.test.cjs", + "install-update-marker.test.cjs", + "install.test.cjs" + ], + "issue": "TBD" + } } } diff --git a/scripts/lint-test-file-count.cjs b/scripts/lint-test-file-count.cjs index c65f4f40f..22e215bbe 100644 --- a/scripts/lint-test-file-count.cjs +++ b/scripts/lint-test-file-count.cjs @@ -4,16 +4,19 @@ * * Scans sdk/src/query/, sdk/src/, get-shit-done/bin/lib/, bin/ for production * modules, then counts matching test files in tests/ and sdk/src (recursive). Cap is 2 - * (primary + one integration). Over-limit clusters must be in the allowlist at - * their frozen count (ratchet: may only decrease). --json emits structured output. + * (primary + one integration). Over-limit clusters must be in the allowlist with the + * EXACT set of test filenames grandfathered (identity ratchet via allowlist-ratchet.cjs). + * Adding a new test file to a capped module is a novel offender; removing one requires + * pruning the allowlist entry (stale). --json emits structured output. * * Verdicts: OK_UNDER_LIMIT | OK_IN_ALLOWLIST | FAIL_EXCEEDS_LIMIT | - * FAIL_EXCEEDS_ALLOWLIST | HINT_CAN_REMOVE_FROM_ALLOWLIST + * FAIL_NOVEL_FILES | FAIL_STALE_ALLOWLIST */ 'use strict'; const fs = require('fs'); const path = require('path'); +const { assertWithinAllowlist } = require('./lib/allowlist-ratchet.cjs'); const ROOT = path.join(__dirname, '..'); const PROD_DIRS = [ @@ -30,11 +33,11 @@ const ALLOWLIST_PATH = path.join(__dirname, 'lint-test-file-count.allowlist.json const MAX_FILES = 2; const Verdict = Object.freeze({ - OK_UNDER_LIMIT: 'OK_UNDER_LIMIT', - OK_IN_ALLOWLIST: 'OK_IN_ALLOWLIST', - FAIL_EXCEEDS_LIMIT: 'FAIL_EXCEEDS_LIMIT', - FAIL_EXCEEDS_ALLOWLIST: 'FAIL_EXCEEDS_ALLOWLIST', - HINT_CAN_REMOVE_FROM_ALLOWLIST: 'HINT_CAN_REMOVE_FROM_ALLOWLIST', + OK_UNDER_LIMIT: 'OK_UNDER_LIMIT', + OK_IN_ALLOWLIST: 'OK_IN_ALLOWLIST', + FAIL_EXCEEDS_LIMIT: 'FAIL_EXCEEDS_LIMIT', + FAIL_NOVEL_FILES: 'FAIL_NOVEL_FILES', + FAIL_STALE_ALLOWLIST: 'FAIL_STALE_ALLOWLIST', }); function isTestFile(name) { @@ -120,17 +123,62 @@ function loadAllowlist() { catch (_) { return {}; } } +/** + * Evaluate one module's test files against the allowlist. + * + * For modules under the default cap (≤ MAX_FILES): simple count check. + * For modules with an allowlist entry: identity check via assertWithinAllowlist. + * - count now ≤ MAX_FILES → FAIL_STALE_ALLOWLIST (all known files are stale; prune entry) + * - novel files (in current but not in known) → FAIL_NOVEL_FILES + * - stale files (in known but not in current) → FAIL_STALE_ALLOWLIST + * - exact match → OK_IN_ALLOWLIST + * For modules over the cap with no allowlist entry: FAIL_EXCEEDS_LIMIT. + * + * Returns { verdict, prefix, count, knownFiles, novel, stale, files } + */ function evaluateLint({ prefix, testFiles, allowlist }) { const count = testFiles.length; const entry = allowlist[prefix]; - const ceiling = entry ? entry.current : null; + const currentNames = testFiles.map(f => path.basename(f)); + if (entry !== undefined) { - if (count <= MAX_FILES) return { verdict: Verdict.HINT_CAN_REMOVE_FROM_ALLOWLIST, prefix, count, ceiling, files: testFiles }; - if (count <= ceiling) return { verdict: Verdict.OK_IN_ALLOWLIST, prefix, count, ceiling, files: testFiles }; - return { verdict: Verdict.FAIL_EXCEEDS_ALLOWLIST, prefix, count, ceiling, files: testFiles }; + const knownFiles = Array.isArray(entry.files) ? entry.files : []; + // If the module is now at or under the default cap, the entire allowlist entry is + // stale and must be removed — this is a ratchet-DOWN failure, not a hint. + // All known files are stale (the whole entry can go). + if (count <= MAX_FILES) { + return { + verdict: Verdict.FAIL_STALE_ALLOWLIST, + prefix, count, + knownFiles, + novel: [], + stale: knownFiles.slice().sort(), + files: testFiles, + }; + } + // Identity check via assertWithinAllowlist + const messages = []; + const { novel, stale } = assertWithinAllowlist({ + label: prefix, + current: currentNames, + known: knownFiles, + fail: (msg) => messages.push(msg), + pruneHint: 'scripts/lint-test-file-count.allowlist.json', + }); + + if (novel.length > 0) { + return { verdict: Verdict.FAIL_NOVEL_FILES, prefix, count, knownFiles, novel, stale, files: testFiles }; + } + if (stale.length > 0) { + return { verdict: Verdict.FAIL_STALE_ALLOWLIST, prefix, count, knownFiles, novel, stale, files: testFiles }; + } + return { verdict: Verdict.OK_IN_ALLOWLIST, prefix, count, knownFiles, novel: [], stale: [], files: testFiles }; } - if (count <= MAX_FILES) return { verdict: Verdict.OK_UNDER_LIMIT, prefix, count, ceiling: null, files: testFiles }; - return { verdict: Verdict.FAIL_EXCEEDS_LIMIT, prefix, count, ceiling: null, files: testFiles }; + + if (count <= MAX_FILES) { + return { verdict: Verdict.OK_UNDER_LIMIT, prefix, count, knownFiles: null, novel: [], stale: [], files: testFiles }; + } + return { verdict: Verdict.FAIL_EXCEEDS_LIMIT, prefix, count, knownFiles: null, novel: [], stale: [], files: testFiles }; } function run() { @@ -147,35 +195,42 @@ function run() { } const failures = results.filter(r => - r.verdict === Verdict.FAIL_EXCEEDS_LIMIT || r.verdict === Verdict.FAIL_EXCEEDS_ALLOWLIST); - const hints = results.filter(r => r.verdict === Verdict.HINT_CAN_REMOVE_FROM_ALLOWLIST); + r.verdict === Verdict.FAIL_EXCEEDS_LIMIT || + r.verdict === Verdict.FAIL_NOVEL_FILES || + r.verdict === Verdict.FAIL_STALE_ALLOWLIST); if (jsonMode) { - console.log(JSON.stringify({ ok: failures.length === 0, results, failures, hints }, null, 2)); + console.log(JSON.stringify({ ok: failures.length === 0, results, failures, hints: [] }, null, 2)); process.exit(failures.length > 0 ? 1 : 0); } if (failures.length === 0) { const inAllowlist = results.filter(r => r.verdict === Verdict.OK_IN_ALLOWLIST).length; console.log(`ok lint-test-file-count: ${results.length} module(s) checked, 0 failures` + - (inAllowlist > 0 ? `, ${inAllowlist} allowlisted` : '') + - (hints.length > 0 ? `, ${hints.length} hint(s)` : '')); - for (const h of hints) { - console.log(` hint: "${h.prefix}" is allowlisted at ${h.ceiling} but now has ${h.count} — remove from allowlist`); - } + (inAllowlist > 0 ? `, ${inAllowlist} allowlisted` : '')); process.exit(0); } process.stderr.write(`\nERROR lint-test-file-count: ${failures.length} module(s) exceed the test-file limit\n\n`); for (const f of failures) { - const tag = f.verdict === Verdict.FAIL_EXCEEDS_LIMIT - ? `${f.count} files (limit ${MAX_FILES})` - : `${f.count} files (allowlist ceiling ${f.ceiling})`; - process.stderr.write(` ${f.prefix}: ${tag}\n`); - for (const tf of f.files) process.stderr.write(` ${path.relative(ROOT, tf)}\n`); + if (f.verdict === Verdict.FAIL_EXCEEDS_LIMIT) { + process.stderr.write(` ${f.prefix}: ${f.count} files (limit ${MAX_FILES}) — not in allowlist\n`); + for (const tf of f.files) process.stderr.write(` ${path.relative(ROOT, tf)}\n`); + } else if (f.verdict === Verdict.FAIL_NOVEL_FILES) { + process.stderr.write(` ${f.prefix}: ${f.novel.length} NEW test file(s) not in allowlist\n`); + for (const n of f.novel) process.stderr.write(` + ${n}\n`); + } else if (f.verdict === Verdict.FAIL_STALE_ALLOWLIST) { + if (f.count <= MAX_FILES) { + const staleList = f.stale.join(', '); + process.stderr.write(` "${f.prefix}": now at ${f.count} file(s) (≤ ${MAX_FILES}) — remove its entry from the allowlist (stale: ${staleList})\n`); + } else { + process.stderr.write(` ${f.prefix}: ${f.stale.length} allowlisted file(s) no longer present — prune allowlist\n`); + for (const s of f.stale) process.stderr.write(` - ${s}\n`); + } + } } process.stderr.write('\nFix: consolidate test files per module (one primary + one integration).\n'); - process.stderr.write('Or add the module to scripts/lint-test-file-count.allowlist.json with PR justification.\n\n'); + process.stderr.write('Or update scripts/lint-test-file-count.allowlist.json with PR justification.\n\n'); process.exit(1); } diff --git a/tests/26-w005-w006-i001-cjs-drift-regression.test.cjs b/tests/26-w005-w006-i001-cjs-drift-regression.test.cjs index 91529a324..ce918e35b 100644 --- a/tests/26-w005-w006-i001-cjs-drift-regression.test.cjs +++ b/tests/26-w005-w006-i001-cjs-drift-regression.test.cjs @@ -24,7 +24,7 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { runGsdTools } = require('./helpers.cjs'); +const { runGsdTools, cleanup } = require('./helpers.cjs'); function mkplanning(base) { const planningDir = path.join(base, '.planning'); @@ -84,7 +84,7 @@ describe('Drift item W005 — phaseDirNameRe: 999.X-name dirs must not trigger W fs.mkdirSync(path.join(phasesDir, '999.1-foo'), { recursive: true }); }); - after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + after(() => { cleanup(tmpDir); }); test('no W005 for 999.1-foo (multi-digit sub-phase prefix)', () => { const result = runGsdTools(['validate', 'health', '--json'], tmpDir); @@ -159,7 +159,7 @@ describe('Drift item W006-archived — MILESTONE_ARCHIVE_DIR_RE and PHASE_TOKEN_ ); }); - after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + after(() => { cleanup(tmpDir); }); test('no W006 for Phase 64 archived under milestones/v1.0-phases/', () => { const result = runGsdTools(['validate', 'health', '--json'], tmpDir); @@ -227,7 +227,7 @@ describe('Drift item I001 — canonicalPlanStem: long PLAN stem matches short SU fs.writeFileSync(path.join(phaseDir, '68-01-SUMMARY.md'), '# Summary\n'); }); - after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + after(() => { cleanup(tmpDir); }); test('no I001 when 68-01-scaffolding-PLAN.md matches 68-01-SUMMARY.md via canonicalPlanStem', () => { const result = runGsdTools(['validate', 'health', '--json'], tmpDir); diff --git a/tests/4-phase-complete-cjs-regression.test.cjs b/tests/4-phase-complete-cjs-regression.test.cjs index a0ca0ae2c..01683f152 100644 --- a/tests/4-phase-complete-cjs-regression.test.cjs +++ b/tests/4-phase-complete-cjs-regression.test.cjs @@ -30,6 +30,8 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); +const { cleanup } = require('./helpers.cjs'); + // ── Load cmdPhaseComplete directly from phase.cjs (bypass the SDK router) ──── // phase-command-router.cjs delegates to SDK when available; we must test the // CJS implementation directly since that is where the bug lives. @@ -216,7 +218,7 @@ describe('issue #4 (CJS): cmdPhaseComplete — idempotency (blind-increment bug) }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + cleanup(tmpDir); }); test('T1: double invocation does NOT double-increment Completed Phases in STATE.md body', () => { @@ -361,7 +363,7 @@ describe('issue #4 (CJS): cmdPhaseComplete — progress percent clamp', () => { let tmpDir; afterEach(() => { - if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + cleanup(tmpDir); }); test('T2: Progress percent never exceeds 100 after double invocation', () => { diff --git a/tests/6-validate-cjs-drift-regression.test.cjs b/tests/6-validate-cjs-drift-regression.test.cjs index 822320e70..ba5679561 100644 --- a/tests/6-validate-cjs-drift-regression.test.cjs +++ b/tests/6-validate-cjs-drift-regression.test.cjs @@ -32,7 +32,7 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { runGsdTools } = require('./helpers.cjs'); +const { runGsdTools, cleanup } = require('./helpers.cjs'); // ── Fixture helpers ────────────────────────────────────────────────────────── @@ -124,7 +124,7 @@ describe('Drift item 1 — W007 activeDiskPhases: no false W007 for archived pha }); after(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('no W007 for archived phase "1" absent from current ROADMAP', () => { @@ -198,7 +198,7 @@ describe('Drift item 2 — phaseVariants() normalization: letter-suffix zero-pad }); after(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('no false W006 when ROADMAP says 01A and disk has 1A-... (phaseVariants normalizes)', () => { @@ -275,7 +275,7 @@ describe('Drift item 3 — W006 false positive when disk has zero-padded letter }); after(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('no false W006 when ROADMAP says 3B and disk has 03B-... (phaseVariants covers zero-padded)', () => { diff --git a/tests/active-workstream-store.test.cjs b/tests/active-workstream-store.test.cjs index 058263959..b9c094712 100644 --- a/tests/active-workstream-store.test.cjs +++ b/tests/active-workstream-store.test.cjs @@ -3,6 +3,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { cleanup } = require('./helpers.cjs'); const { validateWorkstreamName, @@ -119,7 +120,7 @@ describe('active-workstream-store', () => { } finally { if (savedSession !== undefined) process.env.GSD_SESSION_KEY = savedSession; else delete process.env.GSD_SESSION_KEY; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); @@ -139,7 +140,7 @@ describe('active-workstream-store', () => { assert.equal(active, null); assert.equal(adapter.read(), null); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); diff --git a/tests/affected-tests-lib.test.cjs b/tests/affected-tests-lib.test.cjs index 61987f504..541a06019 100644 --- a/tests/affected-tests-lib.test.cjs +++ b/tests/affected-tests-lib.test.cjs @@ -18,6 +18,8 @@ const { resolveRelativeDependency, } = require('../scripts/affected-tests-lib.cjs'); +const { cleanup } = require('./helpers.cjs'); + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -188,7 +190,7 @@ test('transitive: changing a deep dependency selects the test that depends on it 'get-shit-done/bin/lib/depA.cjs': `'use strict';\nconst depB = require('./depB.cjs');\nmodule.exports = { a: depB };\n`, 'tests/t.test.cjs': `'use strict';\nconst depA = require('../get-shit-done/bin/lib/depA.cjs');\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const reverseIndex = buildTransitiveReverseIndex(dir, ['tests/t.test.cjs']); const selected = pickAffectedTests( @@ -213,7 +215,7 @@ test('adversarial(a): cycle depA<->depB — changing depA selects test, no hang' 'get-shit-done/bin/lib/depB.cjs': `'use strict';\nconst depA = require('./depA.cjs');\nmodule.exports = {};\n`, 'tests/cycle.test.cjs': `'use strict';\nconst depA = require('../get-shit-done/bin/lib/depA.cjs');\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); // Must complete without hanging const reverseIndex = buildTransitiveReverseIndex(dir, ['tests/cycle.test.cjs']); @@ -234,7 +236,7 @@ test('adversarial(b): missing require (gone file) — null resolve, no crash', ( // Requires a file that does not exist 'tests/missing.test.cjs': `'use strict';\nconst x = require('../get-shit-done/bin/lib/gone.cjs');\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); // Should not throw let reverseIndex; @@ -258,7 +260,7 @@ test('adversarial(c): .json dependency — changing data.json selects the test', 'get-shit-done/bin/lib/data.json': `{"key":"value"}`, 'tests/json.test.cjs': `'use strict';\nconst data = require('../get-shit-done/bin/lib/data.json');\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const reverseIndex = buildTransitiveReverseIndex(dir, ['tests/json.test.cjs']); const selected = pickAffectedTests( @@ -279,7 +281,7 @@ test('adversarial(d): re-export chain — changing depB selects the test that re 'get-shit-done/bin/lib/reexporter.cjs': `'use strict';\nmodule.exports = require('./depB.cjs');\n`, 'tests/reexport.test.cjs': `'use strict';\nconst x = require('../get-shit-done/bin/lib/reexporter.cjs');\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const reverseIndex = buildTransitiveReverseIndex(dir, ['tests/reexport.test.cjs']); const selected = pickAffectedTests( @@ -313,7 +315,7 @@ test('adversarial(f): WIDEN — changing a src file with no test dependents wide 'get-shit-done/bin/lib/orphan.cjs': `'use strict';\nmodule.exports = {};\n`, 'tests/unrelated.test.cjs': `'use strict';\n// requires nothing\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const reverseIndex = buildTransitiveReverseIndex(dir, ['tests/unrelated.test.cjs']); @@ -345,7 +347,7 @@ test('adversarial(g): dynamic require in changed file with no static dependents 'get-shit-done/bin/lib/dynamic.cjs': `'use strict';\nconst x = 'foo';\nconst m = require(\`./\${x}\`);\nmodule.exports = {};\n`, 'tests/unrelated.test.cjs': `'use strict';\n// does not require dynamic.cjs\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const reverseIndex = buildTransitiveReverseIndex(dir, ['tests/unrelated.test.cjs']); @@ -367,7 +369,7 @@ test('resolveRelativeDependency resolves .ts, .json extensions', (t) => { 'src/helper.ts': `export const x = 1;\n`, 'src/data.json': `{"k":1}`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const fromAbs = path.join(dir, 'tests/consumer.cjs'); @@ -444,7 +446,7 @@ test('regression(mixed-diff): widen plan covers integration suite; concrete matc 'tests/server.integration.test.cjs': `'use strict';\nconst s = require('../bin/lib/server.cjs');\n`, 'tests/unrelated.test.cjs': `'use strict';\n// requires nothing\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); // Act: build graph over both test files const allTests = [ @@ -532,7 +534,7 @@ test('regression(delete-only-source): deleting a source file triggers widen, nev 'get-shit-done/bin/lib/other.cjs': `'use strict';\nmodule.exports = {};\n`, 'tests/unrelated.test.cjs': `'use strict';\n// requires nothing from gone.cjs\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); // Act const allTests = ['tests/unrelated.test.cjs']; @@ -592,7 +594,7 @@ test('regression(rename-stale-old-path): deleted old path triggers widen, protec 'tests/newname.test.cjs': `'use strict';\nconst x = require('../get-shit-done/bin/lib/newname.cjs');\n`, 'tests/unrelated.test.cjs': `'use strict';\n// no dependency on oldname or newname\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); // Act const allTests = ['tests/newname.test.cjs', 'tests/unrelated.test.cjs']; @@ -646,7 +648,7 @@ test('regression(delete-only-test): deleting a test file does not trigger widen const dir = makeFixture({ 'tests/surviving.test.cjs': `'use strict';\n// a plain surviving unit test\n`, }); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); // Act // allTests comes from the fixture's tests/ directory — gone.test.cjs is absent. diff --git a/tests/agent-size-budget.test.cjs b/tests/agent-size-budget.test.cjs index 6bcb17629..857097c23 100644 --- a/tests/agent-size-budget.test.cjs +++ b/tests/agent-size-budget.test.cjs @@ -19,6 +19,10 @@ * rationale in the PR, and make sure the bloat is not duplicated content * that belongs in `get-shit-done/references/`. * + * Tighten-only invariant (issue #597): ceilings track the tier high-water mark + * within GRACE lines. Budgets may only decrease, never silently creep upward. + * The assertTightCeiling() call below enforces this automatically. + * * See: https://github.com/open-gsd/gsd-core/issues/2361 */ @@ -26,13 +30,23 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const { assertTightCeiling } = require('../scripts/lib/allowlist-ratchet.cjs'); const AGENTS_DIR = path.join(__dirname, '..', 'agents'); -const XL_BUDGET = 1600; +// Ceilings tightened to actualMax + GRACE per the ratchet-down rule (#597). +// XL ceiling lowered from 1600 → 1512 (actualMax=1452, gsd-debugger). +const XL_BUDGET = 1512; +// LARGE ceiling kept at 1000 (actualMax=978, slack=22 ≤ GRACE=60). const LARGE_BUDGET = 1000; +// DEFAULT ceiling kept at 500 (actualMax=495, slack=5 ≤ GRACE=60). const DEFAULT_BUDGET = 500; +// Grace band: maximum allowed slack (ceiling − actualMax) before a ceiling is +// considered too loose. 60 lines gives one reasonable screen of breathing room +// without permitting gross inflation. +const GRACE = 60; + const XL_AGENTS = new Set([ 'gsd-debugger', 'gsd-planner', @@ -83,6 +97,35 @@ describe('SIZE: agent line-count budget', () => { } }); +describe('SIZE: tier anti-creep (tighten-only ceilings, issue #597)', () => { + // For each tier, compute the high-water mark across all files in that tier + // and assert the ceiling stays tight. Prevents budgets from silently drifting + // upward: ceiling − actualMax must not exceed GRACE. + test('XL tier: ceiling tracks high-water mark within GRACE', () => { + const values = ALL_AGENTS + .filter(a => XL_AGENTS.has(a)) + .map(a => lineCount(path.join(AGENTS_DIR, a + '.md'))); + const actualMax = Math.max(...values); + assertTightCeiling({ label: 'XL', actualMax, ceiling: XL_BUDGET, grace: GRACE, fail: assert.fail }); + }); + + test('LARGE tier: ceiling tracks high-water mark within GRACE', () => { + const values = ALL_AGENTS + .filter(a => LARGE_AGENTS.has(a)) + .map(a => lineCount(path.join(AGENTS_DIR, a + '.md'))); + const actualMax = Math.max(...values); + assertTightCeiling({ label: 'LARGE', actualMax, ceiling: LARGE_BUDGET, grace: GRACE, fail: assert.fail }); + }); + + test('DEFAULT tier: ceiling tracks high-water mark within GRACE', () => { + const values = ALL_AGENTS + .filter(a => !XL_AGENTS.has(a) && !LARGE_AGENTS.has(a)) + .map(a => lineCount(path.join(AGENTS_DIR, a + '.md'))); + const actualMax = Math.max(...values); + assertTightCeiling({ label: 'DEFAULT', actualMax, ceiling: DEFAULT_BUDGET, grace: GRACE, fail: assert.fail }); + }); +}); + describe('SIZE: every agent is classified', () => { test('every agent falls in exactly one tier', () => { for (const agent of ALL_AGENTS) { diff --git a/tests/agent-skills.test.cjs b/tests/agent-skills.test.cjs index e8a56e824..ec9fe8b77 100644 --- a/tests/agent-skills.test.cjs +++ b/tests/agent-skills.test.cjs @@ -263,7 +263,7 @@ describe('agent-skills global: prefix', () => { afterEach(() => { cleanup(tmpDir); - fs.rmSync(fakeHome, { recursive: true, force: true }); + cleanup(fakeHome); }); function createGlobalSkill(name) { diff --git a/tests/allowlist-ratchet.test.cjs b/tests/allowlist-ratchet.test.cjs new file mode 100644 index 000000000..ab2eecdc5 --- /dev/null +++ b/tests/allowlist-ratchet.test.cjs @@ -0,0 +1,297 @@ +'use strict'; + +/** + * Tests for scripts/lib/allowlist-ratchet.cjs + * + * Covers assertWithinAllowlist and assertTightCeiling. + * Uses a non-throwing fake `fail` that records messages into an array so we can + * assert on call count and message content without early-exit on first failure. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); + +const { + assertWithinAllowlist, + assertTightCeiling, +} = require('../scripts/lib/allowlist-ratchet.cjs'); + +// ─── Fake fail helper ──────────────────────────────────────────────────────── + +/** + * Returns a { fail, calls } pair. `fail` records its message without throwing, + * so tests can observe every violation rather than stopping at the first. + */ +function makeFail() { + const calls = []; + return { + calls, + fail(msg) { + calls.push(msg); + }, + }; +} + +// ─── assertWithinAllowlist ─────────────────────────────────────────────────── + +describe('assertWithinAllowlist', () => { + test('clean case: current subset of known, no stale entries — fail never called', () => { + const { fail, calls } = makeFail(); + const result = assertWithinAllowlist({ + label: 'test-guard', + current: ['a.ts', 'b.ts'], + known: ['a.ts', 'b.ts'], + fail, + }); + assert.strictEqual(calls.length, 0, 'fail should not be called'); + assert.deepStrictEqual(result.novel, []); + assert.deepStrictEqual(result.stale, []); + }); + + test('novel detected: id in current but not in known — fail called with that id', () => { + const { fail, calls } = makeFail(); + const result = assertWithinAllowlist({ + label: 'novel-guard', + current: ['a.ts', 'b.ts', 'c.ts'], + known: ['a.ts', 'b.ts'], + fail, + }); + assert.strictEqual(calls.length, 1, 'fail should be called once for novel'); + assert.ok(calls[0].includes('c.ts'), 'message should mention the novel id'); + assert.ok( + calls[0].includes('fix at the source'), + 'message should include fix-at-source guidance' + ); + assert.deepStrictEqual(result.novel, ['c.ts']); + assert.deepStrictEqual(result.stale, []); + }); + + test('stale detected: id in known but not in current — fail called with that id', () => { + const { fail, calls } = makeFail(); + const result = assertWithinAllowlist({ + label: 'stale-guard', + current: ['a.ts'], + known: ['a.ts', 'b.ts'], + fail, + }); + assert.strictEqual(calls.length, 1, 'fail should be called once for stale'); + assert.ok(calls[0].includes('b.ts'), 'message should mention the stale id'); + assert.ok( + calls[0].includes('ratchets toward zero'), + 'message should include ratchet-toward-zero language' + ); + assert.deepStrictEqual(result.novel, []); + assert.deepStrictEqual(result.stale, ['b.ts']); + }); + + test('stale message includes pruneHint when provided', () => { + const { fail, calls } = makeFail(); + assertWithinAllowlist({ + label: 'prune-guard', + current: ['a.ts'], + known: ['a.ts', 'b.ts'], + fail, + pruneHint: 'edit scripts/my-allowlist.json', + }); + assert.ok( + calls[0].includes('edit scripts/my-allowlist.json'), + 'message should include the pruneHint' + ); + }); + + test('both novel and stale at once — fail called twice', () => { + const { fail, calls } = makeFail(); + const result = assertWithinAllowlist({ + label: 'both-guard', + current: ['a.ts', 'c.ts'], // c.ts is new, b.ts is fixed + known: ['a.ts', 'b.ts'], + fail, + }); + assert.strictEqual(calls.length, 2, 'fail should be called once for novel and once for stale'); + const allMessages = calls.join('\n'); + assert.ok(allMessages.includes('c.ts'), 'should mention novel id c.ts'); + assert.ok(allMessages.includes('b.ts'), 'should mention stale id b.ts'); + assert.deepStrictEqual(result.novel, ['c.ts']); + assert.deepStrictEqual(result.stale, ['b.ts']); + }); + + test('empty inputs — fail never called', () => { + const { fail, calls } = makeFail(); + const result = assertWithinAllowlist({ + label: 'empty-guard', + current: [], + known: [], + fail, + }); + assert.strictEqual(calls.length, 0); + assert.deepStrictEqual(result.novel, []); + assert.deepStrictEqual(result.stale, []); + }); + + test('order-independence: Sets and arrays produce the same result', () => { + const callsArr = makeFail(); + const callsSet = makeFail(); + + const resultArr = assertWithinAllowlist({ + label: 'order-array', + current: ['z.ts', 'a.ts', 'm.ts'], + known: ['a.ts', 'm.ts'], + fail: callsArr.fail, + }); + + const resultSet = assertWithinAllowlist({ + label: 'order-set', + current: new Set(['z.ts', 'a.ts', 'm.ts']), + known: new Set(['a.ts', 'm.ts']), + fail: callsSet.fail, + }); + + assert.deepStrictEqual(resultArr.novel, resultSet.novel, 'novel should be identical regardless of input type'); + assert.deepStrictEqual(resultArr.stale, resultSet.stale, 'stale should be identical regardless of input type'); + assert.deepStrictEqual(resultArr.novel, ['z.ts'], 'novel should be sorted'); + }); + + test('returned novel and stale arrays are sorted', () => { + const { fail } = makeFail(); + const result = assertWithinAllowlist({ + label: 'sort-guard', + current: ['z.ts', 'a.ts', 'm.ts', 'new.ts'], + known: ['z.ts', 'a.ts', 'm.ts', 'old.ts'], + fail, + }); + assert.deepStrictEqual(result.novel, ['new.ts']); + assert.deepStrictEqual(result.stale, ['old.ts']); + }); + + test('current empty, known non-empty — all known are stale', () => { + const { fail, calls } = makeFail(); + const result = assertWithinAllowlist({ + label: 'all-stale', + current: [], + known: ['a.ts', 'b.ts'], + fail, + }); + assert.strictEqual(calls.length, 1); + assert.deepStrictEqual(result.stale, ['a.ts', 'b.ts']); + assert.deepStrictEqual(result.novel, []); + }); + + test('known empty, current non-empty — all current are novel', () => { + const { fail, calls } = makeFail(); + const result = assertWithinAllowlist({ + label: 'all-novel', + current: ['a.ts', 'b.ts'], + known: [], + fail, + }); + assert.strictEqual(calls.length, 1); + assert.deepStrictEqual(result.novel, ['a.ts', 'b.ts']); + assert.deepStrictEqual(result.stale, []); + }); +}); + +// ─── assertTightCeiling ────────────────────────────────────────────────────── + +describe('assertTightCeiling', () => { + test('actualMax under ceiling within grace — ok, fail never called', () => { + const { fail, calls } = makeFail(); + const result = assertTightCeiling({ + label: 'size-guard', + actualMax: 90, + ceiling: 100, + grace: 15, + fail, + }); + assert.strictEqual(calls.length, 0, 'fail should not be called'); + assert.strictEqual(result.ok, true); + assert.strictEqual(result.slack, 10); + }); + + test('actualMax over ceiling — fail called with regression message', () => { + const { fail, calls } = makeFail(); + const result = assertTightCeiling({ + label: 'size-guard', + actualMax: 110, + ceiling: 100, + grace: 5, + fail, + }); + assert.strictEqual(calls.length, 1, 'fail should be called once'); + assert.ok(calls[0].includes('Regression'), 'message should say Regression'); + assert.ok(calls[0].includes('110'), 'message should include actualMax'); + assert.ok(calls[0].includes('100'), 'message should include ceiling'); + assert.strictEqual(result.ok, false); + assert.strictEqual(result.slack, -10); + }); + + test('ceiling too loose (slack > grace) — fail called with tighten message', () => { + const { fail, calls } = makeFail(); + const result = assertTightCeiling({ + label: 'loose-guard', + actualMax: 50, + ceiling: 100, + grace: 10, + fail, + }); + assert.strictEqual(calls.length, 1, 'fail should be called once'); + assert.ok( + calls[0].toLowerCase().includes('tighten') || calls[0].includes('too far'), + 'message should mention tightening' + ); + assert.ok(calls[0].includes('Budgets may only decrease'), 'message should include budget policy'); + assert.strictEqual(result.ok, false); + assert.strictEqual(result.slack, 50); + }); + + test('boundary: slack === grace — ok (exactly at the grace limit)', () => { + const { fail, calls } = makeFail(); + const result = assertTightCeiling({ + label: 'boundary-guard', + actualMax: 90, + ceiling: 100, + grace: 10, + fail, + }); + assert.strictEqual(calls.length, 0, 'fail should not be called at exact grace boundary'); + assert.strictEqual(result.ok, true); + assert.strictEqual(result.slack, 10); + }); + + test('actualMax equals ceiling — ok, slack is zero', () => { + const { fail, calls } = makeFail(); + const result = assertTightCeiling({ + label: 'exact-guard', + actualMax: 100, + ceiling: 100, + grace: 0, + fail, + }); + assert.strictEqual(calls.length, 0); + assert.strictEqual(result.ok, true); + assert.strictEqual(result.slack, 0); + }); + + test('grace 0: any slack triggers fail', () => { + const { fail, calls } = makeFail(); + assertTightCeiling({ + label: 'tight-guard', + actualMax: 99, + ceiling: 100, + grace: 0, + fail, + }); + assert.strictEqual(calls.length, 1, 'any slack above 0 should fail when grace is 0'); + }); + + test('label appears in failure messages', () => { + const { fail, calls } = makeFail(); + assertTightCeiling({ + label: 'my-special-guard', + actualMax: 200, + ceiling: 100, + grace: 5, + fail, + }); + assert.ok(calls[0].includes('my-special-guard'), 'label should appear in message'); + }); +}); diff --git a/tests/bug-131-release-tarball-smoke-explicit-home.test.cjs b/tests/bug-131-release-tarball-smoke-explicit-home.test.cjs index fa531f98e..996725bb3 100644 --- a/tests/bug-131-release-tarball-smoke-explicit-home.test.cjs +++ b/tests/bug-131-release-tarball-smoke-explicit-home.test.cjs @@ -25,7 +25,7 @@ const path = require('node:path'); const { execFileSync } = require('node:child_process'); // The helpers under test. -const { runNpm, isolatedNpmEnv } = require('./helpers.cjs'); +const { runNpm, isolatedNpmEnv, cleanup } = require('./helpers.cjs'); // Resolve a filesystem path to its canonical (symlink-free) form even if the // leaf does not exist yet (e.g. ~/.npm before npm has written its cache). @@ -113,9 +113,9 @@ describe('bug-131: runNpm isolates HOME from the caller environment', () => { `expected semver output from npm --version, got: ${stdout}`, ); } finally { - // Restore write permission before cleanup so rmSync can delete it. + // Restore write permission before cleanup so the directory can be deleted. try { fs.chmodSync(poisonedHome, 0o700); } catch (_) { /* best-effort */ } - fs.rmSync(poisonedHome, { recursive: true, force: true }); + cleanup(poisonedHome); } }); diff --git a/tests/bug-1834-sh-hooks-installed.test.cjs b/tests/bug-1834-sh-hooks-installed.test.cjs index 38a552700..1ba7e3beb 100644 --- a/tests/bug-1834-sh-hooks-installed.test.cjs +++ b/tests/bug-1834-sh-hooks-installed.test.cjs @@ -49,6 +49,7 @@ function createTempDir(prefix) { } function cleanup(dir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup wrapper; try/catch swallows ENOENT so runInstaller teardown never fails the test try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } } diff --git a/tests/bug-1908-uninstall-manifest.test.cjs b/tests/bug-1908-uninstall-manifest.test.cjs index 85a144546..a0afb764d 100644 --- a/tests/bug-1908-uninstall-manifest.test.cjs +++ b/tests/bug-1908-uninstall-manifest.test.cjs @@ -50,6 +50,7 @@ function createFakeInstall(prefix = 'gsd-uninstall-test-') { } function cleanup(dir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local teardown helper predates helpers.cjs; renaming would collide with the imported cleanup try { fs.rmSync(dir, { recursive: true, force: true }); } catch {} } diff --git a/tests/bug-1924-preserve-user-artifacts.test.cjs b/tests/bug-1924-preserve-user-artifacts.test.cjs index 98e283fd2..6b286e027 100644 --- a/tests/bug-1924-preserve-user-artifacts.test.cjs +++ b/tests/bug-1924-preserve-user-artifacts.test.cjs @@ -51,6 +51,7 @@ function createTempDir(prefix) { } function cleanup(dir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup() helper wrapping rmSync; cannot use imported cleanup() without naming collision try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } } diff --git a/tests/bug-1967-cache-invalidation.test.cjs b/tests/bug-1967-cache-invalidation.test.cjs index 789a2c22c..682345e5d 100644 --- a/tests/bug-1967-cache-invalidation.test.cjs +++ b/tests/bug-1967-cache-invalidation.test.cjs @@ -23,6 +23,7 @@ const path = require('node:path'); const os = require('node:os'); const state = require('../get-shit-done/bin/lib/state.cjs'); +const { cleanup } = require('./helpers.cjs'); describe('buildStateFrontmatter cache invalidation (#1967)', () => { let tmpDir; @@ -59,7 +60,7 @@ describe('buildStateFrontmatter cache invalidation (#1967)', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('writeStateMd invalidates cache so subsequent reads see new disk state', () => { diff --git a/tests/bug-211-launcher-home-fallback.test.cjs b/tests/bug-211-launcher-home-fallback.test.cjs index 444098b46..403013711 100644 --- a/tests/bug-211-launcher-home-fallback.test.cjs +++ b/tests/bug-211-launcher-home-fallback.test.cjs @@ -22,6 +22,7 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'get-shit-done', 'workflows'); const SNIPPET_FILE = path.join(WORKFLOWS_DIR, '_runtime-launcher.snippet.sh'); @@ -126,8 +127,8 @@ describe('bug-211: launcher ~/.claude home fallback', () => { `Expected stub output "CLAUDE_HOME_STUB:ping,test", got:\n${stdout.trim()}`, ); } finally { - fs.rmSync(fakeHome, { recursive: true, force: true }); - fs.rmSync(fakeRuntime, { recursive: true, force: true }); + cleanup(fakeHome); + cleanup(fakeRuntime); } }); @@ -182,8 +183,8 @@ describe('bug-211: launcher ~/.claude home fallback', () => { `Expected stderr to contain "not found" or "ERROR", got: ${stderrOutput.trim()}`, ); } finally { - fs.rmSync(fakeHome, { recursive: true, force: true }); - fs.rmSync(fakeRuntime, { recursive: true, force: true }); + cleanup(fakeHome); + cleanup(fakeRuntime); } }); }); diff --git a/tests/bug-2136-sh-hook-version.test.cjs b/tests/bug-2136-sh-hook-version.test.cjs index e3b1e99a6..bcf045db3 100644 --- a/tests/bug-2136-sh-hook-version.test.cjs +++ b/tests/bug-2136-sh-hook-version.test.cjs @@ -72,6 +72,7 @@ function createTempDir(prefix) { } function cleanup(dir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup() helper wrapping rmSync; cannot use imported cleanup() without naming collision try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } } diff --git a/tests/bug-2256-model-overrides-transport.test.cjs b/tests/bug-2256-model-overrides-transport.test.cjs index 967d634bd..6b40ac7ee 100644 --- a/tests/bug-2256-model-overrides-transport.test.cjs +++ b/tests/bug-2256-model-overrides-transport.test.cjs @@ -28,7 +28,7 @@ const { getCodexSkillAdapterHeader, } = require('../bin/install.js'); -const { createTempDir } = require('./helpers.cjs'); +const { createTempDir, cleanup } = require('./helpers.cjs'); const makeTmp = (prefix) => createTempDir(`gsd-2256-${prefix}-`); function writeJson(p, obj) { @@ -36,10 +36,6 @@ function writeJson(p, obj) { fs.writeFileSync(p, JSON.stringify(obj, null, 2)); } -function rmr(p) { - try { fs.rmSync(p, { recursive: true, force: true }); } catch { /* noop */ } -} - describe('bug #2256 — readGsdEffectiveModelOverrides', () => { let projectDir; let homeDir; @@ -65,8 +61,8 @@ describe('bug #2256 — readGsdEffectiveModelOverrides', () => { if (origUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = origUserProfile; } - rmr(projectDir); - rmr(homeDir); + cleanup(projectDir); + cleanup(homeDir); }); test('returns null when neither source defines model_overrides', () => { diff --git a/tests/bug-260-worktree-path-guard.test.cjs b/tests/bug-260-worktree-path-guard.test.cjs index 4de81ee09..02ec3cd2b 100644 --- a/tests/bug-260-worktree-path-guard.test.cjs +++ b/tests/bug-260-worktree-path-guard.test.cjs @@ -24,6 +24,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { spawnSync, execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-worktree-path-guard.js'); const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js'); @@ -97,8 +98,8 @@ before(() => { after(() => { // Remove worktree registration before deleting the directory try { git(mainRepo, ['worktree', 'remove', '--force', worktreeDir]); } catch { /* ignore */ } - try { fs.rmSync(mainRepo, { recursive: true, force: true }); } catch { /* ignore */ } - try { fs.rmSync(worktreeDir, { recursive: true, force: true }); } catch { /* ignore */ } + cleanup(mainRepo); + cleanup(worktreeDir); }); // --------------------------------------------------------------------------- diff --git a/tests/bug-261-worktree-force-add-guard.test.cjs b/tests/bug-261-worktree-force-add-guard.test.cjs index 75ff70b7a..070b91821 100644 --- a/tests/bug-261-worktree-force-add-guard.test.cjs +++ b/tests/bug-261-worktree-force-add-guard.test.cjs @@ -7,6 +7,8 @@ const os = require('node:os'); const path = require('node:path'); const { execFileSync, spawnSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); + const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-workflow-guard.js'); function git(cwd, args) { @@ -61,7 +63,7 @@ describe('bug #261: workflow guard blocks forced git add on worktree-agent branc assert.strictEqual(envelope.decision, 'block'); assert.strictEqual(envelope.code, 'WORKTREE_AGENT_FORCE_ADD_FORBIDDEN'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -73,7 +75,7 @@ describe('bug #261: workflow guard blocks forced git add on worktree-agent branc assert.strictEqual(result.status, 2); assert.strictEqual(JSON.parse(result.stdout).code, 'WORKTREE_AGENT_FORCE_ADD_FORBIDDEN'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -85,7 +87,7 @@ describe('bug #261: workflow guard blocks forced git add on worktree-agent branc assert.strictEqual(result.status, 0); assert.strictEqual(result.stdout, ''); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -97,7 +99,7 @@ describe('bug #261: workflow guard blocks forced git add on worktree-agent branc assert.strictEqual(result.status, 0); assert.strictEqual(result.stdout, ''); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -109,7 +111,7 @@ describe('bug #261: workflow guard blocks forced git add on worktree-agent branc assert.strictEqual(result.status, 0); assert.strictEqual(result.stdout, ''); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -121,7 +123,7 @@ describe('bug #261: workflow guard blocks forced git add on worktree-agent branc assert.strictEqual(result.status, 0); assert.strictEqual(result.stdout, ''); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -132,7 +134,7 @@ describe('bug #261: workflow guard blocks forced git add on worktree-agent branc assert.strictEqual(result.status, 0); assert.strictEqual(result.stdout, ''); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -154,7 +156,7 @@ describe('bug #261: workflow guard blocks forced git add on worktree-agent branc /WORKFLOW ADVISORY/ ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); diff --git a/tests/bug-2760-codex-install-defensive.test.cjs b/tests/bug-2760-codex-install-defensive.test.cjs index fd1aba7b6..0517494c6 100644 --- a/tests/bug-2760-codex-install-defensive.test.cjs +++ b/tests/bug-2760-codex-install-defensive.test.cjs @@ -45,6 +45,8 @@ const { parseTomlToObject, } = require('../bin/install.js'); +const { cleanup } = require('./helpers.cjs'); + if (previousGsdTestMode === undefined) { delete process.env.GSD_TEST_MODE; } else { @@ -108,7 +110,7 @@ describe('#2760 defect 3 — Hooks AoT preservation across install/uninstall/rei }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('fresh install emits the two-level nested AoT schema (#2773)', () => { @@ -249,7 +251,7 @@ describe('#2760 fix 2 — Strip purges invalid legacy [agents] / [[agents]] rega }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('strips bare [agents] single-bracket block (no GSD marker, arbitrary user keys)', () => { @@ -345,7 +347,7 @@ describe('#2760 fix 3 — Post-write Codex schema validation', { concurrency: fa const result = validateCodexConfigSchema(content); assert.equal(result.ok, true, 'GSD-emitted config passes schema validation'); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); @@ -416,7 +418,7 @@ describe('#2760 fix 3 — Post-write Codex schema validation', { concurrency: fa ); } finally { delete installModule.__codexSchemaValidator; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); }); @@ -474,7 +476,7 @@ describe('#2760 fix 4 — Write-failure rollback (atomic write + snapshot restor afterEach(() => { fs.renameSync = originalRenameSync; fs.writeFileSync = originalWriteFileSync; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('pre-install config bytes survive when fs.renameSync throws over configPath', () => { @@ -616,7 +618,7 @@ describe('#2760 CR4 finding 2 — Legacy flat [[hooks]] block migrates to namesp }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('pre-install legacy flat [[hooks]] gsd-check-update + user namespaced [[hooks.SessionStart]] → post-install converges on namespaced AoT', () => { @@ -766,7 +768,7 @@ describe('#2760 CR4 finding 1 — atomicWriteFileSync failure aborts install (po afterEach(() => { fs.renameSync = originalRenameSync; console.log = originalConsoleLog; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('install throws and never prints "Done!" when atomicWriteFileSync fails on configPath', () => { @@ -846,7 +848,7 @@ describe('#2760 CR5 finding 1 — pre-write failures abort install (outer catch afterEach(() => { console.log = originalConsoleLog; delete installModule.__codexSchemaValidator; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('pre-write throw (validator throws, not returns {ok:false}) is fatal and restores snapshot', () => { @@ -996,7 +998,7 @@ describe('#2760 CR5 finding 3 — migration emits namespaced AoT (no flat/namesp }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('user has [[hooks.AfterTool]] AND legacy [hooks.SessionStart] → post-install both namespaced, no flat AoT', () => { diff --git a/tests/bug-2771-user-profile-manifest.test.cjs b/tests/bug-2771-user-profile-manifest.test.cjs index 7f4a6cf2a..0ca38a265 100644 --- a/tests/bug-2771-user-profile-manifest.test.cjs +++ b/tests/bug-2771-user-profile-manifest.test.cjs @@ -228,7 +228,7 @@ describe('manifest path safety', () => { outside = path.join(tmpDir, '..', `outside-managed-file-${path.basename(tmpDir)}.txt`); }); afterEach(() => { - if (outside) fs.rmSync(outside, { recursive: true, force: true }); + cleanup(outside); cleanup(tmpDir); }); diff --git a/tests/bug-2794-opencode-model-profile-overrides.test.cjs b/tests/bug-2794-opencode-model-profile-overrides.test.cjs index adbcca1e1..d00b6d7cb 100644 --- a/tests/bug-2794-opencode-model-profile-overrides.test.cjs +++ b/tests/bug-2794-opencode-model-profile-overrides.test.cjs @@ -35,7 +35,7 @@ const { install, } = require('../bin/install.js'); -const { createTempDir } = require('./helpers.cjs'); +const { createTempDir, cleanup } = require('./helpers.cjs'); const makeTmp = (prefix) => createTempDir(`gsd-2794-${prefix}-`); function writeJson(p, obj) { @@ -43,9 +43,6 @@ function writeJson(p, obj) { fs.writeFileSync(p, JSON.stringify(obj, null, 2), 'utf-8'); } -function rmr(p) { - try { fs.rmSync(p, { recursive: true, force: true }); } catch { /* noop */ } -} describe('bug-2794: readGsdRuntimeProfileResolver resolves opencode tier overrides', () => { let projectDir; @@ -62,8 +59,8 @@ describe('bug-2794: readGsdRuntimeProfileResolver resolves opencode tier overrid afterEach(() => { if (origHome === undefined) delete process.env.HOME; else process.env.HOME = origHome; - rmr(projectDir); - rmr(homeDir); + cleanup(projectDir); + cleanup(homeDir); }); test('resolves opencode sonnet tier to user-supplied model ID', () => { @@ -127,8 +124,8 @@ describe('bug-2794: OpenCode agent install embeds model_profile_overrides model' if (origHome === undefined) delete process.env.HOME; else process.env.HOME = origHome; process.chdir(origCwd); - rmr(projectDir); - rmr(homeDir); + cleanup(projectDir); + cleanup(homeDir); }); test('generated OpenCode agent frontmatter includes model from model_profile_overrides', () => { diff --git a/tests/bug-2831-opencode-home-path-prefix.test.cjs b/tests/bug-2831-opencode-home-path-prefix.test.cjs index bc1820f90..74e306628 100644 --- a/tests/bug-2831-opencode-home-path-prefix.test.cjs +++ b/tests/bug-2831-opencode-home-path-prefix.test.cjs @@ -27,6 +27,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('os'); +const { cleanup } = require('./helpers.cjs'); let computePathPrefix; @@ -148,7 +149,7 @@ describe('bug-2831: OpenCode pathPrefix uses absolute path on all platforms', () `output should include absolute path with @ prefix; got:\n${content}` ); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); }); diff --git a/tests/bug-2836-audit-open-summary-uat-drift.test.cjs b/tests/bug-2836-audit-open-summary-uat-drift.test.cjs index 367b52247..7696a5ff3 100644 --- a/tests/bug-2836-audit-open-summary-uat-drift.test.cjs +++ b/tests/bug-2836-audit-open-summary-uat-drift.test.cjs @@ -23,15 +23,12 @@ const os = require('node:os'); const auditModule = require('../get-shit-done/bin/lib/audit.cjs'); const { auditOpenArtifacts } = auditModule; +const { cleanup } = require('./helpers.cjs'); function mkTmp() { return fs.mkdtempSync(path.join(os.tmpdir(), 'bug-2836-')); } -function rmTmp(dir) { - try { fs.rmSync(dir, { recursive: true, force: true }); } catch {} -} - describe('bug #2836: audit-open quick-task summary filename + UAT terminal status', () => { // Ensure GSD env vars do not redirect planningDir() away from our fixture. let prevProject, prevWorkstream; @@ -70,7 +67,7 @@ describe('bug #2836: audit-open quick-task summary filename + UAT terminal statu ); assert.equal(result.counts.quick_tasks, 0); } finally { - rmTmp(cwd); + cleanup(cwd); } }); @@ -95,7 +92,7 @@ describe('bug #2836: audit-open quick-task summary filename + UAT terminal statu ); assert.equal(result.counts.uat_gaps, 0); } finally { - rmTmp(cwd); + cleanup(cwd); } }); @@ -114,7 +111,7 @@ describe('bug #2836: audit-open quick-task summary filename + UAT terminal statu const realUatGaps = result.items.uat_gaps.filter(i => !i.scan_error); assert.equal(realUatGaps.length, 0); } finally { - rmTmp(cwd); + cleanup(cwd); } }); @@ -134,7 +131,7 @@ describe('bug #2836: audit-open quick-task summary filename + UAT terminal statu assert.equal(realUatGaps.length, 1, 'pending UAT must still be flagged'); assert.equal(realUatGaps[0].status, 'pending'); } finally { - rmTmp(cwd); + cleanup(cwd); } }); @@ -153,7 +150,7 @@ describe('bug #2836: audit-open quick-task summary filename + UAT terminal statu assert.equal(realQuickTasks.length, 1); assert.equal(realQuickTasks[0].status, 'missing'); } finally { - rmTmp(cwd); + cleanup(cwd); } }); }); diff --git a/tests/bug-2916-handle-branching-default-base.test.cjs b/tests/bug-2916-handle-branching-default-base.test.cjs index a51cbf2c0..633975531 100644 --- a/tests/bug-2916-handle-branching-default-base.test.cjs +++ b/tests/bug-2916-handle-branching-default-base.test.cjs @@ -26,6 +26,8 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); + const EXECUTE_PHASE_PATH = path.join( __dirname, '..', @@ -163,7 +165,7 @@ function runHandleBranchingStep(bash, cwd, branchName) { stdio: ['pipe', 'pipe', 'pipe'], }).toString(); } finally { - fs.rmSync(scriptDir, { recursive: true, force: true }); + cleanup(scriptDir); } } @@ -209,7 +211,7 @@ describe('handle_branching branches off origin/HEAD, not current HEAD (#2916)', `new phase branch tip must equal ${upstream} tip` ); } finally { - fs.rmSync(root, { recursive: true, force: true }); + cleanup(root); } }); } @@ -240,7 +242,7 @@ describe('handle_branching branches off origin/HEAD, not current HEAD (#2916)', 'existing-branch tip must be preserved (no rebase/reset)' ); } finally { - fs.rmSync(root, { recursive: true, force: true }); + cleanup(root); } }); }); diff --git a/tests/bug-2943-config-get-context-window-default.test.cjs b/tests/bug-2943-config-get-context-window-default.test.cjs index b986b5ff5..d0488a68e 100644 --- a/tests/bug-2943-config-get-context-window-default.test.cjs +++ b/tests/bug-2943-config-get-context-window-default.test.cjs @@ -27,6 +27,7 @@ const { execFileSync } = require('node:child_process'); const GSD_TOOLS = path.join(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); const { ERROR_REASON } = require(path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'core.cjs')); +const { cleanup } = require('./helpers.cjs'); describe('bug-2943: config-get returns schema default for context_window', () => { let tmpDir; @@ -39,7 +40,7 @@ describe('bug-2943: config-get returns schema default for context_window', () => }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); /** diff --git a/tests/bug-2969-verify-reapply-patches.test.cjs b/tests/bug-2969-verify-reapply-patches.test.cjs index 90a8fed89..dd7fbcfd9 100644 --- a/tests/bug-2969-verify-reapply-patches.test.cjs +++ b/tests/bug-2969-verify-reapply-patches.test.cjs @@ -27,6 +27,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const cp = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); // Script lives at get-shit-done/bin/ so the installer ships it under @@ -47,7 +48,7 @@ function writeFile(absPath, content) { function resetFixture({ withPristine = true } = {}) { for (const dir of [patchesDir, configDir, pristineDir]) { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } fs.mkdirSync(patchesDir); fs.mkdirSync(configDir); @@ -79,7 +80,7 @@ before(() => { }); after(() => { - fs.rmSync(tmpRoot, { recursive: true, force: true }); + cleanup(tmpRoot); }); describe('Bug #2969: deterministic Step 5 verification gate', () => { diff --git a/tests/bug-2973-profile-user-skills-path.test.cjs b/tests/bug-2973-profile-user-skills-path.test.cjs index 61609cacf..11de58da1 100644 --- a/tests/bug-2973-profile-user-skills-path.test.cjs +++ b/tests/bug-2973-profile-user-skills-path.test.cjs @@ -40,6 +40,8 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); +const { cleanup } = require('./helpers.cjs'); + const ROOT = path.join(__dirname, '..'); const PROFILE_OUTPUT = path.join(ROOT, 'get-shit-done', 'bin', 'lib', 'profile-output.cjs'); const WORKFLOW = path.join(ROOT, 'get-shit-done', 'workflows', 'profile-user.md'); @@ -85,7 +87,7 @@ describe('Bug #2973: dev-preferences default writer path is skills/gsd-dev-prefe assert.equal(fs.existsSync(legacyPath), false, `writer must not create ${legacyPath} (#2973)`); } finally { - fs.rmSync(tmpHome, { recursive: true, force: true }); + cleanup(tmpHome); } }); }); @@ -130,7 +132,7 @@ describe('Bug #2973: installer migrates existing legacy dev-preferences.md to sk assert.equal(fs.existsSync(skillFile), true, `expected SKILL.md at ${skillFile}`); assert.equal(fs.readFileSync(skillFile, 'utf-8'), '# my legacy preferences\n'); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); @@ -148,7 +150,7 @@ describe('Bug #2973: installer migrates existing legacy dev-preferences.md to sk // Existing content untouched. assert.equal(fs.readFileSync(skillFile, 'utf-8'), '# user-customized skill\n'); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); }); @@ -200,7 +202,7 @@ describe('Bug #2973 (#3003 CR): installRuntimeArtifacts preserves user-owned gsd assert.equal(fs.readFileSync(skillFile, 'utf-8'), userContent, 'user content must be byte-identical after the install'); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -239,7 +241,7 @@ describe('Bug #2973 (#3003 CR): installRuntimeArtifacts preserves user-owned gsd assert.equal(fs.existsSync(path.join(staleSkillDir, 'SKILL.md')), true, 'fresh SKILL.md from source must be installed after wipe'); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); }); diff --git a/tests/bug-2995-post-install-script-paths.test.cjs b/tests/bug-2995-post-install-script-paths.test.cjs index 6ef75ba00..e9cc404d5 100644 --- a/tests/bug-2995-post-install-script-paths.test.cjs +++ b/tests/bug-2995-post-install-script-paths.test.cjs @@ -11,6 +11,7 @@ const ROOT = path.join(__dirname, '..'); const { auditWorkflowScriptPaths, AUDIT_FINDING } = require( path.join(ROOT, 'scripts', 'audit-workflow-script-paths.cjs'), ); +const { cleanup } = require('./helpers.cjs'); // auditWorkflowScriptPaths is a pure function: it walks workflowsDir, // extracts every ${GSD_HOME}/ script reference, and returns a @@ -39,7 +40,7 @@ function fixtureRepo({ workflows, files }) { } before(() => { tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2995-')); }); -after(() => { fs.rmSync(tmpRoot, { recursive: true, force: true }); }); +after(() => { cleanup(tmpRoot); }); describe('Bug #2995: post-install script-paths audit (#2995)', () => { test('AUDIT_FINDING enum exposes the documented codes', () => { diff --git a/tests/bug-2998-pristine-dir-populated.test.cjs b/tests/bug-2998-pristine-dir-populated.test.cjs index ed594ae53..52433e367 100644 --- a/tests/bug-2998-pristine-dir-populated.test.cjs +++ b/tests/bug-2998-pristine-dir-populated.test.cjs @@ -28,6 +28,7 @@ const crypto = require('node:crypto'); const ROOT = path.join(__dirname, '..'); const INSTALL = require(path.join(ROOT, 'bin', 'install.js')); +const { cleanup } = require('./helpers.cjs'); function sha256(content) { return crypto.createHash('sha256').update(content).digest('hex'); @@ -52,7 +53,7 @@ describe('Bug #2998: populatePristineDir is exported and writes pristine for mod }); assert.equal(written, 0); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -87,7 +88,7 @@ describe('Bug #2998: populatePristineDir is exported and writes pristine for mod const content = fs.readFileSync(out, 'utf-8'); assert.ok(content.length > 0, 'pristine file should be non-empty'); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -107,7 +108,7 @@ describe('Bug #2998: populatePristineDir is exported and writes pristine for mod const out = path.join(pristineDir, 'get-shit-done/this-path-does-not-exist.md'); assert.equal(fs.existsSync(out), false, 'pristine should not contain ghost paths'); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -132,8 +133,8 @@ describe('Bug #2998: populatePristineDir is exported and writes pristine for mod const b = fs.readFileSync(path.join(tmp2, 'gsd-pristine', candidate)); assert.equal(sha256(a), sha256(b), 'two runs of the same inputs must yield identical pristine content'); } finally { - fs.rmSync(tmp1, { recursive: true, force: true }); - fs.rmSync(tmp2, { recursive: true, force: true }); + cleanup(tmp1); + cleanup(tmp2); } }); }); @@ -160,7 +161,7 @@ describe('Bug #2998 (#3004 CR): pristine expansion covers every manifest install assert.equal(written, 1, 'expected agents/ path to be staged and copied to pristine'); assert.equal(fs.existsSync(path.join(pristineDir, candidate)), true); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -184,7 +185,7 @@ describe('Bug #2998 (#3004 CR): pristine expansion covers every manifest install assert.equal(fs.existsSync(path.join(pristineDir, a)), true); assert.equal(fs.existsSync(path.join(pristineDir, b)), true); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); }); diff --git a/tests/bug-3127-state-begin-phase-idempotent.test.cjs b/tests/bug-3127-state-begin-phase-idempotent.test.cjs index 2fb9a075b..6808c9309 100644 --- a/tests/bug-3127-state-begin-phase-idempotent.test.cjs +++ b/tests/bug-3127-state-begin-phase-idempotent.test.cjs @@ -23,6 +23,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); @@ -105,7 +106,7 @@ describe('bug #3127: state.begin-phase idempotency guard', () => { 'begin-phase reset Current Plan to 1 on a mid-flight phase — idempotency guard not applied'); } } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -123,7 +124,7 @@ describe('bug #3127: state.begin-phase idempotency guard', () => { 'begin-phase overwrote stopped_at narrative on a mid-flight phase', ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -142,7 +143,7 @@ describe('bug #3127: state.begin-phase idempotency guard', () => { 'begin-phase should set Current Plan to 1 on a fresh phase'); } } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -172,7 +173,7 @@ describe('bug #3127: state.begin-phase idempotency guard', () => { else process.env.GSD_TEST_MODE = origTestMode; if (origNowMs === undefined) delete process.env.GSD_NOW_MS; else process.env.GSD_NOW_MS = origNowMs; - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); diff --git a/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs b/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs index 524dd29ce..e2f46fdf2 100644 --- a/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs +++ b/tests/bug-3212-execute-phase-stall-safe-resume.test.cjs @@ -9,6 +9,7 @@ const { spawnSync } = require('node:child_process'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); @@ -47,7 +48,7 @@ describe('bug #3212 execute-phase stall detection and safe resume', () => { test('config-get returns schema defaults for executor stall detector keys', (t) => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3212-')); - t.after(() => fs.rmSync(tmp, { recursive: true, force: true })); + t.after(() => cleanup(tmp)); fs.mkdirSync(path.join(tmp, '.planning')); fs.writeFileSync(path.join(tmp, '.planning/config.json'), '{}\n'); diff --git a/tests/bug-3245-codex-toml-floats.test.cjs b/tests/bug-3245-codex-toml-floats.test.cjs index 1b0dd5e00..8f8695800 100644 --- a/tests/bug-3245-codex-toml-floats.test.cjs +++ b/tests/bug-3245-codex-toml-floats.test.cjs @@ -31,6 +31,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { execFileSync } = require('child_process'); +const { cleanup } = require('./helpers.cjs'); const { parseTomlToObject, validateCodexConfigSchema, install } = require('../bin/install.js'); const installModule = require('../bin/install.js'); @@ -224,7 +225,7 @@ describe('#3245 — install succeeds with TOML float in pre-existing config', { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('install completes when config.toml contains tool_timeout_sec = 20.0', () => { @@ -353,7 +354,7 @@ describe('#3245 — idempotent rollback reverts skills/, agents/, and VERSION', afterEach(() => { delete installModule.__codexSchemaValidator; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('validation failure rolls back skills/, agents/, and VERSION to pre-install state', () => { diff --git a/tests/bug-3285-codex-hooks-state-allowed.test.cjs b/tests/bug-3285-codex-hooks-state-allowed.test.cjs index abca2a395..9c2f1ab13 100644 --- a/tests/bug-3285-codex-hooks-state-allowed.test.cjs +++ b/tests/bug-3285-codex-hooks-state-allowed.test.cjs @@ -27,6 +27,7 @@ const { execFileSync } = require('child_process'); const { validateCodexConfigSchema, install } = require('../bin/install.js'); const installModule = require('../bin/install.js'); +const { cleanup } = require('./helpers.cjs'); if (previousGsdTestMode === undefined) { delete process.env.GSD_TEST_MODE; @@ -219,7 +220,7 @@ describe('#3285 — install succeeds when config.toml contains hooks.state entri }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('install does not throw when config.toml contains hooks.state trust entries', () => { diff --git a/tests/bug-3407-pristine-stale-content.test.cjs b/tests/bug-3407-pristine-stale-content.test.cjs index 072297a06..36d5ff29b 100644 --- a/tests/bug-3407-pristine-stale-content.test.cjs +++ b/tests/bug-3407-pristine-stale-content.test.cjs @@ -41,6 +41,7 @@ const crypto = require('node:crypto'); const ROOT = path.join(__dirname, '..'); const INSTALL = require(path.join(ROOT, 'bin', 'install.js')); +const { cleanup } = require('./helpers.cjs'); const MANIFEST_NAME = 'gsd-file-manifest.json'; const PATCHES_DIR_NAME = 'gsd-local-patches'; @@ -63,7 +64,7 @@ describe('Bug #3407: saveLocalPatches preserves old-release pristine across upgr fs.mkdirSync(configDir, { recursive: true }); fs.mkdirSync(fakeSrcDir, { recursive: true }); t.after(() => { - try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* best-effort */ } + cleanup(tmpDir); }); }); diff --git a/tests/bug-3441-path-action-projection.test.cjs b/tests/bug-3441-path-action-projection.test.cjs index dcf010470..d396815bf 100644 --- a/tests/bug-3441-path-action-projection.test.cjs +++ b/tests/bug-3441-path-action-projection.test.cjs @@ -17,15 +17,12 @@ const projection = require(path.join( 'shell-command-projection.cjs', )); const install = require(path.join(__dirname, '..', 'bin', 'install.js')); -const { withIsolatedProcessState } = require('./helpers.cjs'); +const { withIsolatedProcessState, cleanup } = require('./helpers.cjs'); function createTempHome() { return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-home-3441-')); } -function cleanup(dir) { - fs.rmSync(dir, { recursive: true, force: true }); -} describe('bug #3441: PATH guidance is projected from typed shell action IR', () => { test('projection module exports PATH action projection helper', () => { diff --git a/tests/bug-3442-shim-projection-drift-guard.test.cjs b/tests/bug-3442-shim-projection-drift-guard.test.cjs index 78a8f09bd..c959b7d13 100644 --- a/tests/bug-3442-shim-projection-drift-guard.test.cjs +++ b/tests/bug-3442-shim-projection-drift-guard.test.cjs @@ -8,6 +8,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { spawnSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.resolve(__dirname, '..'); const DRIFT_LINT = path.join(ROOT, 'scripts', 'lint-shell-command-projection-drift.cjs'); @@ -44,7 +45,7 @@ describe('bug #3442: shim/wrapper serialized-command drift guard', () => { const result = runLint(fixture); assert.notEqual(result.status, 0, 'inline shim renderer should be rejected by the drift guard'); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -64,7 +65,7 @@ describe('bug #3442: shim/wrapper serialized-command drift guard', () => { const result = runLint(fixture); assert.equal(result.status, 0, `spawnSync/execFileSync should remain allowed:\n${result.stderr || result.stdout}`); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); }); diff --git a/tests/bug-3509-path-spaces.test.cjs b/tests/bug-3509-path-spaces.test.cjs index 5e09ff89d..2b233bc1f 100644 --- a/tests/bug-3509-path-spaces.test.cjs +++ b/tests/bug-3509-path-spaces.test.cjs @@ -16,7 +16,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('os'); -const { runGsdTools } = require('./helpers.cjs'); +const { runGsdTools, cleanup } = require('./helpers.cjs'); // Create a tmpdir whose name always contains a space — this is the invariant // that was violated on /Volumes/Mini Me/... machines. @@ -24,10 +24,6 @@ function createSpacedTmpDir(prefix = 'path with spaces-') { return fs.mkdtempSync(path.join(os.tmpdir(), prefix)); } -function cleanup(dir) { - fs.rmSync(dir, { recursive: true, force: true }); -} - // ─── dispatcher --cwd= with space in path ──────────────────────────────────── describe('bug-3509: --cwd= survives spaces in path', () => { diff --git a/tests/bug-3537-padded-id-against-unpadded-roadmap.test.cjs b/tests/bug-3537-padded-id-against-unpadded-roadmap.test.cjs index 54a06b6c4..d6a15427c 100644 --- a/tests/bug-3537-padded-id-against-unpadded-roadmap.test.cjs +++ b/tests/bug-3537-padded-id-against-unpadded-roadmap.test.cjs @@ -27,6 +27,8 @@ const path = require('node:path'); const os = require('node:os'); const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); + const gsdTools = path.resolve(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); function run(args, cwd) { @@ -159,8 +161,8 @@ function expectParity({ verbWithPadded, verbWithUnpadded, fixtureOpts }) { return { aRoadmap, bRoadmap, ra, rb }; } finally { - fs.rmSync(tmpA, { recursive: true, force: true }); - fs.rmSync(tmpB, { recursive: true, force: true }); + cleanup(tmpA); + cleanup(tmpB); } } @@ -207,7 +209,7 @@ describe('bug #3537: phase verbs accept padded ids against un-padded ROADMAP pro 'verb must return non-empty section under both invocations' ); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -247,7 +249,7 @@ describe('bug #3537: phase verbs accept padded ids against un-padded ROADMAP pro 'must not propose 2.1 when 2.7 already exists in ROADMAP' ); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -298,8 +300,8 @@ describe('bug #3537: phase verbs accept padded ids against un-padded ROADMAP pro // crash mid-run. assert.ok(fs.existsSync(path.join(tmpB, '.planning', 'ROADMAP.md'))); } finally { - fs.rmSync(tmpA, { recursive: true, force: true }); - fs.rmSync(tmpB, { recursive: true, force: true }); + cleanup(tmpA); + cleanup(tmpB); } }); @@ -339,10 +341,10 @@ describe('bug #3537: phase verbs accept padded ids against un-padded ROADMAP pro // and to assert the run did not corrupt the rest of the file. assert.ok(before.length > 0); } finally { - fs.rmSync(tmp2, { recursive: true, force: true }); + cleanup(tmp2); } } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); diff --git a/tests/bug-3542-executor-git-stash-prohibition.test.cjs b/tests/bug-3542-executor-git-stash-prohibition.test.cjs index 2ae3129ff..0f48fea7b 100644 --- a/tests/bug-3542-executor-git-stash-prohibition.test.cjs +++ b/tests/bug-3542-executor-git-stash-prohibition.test.cjs @@ -35,6 +35,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { execSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const EXECUTOR_PATH = path.join(__dirname, '..', 'agents', 'gsd-executor.md'); @@ -172,6 +173,6 @@ test('bug-3542: stash pushed in main checkout is visible inside a linked worktre 'contamination the executor prohibition exists to prevent.', ); } finally { - fs.rmSync(tmpRoot, { recursive: true, force: true }); + cleanup(tmpRoot); } }); diff --git a/tests/bug-3571-configuration-manifest-install-path.test.cjs b/tests/bug-3571-configuration-manifest-install-path.test.cjs index 8e4af7f59..28253d5e5 100644 --- a/tests/bug-3571-configuration-manifest-install-path.test.cjs +++ b/tests/bug-3571-configuration-manifest-install-path.test.cjs @@ -20,7 +20,7 @@ const SHARED_DIR = path.join(REPO_ROOT, 'get-shit-done', 'bin', 'shared'); const { install } = require('../bin/install.js'); -const { createTempDir } = require('./helpers.cjs'); +const { createTempDir, cleanup } = require('./helpers.cjs'); const makeTmpDir = () => createTempDir('gsd-3571-'); function silenceConsole(fn) { @@ -65,7 +65,7 @@ describe('bug #3571: configuration generated manifests resolve in install layout } else { process.env.GSD_EXPLICIT_CONFIG_DIR = savedExplicitConfigDir; } - fs.rmSync(tmpRoot, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 }); + cleanup(tmpRoot); }); test('co-located bin/shared manifests let configuration.cjs load without sdk/shared', () => { diff --git a/tests/bug-3584-runtime-slash-formatter.test.cjs b/tests/bug-3584-runtime-slash-formatter.test.cjs index f96b35189..1c7a21709 100644 --- a/tests/bug-3584-runtime-slash-formatter.test.cjs +++ b/tests/bug-3584-runtime-slash-formatter.test.cjs @@ -25,6 +25,7 @@ const ROOT = path.join(__dirname, '..'); const { formatGsdSlash, resolveRuntime } = require( path.join(ROOT, 'get-shit-done', 'bin', 'lib', 'runtime-slash.cjs'), ); +const { cleanup } = require('./helpers.cjs'); describe('formatGsdSlash — runtime-aware slash command formatter', () => { describe('hyphen-form runtimes (claude, cursor, opencode, kilo, etc.)', () => { @@ -235,7 +236,7 @@ describe('resolveRuntime — env > config > default', () => { const fs = require('fs'); const os = require('os'); const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3584-')); - t.after(() => fs.rmSync(tmp, { recursive: true, force: true })); + t.after(() => cleanup(tmp)); fs.mkdirSync(path.join(tmp, '.planning'), { recursive: true }); fs.writeFileSync( @@ -256,7 +257,7 @@ describe('resolveRuntime — env > config > default', () => { const fs = require('fs'); const os = require('os'); const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3584-')); - t.after(() => fs.rmSync(tmp, { recursive: true, force: true })); + t.after(() => cleanup(tmp)); fs.mkdirSync(path.join(tmp, '.planning'), { recursive: true }); fs.writeFileSync( @@ -277,7 +278,7 @@ describe('resolveRuntime — env > config > default', () => { const fs = require('fs'); const os = require('os'); const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3584-')); - t.after(() => fs.rmSync(tmp, { recursive: true, force: true })); + t.after(() => cleanup(tmp)); fs.mkdirSync(path.join(tmp, '.planning'), { recursive: true }); fs.writeFileSync( diff --git a/tests/bug-3631-router-raw-flag.test.cjs b/tests/bug-3631-router-raw-flag.test.cjs index 1ef627513..50b15b968 100644 --- a/tests/bug-3631-router-raw-flag.test.cjs +++ b/tests/bug-3631-router-raw-flag.test.cjs @@ -24,6 +24,7 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const GSD_TOOLS = path.resolve(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); @@ -103,7 +104,7 @@ describe('bug #3631 — SDK family routers forward --raw to output()', () => { `expected next-decimal of base "1" to be 1.1 or 01.1; got: ${trimmed}` ); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -128,7 +129,7 @@ describe('bug #3631 — SDK family routers forward --raw to output()', () => { `--raw must emit the section body containing the Phase 2 heading; got: ${trimmed.slice(0, 80)}` ); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); }); diff --git a/tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs b/tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs index e25dc2dff..d66765b8d 100644 --- a/tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs +++ b/tests/bug-3657-verify-reapply-patches-pristine-drift.test.cjs @@ -36,6 +36,7 @@ const crypto = require('node:crypto'); const os = require('node:os'); const path = require('node:path'); const cp = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); const SCRIPT = path.join(ROOT, 'get-shit-done', 'bin', 'verify-reapply-patches.cjs'); @@ -66,7 +67,7 @@ function writeBackupMeta(overrides = {}) { function resetFixture() { for (const dir of [patchesDir, configDir, pristineDir]) { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } fs.mkdirSync(patchesDir); fs.mkdirSync(configDir); @@ -98,7 +99,7 @@ before(() => { }); after(() => { - fs.rmSync(tmpRoot, { recursive: true, force: true }); + cleanup(tmpRoot); }); // --------------------------------------------------------------------------- diff --git a/tests/bug-3670-cursor-local-install-migration-lock.test.cjs b/tests/bug-3670-cursor-local-install-migration-lock.test.cjs index c341bb90c..2e866d187 100644 --- a/tests/bug-3670-cursor-local-install-migration-lock.test.cjs +++ b/tests/bug-3670-cursor-local-install-migration-lock.test.cjs @@ -57,6 +57,7 @@ const { INSTALL_MIGRATION_LOCK_NAME, runInstallerMigrations, } = require('../get-shit-done/bin/lib/installer-migrations.cjs'); +const { cleanup } = require('./helpers.cjs'); // --------------------------------------------------------------------------- // Helpers @@ -66,10 +67,6 @@ function createTempDir() { return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3670-')); } -function cleanup(dir) { - fs.rmSync(dir, { recursive: true, force: true }); -} - function lockPath(dir) { return path.join(dir, INSTALL_MIGRATION_LOCK_NAME); } diff --git a/tests/bug-3683-command-colon-namespace-leak.test.cjs b/tests/bug-3683-command-colon-namespace-leak.test.cjs index 06db42156..27dea80d9 100644 --- a/tests/bug-3683-command-colon-namespace-leak.test.cjs +++ b/tests/bug-3683-command-colon-namespace-leak.test.cjs @@ -34,6 +34,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); const INSTALL_PATH = path.join(REPO_ROOT, 'bin', 'install.js'); @@ -148,9 +149,7 @@ describe('bug #3683 — command body colon-namespace leak (Claude local install) }); after(() => { - if (tmpDir) { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); - } + cleanup(tmpDir); }); test('E0: staged commands/gsd/ directory exists after install', () => { diff --git a/tests/bug-3683-workflow-colon-namespace-leak.test.cjs b/tests/bug-3683-workflow-colon-namespace-leak.test.cjs index 65baffeee..0a0d9ae13 100644 --- a/tests/bug-3683-workflow-colon-namespace-leak.test.cjs +++ b/tests/bug-3683-workflow-colon-namespace-leak.test.cjs @@ -37,6 +37,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); const INSTALL_PATH = path.join(REPO_ROOT, 'bin', 'install.js'); @@ -135,9 +136,7 @@ describe('bug #3683 — workflow/reference colon-namespace leak (Claude local in }); after(() => { - if (claudeTmpDir) { - fs.rmSync(claudeTmpDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); - } + cleanup(claudeTmpDir); }); // ------------------------------------------------------------------------- @@ -409,9 +408,7 @@ describe('bug #3683 — workflow/reference colon-namespace leak (Claude local in }); after(() => { - if (tmpDir) { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); - } + cleanup(tmpDir); }); test('G0: staged gemini get-shit-done/workflows/ directory exists after install', () => { diff --git a/tests/bug-3706-ui-safety-gate-false-positives.test.cjs b/tests/bug-3706-ui-safety-gate-false-positives.test.cjs index 531f5d0dd..4823cbb2d 100644 --- a/tests/bug-3706-ui-safety-gate-false-positives.test.cjs +++ b/tests/bug-3706-ui-safety-gate-false-positives.test.cjs @@ -29,6 +29,7 @@ const fs = require('fs'); const path = require('path'); const { spawnSync } = require('node:child_process'); const os = require('node:os'); +const { cleanup } = require('./helpers.cjs'); const HELPER_PATH = path.join(__dirname, '..', 'bin', 'lib', 'ui-safety-gate.cjs'); const PLAN_PHASE_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'plan-phase.md'); @@ -298,7 +299,7 @@ describe('UI gate resolves the helper against RUNTIME_DIR, not the consuming rep assert.strictEqual(res.stdout.trim(), '0', 'helper must be found via RUNTIME_DIR and report UI present — not silently no-op'); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -308,7 +309,7 @@ describe('UI gate resolves the helper against RUNTIME_DIR, not the consuming rep const res = runGateFrom(tmp, 'Requirements: backend REST API only'); assert.strictEqual(res.stdout.trim(), '1'); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -334,8 +335,8 @@ describe('UI gate resolves the helper against RUNTIME_DIR, not the consuming rep assert.strictEqual(res.stdout.trim(), '0', 'helper must be found via get-shit-done/bin/lib/ in installed layout and report UI present'); } finally { - fs.rmSync(fakeRuntime, { recursive: true, force: true }); - fs.rmSync(consumingProject, { recursive: true, force: true }); + cleanup(fakeRuntime); + cleanup(consumingProject); } }); }); diff --git a/tests/bug-3707-locked-worktree-cleanup.test.cjs b/tests/bug-3707-locked-worktree-cleanup.test.cjs index 301834ad1..6fa21f79a 100644 --- a/tests/bug-3707-locked-worktree-cleanup.test.cjs +++ b/tests/bug-3707-locked-worktree-cleanup.test.cjs @@ -12,6 +12,7 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); const { execFileSync, spawnSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const { executeWorktreeWaveCleanupPlan, @@ -135,7 +136,7 @@ describe('bug-3707: executeWorktreeWaveCleanupPlan unlocks and retries on locked }); afterEach(() => { - fs.rmSync(tmpBase, { recursive: true, force: true }); + cleanup(tmpBase); }); test('removes a locked worktree after unlock-retry (real-fs)', () => { @@ -221,7 +222,7 @@ describe('bug-3707: reapOrphanWorktrees', () => { }); afterEach(() => { - fs.rmSync(tmpBase, { recursive: true, force: true }); + cleanup(tmpBase); }); // ── Dead PID + merged branch → reap ──────────────────────────────────────── @@ -424,7 +425,7 @@ describe('bug-3707: reapOrphanWorktrees — adversarial edge cases', () => { }); afterEach(() => { - fs.rmSync(tmpBase, { recursive: true, force: true }); + cleanup(tmpBase); }); // ── Gap 1: Non-numeric lock content (real Claude Code format) → ALIVE (fail-closed) ── diff --git a/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs b/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs index b9ba93697..0cb836bb0 100644 --- a/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs +++ b/tests/bug-376-claude-js-hook-gsd-rewriter.test.cjs @@ -27,6 +27,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); const INSTALL_PATH = path.join(REPO_ROOT, 'bin', 'install.js'); @@ -129,9 +130,7 @@ describe('bug #376 — Suite 1: Claude install rewrites /gsd: → /gsd- in hook }); after(() => { - if (tmpDir) { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); - } + cleanup(tmpDir); }); test('1a: hooks/ directory is created by the Claude local install', () => { @@ -206,9 +205,7 @@ describe('bug #376 — Suite 2: Cursor install still rewrites /gsd: → /gsd- (r }); after(() => { - if (tmpDir) { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); - } + cleanup(tmpDir); }); test('2a: .cursor/ directory is created by the Cursor local install', () => { diff --git a/tests/bug-397-state-preserve-executor-authored.test.cjs b/tests/bug-397-state-preserve-executor-authored.test.cjs index 98406ab9b..8f6c0a76f 100644 --- a/tests/bug-397-state-preserve-executor-authored.test.cjs +++ b/tests/bug-397-state-preserve-executor-authored.test.cjs @@ -33,6 +33,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); const TOOLS_PATH = path.join(ROOT, 'get-shit-done', 'bin', 'gsd-tools.cjs'); @@ -234,7 +235,7 @@ describe('bug #397: executor-authored STATE.md fields must be preserved', () => `record-session overwrote executor-authored Resume File with '${rfMatch[1].trim()}'`, ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -253,7 +254,7 @@ describe('bug #397: executor-authored STATE.md fields must be preserved', () => `Expected 'None' to remain when it was already 'None', got: ${rfMatch[1].trim()}`, ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -272,7 +273,7 @@ describe('bug #397: executor-authored STATE.md fields must be preserved', () => `Expected explicit --resume-file value to be written, got: ${rfMatch[1].trim()}`, ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -293,7 +294,7 @@ describe('bug #397: executor-authored STATE.md fields must be preserved', () => `advance-plan overwrote executor-authored Status: got '${statusMatch[1].trim()}'`, ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -318,7 +319,7 @@ describe('bug #397: executor-authored STATE.md fields must be preserved', () => `Expected phase-complete Status text, got: '${statusMatch[1].trim()}'`, ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -339,7 +340,7 @@ describe('bug #397: executor-authored STATE.md fields must be preserved', () => `advance-plan overwrote executor-authored Last Activity: got '${laMatch[1].trim()}'`, ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -368,7 +369,7 @@ describe('bug #397: executor-authored STATE.md fields must be preserved', () => `advance-plan overwrote executor-authored Current Position Last activity: got '${posActivityMatch[1].trim()}'`, ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); diff --git a/tests/bug-410-install-defaults-test-mode-guard.test.cjs b/tests/bug-410-install-defaults-test-mode-guard.test.cjs index 37d6b93ce..eff6c61b4 100644 --- a/tests/bug-410-install-defaults-test-mode-guard.test.cjs +++ b/tests/bug-410-install-defaults-test-mode-guard.test.cjs @@ -11,6 +11,7 @@ */ const { test, describe } = require('node:test'); +const { cleanup } = require('./helpers.cjs'); const assert = require('node:assert/strict'); const path = require('node:path'); const os = require('node:os'); @@ -109,7 +110,7 @@ describe('Bug #410: finishInstall non-Claude runtime + GSD_TEST_MODE side-effect } finally { // Restore GSD_TEST_MODE and clean up the written file. process.env.GSD_TEST_MODE = saved; - try { fs.rmSync(DEFAULTS_PATH); } catch { /* already gone */ } + cleanup(DEFAULTS_PATH); try { fs.rmdirSync(GSD_DIR); } catch { /* not empty or already gone */ } } }); diff --git a/tests/bug-416-archive-dir-null.test.cjs b/tests/bug-416-archive-dir-null.test.cjs index 113391092..2fadcbb7f 100644 --- a/tests/bug-416-archive-dir-null.test.cjs +++ b/tests/bug-416-archive-dir-null.test.cjs @@ -25,7 +25,7 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); -const { runGsdTools } = require('./helpers.cjs'); +const { runGsdTools, cleanup } = require('./helpers.cjs'); // ── helpers ────────────────────────────────────────────────────────────────── @@ -108,7 +108,7 @@ describe('bug #416 case 1: STATE.md v6.0 with only v5.0-phases/ on disk → null ]); }); - after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + after(() => { cleanup(tmpDir); }); test('validate health emits zero W007 warnings (no prior-milestone phases surfaced)', () => { const result = runGsdTools(['validate', 'health', '--json'], tmpDir); @@ -150,7 +150,7 @@ describe('bug #416 case 2: no STATE.md + multiple archives → version-sort fall ]); }); - after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + after(() => { cleanup(tmpDir); }); test('validate health succeeds and does not emit W007 for v5.0 archive phases', () => { const result = runGsdTools(['validate', 'health', '--json'], tmpDir); @@ -196,7 +196,7 @@ describe('bug #416 case 3: STATE.md v5.0 with matching v5.0-phases/ → returns ]); }); - after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + after(() => { cleanup(tmpDir); }); test('validate health emits zero W007 — archive phases are in ROADMAP and active', () => { const result = runGsdTools(['validate', 'health', '--json'], tmpDir); diff --git a/tests/bug-444-resolver-local-claude-install.test.cjs b/tests/bug-444-resolver-local-claude-install.test.cjs index 8a48c9e70..334ee3a04 100644 --- a/tests/bug-444-resolver-local-claude-install.test.cjs +++ b/tests/bug-444-resolver-local-claude-install.test.cjs @@ -22,6 +22,7 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'get-shit-done', 'workflows'); const SNIPPET_FILE = path.join(WORKFLOWS_DIR, '_runtime-launcher.snippet.sh'); @@ -142,8 +143,8 @@ describe('bug-444: resolver finds repo-local .claude install', () => { `Expected stub output "LOCAL_CLAUDE_STUB:ping,test" but got:\n${stdout.trim()}`, ); } finally { - fs.rmSync(fakeRoot, { recursive: true, force: true }); - fs.rmSync(fakeHome, { recursive: true, force: true }); + cleanup(fakeRoot); + cleanup(fakeHome); } }); @@ -204,8 +205,8 @@ describe('bug-444: resolver finds repo-local .claude install', () => { `Expected $HOME/.claude stub NOT to be invoked, but got:\n${stdout.trim()}`, ); } finally { - fs.rmSync(fakeRoot, { recursive: true, force: true }); - fs.rmSync(fakeHome, { recursive: true, force: true }); + cleanup(fakeRoot); + cleanup(fakeHome); } }); }); diff --git a/tests/bug-474-clock-seam-date-determinism.test.cjs b/tests/bug-474-clock-seam-date-determinism.test.cjs index 5239eaaf4..1bf51a760 100644 --- a/tests/bug-474-clock-seam-date-determinism.test.cjs +++ b/tests/bug-474-clock-seam-date-determinism.test.cjs @@ -228,7 +228,7 @@ describe('bug-474: installer-migrations lock-loop timeout is deterministic via c const configDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-474-lock-')); t.after(() => { - fs.rmSync(configDir, { recursive: true, force: true }); + cleanup(configDir); }); const LOCK_NAME = 'gsd-install-migration.lock'; diff --git a/tests/changeset-cli.test.cjs b/tests/changeset-cli.test.cjs index 28e6f75a1..f832df20a 100644 --- a/tests/changeset-cli.test.cjs +++ b/tests/changeset-cli.test.cjs @@ -7,6 +7,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const cp = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); const SCRIPT = path.join(ROOT, 'scripts', 'changeset', 'cli.cjs'); @@ -36,7 +37,7 @@ function runRender(args = []) { } before(() => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-changeset-')); }); -after(() => { fs.rmSync(tmp, { recursive: true, force: true }); }); +after(() => { cleanup(tmp); }); // Fixtures for extract tests (#3496) // Written as arrays to avoid template-literal indentation injecting @@ -349,7 +350,7 @@ describe('changeset cli extract: version-range changelog extraction (#3496)', () describe('changeset cli render: file-I/O wrapper (#2975)', () => { test('exits 0 with consumed=N when N fragments are folded into CHANGELOG.md and deleted', () => { - fs.rmSync(path.join(tmp, '.changeset'), { recursive: true, force: true }); + cleanup(path.join(tmp, '.changeset')); fs.writeFileSync( path.join(tmp, 'CHANGELOG.md'), '# Changelog\n\n## [Unreleased]\n\n## [1.0.0] - 2026-01-01\n\n### Fixed\n\n- prior fix (#1)\n', diff --git a/tests/changeset-new.test.cjs b/tests/changeset-new.test.cjs index eb2adbddd..ad3a5313c 100644 --- a/tests/changeset-new.test.cjs +++ b/tests/changeset-new.test.cjs @@ -13,10 +13,11 @@ const { generateFragmentName, scaffoldFragment, parseFragment } = (() => { const parse = require(path.join(ROOT, 'scripts', 'changeset', 'parse.cjs')); return { ...newCs, parseFragment: parse.parseFragment }; })(); +const { cleanup } = require('./helpers.cjs'); let tmp; before(() => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-new-changeset-')); }); -after(() => { fs.rmSync(tmp, { recursive: true, force: true }); }); +after(() => { cleanup(tmp); }); describe('changeset new: name generator + scaffold writer (#2975)', () => { test('generateFragmentName returns three lowercase words separated by hyphens', () => { diff --git a/tests/check-update-config-dir.test.cjs b/tests/check-update-config-dir.test.cjs index d7be9531c..0c727de87 100644 --- a/tests/check-update-config-dir.test.cjs +++ b/tests/check-update-config-dir.test.cjs @@ -18,6 +18,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { execFileSync } = require('child_process'); +const { cleanup } = require('./helpers.cjs'); const CHECK_UPDATE_PATH = path.join(__dirname, '..', 'hooks', 'gsd-check-update.js'); @@ -60,7 +61,7 @@ describe('detectConfigDir runtime behavior (#1860)', () => { }); afterEach(() => { - fs.rmSync(tmpHome, { recursive: true, force: true }); + cleanup(tmpHome); }); test('returns .claude config dir when both .claude and .config/opencode exist', () => { diff --git a/tests/ci-rebase-check.test.cjs b/tests/ci-rebase-check.test.cjs index 9984b5bc4..07aafdb77 100644 --- a/tests/ci-rebase-check.test.cjs +++ b/tests/ci-rebase-check.test.cjs @@ -24,6 +24,7 @@ const os = require('node:os'); const ROOT = path.resolve(__dirname, '..'); const SCRIPT = path.join(ROOT, 'scripts', 'ci-rebase-check.cjs'); const NODE = process.execPath; +const { cleanup } = require('./helpers.cjs'); // --------------------------------------------------------------------------- // Helper: run a small inline Node snippet that requires the run() helper @@ -167,7 +168,7 @@ describe('ci-rebase-check: fetch-retry loop resolves when git fetch succeeds', ( `Script must not emit "failed after 3 attempts" when fetch succeeded.\nstderr: ${r.stderr}` ); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); diff --git a/tests/claude-skills-migration.test.cjs b/tests/claude-skills-migration.test.cjs index 55e8f9e6b..993292d75 100644 --- a/tests/claude-skills-migration.test.cjs +++ b/tests/claude-skills-migration.test.cjs @@ -18,6 +18,7 @@ const assert = require('node:assert/strict'); const path = require('path'); const os = require('os'); const fs = require('fs'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); @@ -201,7 +202,7 @@ describe('installRuntimeArtifacts (claude global) — skill layout', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('creates correct directory structure skills/gsd-xxx/SKILL.md', () => { @@ -315,7 +316,7 @@ describe('installRuntimeArtifacts path replacement in Claude global skills (#165 }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('replaces ~/.claude/ and $HOME/.claude/ paths with absolute configDir prefix on global install', () => { @@ -405,7 +406,7 @@ describe('Legacy commands/gsd/ cleanup', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('install removes legacy commands/gsd/ directory when present', () => { @@ -440,7 +441,7 @@ describe('writeManifest tracks skills/ for Claude', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('manifest includes skills/gsd-xxx/SKILL.md entries for Claude runtime', () => { diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index 15704881c..c0e9b77a2 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -19,6 +19,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { execFileSync } = require('child_process'); +const { cleanup } = require('./helpers.cjs'); // #2153 follow-up: ensure hooks/dist/ exists before any install integration // test runs. The Codex install path copies hook files from hooks/dist/, which @@ -953,7 +954,7 @@ describe('Codex hooks emit: migration produces namespaced AoT so managed-emit co tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-codex-fieldparity-')); }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('migration of legacy [hooks.SessionStart] produces two-level nested AoT (#2773)', () => { @@ -1002,7 +1003,7 @@ describe('mergeCodexConfig', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); const sampleBlock = generateCodexConfigBlock([ @@ -1321,7 +1322,7 @@ describe('installCodexConfig (integration)', () => { }); afterEach(() => { - fs.rmSync(tmpTarget, { recursive: true, force: true }); + cleanup(tmpTarget); }); // Only run if agents/ directory exists (not in CI without full checkout) @@ -1430,7 +1431,7 @@ describe('Codex install hook configuration (e2e)', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('Codex install copies hook file that is referenced in hooks.json (#2153)', () => { @@ -2141,7 +2142,7 @@ describe('Codex uninstall symmetry for hook-enabled configs', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('fresh install removes the GSD-added codex_hooks feature on uninstall', () => { @@ -2275,7 +2276,7 @@ describe('Codex uninstall symmetry for hook-enabled configs', () => { const cleaned = stripGsdFromCodexConfig(readCodexConfig(codexHome)); assert.strictEqual(cleaned, initialContent, `preserves short-circuited root features assignment: ${initialContent.split('\n')[0]}`); - fs.rmSync(codexHome, { recursive: true, force: true }); + cleanup(codexHome); fs.mkdirSync(codexHome, { recursive: true }); } }); diff --git a/tests/config-get-default.test.cjs b/tests/config-get-default.test.cjs index 3bc3279c4..f29e53259 100644 --- a/tests/config-get-default.test.cjs +++ b/tests/config-get-default.test.cjs @@ -15,6 +15,7 @@ const fs = require('fs'); const path = require('path'); const { execFileSync } = require('child_process'); const os = require('os'); +const { cleanup } = require('./helpers.cjs'); const GSD_TOOLS = path.join(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); @@ -29,7 +30,7 @@ describe('config-get --default flag (#1893)', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); function run(...args) { diff --git a/tests/copilot-install.test.cjs b/tests/copilot-install.test.cjs index 786a723b4..62ac66a2f 100644 --- a/tests/copilot-install.test.cjs +++ b/tests/copilot-install.test.cjs @@ -877,7 +877,7 @@ describe('Copilot instructions merge/strip', () => { }); afterEach(() => { - fs.rmSync(tmpMergeDir, { recursive: true, force: true }); + cleanup(tmpMergeDir); }); test('creates file from scratch when none exists', () => { @@ -1021,7 +1021,7 @@ describe('Copilot uninstall skill removal', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('identifies gsd-* skill directories for removal', () => { @@ -1083,7 +1083,7 @@ describe('Copilot manifest and patches fixes', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('writeManifest hashes skills for Copilot runtime', () => { @@ -1210,7 +1210,7 @@ describe('E2E: Copilot full install verification', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('installs expected number of skill directories', () => { @@ -1365,7 +1365,7 @@ describe('E2E: Copilot uninstall verification', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('removes engine directory', () => { @@ -1409,7 +1409,7 @@ describe('E2E: Copilot uninstall verification', () => { }); afterEach(() => { - fs.rmSync(td, { recursive: true, force: true }); + cleanup(td); }); test('preserves non-GSD content in skills directory', () => { @@ -1470,7 +1470,7 @@ describe('Claude uninstall preserves user-generated files (#1423)', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('preserves USER-PROFILE.md across uninstall', () => { diff --git a/tests/core.test.cjs b/tests/core.test.cjs index 45ca1de6b..aa133a222 100644 --- a/tests/core.test.cjs +++ b/tests/core.test.cjs @@ -1348,7 +1348,7 @@ describe('resolveWorktreeRoot with linked worktree .planning/', () => { afterEach(() => { if (worktreeDir) { try { execSyncLocal(`git worktree remove "${worktreeDir}" --force`, { cwd: mainDir, stdio: 'pipe' }); } catch { /* ok */ } - try { fs.rmSync(worktreeDir, { recursive: true, force: true }); } catch { /* ok */ } + cleanup(worktreeDir); } cleanup(mainDir); }); @@ -1359,7 +1359,7 @@ describe('resolveWorktreeRoot with linked worktree .planning/', () => { // Create a linked worktree worktreeDir = normalizePath(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-linked-'))); - fs.rmSync(worktreeDir, { recursive: true, force: true }); + cleanup(worktreeDir); execSyncLocal(`git worktree add "${worktreeDir}" -b test-linked`, { cwd: mainDir, stdio: 'pipe' }); // Give the linked worktree its own .planning/ @@ -1374,7 +1374,7 @@ describe('resolveWorktreeRoot with linked worktree .planning/', () => { test('returns main repo root when linked worktree has no .planning/', () => { // Create a linked worktree (no .planning/ in main or worktree) worktreeDir = normalizePath(fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-wt-linked-'))); - fs.rmSync(worktreeDir, { recursive: true, force: true }); + cleanup(worktreeDir); execSyncLocal(`git worktree add "${worktreeDir}" -b test-linked-no-plan`, { cwd: mainDir, stdio: 'pipe' }); // resolveWorktreeRoot should return the main repo root @@ -1473,7 +1473,7 @@ describe('detectSubRepos', () => { }); afterEach(() => { - fs.rmSync(projectRoot, { recursive: true, force: true }); + cleanup(projectRoot); }); test('returns empty array when no child directories have .git', () => { @@ -1520,7 +1520,7 @@ describe('loadConfig sub_repos auto-sync', () => { }); afterEach(() => { - fs.rmSync(projectRoot, { recursive: true, force: true }); + cleanup(projectRoot); }); test('migrates multiRepo: true to sub_repos array', () => { @@ -1590,7 +1590,7 @@ describe('findProjectRoot', () => { }); afterEach(() => { - fs.rmSync(projectRoot, { recursive: true, force: true }); + cleanup(projectRoot); }); test('returns startDir when no .planning/ exists anywhere', () => { diff --git a/tests/dispatch/trace-correlation.test.cjs b/tests/dispatch/trace-correlation.test.cjs index 293d3cf8c..481aedbcb 100644 --- a/tests/dispatch/trace-correlation.test.cjs +++ b/tests/dispatch/trace-correlation.test.cjs @@ -22,6 +22,7 @@ const os = require('os'); const { createHub } = require('../../get-shit-done/bin/lib/command-routing-hub.cjs'); const { createDefaultLogger } = require('../../get-shit-done/bin/lib/observability/logger.cjs'); +const { cleanup } = require('../helpers.cjs'); // ─── Test fixture setup ─────────────────────────────────────────────────────── @@ -99,7 +100,7 @@ describe('trace correlation — end-to-end parentTraceId propagation', () => { process.env.GSD_AUDIT = savedAudit; } // Clean up the temp directory - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); // ── Assertions ──────────────────────────────────────────────────────────── @@ -210,7 +211,7 @@ describe('trace correlation — end-to-end parentTraceId propagation', () => { } else { process.env.GSD_AUDIT = isolatedSavedAudit; } - fs.rmSync(isolatedTmp, { recursive: true, force: true }); + cleanup(isolatedTmp); } }); diff --git a/tests/enh-2538-statusline-last-command.test.cjs b/tests/enh-2538-statusline-last-command.test.cjs index a6e71bb65..69295979b 100644 --- a/tests/enh-2538-statusline-last-command.test.cjs +++ b/tests/enh-2538-statusline-last-command.test.cjs @@ -17,6 +17,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); +const { cleanup } = require('./helpers.cjs'); const statusline = require('../hooks/gsd-statusline.js'); const { VALID_CONFIG_KEYS } = require('../get-shit-done/bin/lib/config-schema.cjs'); @@ -35,7 +36,7 @@ function makeProject({ flag, transcript }) { transcriptPath = path.join(dir, 'transcript.jsonl'); fs.writeFileSync(transcriptPath, transcript); } - return { dir, transcriptPath, cleanup: () => fs.rmSync(dir, { recursive: true, force: true }) }; + return { dir, transcriptPath, cleanup: () => cleanup(dir) }; } function buildInput(dir, transcriptPath) { diff --git a/tests/enh-2789-description-budget.test.cjs b/tests/enh-2789-description-budget.test.cjs index 47fb0afd8..41337bde8 100644 --- a/tests/enh-2789-description-budget.test.cjs +++ b/tests/enh-2789-description-budget.test.cjs @@ -21,6 +21,7 @@ const fs = require('node:fs'); const path = require('node:path'); const { spawnSync } = require('node:child_process'); const os = require('node:os'); +const { cleanup } = require('./helpers.cjs'); const COMMANDS_DIR = path.join(__dirname, '../commands/gsd'); const LINT_SCRIPT = path.join(__dirname, '../scripts/lint-descriptions.cjs'); @@ -137,7 +138,7 @@ describe('lint-descriptions.cjs', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('rejects a command file with a description over 100 chars', () => { diff --git a/tests/enh-2790-skill-consolidation.test.cjs b/tests/enh-2790-skill-consolidation.test.cjs index 52b4feaf1..64c3ad797 100644 --- a/tests/enh-2790-skill-consolidation.test.cjs +++ b/tests/enh-2790-skill-consolidation.test.cjs @@ -8,6 +8,77 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const { assertWithinAllowlist } = require('../scripts/lib/allowlist-ratchet.cjs'); + +// --------------------------------------------------------------------------- +// Allowlisted set of user-invocable skills (commands/gsd/*.md, ns-* excluded). +// Consolidation target ~58; this set may only SHRINK. +// Adding a new skill requires adding it here with justification. +// Removing a consolidated skill requires pruning it here. +// --------------------------------------------------------------------------- +const KNOWN_SKILLS = new Set([ + 'add-tests.md', + 'ai-integration-phase.md', + 'audit-fix.md', + 'audit-milestone.md', + 'audit-uat.md', + 'autonomous.md', + 'capture.md', + 'cleanup.md', + 'code-review.md', + 'complete-milestone.md', + 'config.md', + 'debug.md', + 'discuss-phase.md', + 'docs-update.md', + 'eval-review.md', + 'execute-phase.md', + 'explore.md', + 'extract-learnings.md', + 'fast.md', + 'forensics.md', + 'graphify.md', + 'health.md', + 'help.md', + 'import.md', + 'inbox.md', + 'ingest-docs.md', + 'manager.md', + 'map-codebase.md', + 'milestone-summary.md', + 'mvp-phase.md', + 'new-milestone.md', + 'new-project.md', + 'pause-work.md', + 'phase.md', + 'plan-phase.md', + 'plan-review-convergence.md', + 'pr-branch.md', + 'profile-user.md', + 'progress.md', + 'quick.md', + 'resume-work.md', + 'review-backlog.md', + 'review.md', + 'secure-phase.md', + 'settings.md', + 'ship.md', + 'sketch.md', + 'spec-phase.md', + 'spike.md', + 'stats.md', + 'surface.md', + 'thread.md', + 'ui-phase.md', + 'ui-review.md', + 'ultraplan-phase.md', + 'undo.md', + 'update.md', + 'validate-phase.md', + 'verify-work.md', + 'workspace.md', + 'workstreams.md', +]); const COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); @@ -339,22 +410,22 @@ describe('settings.md is kept (merged into config entry point or remains standal }); // --------------------------------------------------------------------------- -// Group: Skill count reduced +// Group: Skill set allowlisted (identity-based, consolidating toward ~58) // --------------------------------------------------------------------------- -describe('skill count', () => { - test('total user-invocable files in commands/gsd/*.md is <= 63', () => { - // Exclude `ns-*.md` namespace meta-skills (#2792) from this cap. +describe('skill set', () => { + test('user-invocable skill set is allowlisted (consolidating toward ~58)', () => { + // Exclude `ns-*.md` namespace meta-skills (#2792) from this guard. // Those are descriptor-only routers selected first by the model and // are not part of the consolidation surface this test tracks; their // own contract is enforced by tests/enh-2792-namespace-skills.test.cjs. - const files = fs.readdirSync(COMMANDS_DIR) + const currentBasenames = fs.readdirSync(COMMANDS_DIR) .filter((f) => f.endsWith('.md') && !f.startsWith('ns-')); - assert.ok( - files.length <= 63, - [ - `Expected <= 63 user-invocable skill files, found ${files.length}.`, - 'Consolidation target is ~58.', - ].join(' '), - ); + assertWithinAllowlist({ + label: 'user-invocable skills (commands/gsd)', + current: currentBasenames, + known: KNOWN_SKILLS, + fail: assert.fail, + pruneHint: 'edit KNOWN_SKILLS in tests/enh-2790-skill-consolidation.test.cjs', + }); }); }); diff --git a/tests/eslint-rules.test.cjs b/tests/eslint-rules.test.cjs index 38f8473eb..16fbb45da 100644 --- a/tests/eslint-rules.test.cjs +++ b/tests/eslint-rules.test.cjs @@ -3,10 +3,11 @@ /** * eslint-rules.test.cjs * - * RuleTester unit tests for the three local ESLint rules: + * RuleTester unit tests for the local ESLint rules: * - local/no-source-grep * - local/no-magic-sleep-in-tests * - local/no-elapsed-assertion + * - local/no-raw-rmsync-in-tests */ const { test, describe } = require('node:test'); @@ -16,6 +17,7 @@ const { RuleTester } = require('eslint'); const noSourceGrep = require('../eslint-rules/no-source-grep.cjs'); const noMagicSleepInTests = require('../eslint-rules/no-magic-sleep-in-tests.cjs'); const noElapsedAssertion = require('../eslint-rules/no-elapsed-assertion.cjs'); +const noRawRmsyncInTests = require('../eslint-rules/no-raw-rmsync-in-tests.cjs'); const ruleTester = new RuleTester({ languageOptions: { @@ -317,3 +319,173 @@ describe('no-elapsed-assertion rule', () => { assert.ok(true, 'no-elapsed-assertion flags assert.equal with timing comparison'); }); }); + +// ─── no-raw-rmsync-in-tests ────────────────────────────────────────────────── + +describe('no-raw-rmsync-in-tests rule', () => { + // ── INVALID cases (must error) ──────────────────────────────────────────── + + test('invalid: fs.rmSync() in a test file', () => { + ruleTester.run('no-raw-rmsync-in-tests', noRawRmsyncInTests, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + fs.rmSync(tmpDir, { recursive: true, force: true }); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noRawRmSync' }], + }, + ], + }); + assert.ok(true, 'no-raw-rmsync-in-tests flags fs.rmSync() in test file'); + }); + + test('invalid: computed member fs["rmSync"]() in a test file', () => { + ruleTester.run('no-raw-rmsync-in-tests', noRawRmsyncInTests, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + fs['rmSync'](d, { recursive: true, force: true }); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noRawRmSync' }], + }, + ], + }); + assert.ok(true, 'no-raw-rmsync-in-tests flags fs["rmSync"]() in test file'); + }); + + test('invalid: destructured rmSync from require("fs") in a test file', () => { + ruleTester.run('no-raw-rmsync-in-tests', noRawRmsyncInTests, { + valid: [], + invalid: [ + { + code: ` + const { rmSync } = require('fs'); + rmSync(d, { recursive: true, force: true }); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noRawRmSync' }], + }, + ], + }); + assert.ok(true, 'no-raw-rmsync-in-tests flags destructured rmSync from require("fs")'); + }); + + test('invalid: aliased const del = fs.rmSync; del() in a test file', () => { + ruleTester.run('no-raw-rmsync-in-tests', noRawRmsyncInTests, { + valid: [], + invalid: [ + { + code: ` + const fs = require('fs'); + const del = fs.rmSync; + del(d, { recursive: true, force: true }); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noRawRmSync' }], + }, + ], + }); + assert.ok(true, 'no-raw-rmsync-in-tests flags aliased fs.rmSync'); + }); + + test('invalid: allow-test-rule annotation no longer suppresses this rule (Defect 1 fixed)', () => { + // A file with // allow-test-rule: must still error + // on raw rmSync calls. The file-level annotation is for no-source-grep only. + ruleTester.run('no-raw-rmsync-in-tests', noRawRmsyncInTests, { + valid: [], + invalid: [ + { + code: ` + // allow-test-rule: source-text-is-the-product + const fs = require('fs'); + fs.rmSync(d, { recursive: true, force: true }); + `, + filename: 'tests/foo.test.cjs', + errors: [{ messageId: 'noRawRmSync' }], + }, + ], + }); + assert.ok(true, 'no-raw-rmsync-in-tests is NOT suppressed by allow-test-rule annotation'); + }); + + // ── VALID cases (must NOT error) ────────────────────────────────────────── + + test('valid: helpers.cleanup() in a test file (no error)', () => { + ruleTester.run('no-raw-rmsync-in-tests', noRawRmsyncInTests, { + valid: [ + { + code: ` + const { cleanup } = require('../helpers.cjs'); + cleanup(tmpDir); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + assert.ok(true, 'no-raw-rmsync-in-tests allows helpers.cleanup()'); + }); + + test('valid: bare rmSync() that is NOT fs-derived (local function) is not flagged', () => { + // A locally defined function named rmSync must not be flagged — the rule + // only tracks names that were bound from require("fs"). + ruleTester.run('no-raw-rmsync-in-tests', noRawRmsyncInTests, { + valid: [ + { + code: ` + const rmSync = () => {}; + rmSync(d); + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + assert.ok(true, 'no-raw-rmsync-in-tests does not flag a locally-defined rmSync()'); + }); + + // NOTE: The inline `// eslint-disable-next-line local/no-raw-rmsync-in-tests -- reason` + // escape hatch is handled entirely by ESLint's own disable-comment mechanism and + // cannot be unit-tested here via RuleTester (RuleTester runs the rule under a + // different internal namespace so the comment's rule-id doesn't match). The escape + // hatch works correctly when ESLint processes real files via `npx eslint`. + + test('valid: fs.rmSync() in a non-test file (rule is inert outside *.test.cjs)', () => { + ruleTester.run('no-raw-rmsync-in-tests', noRawRmsyncInTests, { + valid: [ + { + code: ` + const fs = require('fs'); + fs.rmSync(tmpDir, { recursive: true, force: true }); + `, + filename: 'scripts/foo.cjs', + }, + ], + invalid: [], + }); + assert.ok(true, 'no-raw-rmsync-in-tests is inert in non-test files'); + }); + + test('valid: member access / assignment without calling (not a CallExpression)', () => { + ruleTester.run('no-raw-rmsync-in-tests', noRawRmsyncInTests, { + valid: [ + { + code: ` + const fs = require('fs'); + const orig = fs.rmSync; + fs.rmSync = orig; + `, + filename: 'tests/foo.test.cjs', + }, + ], + invalid: [], + }); + assert.ok(true, 'no-raw-rmsync-in-tests ignores member access / assignment without call'); + }); +}); diff --git a/tests/feat-2795-update-banner.test.cjs b/tests/feat-2795-update-banner.test.cjs index 13e1b7901..5b3ee0462 100644 --- a/tests/feat-2795-update-banner.test.cjs +++ b/tests/feat-2795-update-banner.test.cjs @@ -19,6 +19,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { spawnSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const HOOK_PATH = path.join(__dirname, '..', 'hooks', 'gsd-update-banner.js'); const { @@ -123,7 +124,7 @@ describe('shouldSuppressFailureWarning', () => { ); assert.equal(result, false); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -135,7 +136,7 @@ describe('shouldSuppressFailureWarning', () => { const result = shouldSuppressFailureWarning(f, 1000 + RATE_LIMIT_SECONDS - 1); assert.equal(result, true); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -147,7 +148,7 @@ describe('shouldSuppressFailureWarning', () => { const result = shouldSuppressFailureWarning(f, 1000 + RATE_LIMIT_SECONDS + 1); assert.equal(result, false); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -159,7 +160,7 @@ describe('shouldSuppressFailureWarning', () => { const result = shouldSuppressFailureWarning(f, 100); assert.equal(result, false); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); @@ -194,7 +195,7 @@ describe('gsd-update-banner.js end-to-end', () => { assert.equal(r.status, 0, `expected exit 0, got ${r.status} stderr=${r.stderr}`); assert.equal(r.stdout.trim(), ''); } finally { - fs.rmSync(home, { recursive: true, force: true }); + cleanup(home); } }); @@ -213,7 +214,7 @@ describe('gsd-update-banner.js end-to-end', () => { assert.ok(parsed.systemMessage.includes('1.40.0')); assert.ok(parsed.systemMessage.includes('/gsd:update')); } finally { - fs.rmSync(home, { recursive: true, force: true }); + cleanup(home); } }); @@ -229,7 +230,7 @@ describe('gsd-update-banner.js end-to-end', () => { assert.equal(r.status, 0); assert.equal(r.stdout.trim(), ''); } finally { - fs.rmSync(home, { recursive: true, force: true }); + cleanup(home); } }); @@ -243,7 +244,7 @@ describe('gsd-update-banner.js end-to-end', () => { assert.equal(typeof parsed.systemMessage, 'string'); assert.ok(/check failed/i.test(parsed.systemMessage)); } finally { - fs.rmSync(home, { recursive: true, force: true }); + cleanup(home); } }); @@ -272,7 +273,7 @@ describe('gsd-update-banner.js end-to-end', () => { 'subsequent run within rate-limit window must stay silent' ); } finally { - fs.rmSync(home, { recursive: true, force: true }); + cleanup(home); } }); @@ -284,7 +285,7 @@ describe('gsd-update-banner.js end-to-end', () => { assert.equal(r.status, 0); assert.equal(r.stdout.trim(), ''); } finally { - fs.rmSync(home, { recursive: true, force: true }); + cleanup(home); } }); }); diff --git a/tests/feat-3023-model-phase-types.test.cjs b/tests/feat-3023-model-phase-types.test.cjs index 6265e86da..f2c7c2ebc 100644 --- a/tests/feat-3023-model-phase-types.test.cjs +++ b/tests/feat-3023-model-phase-types.test.cjs @@ -35,7 +35,7 @@ const { } = require('../get-shit-done/bin/lib/model-profiles.cjs'); const { isValidConfigKey } = require('../get-shit-done/bin/lib/config-schema.cjs'); -const { createTempDir } = require('./helpers.cjs'); +const { createTempDir, cleanup } = require('./helpers.cjs'); const makeTmp = (prefix) => createTempDir(`gsd-3023-${prefix}-`); function writeConfig(projectDir, config) { @@ -45,7 +45,7 @@ function writeConfig(projectDir, config) { } function rmr(p) { - try { fs.rmSync(p, { recursive: true, force: true }); } catch { /* noop */ } + cleanup(p); } // ─── Schema: AGENT_TO_PHASE_TYPE table + VALID_PHASE_TYPES ────────────────── diff --git a/tests/feat-3024-dynamic-routing.test.cjs b/tests/feat-3024-dynamic-routing.test.cjs index 091d479fe..928386add 100644 --- a/tests/feat-3024-dynamic-routing.test.cjs +++ b/tests/feat-3024-dynamic-routing.test.cjs @@ -60,14 +60,14 @@ const { } = require('../get-shit-done/bin/lib/model-profiles.cjs'); const { isValidConfigKey } = require('../get-shit-done/bin/lib/config-schema.cjs'); -const { createTempDir } = require('./helpers.cjs'); +const { createTempDir, cleanup } = require('./helpers.cjs'); const makeTmp = (prefix) => createTempDir(`gsd-3024-${prefix}-`); function writeConfig(dir, config) { const planningDir = path.join(dir, '.planning'); fs.mkdirSync(planningDir, { recursive: true }); fs.writeFileSync(path.join(planningDir, 'config.json'), JSON.stringify(config, null, 2)); } -function rmr(p) { try { fs.rmSync(p, { recursive: true, force: true }); } catch { /* noop */ } } +function rmr(p) { cleanup(p); } // ─── Schema: AGENT_DEFAULT_TIERS coverage + valid tier set ────────────────── diff --git a/tests/feat-3039-help-tiered.test.cjs b/tests/feat-3039-help-tiered.test.cjs index 7759c16e1..a21220710 100644 --- a/tests/feat-3039-help-tiered.test.cjs +++ b/tests/feat-3039-help-tiered.test.cjs @@ -29,12 +29,17 @@ * 8. Every full.md heading is either aliased or in the intentional-orphan allowlist. * 9. The `commands/gsd/help.md` shim passes `$ARGUMENTS` through and advertises * the composable `--brief ` form. + * + * Tighten-only invariant (issue #597): ceilings track the per-tier high-water mark + * within GRACE lines. Budgets may only decrease, never silently creep upward. + * The assertTightCeiling() calls below enforce this automatically. */ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); +const { assertTightCeiling } = require('../scripts/lib/allowlist-ratchet.cjs'); const ROOT = path.join(__dirname, '..'); const WORKFLOWS = path.join(ROOT, 'get-shit-done', 'workflows'); @@ -47,11 +52,25 @@ const MODE_FILES = ['brief.md', 'default.md', 'full.md', 'topic.md']; // "One screen" budgets, including frontmatter// tags. // These are conservative (one-page conceptual size of ~25 lines of usable // content) but allow for the wrapping tags. Tighten as content stabilizes. +// +// Ceilings tightened to actualMax + SMALL_GRACE per the ratchet-down rule (#597). +// BRIEF ceiling kept at 30 (actualMax=22, slack=8 ≤ SMALL_GRACE=10). const BRIEF_BUDGET = 30; -const DEFAULT_BUDGET = 70; +// DEFAULT ceiling lowered from 70 → 60 (actualMax=50; #597 ratchet-down). +const DEFAULT_BUDGET = 60; // full.md is the LARGE tier (see workflow-size-budget.test.cjs — LARGE_BUDGET = 1500). // The size-budget test is non-recursive so full.md is not covered there; cap it here. -const FULL_BUDGET = 1500; +// FULL ceiling lowered from 1500 → 844 (actualMax=784; #597 ratchet-down). +const FULL_BUDGET = 844; + +// Grace bands: +// SMALL_GRACE — for the tiny brief/default/dispatcher files (≤ ~70 lines): +// 10 lines of breathing room is proportionate and prevents trivial edits from +// failing while still catching any meaningful upward creep. +// LARGE_GRACE — for full.md where content fluctuates more: +// 60 lines matches the line-budget GRACE used in the other size-budget tests. +const SMALL_GRACE = 10; +const LARGE_GRACE = 60; function read(file) { return fs.readFileSync(file, 'utf8'); @@ -71,10 +90,13 @@ describe('feature #3039: tiered help — file structure', () => { }); } - test('dispatcher exists and is small (≤ 40 lines)', () => { + // Dispatcher ceiling lowered from 40 → 34 (actualMax=24; #597 ratchet-down). + const DISPATCHER_BUDGET = 34; + test(`dispatcher exists and is small (≤ ${DISPATCHER_BUDGET} lines)`, () => { assert.ok(fs.existsSync(DISPATCHER)); const n = lineCount(DISPATCHER); - assert.ok(n <= 40, `dispatcher should be small; got ${n} lines`); + assert.ok(n <= DISPATCHER_BUDGET, `dispatcher should be small; got ${n} lines`); + assertTightCeiling({ label: 'dispatcher', actualMax: n, ceiling: DISPATCHER_BUDGET, grace: SMALL_GRACE, fail: assert.fail }); }); for (const f of MODE_FILES) { @@ -94,11 +116,13 @@ describe('feature #3039: tiered help — size budgets', () => { test(`brief.md fits one screen (≤ ${BRIEF_BUDGET} lines)`, () => { const n = lineCount(path.join(MODES, 'brief.md')); assert.ok(n <= BRIEF_BUDGET, `brief.md is ${n} lines, budget ${BRIEF_BUDGET}`); + assertTightCeiling({ label: 'BRIEF', actualMax: n, ceiling: BRIEF_BUDGET, grace: SMALL_GRACE, fail: assert.fail }); }); test(`default.md fits one screen (≤ ${DEFAULT_BUDGET} lines)`, () => { const n = lineCount(path.join(MODES, 'default.md')); assert.ok(n <= DEFAULT_BUDGET, `default.md is ${n} lines, budget ${DEFAULT_BUDGET}`); + assertTightCeiling({ label: 'DEFAULT', actualMax: n, ceiling: DEFAULT_BUDGET, grace: SMALL_GRACE, fail: assert.fail }); }); test('full.md preserves the complete reference (≥ 600 lines)', () => { @@ -113,6 +137,7 @@ describe('feature #3039: tiered help — size budgets', () => { // workflow-size-budget.test.cjs. Cap it here at the LARGE tier limit. const n = lineCount(path.join(MODES, 'full.md')); assert.ok(n <= FULL_BUDGET, `full.md grew to ${n} lines (LARGE budget: ${FULL_BUDGET})`); + assertTightCeiling({ label: 'FULL', actualMax: n, ceiling: FULL_BUDGET, grace: LARGE_GRACE, fail: assert.fail }); }); }); diff --git a/tests/feat-3210-fallow-integration.test.cjs b/tests/feat-3210-fallow-integration.test.cjs index 4e977d75a..1131cdfa3 100644 --- a/tests/feat-3210-fallow-integration.test.cjs +++ b/tests/feat-3210-fallow-integration.test.cjs @@ -61,7 +61,7 @@ describe('feat-3210: fallow integration module', () => { const resolved = resolveFallowBinary({ cwd: tmp, envPath: '' }); assert.strictEqual(resolved, fallowPath); - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); }); // H6: replaced wholesale win32 skip with platform-adapted assertion @@ -87,7 +87,7 @@ describe('feat-3210: fallow integration module', () => { 'Windows: .cmd candidate must be preferred over bare extensionless file', ); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } } else { // H6: non-Windows — non-executable file in PATH must be ignored @@ -101,7 +101,7 @@ describe('feat-3210: fallow integration module', () => { const resolved = resolveFallowBinary({ cwd: tmp, envPath: pathDir }); assert.strictEqual(resolved, null); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } } }); @@ -130,7 +130,7 @@ describe('feat-3210: fallow integration module', () => { () => requireFallowBinary({ cwd: tmp, envPath: '' }), /install fallow via `npm install -D fallow` or `cargo install fallow`/, ); - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); }); // L3: runFallowAudit against a non-zero-exit binary must surface error state @@ -166,7 +166,7 @@ describe('feat-3210: fallow integration module', () => { 'runFallowAudit must return error state (error/exitCode/failed) when binary exits non-zero', ); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); @@ -267,7 +267,7 @@ describe('feat-3210: M2 - node_modules/.bin resolution order', () => { const resolved = resolveFallowBinary({ cwd: tmp, envPath: pathDir }); assert.strictEqual(resolved, localFallow, 'node_modules/.bin/fallow must win over PATH fallow'); } finally { - fs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); }); diff --git a/tests/feat-3251-command-aliases-manifest-coverage.test.cjs b/tests/feat-3251-command-aliases-manifest-coverage.test.cjs index fd3e44b59..45e11b6b2 100644 --- a/tests/feat-3251-command-aliases-manifest-coverage.test.cjs +++ b/tests/feat-3251-command-aliases-manifest-coverage.test.cjs @@ -15,6 +15,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('path'); const { spawnSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const REPO_ROOT = path.join(__dirname, '..'); const COMMAND_ALIASES_FILE = path.join( @@ -183,7 +184,7 @@ describe('feat-3251: generated aliases dispatch through real gsd-tools behavior' cli_flag_present: true, }); } finally { - fs.rmSync(projectDir, { recursive: true, force: true }); + cleanup(projectDir); } }); @@ -216,7 +217,7 @@ describe('feat-3251: generated aliases dispatch through real gsd-tools behavior' assert.equal(output.cli_flag_present, false); assert.deepEqual(snapshotProjectState(projectDir), beforeFiles); } finally { - fs.rmSync(projectDir, { recursive: true, force: true }); + cleanup(projectDir); } }); @@ -250,7 +251,7 @@ describe('feat-3251: generated aliases dispatch through real gsd-tools behavior' assert.equal(output.roadmap_mode, null); assert.deepEqual(snapshotProjectState(projectDir), beforeFiles); } finally { - fs.rmSync(projectDir, { recursive: true, force: true }); + cleanup(projectDir); } }); @@ -270,7 +271,7 @@ describe('feat-3251: generated aliases dispatch through real gsd-tools behavior' assert.equal(/\n\s*at\s/.test(result.stderr), false, 'non-debug failure must not print a stack trace'); assert.deepEqual(snapshotProjectState(projectDir), beforeFiles); } finally { - fs.rmSync(projectDir, { recursive: true, force: true }); + cleanup(projectDir); } }); }); diff --git a/tests/feat-3262-scan-phase-plans.test.cjs b/tests/feat-3262-scan-phase-plans.test.cjs index fb600f6e7..e59781cf3 100644 --- a/tests/feat-3262-scan-phase-plans.test.cjs +++ b/tests/feat-3262-scan-phase-plans.test.cjs @@ -17,6 +17,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('os'); +const { cleanup } = require('./helpers.cjs'); // Helper under test — must exist at this path (GREEN phase wires it up) const scanPhasePlans = require('../get-shit-done/bin/lib/plan-scan.cjs'); @@ -44,7 +45,7 @@ beforeEach(() => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); // --------------------------------------------------------------------------- diff --git a/tests/feat-3595-fs-fault-injection-atomic-write.test.cjs b/tests/feat-3595-fs-fault-injection-atomic-write.test.cjs index a15bb371c..71863ac1e 100644 --- a/tests/feat-3595-fs-fault-injection-atomic-write.test.cjs +++ b/tests/feat-3595-fs-fault-injection-atomic-write.test.cjs @@ -48,7 +48,7 @@ const { * Create a fresh real-fs scratch dir per test so no two faults share * state. Returns the directory; caller must clean up. */ -const { createTempDir } = require('./helpers.cjs'); +const { createTempDir, cleanup } = require('./helpers.cjs'); const mkScratch = (name) => createTempDir(`fs-fault-${name}-`); /** @@ -66,7 +66,7 @@ function orphanTmpFiles(dir) { test('platformWriteSync happy path writes content atomically (baseline for fault tests)', (t) => { const dir = mkScratch('happy'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const file = path.join(dir, 'config.json'); platformWriteSync(file, '{"k":"v"}\n'); assert.equal(fs.readFileSync(file, 'utf-8'), '{"k":"v"}\n'); @@ -77,7 +77,7 @@ test('platformWriteSync happy path writes content atomically (baseline for fault test('platformWriteSync recovers when renameSync fails (EXDEV cross-device fallback)', (t) => { const dir = mkScratch('exdev'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const file = path.join(dir, 'config.json'); // Simulate rename failing once (e.g. cross-device move on a CI runner @@ -104,7 +104,7 @@ test('platformWriteSync recovers when renameSync fails (EXDEV cross-device fallb test('platformWriteSync falls back when initial tmp writeFileSync fails (ENOSPC)', (t) => { const dir = mkScratch('enospc'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const file = path.join(dir, 'config.json'); // Make the FIRST writeFileSync (to .tmp.) fail with ENOSPC. The @@ -140,7 +140,7 @@ test('platformWriteSync falls back when initial tmp writeFileSync fails (ENOSPC) test('platformWriteSync propagates the FALLBACK error when both tmp and fallback writes fail', (t) => { const dir = mkScratch('double-fail'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const file = path.join(dir, 'config.json'); let writeCalls = 0; @@ -178,7 +178,7 @@ test('platformWriteSync propagates the FALLBACK error when both tmp and fallback test('platformWriteSync propagates mkdirSync failure unchanged (no swallowed parent-dir errors)', (t) => { const dir = mkScratch('mkdir-fail'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const file = path.join(dir, 'deep', 'nested', 'config.json'); const mkdirMock = mock.method(fs, 'mkdirSync', () => { @@ -204,7 +204,7 @@ test('platformWriteSync propagates mkdirSync failure unchanged (no swallowed par test('platformWriteSync against a target path that is an existing directory fails cleanly', (t) => { const dir = mkScratch('target-is-dir'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const file = path.join(dir, 'collides'); // Pre-create the target AS a directory so the rename step would // collide with a directory at the destination. @@ -229,7 +229,7 @@ test('platformWriteSync against a target path that is an existing directory fail test('platformWriteSync handles paths with spaces, unicode, and newline characters', (t) => { const dir = mkScratch('weird-path'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const cases = [ 'has spaces in name.json', 'unicode-日本語-name.json', @@ -258,7 +258,7 @@ test('platformWriteSync handles paths with spaces, unicode, and newline characte test('platformWriteSync never leaks a tmp file after a successful happy-path write', (t) => { const dir = mkScratch('no-orphan-happy'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); for (let i = 0; i < 25; i++) { platformWriteSync(path.join(dir, `f-${i}.json`), `{"i":${i}}\n`); } @@ -274,7 +274,7 @@ test('platformWriteSync never leaks a tmp file after a successful happy-path wri test('platformEnsureDir is idempotent on an existing directory', (t) => { const dir = mkScratch('ensure-idem'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const target = path.join(dir, 'a', 'b', 'c'); // First call creates; subsequent calls must not throw EEXIST. platformEnsureDir(target); @@ -287,7 +287,7 @@ test('platformEnsureDir is idempotent on an existing directory', (t) => { test('platformEnsureDir propagates EACCES when parent dir is unwritable', (t) => { const dir = mkScratch('ensure-fail'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const mkdirMock = mock.method(fs, 'mkdirSync', () => { const err = new Error('EACCES: permission denied'); @@ -328,7 +328,7 @@ test('platformWriteSync REPLACES a symlink with a regular file rather than follo } const dir = mkScratch('symlink-replace'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const realTarget = path.join(dir, 'real-target.json'); fs.writeFileSync(realTarget, 'original — must not be touched\n'); @@ -356,7 +356,7 @@ test('platformWriteSync against a broken symlink replaces it with the intended f return; } const dir = mkScratch('symlink-broken'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const link = path.join(dir, 'dangling.json'); fs.symlinkSync(path.join(dir, 'does-not-exist'), link); assert.equal(fs.lstatSync(link).isSymbolicLink(), true, 'pre-check: link is dangling'); @@ -380,7 +380,7 @@ test('platformWriteSync survives a concurrent collision on the same target path' // the writer is sync. This exercises mid-flight error recovery, which // is the same concurrency hazard at a lower granularity.) const dir = mkScratch('concurrent'); - t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + t.after(() => cleanup(dir)); const file = path.join(dir, 'race.json'); // First write completes normally. diff --git a/tests/feat-443-effort-install-wiring.install.test.cjs b/tests/feat-443-effort-install-wiring.install.test.cjs index b3cc536c9..a4cf1a7cc 100644 --- a/tests/feat-443-effort-install-wiring.install.test.cjs +++ b/tests/feat-443-effort-install-wiring.install.test.cjs @@ -28,6 +28,7 @@ const path = require('node:path'); const os = require('node:os'); const { install } = require('../bin/install.js'); +const { cleanup } = require('./helpers.cjs'); const REPO_ROOT = path.resolve(__dirname, '..'); const SOURCE_AGENTS_DIR = path.join(REPO_ROOT, 'agents'); @@ -107,7 +108,7 @@ function runGlobalInstall(runtime, tmpHome) { if (prevSkipStale === undefined) delete process.env.GSD_SKIP_STALE_SDK_CHECK; else process.env.GSD_SKIP_STALE_SDK_CHECK = prevSkipStale; // Clean up the isolated HOME dir - try { fs.rmSync(isolatedHome, { recursive: true, force: true }); } catch (_) { /* best-effort */ } + cleanup(isolatedHome); } return tmpHome; @@ -132,7 +133,7 @@ describe('#443 Claude install: effort: injected into frontmatter', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('gsd-planner.md contains effort: xhigh (heavy tier default)', () => { @@ -170,7 +171,7 @@ describe('#443 Gemini install: effort: absent (Gemini-safe)', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('gsd-planner.md does NOT contain effort: (Gemini install)', () => { @@ -201,7 +202,7 @@ describe('#443 Codex install: model_reasoning_effort in .toml (unified resolver) }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('gsd-planner.toml contains model_reasoning_effort = "xhigh" (heavy tier)', () => { @@ -255,7 +256,7 @@ describe('#443 Config-driven: effort.agent_overrides drives install-time effort' }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('Claude .md gets effort: low when agent_overrides.gsd-planner=low', () => { @@ -329,7 +330,7 @@ describe('#443 resolveInstallTimeEffort: invalid tokens fall through to valid ef }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); function writeProjectConfig(config) { diff --git a/tests/feat-488-effort-sync.test.cjs b/tests/feat-488-effort-sync.test.cjs index 80e5f59b2..d36236240 100644 --- a/tests/feat-488-effort-sync.test.cjs +++ b/tests/feat-488-effort-sync.test.cjs @@ -13,6 +13,8 @@ const path = require('node:path'); const os = require('node:os'); const { spawnSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); + const GSD_TOOLS = path.resolve(__dirname, '../get-shit-done/bin/gsd-tools.cjs'); function runCli(args, env = {}) { @@ -93,7 +95,7 @@ describe('feat-488: effort sync command', () => { // dry-run must not modify the file assert.ok(fs.readFileSync(agentPath, 'utf8').includes('effort: medium'), 'dry-run must not write file'); - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('--apply mode rewrites effort: frontmatter to new config value', () => { @@ -115,7 +117,7 @@ describe('feat-488: effort sync command', () => { assert.ok(updated.includes('effort: xhigh'), 'file must be updated to xhigh'); assert.ok(!updated.includes('effort: medium'), 'old effort value must be gone'); - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('skips agents where effort: already matches config', () => { @@ -134,7 +136,7 @@ describe('feat-488: effort sync command', () => { assert.equal(result.synced, 0, 'nothing to sync when already matching'); assert.equal(result.skipped, 1); - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('injects effort: into agent files that lack the frontmatter key', () => { @@ -154,7 +156,7 @@ describe('feat-488: effort sync command', () => { assert.equal(result.changes[0].to, 'max'); assert.ok(fs.readFileSync(agentPath, 'utf8').includes('effort: max'), 'effort must be injected'); - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('non-claude runtime exits cleanly with informative reason field', () => { @@ -168,7 +170,7 @@ describe('feat-488: effort sync command', () => { assert.ok(result.reason, 'should include a reason message for unsupported runtime'); assert.equal(result.synced, 0); - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('home-default effort config gap: applies home-level effort when project config has no effort section', () => { @@ -211,8 +213,8 @@ describe('feat-488: effort sync command', () => { assert.ok(typeof result.synced === 'number', 'synced must be a number'); assert.ok(Array.isArray(result.changes), 'changes must be array'); - fs.rmSync(tmpHome, { recursive: true, force: true }); - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpHome); + cleanup(tmpDir); }); test('CLI dispatcher: positional args after effort sync are rejected', () => { @@ -245,6 +247,6 @@ describe('feat-488: effort sync command', () => { 'CLI --apply must write the updated effort value' ); - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); }); diff --git a/tests/feat-49-model-policy-presets.test.cjs b/tests/feat-49-model-policy-presets.test.cjs index 2d44cd211..d0ba37810 100644 --- a/tests/feat-49-model-policy-presets.test.cjs +++ b/tests/feat-49-model-policy-presets.test.cjs @@ -72,7 +72,7 @@ const { const modelCatalog = require('../get-shit-done/bin/lib/model-catalog.cjs'); const { isValidConfigKey } = require('../get-shit-done/bin/lib/config-schema.cjs'); -const { createTempDir } = require('./helpers.cjs'); +const { createTempDir, cleanup } = require('./helpers.cjs'); const makeTmp = (prefix) => createTempDir(`gsd-49-${prefix}-`); @@ -83,7 +83,7 @@ function writeConfig(dir, config) { } function rmr(p) { - try { fs.rmSync(p, { recursive: true, force: true }); } catch { /* noop */ } + cleanup(p); } // ─── resolveModelPolicy unit tests ────────────────────────────────────────── diff --git a/tests/forensics.test.cjs b/tests/forensics.test.cjs index 9c08195fd..5027e69be 100644 --- a/tests/forensics.test.cjs +++ b/tests/forensics.test.cjs @@ -15,6 +15,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('os'); +const { cleanup } = require('./helpers.cjs'); const repoRoot = path.resolve(__dirname, '..'); const commandPath = path.join(repoRoot, 'commands', 'gsd', 'forensics.md'); @@ -220,7 +221,7 @@ describe('forensics fixture-based tests', () => { }); afterEach(() => { - if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('detects missing artifacts in phase structure', () => { diff --git a/tests/graphify-auto-update.test.cjs b/tests/graphify-auto-update.test.cjs index f24da1b7b..219b0ad42 100644 --- a/tests/graphify-auto-update.test.cjs +++ b/tests/graphify-auto-update.test.cjs @@ -130,7 +130,7 @@ describe('auto-update', () => { head_at_build: 'abcdef0', graphify_version: null, }); - t.after(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + t.after(() => cleanup(tmpDir)); const s = graphifyStatus(tmpDir); assert.strictEqual(s.stale, true, 'auto-build failure must set stale=true'); assert.ok(s.last_build_auto_update, 'last_build_auto_update must be exposed'); @@ -147,7 +147,7 @@ describe('auto-update', () => { head_at_build: 'abcdef0', graphify_version: null, }); - t.after(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + t.after(() => cleanup(tmpDir)); const s = graphifyStatus(tmpDir); assert.strictEqual(s.stale, true, 'auto-build in-flight must set stale=true'); assert.strictEqual(s.last_build_auto_update.status, 'running'); @@ -162,7 +162,7 @@ describe('auto-update', () => { head_at_build: 'abcdef0', graphify_version: null, }); - t.after(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + t.after(() => cleanup(tmpDir)); const s = graphifyStatus(tmpDir); assert.strictEqual(s.stale, false, 'fresh graph + ok auto-build => not stale'); assert.strictEqual(s.last_build_auto_update.status, 'ok'); @@ -170,7 +170,7 @@ describe('auto-update', () => { test('graphifyStatus exposes last_build_auto_update: null when status file absent', (t) => { const tmpDir = makeStatusProject(null); - t.after(() => fs.rmSync(tmpDir, { recursive: true, force: true })); + t.after(() => cleanup(tmpDir)); const s = graphifyStatus(tmpDir); assert.strictEqual(s.last_build_auto_update, null); assert.strictEqual(s.stale, false, 'no status file => stale follows mtime only'); @@ -316,6 +316,7 @@ describe('auto-update', () => { atomicSleep(50); // yield 50 ms, then re-check (replaces execFileSync('sleep')) } try { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- best-effort teardown: error is swallowed so cleanup() (which propagates) cannot be used here; a residual temp dir after a detached-subprocess race is harmless (#382) fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 8, retryDelay: 100 }); } catch { /* best-effort teardown: a residual temp dir is harmless; never fail the test (#382) */ } } diff --git a/tests/graphify-visualization.test.cjs b/tests/graphify-visualization.test.cjs index 377dbc0c2..0f7127a7e 100644 --- a/tests/graphify-visualization.test.cjs +++ b/tests/graphify-visualization.test.cjs @@ -546,9 +546,7 @@ describe('regressions', () => { }); after(() => { - if (tmpDir) { - try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* ignore */ } - } + cleanup(tmpDir); }); test('hooks/gsd-graphify-update.sh present at install target', () => { diff --git a/tests/gsd-statusline.test.cjs b/tests/gsd-statusline.test.cjs index dae5a2505..4960a6d04 100644 --- a/tests/gsd-statusline.test.cjs +++ b/tests/gsd-statusline.test.cjs @@ -21,6 +21,7 @@ const { readGsdState, isInstalledAheadOfLatest, } = require('../hooks/gsd-statusline.js'); +const { cleanup } = require('./helpers.cjs'); // ─── parseStateMd ─────────────────────────────────────────────────────────── @@ -396,7 +397,7 @@ describe('todo-resolution: resolves in_progress task from the newest matching to test('resolves in_progress task from the newest matching todos file (#305)', (t) => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-305-')); t.after(() => { - try { fs.rmSync(tempDir, { recursive: true, force: true }); } catch {} + cleanup(tempDir); }); const todosDir = path.join(tempDir, 'todos'); diff --git a/tests/helpers-cleanup.test.cjs b/tests/helpers-cleanup.test.cjs new file mode 100644 index 000000000..16b6e517e --- /dev/null +++ b/tests/helpers-cleanup.test.cjs @@ -0,0 +1,101 @@ +/** + * GSD Tools Test Helpers – cleanup() behavioral tests + * + * Three deterministic, cross-platform tests that verify cleanup()'s + * observable contract at the seam rather than probing its internals. + */ + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const os = require('os'); + +const { cleanup, createTempDir } = require('./helpers.cjs'); + +// ─── Test 1: Real-FS happy path ────────────────────────────────────────────── + +test('cleanup removes a real temp dir with nested subdirs and files', () => { + const dir = createTempDir('gsd-cleanup-test-'); + const nested = path.join(dir, 'a', 'b', 'c'); + fs.mkdirSync(nested, { recursive: true }); + fs.writeFileSync(path.join(nested, 'file.txt'), 'hello'); + fs.writeFileSync(path.join(dir, 'root.txt'), 'world'); + + cleanup(dir); + + assert.strictEqual(fs.existsSync(dir), false, 'temp dir should not exist after cleanup'); +}); + +// ─── Test 2: Retry-budget contract ─────────────────────────────────────────── + +test('cleanup passes recursive/force/maxRetries/retryDelay options to fs.rmSync', () => { + // Use a real temp dir as the target so cleanup() has a valid path argument. + // We chdir AWAY from it first so cleanup() does not try to chdir either. + const dir = createTempDir('gsd-cleanup-opts-test-'); + + // Capture original cwd and shift away from the target. + const originalCwd = process.cwd(); + // Chdir to the parent of the target so cleanup's cwd-guard is a no-op. + process.chdir(path.dirname(dir)); + + let capturedOptions = null; + const realRmSync = fs.rmSync; + + try { + // Replace fs.rmSync with a probe that captures options then does nothing. + // This is an assignment expression (not a CallExpression) so it satisfies + // the ESLint rule that bans raw fs.rmSync(...) call expressions in tests. + fs.rmSync = (targetPath, opts) => { + capturedOptions = opts; + // Do NOT call through — we don't want the dir actually removed here; + // we're only testing the options shape. + }; + + cleanup(dir); + } finally { + fs.rmSync = realRmSync; + process.chdir(originalCwd); + // Remove the dir with the real rmSync now that we restored it. + cleanup(dir); + } + + assert.ok(capturedOptions !== null, 'fs.rmSync should have been called'); + assert.strictEqual(capturedOptions.recursive, true, 'recursive must be true'); + assert.strictEqual(capturedOptions.force, true, 'force must be true'); + assert.ok( + typeof capturedOptions.maxRetries === 'number' && capturedOptions.maxRetries > 0, + 'maxRetries must be a positive number' + ); + assert.ok( + typeof capturedOptions.retryDelay === 'number' && capturedOptions.retryDelay > 0, + 'retryDelay must be a positive number' + ); +}); + +// ─── Test 3: cwd-guard ─────────────────────────────────────────────────────── + +test('cleanup does not throw when cwd is inside the target dir, and removes the dir', () => { + const dir = createTempDir('gsd-cleanup-cwd-test-'); + const nested = path.join(dir, 'deep', 'nested'); + fs.mkdirSync(nested, { recursive: true }); + + const originalCwd = process.cwd(); + + try { + // Step INTO the nested subdir so cwd is inside the cleanup target. + process.chdir(nested); + + assert.doesNotThrow(() => { + cleanup(dir); + }, 'cleanup should not throw even when cwd is inside the target'); + } finally { + // Restore original cwd. cleanup() will have chdir'd to dirname(dir), + // so we always restore explicitly regardless. + if (process.cwd() !== originalCwd) { + process.chdir(originalCwd); + } + } + + assert.strictEqual(fs.existsSync(dir), false, 'temp dir should not exist after cleanup'); +}); diff --git a/tests/hermes-skills-migration.test.cjs b/tests/hermes-skills-migration.test.cjs index c35da1378..488596376 100644 --- a/tests/hermes-skills-migration.test.cjs +++ b/tests/hermes-skills-migration.test.cjs @@ -23,7 +23,7 @@ const { convertClaudeCommandToClaudeSkill, installRuntimeArtifacts, } = require('../bin/install.js'); -const { parseFrontmatter } = require('./helpers.cjs'); +const { parseFrontmatter, cleanup } = require('./helpers.cjs'); const pkg = require('../package.json'); const { @@ -138,9 +138,7 @@ describe('Hermes Agent: installRuntimeArtifacts', () => { }); afterEach(() => { - if (fs.existsSync(tmpDir)) { - fs.rmSync(tmpDir, { recursive: true }); - } + cleanup(tmpDir); }); test('creates skills/gsd/quick/SKILL.md directory structure (Hermes bare-stem layout)', () => { diff --git a/tests/hooks-opt-in.test.cjs b/tests/hooks-opt-in.test.cjs index fb26ed1aa..e7a26135d 100644 --- a/tests/hooks-opt-in.test.cjs +++ b/tests/hooks-opt-in.test.cjs @@ -46,6 +46,7 @@ function createTempProject(prefix = 'gsd-hook-test-') { } function cleanup(tmpDir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- this IS the local teardown helper; wrapping helpers.cjs cleanup would create a circular dependency try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch {} } @@ -203,7 +204,7 @@ describe('opt-in gating behavior', { skip: isWindows ? 'bash hooks require unix test('validate-commit is a no-op when config.json is absent', (t) => { // No config.json at all const bareDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-hook-bare-')); - t.after(() => { fs.rmSync(bareDir, { recursive: true, force: true }); }); + t.after(() => { cleanup(bareDir); }); const hookPath = path.join(HOOKS_DIR, 'gsd-validate-commit.sh'); const input = JSON.stringify({ tool_input: { command: 'git commit -m "WIP save"' } @@ -353,7 +354,7 @@ describe('hook execution when enabled', { skip: isWindows ? 'bash hooks require test('session-state exits 0 without .planning/ (in enabled project)', (t) => { // Create a dir with config but no STATE.md const noStateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-hook-nostate-')); - t.after(() => { fs.rmSync(noStateDir, { recursive: true, force: true }); }); + t.after(() => { cleanup(noStateDir); }); fs.mkdirSync(path.join(noStateDir, '.planning'), { recursive: true }); writeConfigWithHooks(noStateDir, true); const hookPath = path.join(HOOKS_DIR, 'gsd-session-state.sh'); diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index f4f892929..6dea5dae2 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -140,7 +140,7 @@ describe('install-profiles: stageSkillsForMode', () => { try { assert.strictEqual(stageSkillsForMode(src, 'full'), src); } finally { - fs.rmSync(src, { recursive: true, force: true }); + cleanup(src); } }); @@ -156,8 +156,8 @@ describe('install-profiles: stageSkillsForMode', () => { 'phase.md', 'plan-phase.md', 'surface.md', 'update.md'], ); } finally { - fs.rmSync(src, { recursive: true, force: true }); - if (staged) fs.rmSync(staged, { recursive: true, force: true }); + cleanup(src); + cleanup(staged); } }); @@ -170,8 +170,8 @@ describe('install-profiles: stageSkillsForMode', () => { const copied = fs.readFileSync(path.join(staged, 'plan-phase.md'), 'utf8'); assert.strictEqual(copied, original); } finally { - fs.rmSync(src, { recursive: true, force: true }); - if (staged) fs.rmSync(staged, { recursive: true, force: true }); + cleanup(src); + cleanup(staged); } }); @@ -191,8 +191,8 @@ describe('install-profiles: stageSkillsForMode', () => { staged = stageSkillsForMode(src, 'minimal'); assert.deepStrictEqual(fs.readdirSync(staged), ['plan-phase.md']); } finally { - fs.rmSync(src, { recursive: true, force: true }); - if (staged) fs.rmSync(staged, { recursive: true, force: true }); + cleanup(src); + cleanup(staged); } }); }); @@ -211,7 +211,7 @@ describe('install-profiles: cleanupStagedSkills', () => { assert.ok(!fs.existsSync(a)); assert.ok(!fs.existsSync(b)); } finally { - fs.rmSync(src, { recursive: true, force: true }); + cleanup(src); } }); @@ -230,7 +230,7 @@ describe('install-profiles: cleanupStagedSkills', () => { const after = process.listenerCount('exit'); assert.ok(after - before <= 1, `expected <=1 new exit listener, got ${after - before}`); } finally { - fs.rmSync(src, { recursive: true, force: true }); + cleanup(src); cleanupStagedSkills(); } }); @@ -261,7 +261,7 @@ describe('install-profiles: cleanupStagedSkills', () => { } finally { fs.copyFileSync = realCopy; fs.mkdtempSync = realMkdtemp; - fs.rmSync(src, { recursive: true, force: true }); + cleanup(src); cleanupStagedSkills(); } }); @@ -296,7 +296,7 @@ describe('install: --minimal honoured for every runtime in --global mode', () => ); assert.strictEqual(manifestAgentCount(manifest), 0); } finally { - fs.rmSync(root, { recursive: true, force: true }); + cleanup(root); } }); } @@ -315,7 +315,7 @@ describe('install: --minimal honoured for every runtime in --local mode', () => ); assert.strictEqual(manifestAgentCount(manifest), 0); } finally { - fs.rmSync(root, { recursive: true, force: true }); + cleanup(root); } }); } @@ -333,7 +333,7 @@ describe('install: Cline --minimal (rules-based, no skills/ dir)', () => { assert.strictEqual(manifestAgentCount(manifest), 0); assert.ok(fs.existsSync(path.join(configDir, '.clinerules'))); } finally { - fs.rmSync(root, { recursive: true, force: true }); + cleanup(root); } }); } @@ -358,7 +358,7 @@ describe('install: on-disk skill files match manifest for --minimal', () => { assert.deepStrictEqual(gsdAgents, []); } } finally { - fs.rmSync(root, { recursive: true, force: true }); + cleanup(root); } }); } @@ -385,7 +385,7 @@ describe('install: manifest records mode for both profiles', () => { const agentCount = Object.keys(m.files || {}).filter(k => k.startsWith('agents/')).length; return { mode: m.mode, skillCount, agentCount }; } finally { - fs.rmSync(targetDir, { recursive: true, force: true }); + cleanup(targetDir); } } @@ -439,7 +439,7 @@ describe('install-minimal-backcompat: --minimal and --profile=core produce same const profileMarker = fs.existsSync(markerPath) ? fs.readFileSync(markerPath, 'utf8').trim() : null; return { mode: m.mode, skillCount, profileMarker }; } finally { - fs.rmSync(targetDir, { recursive: true, force: true }); + cleanup(targetDir); } } @@ -518,7 +518,7 @@ describe('install: Codex full → minimal downgrade cleans stale agent state', ( } assert.ok(fs.existsSync(configPath)); } finally { - fs.rmSync(targetDir, { recursive: true, force: true }); + cleanup(targetDir); } }); }); @@ -545,7 +545,7 @@ describe('install: Claude full → minimal downgrade removes stale agents', () = assert.ok(remaining.includes('my-custom-agent.md')); assert.deepStrictEqual(remaining.filter(f => f.startsWith('gsd-')), []); } finally { - fs.rmSync(targetDir, { recursive: true, force: true }); + cleanup(targetDir); } }); }); diff --git a/tests/install-path-detection.test.cjs b/tests/install-path-detection.test.cjs index 81b6107e6..a9c7ce040 100644 --- a/tests/install-path-detection.test.cjs +++ b/tests/install-path-detection.test.cjs @@ -39,6 +39,7 @@ function createTempHome() { } function cleanup(dir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup predates helpers.cjs; name collision prevents import fs.rmSync(dir, { recursive: true, force: true }); } diff --git a/tests/install-update-marker.test.cjs b/tests/install-update-marker.test.cjs index 6116b356f..8e33b3d50 100644 --- a/tests/install-update-marker.test.cjs +++ b/tests/install-update-marker.test.cjs @@ -16,6 +16,8 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); +const { cleanup } = require('./helpers.cjs'); + const { resolveEffectiveProfile, mostRestrictiveProfile, @@ -36,7 +38,7 @@ describe('resolveEffectiveProfile', () => { const result = resolveEffectiveProfile({ requestedProfileName: null, targetDir: dir }); assert.strictEqual(result, 'full'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -47,7 +49,7 @@ describe('resolveEffectiveProfile', () => { const result = resolveEffectiveProfile({ requestedProfileName: null, targetDir: dir }); assert.strictEqual(result, 'standard'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -58,7 +60,7 @@ describe('resolveEffectiveProfile', () => { const result = resolveEffectiveProfile({ requestedProfileName: null, targetDir: dir }); assert.strictEqual(result, 'core'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -69,7 +71,7 @@ describe('resolveEffectiveProfile', () => { const result = resolveEffectiveProfile({ requestedProfileName: 'full', targetDir: dir }); assert.strictEqual(result, 'full'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -80,7 +82,7 @@ describe('resolveEffectiveProfile', () => { const result = resolveEffectiveProfile({ requestedProfileName: 'standard', targetDir: dir }); assert.strictEqual(result, 'standard'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -91,7 +93,7 @@ describe('resolveEffectiveProfile', () => { const result = resolveEffectiveProfile({ requestedProfileName: null, targetDir: dir }); assert.strictEqual(result, 'full'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); @@ -159,7 +161,7 @@ describe('marker-driven profile resolution end-to-end', () => { if (staged) cleanupStagedSkills(); } } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -175,8 +177,8 @@ describe('marker-driven profile resolution end-to-end', () => { assert.strictEqual(resolved, 'core', 'core is smaller than standard — most-restrictive wins'); } finally { - fs.rmSync(dirA, { recursive: true, force: true }); - fs.rmSync(dirB, { recursive: true, force: true }); + cleanup(dirA); + cleanup(dirB); } }); @@ -190,7 +192,7 @@ describe('marker-driven profile resolution end-to-end', () => { const resolved = resolveProfile({ modes: [effective], manifest }); assert.strictEqual(resolved.skills, '*', 'full profile should be sentinel'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); diff --git a/tests/installer-migrations.test.cjs b/tests/installer-migrations.test.cjs index 02e60d67f..5a1dc6103 100644 --- a/tests/installer-migrations.test.cjs +++ b/tests/installer-migrations.test.cjs @@ -22,6 +22,7 @@ function createTempInstall() { } function cleanup(dir) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- local cleanup predates helpers.cjs; name collision prevents import fs.rmSync(dir, { recursive: true, force: true }); } diff --git a/tests/issue-2517-runtime-aware-profiles.test.cjs b/tests/issue-2517-runtime-aware-profiles.test.cjs index 2aff35153..66b75ee36 100644 --- a/tests/issue-2517-runtime-aware-profiles.test.cjs +++ b/tests/issue-2517-runtime-aware-profiles.test.cjs @@ -68,7 +68,7 @@ function isolateHome() { function restoreHome() { if (_origHome === undefined) delete process.env.HOME; else process.env.HOME = _origHome; if (_origGsdHome === undefined) delete process.env.GSD_HOME; else process.env.GSD_HOME = _origGsdHome; - if (_isolatedHome) fs.rmSync(_isolatedHome, { recursive: true, force: true }); + cleanup(_isolatedHome); _isolatedHome = null; } diff --git a/tests/issue-2639-codex-toml-neutralization.test.cjs b/tests/issue-2639-codex-toml-neutralization.test.cjs index c373f2ab7..b8e299cb7 100644 --- a/tests/issue-2639-codex-toml-neutralization.test.cjs +++ b/tests/issue-2639-codex-toml-neutralization.test.cjs @@ -24,6 +24,7 @@ const path = require('path'); const os = require('os'); const { installCodexConfig } = require('../bin/install.js'); +const { cleanup } = require('./helpers.cjs'); function makeTempDir() { return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2639-')); @@ -54,7 +55,7 @@ describe('#2639 — Codex TOML emit routes through full neutralization pipeline' }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('strips CLAUDE.md, .claude/skills/, .claude/commands/, .claude/agents/, and .claudeignore from emitted TOML', () => { diff --git a/tests/issue-498-update-context.test.cjs b/tests/issue-498-update-context.test.cjs index 880adf0df..b0740eeb7 100644 --- a/tests/issue-498-update-context.test.cjs +++ b/tests/issue-498-update-context.test.cjs @@ -16,6 +16,7 @@ const { execFileSync } = require('node:child_process'); const ROOT = path.join(__dirname, '..'); const GSD_TOOLS = path.join(ROOT, 'get-shit-done', 'bin', 'gsd-tools.cjs'); +const { cleanup } = require('./helpers.cjs'); const { resolveUpdateContext } = require( path.join(ROOT, 'get-shit-done', 'bin', 'lib', 'update-context.cjs'), ); @@ -137,7 +138,7 @@ describe('gsd-tools update-context (CLI): emits the JSON contract', () => { assert.equal(ctx.scope, 'GLOBAL'); assert.equal(ctx.runtime, 'kilo'); } finally { - nodeFs.rmSync(tmp, { recursive: true, force: true }); + cleanup(tmp); } }); }); diff --git a/tests/learnings.test.cjs b/tests/learnings.test.cjs index 1b58d2e41..f75b49f26 100644 --- a/tests/learnings.test.cjs +++ b/tests/learnings.test.cjs @@ -37,9 +37,7 @@ function makeTempDir() { * @param {string} dir */ function cleanupDir(dir) { - if (fs.existsSync(dir)) { - fs.rmSync(dir, { recursive: true, force: true }); - } + cleanup(dir); } // ─── Write ─────────────────────────────────────────────────────────────────── diff --git a/tests/lint-docs-required.test.cjs b/tests/lint-docs-required.test.cjs index e0e7db683..6f58521d3 100644 --- a/tests/lint-docs-required.test.cjs +++ b/tests/lint-docs-required.test.cjs @@ -6,6 +6,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); const { evaluateLint, @@ -284,7 +285,7 @@ describe('docs-required lint: readFragmentsFromDisk', () => { fs.mkdirSync(path.join(tmp, '.changeset'), { recursive: true }); fn(tmp); } finally { - fs.rmSync(tmp, { recursive: true, force: true, maxRetries: 20, retryDelay: 250 }); + cleanup(tmp); } } diff --git a/tests/lint-pr-check-project-dir.test.cjs b/tests/lint-pr-check-project-dir.test.cjs index ee84e48f2..6e347a3d9 100644 --- a/tests/lint-pr-check-project-dir.test.cjs +++ b/tests/lint-pr-check-project-dir.test.cjs @@ -6,6 +6,7 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); const { spawnSync } = require('child_process'); +const { cleanup } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); const LINT_SCRIPT = path.join(ROOT, 'scripts', 'lint-pr-check-project-dir.cjs'); @@ -110,7 +111,7 @@ describe('lint-pr-check-project-dir', () => { assert.notStrictEqual(result.status, 0); } finally { - fs.rmSync(dir, { recursive: true, force: true, maxRetries: 20, retryDelay: 250 }); + cleanup(dir); } }); diff --git a/tests/lint-skill-deps.test.cjs b/tests/lint-skill-deps.test.cjs index a13556a06..0e6254f1f 100644 --- a/tests/lint-skill-deps.test.cjs +++ b/tests/lint-skill-deps.test.cjs @@ -13,6 +13,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { spawnSync } = require('child_process'); +const { cleanup } = require('./helpers.cjs'); const LINT_SCRIPT = path.join(__dirname, '..', 'scripts', 'lint-skill-deps.cjs'); @@ -48,7 +49,7 @@ describe('lint-skill-deps: frontmatter ↔ body consistency', () => { const result = runLint(['--dir', dir]); assert.strictEqual(result.status, 0, `Expected exit 0, got ${result.status}\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -65,7 +66,7 @@ describe('lint-skill-deps: frontmatter ↔ body consistency', () => { const result = runLint(['--dir', dir]); assert.notStrictEqual(result.status, 0, 'Should exit non-zero when requires: is missing but body has reference'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -81,7 +82,7 @@ describe('lint-skill-deps: frontmatter ↔ body consistency', () => { const result = runLint(['--dir', dir]); assert.notStrictEqual(result.status, 0, 'Should exit non-zero for undeclared reference'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -92,7 +93,7 @@ describe('lint-skill-deps: frontmatter ↔ body consistency', () => { const result = runLint(['--dir', dir]); assert.strictEqual(result.status, 0); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -108,7 +109,7 @@ describe('lint-skill-deps: frontmatter ↔ body consistency', () => { const result = runLint(['--dir', dir]); assert.notStrictEqual(result.status, 0, 'Unknown skill references must fail lint'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); @@ -138,7 +139,7 @@ describe('lint-skill-deps: script basics', () => { assert.strictEqual(result.stderr, '', `Expected empty stderr on success, got: ${result.stderr}`); assert.ok(result.stdout.length > 0, 'Expected non-empty stdout on success'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); diff --git a/tests/lint-test-file-count.test.cjs b/tests/lint-test-file-count.test.cjs index 02671d473..c16bcc8e5 100644 --- a/tests/lint-test-file-count.test.cjs +++ b/tests/lint-test-file-count.test.cjs @@ -54,7 +54,7 @@ describe('evaluateLint — OK_UNDER_LIMIT', () => { }); assert.strictEqual(result.verdict, Verdict.OK_UNDER_LIMIT); assert.strictEqual(result.count, 1); - assert.strictEqual(result.ceiling, null); + assert.strictEqual(result.knownFiles, null); }); test('2-file module passes (primary + integration)', () => { @@ -84,12 +84,12 @@ describe('evaluateLint — FAIL_EXCEEDS_LIMIT', () => { }); assert.strictEqual(result.verdict, Verdict.FAIL_EXCEEDS_LIMIT); assert.strictEqual(result.count, 3); - assert.strictEqual(result.ceiling, null); + assert.strictEqual(result.knownFiles, null); }); }); -describe('evaluateLint — allowlist behaviour', () => { - test('3-file module allowlisted at 3 passes (OK_IN_ALLOWLIST)', () => { +describe('evaluateLint — allowlist behaviour (identity-based)', () => { + test('3-file module allowlisted with exact filenames passes (OK_IN_ALLOWLIST)', () => { const result = evaluateLint({ prefix: 'phase', testFiles: makeFiles('phase', [ @@ -97,44 +97,120 @@ describe('evaluateLint — allowlist behaviour', () => { 'phase-edge.test.cjs', 'phase-regression.test.cjs', ]), - allowlist: { phase: { current: 3, issue: 'TBD' } }, + allowlist: { + phase: { + files: ['phase.test.cjs', 'phase-edge.test.cjs', 'phase-regression.test.cjs'], + issue: 'TBD', + }, + }, }); assert.strictEqual(result.verdict, Verdict.OK_IN_ALLOWLIST); assert.strictEqual(result.count, 3); - assert.strictEqual(result.ceiling, 3); + assert.deepStrictEqual(result.novel, []); + assert.deepStrictEqual(result.stale, []); }); - test('2-file module allowlisted at 3 emits HINT_CAN_REMOVE_FROM_ALLOWLIST', () => { + test('2-file module allowlisted fails (FAIL_STALE_ALLOWLIST — whole entry must be pruned)', () => { const result = evaluateLint({ prefix: 'phase', testFiles: makeFiles('phase', [ 'phase.test.cjs', 'phase-edge.test.cjs', ]), - allowlist: { phase: { current: 3, issue: 'TBD' } }, + allowlist: { + phase: { + files: ['phase.test.cjs', 'phase-edge.test.cjs', 'phase-regression.test.cjs'], + issue: 'TBD', + }, + }, }); - assert.strictEqual(result.verdict, Verdict.HINT_CAN_REMOVE_FROM_ALLOWLIST); + // Ratchet-DOWN: dropping to ≤ MAX_FILES while allowlisted is a FAILURE, not a hint. + assert.strictEqual(result.verdict, Verdict.FAIL_STALE_ALLOWLIST); assert.strictEqual(result.count, 2); - assert.strictEqual(result.ceiling, 3); + assert.deepStrictEqual(result.novel, []); + // stale should list all known files (the entire entry must be removed) + assert.deepStrictEqual(result.stale.sort(), [ + 'phase-edge.test.cjs', + 'phase-regression.test.cjs', + 'phase.test.cjs', + ]); }); - test('4-file module allowlisted at 3 fails (FAIL_EXCEEDS_ALLOWLIST)', () => { + test('novel file added to capped module fails (FAIL_NOVEL_FILES)', () => { const result = evaluateLint({ prefix: 'phase', testFiles: makeFiles('phase', [ 'phase.test.cjs', - 'phase-a.test.cjs', - 'phase-b.test.cjs', - 'phase-c.test.cjs', + 'phase-edge.test.cjs', + 'phase-regression.test.cjs', + 'phase-new-extra.test.cjs', // <-- novel ]), - allowlist: { phase: { current: 3, issue: 'TBD' } }, + allowlist: { + phase: { + files: ['phase.test.cjs', 'phase-edge.test.cjs', 'phase-regression.test.cjs'], + issue: 'TBD', + }, + }, }); - assert.strictEqual(result.verdict, Verdict.FAIL_EXCEEDS_ALLOWLIST); - assert.strictEqual(result.count, 4); - assert.strictEqual(result.ceiling, 3); + assert.strictEqual(result.verdict, Verdict.FAIL_NOVEL_FILES); + assert.deepStrictEqual(result.novel, ['phase-new-extra.test.cjs']); + assert.deepStrictEqual(result.stale, []); }); - test('ratchet: count equal to ceiling passes', () => { + test('allowlisted file removed from disk while dropping to cap fails (FAIL_STALE_ALLOWLIST)', () => { + const result = evaluateLint({ + prefix: 'phase', + testFiles: makeFiles('phase', [ + 'phase.test.cjs', + 'phase-edge.test.cjs', + // phase-regression.test.cjs removed — count now at MAX_FILES (2) + ]), + allowlist: { + phase: { + files: ['phase.test.cjs', 'phase-edge.test.cjs', 'phase-regression.test.cjs'], + issue: 'TBD', + }, + }, + }); + // count is 2 (≤ MAX_FILES=2) while still allowlisted — ratchet-DOWN FAILURE. + // All known files are stale; the entire entry must be pruned. + assert.strictEqual(result.verdict, Verdict.FAIL_STALE_ALLOWLIST); + assert.deepStrictEqual(result.novel, []); + assert.deepStrictEqual(result.stale.sort(), [ + 'phase-edge.test.cjs', + 'phase-regression.test.cjs', + 'phase.test.cjs', + ]); + }); + + test('allowlisted file removed while still over cap fails (FAIL_STALE_ALLOWLIST)', () => { + // Module has 4 files allowlisted, one removed (3 remain, still > 2) + const result = evaluateLint({ + prefix: 'phase', + testFiles: makeFiles('phase', [ + 'phase.test.cjs', + 'phase-edge.test.cjs', + 'phase-regression.test.cjs', + // phase-extra.test.cjs removed from disk + ]), + allowlist: { + phase: { + files: [ + 'phase.test.cjs', + 'phase-edge.test.cjs', + 'phase-regression.test.cjs', + 'phase-extra.test.cjs', // stale + ], + issue: 'TBD', + }, + }, + }); + assert.strictEqual(result.verdict, Verdict.FAIL_STALE_ALLOWLIST); + assert.deepStrictEqual(result.stale, ['phase-extra.test.cjs']); + assert.deepStrictEqual(result.novel, []); + }); + + test('ratchet: count equal to allowlisted set passes', () => { const result = evaluateLint({ prefix: 'init', testFiles: makeFiles('init', [ @@ -142,10 +218,50 @@ describe('evaluateLint — allowlist behaviour', () => { 'init-manager.test.cjs', 'init-manager-deps.test.cjs', ]), - allowlist: { init: { current: 3, issue: 'TBD' } }, + allowlist: { + init: { + files: ['init.test.cjs', 'init-manager.test.cjs', 'init-manager-deps.test.cjs'], + issue: 'TBD', + }, + }, }); assert.strictEqual(result.verdict, Verdict.OK_IN_ALLOWLIST); }); + + // ------------------------------------------------------------------- + // Masking blind spot: count unchanged but SET changed → must FAIL + // ------------------------------------------------------------------- + test('masking blind spot closed: swapped file (same count, different identity) fails', () => { + // Old allowlist grandfathers 3 files. One is deleted, one new one added. + // Count stays at 3 — the old count-ratchet would have passed. Identity must fail. + const result = evaluateLint({ + prefix: 'phase', + testFiles: makeFiles('phase', [ + 'phase.test.cjs', + 'phase-edge.test.cjs', + 'phase-brand-new.test.cjs', // <-- replaces phase-regression (novel) + ]), + allowlist: { + phase: { + files: [ + 'phase.test.cjs', + 'phase-edge.test.cjs', + 'phase-regression.test.cjs', // <-- no longer on disk (stale) + ], + issue: 'TBD', + }, + }, + }); + // The identity check must catch this: novel = ['phase-brand-new.test.cjs'], + // stale = ['phase-regression.test.cjs']. Count is the same (3), but the + // old count-ratchet would have silently passed. The identity ratchet fails. + assert.ok( + result.verdict === Verdict.FAIL_NOVEL_FILES || result.verdict === Verdict.FAIL_STALE_ALLOWLIST, + `expected FAIL_NOVEL_FILES or FAIL_STALE_ALLOWLIST, got ${result.verdict}` + ); + assert.deepStrictEqual(result.novel, ['phase-brand-new.test.cjs']); + assert.deepStrictEqual(result.stale, ['phase-regression.test.cjs']); + }); }); // --------------------------------------------------------------------------- @@ -207,7 +323,7 @@ describe('CLI --json', () => { assert.ok(typeof data.ok === 'boolean', 'ok must be boolean'); }); - test('each result has verdict, prefix, count, ceiling, files', () => { + test('each result has verdict, prefix, count, knownFiles, files', () => { const { data } = runCliJson(); for (const r of data.results) { assert.ok(typeof r.verdict === 'string', `verdict missing on ${r.prefix}`); @@ -224,4 +340,14 @@ describe('CLI --json', () => { assert.ok(valid.has(r.verdict), `Unknown verdict "${r.verdict}" on prefix "${r.prefix}"`); } }); + + test('OK_IN_ALLOWLIST results have knownFiles array', () => { + const { data } = runCliJson(); + const allowlisted = data.results.filter(r => r.verdict === Verdict.OK_IN_ALLOWLIST); + assert.ok(allowlisted.length > 0, 'expected at least one allowlisted module in real repo'); + for (const r of allowlisted) { + assert.ok(Array.isArray(r.knownFiles), `knownFiles must be array on ${r.prefix}`); + assert.ok(r.knownFiles.length > 0, `knownFiles must be non-empty on ${r.prefix}`); + } + }); }); diff --git a/tests/milestone-archive.test.cjs b/tests/milestone-archive.test.cjs index b41a42f7b..706477a60 100644 --- a/tests/milestone-archive.test.cjs +++ b/tests/milestone-archive.test.cjs @@ -232,6 +232,7 @@ function setupMilestoneArchiveProject(tmpDir, options = {}) { roadmapPhases = ['64'], } = options; + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-fixture setup: removing subdirectory (not temp root teardown) fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true }); const archiveDir = path.join(tmpDir, '.planning', 'milestones', `${milestone}-phases`); @@ -297,6 +298,7 @@ describe('#3164 — validate consistency: milestone-archive layout', () => { }); test('consistency scans only active milestone archive and still validates plans/frontmatter', () => { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test setup: removing subdirectory to establish milestone-archive layout fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true }); const oldDir = path.join(tmpDir, '.planning', 'milestones', 'v1.6-phases', '64-legacy'); @@ -373,6 +375,7 @@ describe('#3164 — find-phase: milestone-archive layout', () => { }); test('find-phase searches milestone archives in deterministic sorted order', () => { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test setup: removing phases subdirectory to establish milestone-archive layout fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true }); const milestonesDir = path.join(tmpDir, '.planning', 'milestones'); diff --git a/tests/milestone-summary.test.cjs b/tests/milestone-summary.test.cjs index 514f0e062..03e0a2dbd 100644 --- a/tests/milestone-summary.test.cjs +++ b/tests/milestone-summary.test.cjs @@ -15,6 +15,7 @@ const { test, describe, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const { cleanup } = require('./helpers.cjs'); const repoRoot = path.resolve(__dirname, '..'); const commandPath = path.join(repoRoot, 'commands', 'gsd', 'milestone-summary.md'); @@ -206,7 +207,7 @@ describe('milestone-summary fixture-based artifact discovery', () => { }); afterEach(() => { - if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('discovers artifacts in archived milestone structure', () => { diff --git a/tests/new-milestone-clear-phases.test.cjs b/tests/new-milestone-clear-phases.test.cjs index 82478a2a0..76e81124a 100644 --- a/tests/new-milestone-clear-phases.test.cjs +++ b/tests/new-milestone-clear-phases.test.cjs @@ -64,6 +64,7 @@ describe('phases clear command', () => { test('succeeds with cleared=0 when phases directory does not exist', () => { // Remove the phases directory entirely + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test removal to simulate absent phases dir (SUT behavior, not teardown) fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true, force: true }); const result = runGsdTools('phases clear --confirm', tmpDir); diff --git a/tests/next-decimal-roadmap-scan.test.cjs b/tests/next-decimal-roadmap-scan.test.cjs index 4f24c65a4..df5b6bbec 100644 --- a/tests/next-decimal-roadmap-scan.test.cjs +++ b/tests/next-decimal-roadmap-scan.test.cjs @@ -155,6 +155,7 @@ describe('phase next-decimal ROADMAP.md scanning (#1865)', () => { test('handles no phases dir and no ROADMAP.md', () => { // Remove the phases directory entirely + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test removal to simulate absent phases dir (SUT behavior, not teardown) fs.rmSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true }); const result = runGsdTools('phase next-decimal 999', tmpDir); diff --git a/tests/observability/hub-logger-integration.test.cjs b/tests/observability/hub-logger-integration.test.cjs index 6168af902..ef1b99464 100644 --- a/tests/observability/hub-logger-integration.test.cjs +++ b/tests/observability/hub-logger-integration.test.cjs @@ -29,6 +29,7 @@ const { createDefaultLogger, createNoOpLogger, } = require('../../get-shit-done/bin/lib/observability/logger.cjs'); +const { cleanup } = require('../helpers.cjs'); // ─── helpers ───────────────────────────────────────────────────────────────── @@ -346,7 +347,7 @@ describe('Hub + createDefaultLogger — end-to-end', () => { afterEach(() => { if (savedAudit === undefined) delete process.env.GSD_AUDIT; else process.env.GSD_AUDIT = savedAudit; if (savedAuditArgs === undefined) delete process.env.GSD_AUDIT_ARGS; else process.env.GSD_AUDIT_ARGS = savedAuditArgs; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('silent on success with default logger', () => { diff --git a/tests/observability/logger.test.cjs b/tests/observability/logger.test.cjs index 4daa70c1c..d2835b6a1 100644 --- a/tests/observability/logger.test.cjs +++ b/tests/observability/logger.test.cjs @@ -18,6 +18,7 @@ const { createDefaultLogger, createNoOpLogger, } = require('../../get-shit-done/bin/lib/observability/logger.cjs'); +const { cleanup } = require('../helpers.cjs'); // ─── helpers ───────────────────────────────────────────────────────────────── @@ -105,7 +106,7 @@ describe('createDefaultLogger — silent on success', () => { afterEach(() => { if (savedAudit === undefined) delete process.env.GSD_AUDIT; else process.env.GSD_AUDIT = savedAudit; if (savedAuditArgs === undefined) delete process.env.GSD_AUDIT_ARGS; else process.env.GSD_AUDIT_ARGS = savedAuditArgs; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('no stderr output on ok result', () => { @@ -140,7 +141,7 @@ describe('createDefaultLogger — stderr on error', () => { afterEach(() => { if (savedAudit === undefined) delete process.env.GSD_AUDIT; else process.env.GSD_AUDIT = savedAudit; if (savedAuditArgs === undefined) delete process.env.GSD_AUDIT_ARGS; else process.env.GSD_AUDIT_ARGS = savedAuditArgs; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('emits exactly one JSON line to stderr on error', () => { @@ -230,7 +231,7 @@ describe('createDefaultLogger — audit file', () => { afterEach(() => { if (savedAudit === undefined) delete process.env.GSD_AUDIT; else process.env.GSD_AUDIT = savedAudit; if (savedAuditArgs === undefined) delete process.env.GSD_AUDIT_ARGS; else process.env.GSD_AUDIT_ARGS = savedAuditArgs; - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('creates .planning/.gsd-trace.jsonl when GSD_AUDIT=1 (ok result)', () => { diff --git a/tests/perf-315-loadconfig-subrepo-scan.test.cjs b/tests/perf-315-loadconfig-subrepo-scan.test.cjs index e212e3595..35c3f80a6 100644 --- a/tests/perf-315-loadconfig-subrepo-scan.test.cjs +++ b/tests/perf-315-loadconfig-subrepo-scan.test.cjs @@ -25,6 +25,7 @@ const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); +const { cleanup } = require('./helpers.cjs'); // Import loadConfig directly (sync, no CLI subprocess needed) const { loadConfig } = require('../get-shit-done/bin/lib/core.cjs'); @@ -63,7 +64,7 @@ describe('perf-315 — loadConfig calls detectSubRepos at most once per invocati afterEach(() => { if (tmpDir) { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); tmpDir = null; } }); diff --git a/tests/perf-316-state-lock-buffer-alloc.test.cjs b/tests/perf-316-state-lock-buffer-alloc.test.cjs index 68a3abcb9..72c4b53d8 100644 --- a/tests/perf-316-state-lock-buffer-alloc.test.cjs +++ b/tests/perf-316-state-lock-buffer-alloc.test.cjs @@ -32,6 +32,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { Worker } = require('worker_threads'); +const { cleanup } = require('./helpers.cjs'); // ───────────────────────────────────────────────────────────────────────────── // Constants @@ -127,7 +128,7 @@ function makeTempDir() { } function removeTempDir(dir) { - try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + try { cleanup(dir); } catch { /* ignore */ } } // ───────────────────────────────────────────────────────────────────────────── diff --git a/tests/perf-317-context-monitor-fs.test.cjs b/tests/perf-317-context-monitor-fs.test.cjs index 2ad5e9e87..c71b72630 100644 --- a/tests/perf-317-context-monitor-fs.test.cjs +++ b/tests/perf-317-context-monitor-fs.test.cjs @@ -18,6 +18,7 @@ const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const MONITOR_PATH = path.join(__dirname, '..', 'hooks', 'gsd-context-monitor.js'); const tmpDir = os.tmpdir(); @@ -157,7 +158,7 @@ describe('perf #317: config.json absent (exercises config-missing → defaults p 'warning output must contain additionalContext' ); } finally { - try { fs.rmSync(testCwd, { recursive: true, force: true }); } catch { /* tolerate */ } + cleanup(testCwd); } }); @@ -194,7 +195,7 @@ describe('perf #317: config.json absent (exercises config-missing → defaults p stdout = e.stdout || ''; } finally { try { fs.unlinkSync(metricsPath); } catch { /* noop */ } - try { fs.rmSync(testCwd, { recursive: true, force: true }); } catch { /* tolerate */ } + cleanup(testCwd); } assert.strictEqual(exitCode, 0, 'hook should exit 0 when context_warnings=false'); diff --git a/tests/perf-407-planning-lock-buffer-alloc.test.cjs b/tests/perf-407-planning-lock-buffer-alloc.test.cjs index 994f905f8..c2897e643 100644 --- a/tests/perf-407-planning-lock-buffer-alloc.test.cjs +++ b/tests/perf-407-planning-lock-buffer-alloc.test.cjs @@ -34,6 +34,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); const os = require('os'); +const { cleanup } = require('./helpers.cjs'); // ───────────────────────────────────────────────────────────────────────────── // Constants @@ -58,7 +59,7 @@ function makeTempDir() { } function removeTempDir(dir) { - try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + try { cleanup(dir); } catch { /* ignore */ } } /** diff --git a/tests/phase.test.cjs b/tests/phase.test.cjs index a61143997..7a3b862df 100644 --- a/tests/phase.test.cjs +++ b/tests/phase.test.cjs @@ -3387,7 +3387,7 @@ describe('bug #1998: phase complete updates overview checkbox', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('checkbox updated when no archived milestones exist', () => { @@ -3506,7 +3506,7 @@ describe('bug #2005: phase complete updates plan count when milestone is inside }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('plan count is updated when current milestone is wrapped in
', () => { @@ -3659,7 +3659,7 @@ describe('bug #2526: phase complete warns about unregistered REQ-IDs', () => { }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('emits warning for REQ-IDs in body but missing from Traceability table', () => { @@ -4296,7 +4296,7 @@ describe('bug-3287 — init plan-phase exposes expected_phase_dir with project_c }); afterEach(() => { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); }); test('completed_phases is derived from ROADMAP, not blindly incremented (idempotency)', () => { diff --git a/tests/planning-workspace.test.cjs b/tests/planning-workspace.test.cjs index 699b0fe73..df3ec9e5d 100644 --- a/tests/planning-workspace.test.cjs +++ b/tests/planning-workspace.test.cjs @@ -3,6 +3,7 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { cleanup } = require('./helpers.cjs'); const { createPlanningWorkspace, @@ -81,7 +82,7 @@ describe('planning-workspace: session adapter precedence', () => { assert.strictEqual(workspace.activeWorkstream.get(), 'session-ws'); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); }); @@ -109,7 +110,7 @@ describe('planning-workspace: self-heal behavior', () => { assert.strictEqual(workspace.activeWorkstream.get(), null); assert.strictEqual(adapter.read(), null); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); }); @@ -122,7 +123,7 @@ describe('planning-workspace: lock seam', () => { assert.strictEqual(result, 'ok'); assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', '.lock'))); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); @@ -141,7 +142,7 @@ describe('planning-workspace: lock seam', () => { assert.strictEqual(attempts, 1); assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', '.lock'))); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); }); @@ -180,7 +181,7 @@ describe('core compatibility adapter: planning workspace functions', () => { setActiveWorkstream(tmpDir, null); assert.strictEqual(core.getActiveWorkstream(tmpDir), null); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); }); diff --git a/tests/prune-orphaned-worktrees.test.cjs b/tests/prune-orphaned-worktrees.test.cjs index 2a4c5930e..b3f492f9c 100644 --- a/tests/prune-orphaned-worktrees.test.cjs +++ b/tests/prune-orphaned-worktrees.test.cjs @@ -181,6 +181,7 @@ describe('pruneOrphanedWorktrees', () => { assert.ok(listedWorktreePaths(repoDir).has(wantedKey), 'worktree should appear in list before deletion'); // Manually delete the worktree directory (simulate orphan) + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test fault injection: simulates an orphaned worktree dir that git still references fs.rmSync(worktreeDir, { recursive: true, force: true }); // Act diff --git a/tests/quick-branching.test.cjs b/tests/quick-branching.test.cjs index d0ac0dacb..4f366f69a 100644 --- a/tests/quick-branching.test.cjs +++ b/tests/quick-branching.test.cjs @@ -19,6 +19,7 @@ const { execFileSync } = require('node:child_process'); const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); +const { cleanup } = require('./helpers.cjs'); const QUICK_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'quick.md'); @@ -149,7 +150,7 @@ function runStep(bash, cwd, branchName) { stdio: ['pipe', 'pipe', 'pipe'], }).toString(); } finally { - fs.rmSync(scriptDir, { recursive: true, force: true }); + cleanup(scriptDir); } } @@ -256,7 +257,7 @@ describe('quick workflow: branching support', () => { `new quick-task branch tip must equal ${upstream} tip` ); } finally { - fs.rmSync(root, { recursive: true, force: true }); + cleanup(root); } }); } @@ -287,7 +288,7 @@ describe('quick workflow: branching support', () => { 'existing-branch tip must be preserved (no rebase/reset)' ); } finally { - fs.rmSync(root, { recursive: true, force: true }); + cleanup(root); } }); }); diff --git a/tests/qwen-skills-migration.test.cjs b/tests/qwen-skills-migration.test.cjs index 31e13a485..6e61732e0 100644 --- a/tests/qwen-skills-migration.test.cjs +++ b/tests/qwen-skills-migration.test.cjs @@ -29,6 +29,8 @@ const { resolveProfile, } = require('../get-shit-done/bin/lib/install-profiles.cjs'); +const { cleanup } = require('./helpers.cjs'); + const manifest = loadSkillsManifest(); const resolvedProfileFull = resolveProfile({ modes: [], manifest }); @@ -136,9 +138,7 @@ describe('Qwen Code: installRuntimeArtifacts', () => { }); afterEach(() => { - if (fs.existsSync(tmpDir)) { - fs.rmSync(tmpDir, { recursive: true }); - } + cleanup(tmpDir); }); test('creates skills/gsd-xxx/SKILL.md directory structure', () => { diff --git a/tests/reapply-patches.test.cjs b/tests/reapply-patches.test.cjs index a494301bc..f896e19eb 100644 --- a/tests/reapply-patches.test.cjs +++ b/tests/reapply-patches.test.cjs @@ -19,14 +19,12 @@ function sha256(content) { return crypto.createHash('sha256').update(content).digest('hex'); } +const { cleanup } = require('./helpers.cjs'); + function createTempDir() { return fs.mkdtempSync(path.join(require('os').tmpdir(), 'gsd-patch-test-')); } -function cleanup(dir) { - try { fs.rmSync(dir, { recursive: true, force: true }); } catch {} -} - /** * Simulate what the installer does: create a manifest, modify a file, * then run the saveLocalPatches detection logic. diff --git a/tests/runtime-artifact-layout-install-profiles.test.cjs b/tests/runtime-artifact-layout-install-profiles.test.cjs index 42970b686..02b631763 100644 --- a/tests/runtime-artifact-layout-install-profiles.test.cjs +++ b/tests/runtime-artifact-layout-install-profiles.test.cjs @@ -468,7 +468,7 @@ describe('loadSkillsManifest', () => { const m = loadSkillsManifest(dir); assert.ok(m instanceof Map, 'should return a Map'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -480,7 +480,7 @@ describe('loadSkillsManifest', () => { assert.ok(m.has('help'), 'help should be in manifest'); assert.deepStrictEqual(m.get('help'), []); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -492,7 +492,7 @@ describe('loadSkillsManifest', () => { assert.ok(m.has('add-tests')); assert.deepStrictEqual(m.get('add-tests'), ['phase']); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -503,7 +503,7 @@ describe('loadSkillsManifest', () => { const m = loadSkillsManifest(dir); assert.deepStrictEqual(m.get('plan-phase'), ['discuss-phase', 'phase', 'review', 'update']); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -518,7 +518,7 @@ describe('loadSkillsManifest', () => { assert.ok(!m.has('README')); assert.ok(!m.has('notes')); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -528,7 +528,7 @@ describe('loadSkillsManifest', () => { const m = loadSkillsManifest(dir); assert.strictEqual(m.size, 0); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -539,7 +539,7 @@ describe('loadSkillsManifest', () => { const m = loadSkillsManifest(dir); assert.deepStrictEqual(m.get('explore'), []); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -564,7 +564,7 @@ describe('readActiveProfile / writeActiveProfile', () => { writeActiveProfile(dir, 'standard'); assert.strictEqual(readActiveProfile(dir), 'standard'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -574,7 +574,7 @@ describe('readActiveProfile / writeActiveProfile', () => { writeActiveProfile(dir, 'core'); assert.strictEqual(readActiveProfile(dir), 'core'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -584,7 +584,7 @@ describe('readActiveProfile / writeActiveProfile', () => { writeActiveProfile(dir, 'core,audit'); assert.strictEqual(readActiveProfile(dir), 'core,audit'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -594,7 +594,7 @@ describe('readActiveProfile / writeActiveProfile', () => { writeActiveProfile(dir, 'full'); assert.strictEqual(readActiveProfile(dir), 'full'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -604,7 +604,7 @@ describe('readActiveProfile / writeActiveProfile', () => { const result = readActiveProfile(dir); assert.strictEqual(result, null); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -621,7 +621,7 @@ describe('readActiveProfile / writeActiveProfile', () => { const result = readActiveProfile(dir); assert.strictEqual(result, null); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -632,7 +632,7 @@ describe('readActiveProfile / writeActiveProfile', () => { const result = readActiveProfile(dir); assert.strictEqual(result, null); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -644,7 +644,7 @@ describe('readActiveProfile / writeActiveProfile', () => { assert.ok(fs.existsSync(nested), 'directory should be created'); assert.strictEqual(readActiveProfile(nested), 'standard'); } finally { - fs.rmSync(base, { recursive: true, force: true }); + cleanup(base); } }); @@ -655,7 +655,7 @@ describe('readActiveProfile / writeActiveProfile', () => { writeActiveProfile(dir, 'full'); assert.strictEqual(readActiveProfile(dir), 'full'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); diff --git a/tests/runtime-artifact-layout-surface.test.cjs b/tests/runtime-artifact-layout-surface.test.cjs index 2e8318283..211c590d1 100644 --- a/tests/runtime-artifact-layout-surface.test.cjs +++ b/tests/runtime-artifact-layout-surface.test.cjs @@ -323,7 +323,7 @@ describe('resolveSurface', () => { 'surface with no state should equal profile resolution' ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -351,7 +351,7 @@ describe('resolveSurface', () => { } } } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -376,7 +376,7 @@ describe('resolveSurface', () => { assert.ok(resolved.skills.has(dep), `transitive dep "${dep}" of sketch must be present`); } } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -395,7 +395,7 @@ describe('resolveSurface', () => { assert.ok(!resolved.skills.has('progress'), '"progress" must be removed by explicitRemoves'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -410,7 +410,7 @@ describe('resolveSurface', () => { assert.ok(typeof resolved.name === 'string'); assert.ok(resolved.agents instanceof Set); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -434,7 +434,7 @@ describe('resolveSurface', () => { 'surface baseProfile takes precedence over marker' ); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -463,7 +463,7 @@ describe('resolveSurface', () => { } } } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); @@ -484,7 +484,7 @@ describe('readSurface / writeSurface', () => { const read = readSurface(dir); assert.deepStrictEqual(read, state); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -500,7 +500,7 @@ describe('readSurface / writeSurface', () => { writeSurface(dir, state); assert.deepStrictEqual(readSurface(dir), state); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -516,7 +516,7 @@ describe('readSurface / writeSurface', () => { writeSurface(dir, state); assert.deepStrictEqual(readSurface(dir), state); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -526,7 +526,7 @@ describe('readSurface / writeSurface', () => { const result = readSurface(dir); assert.strictEqual(result, null); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -543,7 +543,7 @@ describe('readSurface / writeSurface', () => { const result = readSurface(dir); assert.strictEqual(result, null); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -558,7 +558,7 @@ describe('readSurface / writeSurface', () => { const result = readSurface(dir); assert.strictEqual(result, null); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -573,7 +573,7 @@ describe('readSurface / writeSurface', () => { const result = readSurface(dir); assert.strictEqual(result, null); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -587,7 +587,7 @@ describe('readSurface / writeSurface', () => { assert.deepStrictEqual(tmpFiles, [], 'no tmp files should remain after write'); assert.ok(files.includes('.gsd-surface.json')); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -600,7 +600,7 @@ describe('readSurface / writeSurface', () => { assert.strictEqual(read.baseProfile, 'standard'); assert.deepStrictEqual(read.disabledClusters, ['utility']); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -612,7 +612,7 @@ describe('readSurface / writeSurface', () => { assert.ok(fs.existsSync(nested)); assert.ok(readSurface(nested) !== null); } finally { - fs.rmSync(base, { recursive: true, force: true }); + cleanup(base); } }); }); @@ -720,7 +720,7 @@ describe('listSurface', () => { assert.ok(Array.isArray(result.disabled), 'disabled must be array'); assert.ok(typeof result.tokenCost === 'number', 'tokenCost must be number'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -737,7 +737,7 @@ describe('listSurface', () => { assert.ok(typeof result.tokenCost === 'number', 'tokenCost must be number'); assert.ok(result.tokenCost >= 0, 'tokenCost must be non-negative'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -758,7 +758,7 @@ describe('listSurface', () => { assert.ok(coreList.enabled.length + coreList.disabled.length === totalStems, 'enabled + disabled must equal total stems'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -784,7 +784,7 @@ describe('listSurface', () => { assert.ok(afterList.tokenCost <= beforeList.tokenCost, 'disabling a cluster should not increase token cost'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -807,7 +807,7 @@ describe('listSurface', () => { assert.strictEqual(result.tokenCost, expected, 'tokenCost must equal sum of description lengths ÷ 4'); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); @@ -822,7 +822,7 @@ describe('listSurface', () => { assert.deepStrictEqual(result.enabled, [...result.enabled].sort()); assert.deepStrictEqual(result.disabled, [...result.disabled].sort()); } finally { - fs.rmSync(dir, { recursive: true, force: true }); + cleanup(dir); } }); }); diff --git a/tests/runtime-launcher-parity.test.cjs b/tests/runtime-launcher-parity.test.cjs index 717a23fb2..ae6b1b0cf 100644 --- a/tests/runtime-launcher-parity.test.cjs +++ b/tests/runtime-launcher-parity.test.cjs @@ -27,6 +27,7 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); const { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'get-shit-done', 'workflows'); const SNIPPET_FILE = path.join(WORKFLOWS_DIR, '_runtime-launcher.snippet.sh'); @@ -231,7 +232,7 @@ describe('runtime-launcher-parity (#373)', () => { `Expected stdout to contain "STUB:query,state.json" but got: ${stdout.trim()}`, ); } finally { - fs.rmSync(base, { recursive: true, force: true }); + cleanup(base); } }); @@ -285,7 +286,7 @@ describe('runtime-launcher-parity (#373)', () => { `Expected stderr to contain "not found" or "ERROR", got: ${stderrOutput.trim()}`, ); } finally { - fs.rmSync(base, { recursive: true, force: true }); + cleanup(base); } }); @@ -343,7 +344,7 @@ describe('runtime-launcher-parity (#373)', () => { `Expected stdout to contain "installed:query state.json" (PATH stub output), got: ${stdout.trim()}`, ); } finally { - fs.rmSync(base, { recursive: true, force: true }); + cleanup(base); } }); diff --git a/tests/secret-scan-lint.test.cjs b/tests/secret-scan-lint.test.cjs index 79508a59a..5b258dd31 100644 --- a/tests/secret-scan-lint.test.cjs +++ b/tests/secret-scan-lint.test.cjs @@ -38,6 +38,7 @@ const { execFileSync, spawnSync } = require('child_process'); const fs = require('fs'); const os = require('os'); const path = require('path'); +const { cleanup } = require('./helpers.cjs'); const PROJECT_ROOT = path.join(__dirname, '..'); const LINT_SCRIPT = path.join(PROJECT_ROOT, 'scripts', 'secret-scan-lint.sh'); @@ -69,7 +70,7 @@ function runLint(ignoreContent, extraArgs = []) { stderr: result.stderr || '', }; } finally { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 20, retryDelay: 250 }); + cleanup(tmpDir); } } @@ -94,7 +95,7 @@ function runSecretScan(fileContent, extraArgs = []) { stderr: result.stderr || '', }; } finally { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 20, retryDelay: 250 }); + cleanup(tmpDir); } } @@ -364,7 +365,7 @@ describe('secret-scan.sh --strict: reduces effective exclusions', { skip: IS_WIN // A clean file with no secrets must exit 0 under --strict assert.equal(status, 0, `--strict on a clean file should exit 0, got ${status}.\nstdout: ${result.stdout}\nstderr: ${result.stderr}`); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); @@ -422,7 +423,7 @@ describe('secret-scan.sh --strict: reduces effective exclusions', { skip: IS_WIN `Strict mode should scan grandfathered file and find secrets (exit 1), got ${strictStatus}.\nstdout: ${strictResult.stdout}\nstderr: ${strictResult.stderr}` ); } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); + cleanup(tmpDir); } }); }); diff --git a/tests/security-prompt-injection.test.cjs b/tests/security-prompt-injection.test.cjs index 1211b4e85..a17d4eebb 100644 --- a/tests/security-prompt-injection.test.cjs +++ b/tests/security-prompt-injection.test.cjs @@ -643,7 +643,7 @@ describe('validatePath: hostile path values are rejected before write', () => { assert.strictEqual(r.safe, false, 'a symlink whose target is outside the base must fail containment'); // Cleanup the outside dir; the link itself is cleaned by cleanup(tmpDir). - fs.rmSync(outside, { recursive: true, force: true, maxRetries: 20, retryDelay: 250 }); + cleanup(outside); }); }); diff --git a/tests/security-scan.test.cjs b/tests/security-scan.test.cjs index 3d66b1689..7f4bad691 100644 --- a/tests/security-scan.test.cjs +++ b/tests/security-scan.test.cjs @@ -39,6 +39,8 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); +const { cleanup } = require('./helpers.cjs'); + const PROJECT_ROOT = path.join(__dirname, '..'); const SCRIPTS = { injection: path.join(PROJECT_ROOT, 'scripts', 'prompt-injection-scan.sh'), @@ -69,7 +71,7 @@ function runScript(scriptPath, content, extraArgs) { stderr: err.stderr || '', }; } finally { - fs.rmSync(tmpDir, { recursive: true, force: true, maxRetries: 20, retryDelay: 250 }); + cleanup(tmpDir); } } diff --git a/tests/temp-subdir.test.cjs b/tests/temp-subdir.test.cjs index 346711c42..97b29d343 100644 --- a/tests/temp-subdir.test.cjs +++ b/tests/temp-subdir.test.cjs @@ -79,6 +79,7 @@ describe('dedicated gsd temp subdirectory', () => { // Verify it does not exist if (fs.existsSync(uniqueSubdir)) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test pre-condition reset: ensures uniqueSubdir is absent before testing SUT creation behavior fs.rmSync(uniqueSubdir, { recursive: true, force: true }); } assert.ok(!fs.existsSync(uniqueSubdir), 'test subdir should not exist before test'); diff --git a/tests/verify-health.test.cjs b/tests/verify-health.test.cjs index 6589ef184..224275966 100644 --- a/tests/verify-health.test.cjs +++ b/tests/verify-health.test.cjs @@ -74,6 +74,7 @@ describe('validate health command', () => { test("returns 'broken' when .planning directory is missing", () => { // createTempProject creates .planning/phases — remove it entirely + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test SUT setup: removes .planning/ to simulate missing dir condition fs.rmSync(path.join(tmpDir, '.planning'), { recursive: true, force: true }); const result = runGsdTools('validate health', tmpDir); @@ -1008,6 +1009,7 @@ describe('validate health — missing phasesDir', () => { // Remove the phases directory if it exists const phasesDir = path.join(tmpDir, '.planning', 'phases'); if (fs.existsSync(phasesDir)) { + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test SUT setup: removes phases/ to simulate missing phasesDir condition fs.rmSync(phasesDir, { recursive: true, force: true }); } diff --git a/tests/windows-test-parity-guard.test.cjs b/tests/windows-test-parity-guard.test.cjs index 6c562911d..ed1a8922a 100644 --- a/tests/windows-test-parity-guard.test.cjs +++ b/tests/windows-test-parity-guard.test.cjs @@ -3,47 +3,80 @@ process.env.GSD_TEST_MODE = '1'; /** - * Ratchet-style lint guard against Windows-test-parity regressions. + * Named-set allowlist guard against Windows-test-parity regressions. * * PR #3649 cleared ~270 Windows-only test failures from the chunking fix * in #3597 surfaced. Each cluster reduced to a handful of repeating * patterns. This guard prevents the patterns from being re-introduced. * - * Strategy: per-pattern offender list is snapshotted at the count present - * at the time of PR #3649. The test fails if a NEW file is added that - * matches the anti-pattern (count grows above the baseline). Existing - * offenders are acknowledged as technical debt that can be cleared - * incrementally without blocking this PR. + * Strategy (updated from integer-count ratchet): each rule's known offenders + * are enumerated by filename in a frozen KNOWN_OFFENDERS set. The guard uses + * the shared assertWithinAllowlist primitive (scripts/lib/allowlist-ratchet.cjs) + * which enforces BOTH directions: + * - Novel offenders (current \ known) → fail immediately. + * - Stale allowlist entries (known \ current) → also fail, forcing the + * allowlist to shrink as defects are fixed (ratchet-DOWN enforcement). * - * When you fix an existing offender, lower the corresponding BASELINE - * count by 1. When CI breaks because BASELINE is set higher than the - * actual offender count, lower BASELINE to match (one-way ratchet down). + * When you fix an existing offender, you MUST remove its entry from + * KNOWN_OFFENDERS — the guard will fail on stale entries to enforce progress. + * When CI breaks because a new file introduced an anti-pattern, fix the + * anti-pattern — do not just add the filename to the set to silence the guard. + * + * rmSync teardown safety is now enforced at write-time by the ESLint rule + * local/no-raw-rmsync-in-tests (see issue #597); it is no longer ratcheted here. * * Scope: tests/ only. Production-code Windows-compat is enforced via * behavioural tests (see no-unconditional-win32-skip.test.cjs). */ +// allow-test-rule: structural-regression-guard + const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); +const { assertWithinAllowlist } = require('../scripts/lib/allowlist-ratchet.cjs'); const TESTS_DIR = path.join(__dirname); const SELF = path.basename(__filename); -// ── Baseline counts after PR #3649 batch ───────────────────────────────── -// Set these to the exact number of offending files at the time of merge. -// Each rule must not exceed its baseline; CI fails when a new offender appears. -// Decrement when an existing offender is fixed. -const BASELINE = { - splitNewlineOnFileContent: 3, - fenceRegexLiteralNewline: 2, - frontmatterAnchorLiteralNewline: 5, - hardcodedTmpToFsCall: 0, - bareNpmExecWithoutShell: 0, - stubsHomeNoUserProfile: 8, - rmSyncNoMaxRetries: 95, -}; +// ── Known offenders after PR #3649 batch (named-set allowlist) ─────────── +// These are the files that matched each anti-pattern at the time of writing. +// A test fails when a file NOT in the set starts matching (novel regression), +// OR when a file in the set stops matching (stale entry — must be pruned). +// Edit this object to update the allowlists: +// edit KNOWN_OFFENDERS in tests/windows-test-parity-guard.test.cjs +const KNOWN_OFFENDERS = Object.freeze({ + splitNewlineOnFileContent: new Set([ + 'release-coverage-scope.test.cjs', + 'secret-scan-lint.test.cjs', + 'security-scan.test.cjs', + ]), + fenceRegexLiteralNewline: new Set([ + 'bug-2995-post-install-script-paths.test.cjs', + 'security-scan.test.cjs', + ]), + frontmatterAnchorLiteralNewline: new Set([ + 'bug-1967-cache-invalidation.test.cjs', + 'bug-2643-skill-frontmatter-name.test.cjs', + 'bug-2808-skill-hyphen-name.test.cjs', + 'bug-3168-task-to-agent-rename.test.cjs', + 'qwen-skills-migration.test.cjs', + ]), + hardcodedTmpToFsCall: new Set([ + // (none at time of writing) + ]), + bareNpmExecWithoutShell: new Set([ + // (none at time of writing) + ]), + stubsHomeNoUserProfile: new Set([ + 'bug-130-finishinstall-opencode-testmode.test.cjs', + 'bug-2794-opencode-model-profile-overrides.test.cjs', + 'claude-md.test.cjs', + 'feat-443-effort-install-wiring.install.test.cjs', + 'issue-2517-runtime-aware-profiles.test.cjs', + ]), +}); function listTestFiles() { return fs.readdirSync(TESTS_DIR) @@ -77,107 +110,94 @@ function countMatchingFiles(predicate) { return { count, offenders }; } -function ratchetAssert(rule, actualCount, baselineCount, offenders, guidance) { - if (actualCount > baselineCount) { - const newCount = actualCount - baselineCount; - assert.fail( - `Windows-parity guard "${rule}": ${actualCount} offenders, baseline is ${baselineCount} ` + - `(+${newCount} new). New occurrences of this anti-pattern were added. ` + - `${guidance}\n\nFull offender list (${actualCount}):\n ` + - offenders.join('\n '), - ); - } -} +const PRUNE_HINT = 'edit KNOWN_OFFENDERS in tests/windows-test-parity-guard.test.cjs'; -describe('Windows test-parity lint guards (ratchet baseline: PR #3649)', () => { +describe('Windows test-parity lint guards (named-set allowlist: PR #3649)', () => { // ── G1 — CRLF: file-content split on literal '\n' ───────────────────── test('split-on-newline after readFileSync (use /\\r?\\n/)', () => { - const { count, offenders } = countMatchingFiles((text) => { + const { offenders } = countMatchingFiles((text) => { return /\.readFileSync\s*\([^)]*\)[^;]*\.split\(\s*['"]\\n['"]\s*\)/.test(text); }); - ratchetAssert( - 'splitNewlineOnFileContent', count, BASELINE.splitNewlineOnFileContent, offenders, - "Replace .split('\\n') with .split(/\\r?\\n/) so the test tolerates CRLF " + - "checkout (autocrlf=true on Windows leaves trailing \\r on every line).", - ); + assertWithinAllowlist({ + label: 'splitNewlineOnFileContent', + current: offenders, + known: KNOWN_OFFENDERS.splitNewlineOnFileContent, + fail: assert.fail, + pruneHint: PRUNE_HINT, + }); }); // ── G2 — CRLF: ```bash|sh\n fence regex on file content ────────────── test('markdown-fence regex with literal \\n after ```bash/sh', () => { - const { count, offenders } = countMatchingFiles((text) => { + const { offenders } = countMatchingFiles((text) => { return /\/[^/]*```(?:bash|sh)\\n[^/]*\//.test(text); }); - ratchetAssert( - 'fenceRegexLiteralNewline', count, BASELINE.fenceRegexLiteralNewline, offenders, - "Use /```(?:bash|sh)\\r?\\n([\\s\\S]*?)```/g — Windows CRLF makes the byte after " + - "`bash` be \\r, the regex never matches, and bash-block extraction returns empty.", - ); + assertWithinAllowlist({ + label: 'fenceRegexLiteralNewline', + current: offenders, + known: KNOWN_OFFENDERS.fenceRegexLiteralNewline, + fail: assert.fail, + pruneHint: PRUNE_HINT, + }); }); // ── G3 — CRLF: frontmatter regex with literal '\n' ──────────────────── test('frontmatter regex anchors on /^---\\n/', () => { - const { count, offenders } = countMatchingFiles((text) => { + const { offenders } = countMatchingFiles((text) => { return /\/\^---\\n/.test(text); }); - ratchetAssert( - 'frontmatterAnchorLiteralNewline', count, BASELINE.frontmatterAnchorLiteralNewline, offenders, - "Use /^---\\r?\\n/ — on Windows the byte after --- is \\r, not \\n, so the " + - "anchor fails to match and parseFrontmatter returns null/{}.", - ); + assertWithinAllowlist({ + label: 'frontmatterAnchorLiteralNewline', + current: offenders, + known: KNOWN_OFFENDERS.frontmatterAnchorLiteralNewline, + fail: assert.fail, + pruneHint: PRUNE_HINT, + }); }); // ── G4 — POSIX-tmp: hardcoded '/tmp/' literal passed to fs.* ───────── test('fs.* call receives a hardcoded "/tmp/..." literal', () => { - const { count, offenders } = countMatchingFiles((text) => { + const { offenders } = countMatchingFiles((text) => { return /\bfs\.[A-Za-z]+\s*\([^)]*['"]\/tmp\/[^'"]+['"][^)]*\)/.test(text); }); - ratchetAssert( - 'hardcodedTmpToFsCall', count, BASELINE.hardcodedTmpToFsCall, offenders, - "Use os.tmpdir() — on Windows '/tmp/foo' becomes 'D:\\tmp\\foo' where D:\\tmp " + - "doesn't exist by default → ENOENT.", - ); + assertWithinAllowlist({ + label: 'hardcodedTmpToFsCall', + current: offenders, + known: KNOWN_OFFENDERS.hardcodedTmpToFsCall, + fail: assert.fail, + pruneHint: PRUNE_HINT, + }); }); // ── G5 — npm.cmd: bare 'npm' to exec*Sync without shell:true ───────── test('bare npm exec without shell-true Windows fallback', () => { - const { count, offenders } = countMatchingFiles((text) => { + const { offenders } = countMatchingFiles((text) => { const re = /\b(?:execFileSync|spawnSync)\s*\(\s*['"]npm['"]\s*,[^)]*\)/g; const matches = text.match(re) || []; return matches.some((m) => !/shell\s*:\s*true/.test(m) && !/shell\s*:\s*isWindows/.test(m), ); }); - ratchetAssert( - 'bareNpmExecWithoutShell', count, BASELINE.bareNpmExecWithoutShell, offenders, - "On Windows npm is npm.cmd — pass {shell: process.platform === 'win32'} or " + - "use npm.cmd directly, otherwise execFileSync errors ENOENT.", - ); + assertWithinAllowlist({ + label: 'bareNpmExecWithoutShell', + current: offenders, + known: KNOWN_OFFENDERS.bareNpmExecWithoutShell, + fail: assert.fail, + pruneHint: PRUNE_HINT, + }); }); // ── G6 — Test stubs HOME without USERPROFILE ───────────────────────── test('test stubs process.env.HOME but never references USERPROFILE', () => { - const { count, offenders } = countMatchingFiles((text) => { + const { offenders } = countMatchingFiles((text) => { return /process\.env\.HOME\s*=\s*/.test(text) && !/USERPROFILE/.test(text); }); - ratchetAssert( - 'stubsHomeNoUserProfile', count, BASELINE.stubsHomeNoUserProfile, offenders, - "On Windows os.homedir() reads USERPROFILE (not HOME). Tests redirecting ~ " + - "must override both, or the SUT sees the real user's home.", - ); - }); - - // ── G7 — rmSync cleanup without retry budget ───────────────────────── - test('test teardown rmSync without maxRetries', () => { - const { count, offenders } = countMatchingFiles((text) => { - const re = /fs\.rmSync\s*\([^)]*recursive\s*:\s*true[^)]*force\s*:\s*true[^)]*\)/g; - const matches = text.match(re) || []; - return matches.some((m) => !/maxRetries/.test(m)); + assertWithinAllowlist({ + label: 'stubsHomeNoUserProfile', + current: offenders, + known: KNOWN_OFFENDERS.stubsHomeNoUserProfile, + fail: assert.fail, + pruneHint: PRUNE_HINT, }); - ratchetAssert( - 'rmSyncNoMaxRetries', count, BASELINE.rmSyncNoMaxRetries, offenders, - "Use helpers.cleanup() (shared 5s retry budget) or pass " + - "{maxRetries: 10, retryDelay: 100} — Windows AV scanners can hold handles for " + - "seconds after a process exits, surfacing as flaky EBUSY teardown failures.", - ); }); }); diff --git a/tests/workflow-size-budget.test.cjs b/tests/workflow-size-budget.test.cjs index bea11a3d9..0249a94a0 100644 --- a/tests/workflow-size-budget.test.cjs +++ b/tests/workflow-size-budget.test.cjs @@ -21,6 +21,10 @@ * (see `workflows/discuss-phase/modes/` for the progressive-disclosure * pattern introduced by #2551). * + * Tighten-only invariant (issue #597): ceilings track the tier high-water mark + * within GRACE lines. Budgets may only decrease, never silently creep upward. + * The assertTightCeiling() call below enforces this automatically. + * * See: * - https://github.com/open-gsd/gsd-core/issues/2551 (this test) * - https://github.com/open-gsd/gsd-core/issues/2361 (agent budget) @@ -30,18 +34,27 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const { assertTightCeiling } = require('../scripts/lib/allowlist-ratchet.cjs'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'get-shit-done', 'workflows'); +// Grace band: maximum allowed slack (ceiling − actualMax) before a ceiling is +// considered too loose. 60 lines gives one reasonable screen of breathing room +// without permitting gross inflation. +const GRACE = 60; + // Bumped from 1700 → 1800 in #3181 to absorb MVP-mode verb-call additions // in execute-phase.md (1727 → ) and plan-phase.md (1714 → ) from #3178. // Follow-up #3182 (TBD): extract MVP-mode bodies to `/modes/mvp.md` // per the discuss-phase/modes/ precedent and revert this back to 1700. // Bumped from 1800 → 1810 in #3707 to absorb the startup orphan-sweep // block added to execute-phase.md (+2 lines: one comment + one bash command). +// XL ceiling kept at 1810 (actualMax=1810, plan-phase; slack=0 ≤ GRACE=60). const XL_BUDGET = 1810; -const LARGE_BUDGET = 1500; -const DEFAULT_BUDGET = 1000; +// LARGE ceiling lowered from 1500 → 1236 (actualMax=1176, docs-update; #597 ratchet-down). +const LARGE_BUDGET = 1236; +// DEFAULT ceiling lowered from 1000 → 870 (actualMax=810, settings-advanced; #597 ratchet-down). +const DEFAULT_BUDGET = 870; // Top-level orchestrators that own end-to-end multi-phase rubrics. // Grandfathered at current sizes — see PR #2551 for #2551 progressive-disclosure @@ -101,6 +114,35 @@ describe('SIZE: workflow line-count budget', () => { } }); +describe('SIZE: tier anti-creep (tighten-only ceilings, issue #597)', () => { + // For each tier, compute the high-water mark across all files in that tier + // and assert the ceiling stays tight. Prevents budgets from silently drifting + // upward: ceiling − actualMax must not exceed GRACE. + test('XL tier: ceiling tracks high-water mark within GRACE', () => { + const values = ALL_WORKFLOWS + .filter(w => XL_WORKFLOWS.has(w)) + .map(w => lineCount(path.join(WORKFLOWS_DIR, w + '.md'))); + const actualMax = Math.max(...values); + assertTightCeiling({ label: 'XL', actualMax, ceiling: XL_BUDGET, grace: GRACE, fail: assert.fail }); + }); + + test('LARGE tier: ceiling tracks high-water mark within GRACE', () => { + const values = ALL_WORKFLOWS + .filter(w => LARGE_WORKFLOWS.has(w)) + .map(w => lineCount(path.join(WORKFLOWS_DIR, w + '.md'))); + const actualMax = Math.max(...values); + assertTightCeiling({ label: 'LARGE', actualMax, ceiling: LARGE_BUDGET, grace: GRACE, fail: assert.fail }); + }); + + test('DEFAULT tier: ceiling tracks high-water mark within GRACE', () => { + const values = ALL_WORKFLOWS + .filter(w => !XL_WORKFLOWS.has(w) && !LARGE_WORKFLOWS.has(w)) + .map(w => lineCount(path.join(WORKFLOWS_DIR, w + '.md'))); + const actualMax = Math.max(...values); + assertTightCeiling({ label: 'DEFAULT', actualMax, ceiling: DEFAULT_BUDGET, grace: GRACE, fail: assert.fail }); + }); +}); + describe('SIZE: discuss-phase progressive disclosure (issue #2551)', () => { // Issue #2551 explicitly targets discuss-phase.md at <500 lines, separate from // the per-tier grandfathered budgets above. This is the headline metric of the diff --git a/tests/workstream.test.cjs b/tests/workstream.test.cjs index 6738b445f..856296ffc 100644 --- a/tests/workstream.test.cjs +++ b/tests/workstream.test.cjs @@ -291,6 +291,7 @@ describe('pointer lifecycle hardening', () => { runGsdTools(['workstream', 'set', 'alpha', '--raw'], tmpDir, { GSD_SESSION_KEY: 'session-alpha' }); runGsdTools(['workstream', 'set', 'beta', '--raw'], tmpDir, { GSD_SESSION_KEY: 'session-beta' }); + // eslint-disable-next-line local/no-raw-rmsync-in-tests -- mid-test fault injection: simulates a deleted workstream to exercise stale-pointer self-cleanup fs.rmSync(path.join(tmpDir, '.planning', 'workstreams', 'alpha'), { recursive: true, force: true }); const alpha = runGsdTools(['workstream', 'get'], tmpDir, { GSD_SESSION_KEY: 'session-alpha' });