From 874214d96daf2f722ec28bc23858911f8c0dec1f Mon Sep 17 00:00:00 2001 From: brady gaster Date: Fri, 21 Aug 2026 01:44:28 -0700 Subject: [PATCH] fix(test): stop init-scaffolding from mutating tracked repo files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test/init-scaffolding.test.ts created its sandbox at join(process.cwd(), '.test-init-scaffold-') — inside the repository working tree. Several of its cases deliberately run initSquad()/runInit() against a sandbox that has no .git of its own, so init walks upward to find the enclosing git root, finds the *real* repository, and stampVersion() rewrites the tracked .github/agents/squad.agent.md from 0.0.0-source to the package version. The suite passes while doing it. Controlled proof on a clean tree, that file alone: 24 tests passed, then `git status --short` reports `M .github/agents/squad.agent.md`. Any agent running a broad `git add` after a test run commits a spurious version bump. This was known and worked around rather than fixed: test/template-sync.test.ts carries two beforeAll re-syncs whose comments describe it as a race condition to be "minimised to near-zero". The repair ran every time, which is exactly why the damage looked intermittent and environmental. Move the sandbox to the OS temp dir so upward git-root resolution can no longer reach the repository, and add a two-part guard that goes red against the pre-fix state: AssertionError: expected 'M .github/agents/squad.agent.md' to be '' AssertionError: TEST_ROOT (...\.test-init-scaffold-99246f71) must not live under ...\bradygaster-fictional-waddle: expected true to be false The guard compares porcelain status against a file-level beforeAll snapshot rather than asserting emptiness, so a pre-existing local edit to those paths does not produce a false red. The template-sync re-syncs are retained as defence-in-depth — a future suite that reintroduces an in-repo sandbox now fails loudly there instead of leaving a silently mutated tree — and their comments updated to say so. Post-fix: 26 passed, tree clean; 327 passed across the four init/template suites, tree clean. Not addressed here: `npm test` also rewrites test/__snapshots__/parser-contracts.test.ts.snap. That diff is pure EOL with zero content lines (file is i/lf w/crlf; vitest writes LF) — the #1793 CRLF class. Its fix is `*.snap text eol=lf`, which belongs with #1790 so it rides the same renormalize; the attribute alone repairs no existing checkout. Closes #1796 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70370e36-33b0-4786-bd72-4cf15518daa6 --- test/init-scaffolding.test.ts | 56 +++++++++++++++++++++++++++++++++-- test/template-sync.test.ts | 24 ++++++++------- 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/test/init-scaffolding.test.ts b/test/init-scaffolding.test.ts index d39ce62ab..483da122d 100644 --- a/test/init-scaffolding.test.ts +++ b/test/init-scaffolding.test.ts @@ -6,10 +6,11 @@ * Also confirms init works without errors in repos that have no remote. */ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; import { mkdir, rm, readFile } from 'fs/promises'; -import { join } from 'path'; +import { join, resolve, relative, isAbsolute } from 'path'; import { existsSync } from 'fs'; +import { tmpdir } from 'os'; import { randomBytes } from 'crypto'; import { execFileSync } from 'child_process'; import { initSquad } from '@bradygaster/squad-sdk'; @@ -18,7 +19,35 @@ import { runInit } from '@bradygaster/squad-cli/core/init'; import { runDoctor } from '@bradygaster/squad-cli/commands/doctor'; import type { DoctorCheck } from '@bradygaster/squad-cli/commands/doctor'; -const TEST_ROOT = join(process.cwd(), `.test-init-scaffold-${randomBytes(4).toString('hex')}`); +// Sandbox lives in the OS temp dir, NOT inside the repository (#1796). +// runInit()/initSquad() walk upward to find the enclosing git root when the +// target directory is not itself a repo. A sandbox under process.cwd() therefore +// resolves to the *real* repository and stampVersion() rewrites the tracked +// .github/agents/squad.agent.md — a passing test silently mutating source. +const TEST_ROOT = join(tmpdir(), `.test-init-scaffold-${randomBytes(4).toString('hex')}`); + +/** Repository root — used only by the working-tree guard below. */ +const REPO_ROOT = resolve(__dirname, '..'); + +/** Files this suite must never touch. Relative to REPO_ROOT. */ +const GUARDED_PATHS = ['.github/agents/squad.agent.md', '.squad-templates/squad.agent.md']; + +/** Porcelain status of the guarded paths, so pre-existing local edits don't skew the guard. */ +function guardedStatus(): string { + return execFileSync('git', ['status', '--porcelain', '--', ...GUARDED_PATHS], { + cwd: REPO_ROOT, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); +} + +/** Captured before any test in this file runs. */ +let guardedStatusBefore = ''; + +beforeAll(() => { + guardedStatusBefore = guardedStatus(); +}); + /** Create a bare git repo at the given path (no remote). */ function gitInit(dir: string): void { @@ -498,3 +527,24 @@ describe('.gitignore state-backend marker block — initSquad()', () => { expect(occurrences).toBe(1); }); }); + +// ─── Working-tree containment guard (#1796) ──────────────────────────────── + +describe('repository working tree is not mutated by this suite (#1796)', () => { + it('sandbox root is outside the repository working tree', () => { + const rel = relative(REPO_ROOT, TEST_ROOT); + // A path inside REPO_ROOT produces a relative path that neither escapes + // upward nor is absolute. Anything else means the sandbox is contained. + const isInsideRepo = rel !== '' && !rel.startsWith('..') && !isAbsolute(rel); + expect(isInsideRepo, `TEST_ROOT (${TEST_ROOT}) must not live under ${REPO_ROOT}`).toBe(false); + }); + + it('tracked squad.agent.md files are byte-unchanged after init runs', () => { + // Pre-fix this is red: initSquad()/runInit() on a sandbox that has no git + // root of its own walk upward, find the real repository, and stampVersion() + // rewrites .github/agents/squad.agent.md from 0.0.0-source to the package + // version. The suite still passes — the mutation is silent. + expect(guardedStatus()).toBe(guardedStatusBefore); + }); +}); + diff --git a/test/template-sync.test.ts b/test/template-sync.test.ts index 911474b69..c7cf8af7b 100644 --- a/test/template-sync.test.ts +++ b/test/template-sync.test.ts @@ -23,9 +23,14 @@ const __dirname = dirname(fileURLToPath(import.meta.url)); const ROOT = resolve(__dirname, '..'); // Re-sync templates before any byte-comparison checks. -// Other test files (e.g., acceptance tests) may run `squad init` in the -// repo root, overwriting .github/agents/squad.agent.md from the CLI -// template and making it diverge from .squad-templates/squad.agent.md. +// +// Historically this existed because test/init-scaffolding.test.ts sandboxed +// itself *inside* the repo, so init() walked up to the real git root and +// stampVersion() rewrote .github/agents/squad.agent.md mid-run (#1796). That +// sandbox now lives in the OS temp dir and can no longer reach the repository, +// so this sync is defence-in-depth rather than active repair — it is kept so a +// future suite that reintroduces the escape fails loudly here instead of +// leaving a silently mutated working tree. beforeAll(() => { execSync('node scripts/sync-templates.mjs', { cwd: ROOT, @@ -118,13 +123,12 @@ const CASTING_POLICY_LOCATIONS = [ // --------------------------------------------------------------------------- describe('dynamic template enumeration (all synced files)', () => { - // Re-sync immediately before byte comparisons to guard against a race - // condition with test/init-scaffolding.test.ts. That suite runs runInit() - // with a subdirectory of the repo root as cwd; in monorepo mode the init - // command places squad.agent.md at the real git root (.github/agents/) via - // stampVersion(), which races with the file-level beforeAll sync. A - // describe-scoped beforeAll runs synchronously, right before any test in - // this suite, minimising the window to near-zero. + // Re-sync immediately before byte comparisons as a second layer of defence. + // The original race — test/init-scaffolding.test.ts running runInit() against + // a sandbox under the repo root, so monorepo resolution stamped the real + // .github/agents/squad.agent.md — was fixed in #1796 by moving that sandbox + // to the OS temp dir. This describe-scoped beforeAll is retained to keep the + // window closed if any future suite reintroduces an in-repo sandbox. beforeAll(() => { execSync('node scripts/sync-templates.mjs', { cwd: ROOT,