diff --git a/src/resources/extensions/gsd/doctor-environment.ts b/src/resources/extensions/gsd/doctor-environment.ts index 17a266ce8..563afdbb4 100644 --- a/src/resources/extensions/gsd/doctor-environment.ts +++ b/src/resources/extensions/gsd/doctor-environment.ts @@ -149,21 +149,44 @@ function checkDependenciesInstalled(basePath: string): EnvironmentCheckResult | }; } - // Check if lockfile is newer than node_modules - const lockfiles = ["package-lock.json", "yarn.lock", "pnpm-lock.yaml"]; - for (const lockfile of lockfiles) { - const lockPath = join(basePath, lockfile); + // Check if lockfile is newer than the last install. + // + // Each package manager writes a metadata marker inside node_modules on + // every install. Comparing the lockfile mtime against the marker is + // reliable; comparing against the node_modules *directory* mtime is not, + // because directory mtime only changes when entries are added or removed + // — not when files inside it are updated. (#1974) + const lockfiles: Array<{ lock: string; markers: string[] }> = [ + { lock: "package-lock.json", markers: ["node_modules/.package-lock.json"] }, + { lock: "yarn.lock", markers: ["node_modules/.yarn-integrity"] }, + { lock: "pnpm-lock.yaml", markers: ["node_modules/.modules.yaml"] }, + ]; + + for (const { lock, markers } of lockfiles) { + const lockPath = join(basePath, lock); if (!existsSync(lockPath)) continue; try { const lockMtime = statSync(lockPath).mtimeMs; - const nmMtime = statSync(nodeModules).mtimeMs; - if (lockMtime > nmMtime) { + // Prefer the package manager's marker file; fall back to directory mtime + // only when no marker exists (e.g., manually created node_modules). + let installMtime = 0; + for (const marker of markers) { + const markerPath = join(basePath, marker); + if (existsSync(markerPath)) { + installMtime = Math.max(installMtime, statSync(markerPath).mtimeMs); + } + } + if (installMtime === 0) { + installMtime = statSync(nodeModules).mtimeMs; + } + + if (lockMtime > installMtime) { return { name: "dependencies", status: "warning", - message: `${lockfile} is newer than node_modules — dependencies may be stale`, + message: `${lock} is newer than node_modules — dependencies may be stale`, detail: `Run npm install / yarn / pnpm install to update`, }; } diff --git a/src/resources/extensions/gsd/tests/doctor-environment.test.ts b/src/resources/extensions/gsd/tests/doctor-environment.test.ts index 35dfc52e9..59263f2b7 100644 --- a/src/resources/extensions/gsd/tests/doctor-environment.test.ts +++ b/src/resources/extensions/gsd/tests/doctor-environment.test.ts @@ -15,7 +15,7 @@ import assert from 'node:assert/strict'; * - Report formatting */ -import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, utimesSync } from "node:fs"; import { join, dirname } from "node:path"; import { tmpdir } from "node:os"; @@ -102,6 +102,120 @@ describe('doctor-environment', async () => { assert.deepStrictEqual(depsCheck!.status, "ok", "existing node_modules is ok"); }); + // ── Stale Dependencies: marker file check (#1974) ────────────────── + console.log("\n=== env: npm marker file newer than lockfile → ok (#1974) ==="); + { + // Simulate the exact bug scenario: + // 1. node_modules dir mtime is old (no entries added/removed recently) + // 2. package-lock.json mtime is recent (npm rewrote it) + // 3. node_modules/.package-lock.json mtime is between dir and lockfile + // (npm wrote it during the same install that rewrote the lockfile) + // + // The bug: code compares lockfile mtime vs dir mtime → false positive warning + // The fix: compare lockfile mtime vs marker file mtime → correctly ok + const dir = createProjectDir({ + "package.json": JSON.stringify({ name: "test" }), + }); + mkdirSync(join(dir, "node_modules"), { recursive: true }); + + // Simulate the exact bug: npm install with "up to date" rewrites the + // lockfile and the marker, but no packages are added/removed so the + // directory mtime should be old. We write the marker first (which + // bumps dir mtime), then force the dir mtime back to the past. + // + // Timeline: dir(T-120s) < lockfile(T-5s) ≈ marker(T-5s) + // Bug: code compares lockfile vs dir → false positive stale warning + // Fix: code compares lockfile vs marker → correctly reports ok + const dirTime = new Date(Date.now() - 120_000); + const installTime = new Date(Date.now() - 5_000); + + // Write marker file (this bumps dir mtime as a side effect) + writeFileSync(join(dir, "node_modules", ".package-lock.json"), "{}"); + utimesSync(join(dir, "node_modules", ".package-lock.json"), installTime, installTime); + + // Force dir mtime back to the past — simulates no top-level entries changed + utimesSync(join(dir, "node_modules"), dirTime, dirTime); + + // Lockfile written at install time (same as marker, or slightly after) + writeFileSync(join(dir, "package-lock.json"), "{}"); + utimesSync(join(dir, "package-lock.json"), installTime, installTime); + + cleanups.push(dir); + const results = runEnvironmentChecks(dir); + const depsCheck = results.find(r => r.name === "dependencies"); + assertTrue(depsCheck !== undefined, "dependencies check runs"); + assertEq(depsCheck!.status, "ok", "npm marker newer than lockfile → not stale"); + } + + console.log("\n=== env: yarn marker file newer than lockfile → ok (#1974) ==="); + { + const dir = createProjectDir({ + "package.json": JSON.stringify({ name: "test" }), + }); + mkdirSync(join(dir, "node_modules"), { recursive: true }); + + const dirTime = new Date(Date.now() - 120_000); + const installTime = new Date(Date.now() - 5_000); + + writeFileSync(join(dir, "node_modules", ".yarn-integrity"), "{}"); + utimesSync(join(dir, "node_modules", ".yarn-integrity"), installTime, installTime); + utimesSync(join(dir, "node_modules"), dirTime, dirTime); + + writeFileSync(join(dir, "yarn.lock"), ""); + utimesSync(join(dir, "yarn.lock"), installTime, installTime); + + cleanups.push(dir); + const results = runEnvironmentChecks(dir); + const depsCheck = results.find(r => r.name === "dependencies"); + assertTrue(depsCheck !== undefined, "dependencies check runs"); + assertEq(depsCheck!.status, "ok", "yarn marker newer than lockfile → not stale"); + } + + console.log("\n=== env: pnpm marker file newer than lockfile → ok (#1974) ==="); + { + const dir = createProjectDir({ + "package.json": JSON.stringify({ name: "test" }), + }); + mkdirSync(join(dir, "node_modules"), { recursive: true }); + + const dirTime = new Date(Date.now() - 120_000); + const installTime = new Date(Date.now() - 5_000); + + writeFileSync(join(dir, "node_modules", ".modules.yaml"), "{}"); + utimesSync(join(dir, "node_modules", ".modules.yaml"), installTime, installTime); + utimesSync(join(dir, "node_modules"), dirTime, dirTime); + + writeFileSync(join(dir, "pnpm-lock.yaml"), ""); + utimesSync(join(dir, "pnpm-lock.yaml"), installTime, installTime); + + cleanups.push(dir); + const results = runEnvironmentChecks(dir); + const depsCheck = results.find(r => r.name === "dependencies"); + assertTrue(depsCheck !== undefined, "dependencies check runs"); + assertEq(depsCheck!.status, "ok", "pnpm marker newer than lockfile → not stale"); + } + + console.log("\n=== env: no marker file falls back to dir mtime → stale warning (#1974) ==="); + { + // No marker file exists, lockfile newer than dir → should still warn + const dir = createProjectDir({ + "package.json": JSON.stringify({ name: "test" }), + }); + mkdirSync(join(dir, "node_modules"), { recursive: true }); + + const past = new Date(Date.now() - 60_000); + utimesSync(join(dir, "node_modules"), past, past); + + writeFileSync(join(dir, "package-lock.json"), "{}"); + // No marker file written — fallback to dir mtime comparison + + cleanups.push(dir); + const results = runEnvironmentChecks(dir); + const depsCheck = results.find(r => r.name === "dependencies"); + assertTrue(depsCheck !== undefined, "dependencies check runs"); + assertEq(depsCheck!.status, "warning", "no marker + lockfile newer → stale warning"); + } + // ── Env File Check ───────────────────────────────────────────────── test('env: .env.example without .env detected', () => { const dir = createProjectDir({