diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 991e3e1..e43b38b 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -42,13 +42,9 @@ jobs: - name: Install/uninstall smoke (isolated HOME) shell: bash run: bash scripts/smoke-install.sh - # Re-run the unit suite here only on Linux — ci.yml already runs it on the Node 20/22 - # Linux matrix, so this is just belt-and-suspenders on the same platform. - # TODO(macos-suite): the first macOS run surfaced ONE environment-specific unit-test - # failure (the install smoke itself is green on macOS; only a temp-path/case-sensitivity - # assumption in a unit test trips on APFS + /tmp→/private/tmp). Pin and fix that test, - # then drop this `if` to get full macOS unit coverage. The install evidence — P2's - # actual deliverable — already runs on macos above. + # Re-run the unit suite here across the matrix. ci.yml already runs it on the Node + # 20/22 Linux matrix; running it on macOS too proves the >=20 behavior on APFS + + # /tmp→/private/tmp, where tmpdir() is a symlink (dryRun now canonicalizes worktree + # paths so per-file attribution string-matches node --test's realpath'd locations). - name: Test suite - if: matrix.os == 'ubuntu-latest' run: npm test diff --git a/CHANGELOG.md b/CHANGELOG.md index ed9b10e..2c603f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed + +- **`forge imagine --dry-run` per-file attribution on macOS.** `dryRun()` matched node + `--test`'s realpath'd `location:` diagnostics against a raw `mkdtemp` worktree path; on + platforms where `tmpdir()` is itself a symlink (macOS: `/var`→`/private/var`, + `/tmp`→`/private/tmp`) the two never matched, so the pass/fail-per-file breakdown was + silently dropped. The worktree root is now canonicalized with `realpathSync` before + comparison (a no-op on Linux). The CI smoke matrix now runs the full unit suite on + `macos-latest`, which surfaced this. + ## [0.23.0] - 2026-07-19 ### Security diff --git a/src/imagine.js b/src/imagine.js index 6b88d2c..10f8e33 100644 --- a/src/imagine.js +++ b/src/imagine.js @@ -8,7 +8,7 @@ // TAP summary, and always discards the sandbox. Selection stays useful on its own as // "run these, in this order"; dryRun turns the prediction into measured evidence. import { execFileSync, spawnSync } from "node:child_process"; -import { existsSync, mkdtempSync, readFileSync, rmSync, statSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, realpathSync, rmSync, statSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { build as buildAtlas, impact, load as loadAtlas } from "./atlas.js"; @@ -134,6 +134,12 @@ export function dryRun(root, { tests, timeoutMs = 120000 } = {}) { const msg = /** @type {{stderr?: Buffer}} */ (e).stderr?.toString().trim() || String(e); return { ok: false, reason: `git worktree add failed: ${msg}` }; } + // node --test reports each failure's `location:` as a canonical (realpath'd) path. + // On platforms where tmpdir() itself is a symlink (macOS: /var → /private/var), the + // raw `wt` path won't string-match those locations, so per-file attribution below + // must compare against the canonicalized worktree root. On Linux (no symlink) this + // is a no-op. + const wtReal = realpathSync(wt); // Runner policy: always `node --test ` — a custom package test script // (jest, vitest, …) is a WHOLE-SUITE command that can't be scoped per-file safely, // which would defeat minimal selection. We still run node --test and say so, so a @@ -196,7 +202,7 @@ export function dryRun(root, { tests, timeoutMs = 120000 } = {}) { let attributable = true; for (const block of (run.stdout ?? "").split(/^not ok /m).slice(1)) { const file = block.split("\n").map(locFile).find(Boolean); - const t = file && tests.find((c) => file === join(wt, String(c))); + const t = file && tests.find((c) => file === join(wtReal, String(c))); if (t) perFile[String(t)] = "fail"; else attributable = false; }