diff --git a/.changeset/fn-6615-prefer-fresh-src.md b/.changeset/fn-6615-prefer-fresh-src.md new file mode 100644 index 0000000000..77523045fa --- /dev/null +++ b/.changeset/fn-6615-prefer-fresh-src.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Prefer fresher TypeScript plugin source over stale gitignored dist output in dev/worktree plugin resolution when no `bundled.js` exists. Production bundled installs remain unaffected because `bundled.js` still always wins. diff --git a/packages/cli/src/plugins/__tests__/bundled-plugin-install.test.ts b/packages/cli/src/plugins/__tests__/bundled-plugin-install.test.ts index 90887da9af..a597259559 100644 --- a/packages/cli/src/plugins/__tests__/bundled-plugin-install.test.ts +++ b/packages/cli/src/plugins/__tests__/bundled-plugin-install.test.ts @@ -3,17 +3,22 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; // ── Mocks ──────────────────────────────────────────────────────────── // vi.mock factories are hoisted, so we use vi.hoisted() for mock references. -const { mockExistsSync, mockStatSync, mockReadFile, mockFsStat, mockCopyFile, mockValidatePluginManifest } = vi.hoisted(() => ({ - mockExistsSync: vi.fn<(path: string) => boolean>(), - mockStatSync: vi.fn<(path: string) => { isDirectory: () => boolean }>(), - mockReadFile: vi.fn<(path: string, encoding: string) => Promise>(), - mockFsStat: vi.fn<(path: string) => Promise<{ isDirectory: () => boolean }>>(), - mockCopyFile: vi.fn<(src: string, dest: string) => Promise>(), - mockValidatePluginManifest: vi.fn<(manifest: unknown) => { valid: boolean; errors: string[] }>(), -})); +const { mockExistsSync, mockReaddirSync, mockStatSync, mockReadFile, mockFsStat, mockCopyFile, mockValidatePluginManifest } = + vi.hoisted(() => ({ + mockExistsSync: vi.fn<(path: string) => boolean>(), + mockReaddirSync: vi.fn< + (path: string, options: { withFileTypes: true; encoding: "utf8" }) => Array<{ name: string; isDirectory: () => boolean }> + >(), + mockStatSync: vi.fn<(path: string) => { isDirectory: () => boolean; mtimeMs?: number }>(), + mockReadFile: vi.fn<(path: string, encoding: string) => Promise>(), + mockFsStat: vi.fn<(path: string) => Promise<{ isDirectory: () => boolean }>>(), + mockCopyFile: vi.fn<(src: string, dest: string) => Promise>(), + mockValidatePluginManifest: vi.fn<(manifest: unknown) => { valid: boolean; errors: string[] }>(), + })); vi.mock("node:fs", () => ({ existsSync: mockExistsSync, + readdirSync: mockReaddirSync, statSync: mockStatSync, })); @@ -201,7 +206,8 @@ async function getResolvedBundledPath(): Promise { beforeEach(() => { vi.clearAllMocks(); - mockStatSync.mockImplementation(() => ({ isDirectory: () => false })); + mockReaddirSync.mockReturnValue([{ name: "index.ts", isDirectory: () => false }]); + mockStatSync.mockImplementation(() => ({ isDirectory: () => false, mtimeMs: 0 })); mockFsStat.mockImplementation(async () => ({ isDirectory: () => false })); mockCopyFile.mockResolvedValue(); }); @@ -217,8 +223,27 @@ describe("resolvePluginEntryPath", () => { expect(resolvePluginEntryPath("/tmp/plugin")).toBe("/tmp/plugin/bundled.js"); }); - it("prefers dist/index.js when bundled.js is unavailable", () => { + it("prefers src/index.ts when bundled.js is unavailable and src is newer than dist", () => { mockExistsSync.mockImplementation((p: string) => p.endsWith("/src/index.ts") || p.endsWith("/dist/index.js")); + mockStatSync.mockImplementation((p: string) => ({ + isDirectory: () => false, + mtimeMs: p.endsWith("/dist/index.js") ? 1 : 2, + })); + expect(resolvePluginEntryPath("/tmp/plugin")).toBe("/tmp/plugin/src/index.ts"); + }); + + it("prefers dist/index.js when bundled.js is unavailable and dist is newer", () => { + mockExistsSync.mockImplementation((p: string) => p.endsWith("/src/index.ts") || p.endsWith("/dist/index.js")); + mockStatSync.mockImplementation((p: string) => ({ + isDirectory: () => false, + mtimeMs: p.endsWith("/dist/index.js") ? 2 : 1, + })); + expect(resolvePluginEntryPath("/tmp/plugin")).toBe("/tmp/plugin/dist/index.js"); + }); + + it("prefers dist/index.js when bundled.js is unavailable and mtimes are equal", () => { + mockExistsSync.mockImplementation((p: string) => p.endsWith("/src/index.ts") || p.endsWith("/dist/index.js")); + mockStatSync.mockImplementation(() => ({ isDirectory: () => false, mtimeMs: 1 })); expect(resolvePluginEntryPath("/tmp/plugin")).toBe("/tmp/plugin/dist/index.js"); }); @@ -252,6 +277,7 @@ describe("ensureBundledDependencyGraphPluginInstalled", () => { })); vi.doMock("node:fs", () => ({ existsSync: mockExistsSync, + readdirSync: mockReaddirSync, statSync: mockStatSync, })); vi.doMock("node:fs/promises", () => ({ diff --git a/packages/cli/src/plugins/__tests__/resolve-plugin-entry-path-sync.test.ts b/packages/cli/src/plugins/__tests__/resolve-plugin-entry-path-sync.test.ts index 0fa5e80258..525a27082f 100644 --- a/packages/cli/src/plugins/__tests__/resolve-plugin-entry-path-sync.test.ts +++ b/packages/cli/src/plugins/__tests__/resolve-plugin-entry-path-sync.test.ts @@ -12,8 +12,8 @@ * seam that exercises both implementations equally. */ import { describe, it, expect, beforeEach, afterEach } from "vitest"; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; -import { join } from "node:path"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, utimesSync } from "node:fs"; +import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; import { resolvePluginEntryPath as cliResolve } from "../bundled-plugin-install.js"; import { resolvePluginEntryPath as coreResolve } from "@fusion/core"; @@ -31,16 +31,53 @@ describe("resolvePluginEntryPath: CLI copy stays in sync with @fusion/core", () function touch(relative: string) { const full = join(dir, relative); - mkdirSync(join(full, ".."), { recursive: true }); + mkdirSync(dirname(full), { recursive: true }); writeFileSync(full, "// entry\n"); } - const layouts: Array<{ name: string; files: string[]; expected: string | null }> = [ + const older = new Date("2026-01-01T00:00:00.000Z"); + const newer = new Date("2026-01-01T00:01:00.000Z"); + + const layouts: Array<{ + name: string; + files: string[]; + expected: string | null; + mtimes?: Record; + }> = [ { name: "bundled.js only", files: ["bundled.js"], expected: "bundled.js" }, { name: "dist/index.js only", files: ["dist/index.js"], expected: "dist/index.js" }, { name: "src/index.ts only", files: ["src/index.ts"], expected: "src/index.ts" }, { name: "bundled.js preferred over src", files: ["bundled.js", "src/index.ts"], expected: "bundled.js" }, - { name: "dist preferred over src", files: ["dist/index.js", "src/index.ts"], expected: "dist/index.js" }, + { + name: "dist + src, src newer → src/index.ts", + files: ["dist/index.js", "src/index.ts"], + expected: "src/index.ts", + mtimes: { "dist/index.js": older, "src/index.ts": newer }, + }, + { + name: "dist + src, dist newer → dist/index.js", + files: ["dist/index.js", "src/index.ts"], + expected: "dist/index.js", + mtimes: { "dist/index.js": newer, "src/index.ts": older }, + }, + { + name: "dist + src, equal mtimes → dist/index.js", + files: ["dist/index.js", "src/index.ts"], + expected: "dist/index.js", + mtimes: { "dist/index.js": older, "src/index.ts": older }, + }, + { + name: "dist + src, non-index src file newer → src/index.ts", + files: ["dist/index.js", "src/index.ts", "src/settings.ts"], + expected: "src/index.ts", + mtimes: { "dist/index.js": older, "src/index.ts": older, "src/settings.ts": newer }, + }, + { + name: "bundled.js + dist + src, src newer → bundled.js", + files: ["bundled.js", "dist/index.js", "src/index.ts"], + expected: "bundled.js", + mtimes: { "dist/index.js": older, "src/index.ts": newer }, + }, { name: "all three → bundled.js", files: ["bundled.js", "dist/index.js", "src/index.ts"], expected: "bundled.js" }, { name: "no entry files", files: ["README.md"], expected: null }, ]; @@ -48,6 +85,9 @@ describe("resolvePluginEntryPath: CLI copy stays in sync with @fusion/core", () for (const layout of layouts) { it(`resolves identically for: ${layout.name}`, () => { for (const f of layout.files) touch(f); + for (const [file, mtime] of Object.entries(layout.mtimes ?? {})) { + utimesSync(join(dir, file), mtime, mtime); + } const expected = layout.expected === null ? null : join(dir, layout.expected); expect(cliResolve(dir)).toBe(expected); diff --git a/packages/cli/src/plugins/bundled-plugin-install.ts b/packages/cli/src/plugins/bundled-plugin-install.ts index 6f5beede25..6a9b5af702 100644 --- a/packages/cli/src/plugins/bundled-plugin-install.ts +++ b/packages/cli/src/plugins/bundled-plugin-install.ts @@ -1,4 +1,4 @@ -import { existsSync, statSync } from "node:fs"; +import { existsSync, readdirSync, statSync } from "node:fs"; import { readFile } from "node:fs/promises"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; @@ -70,10 +70,17 @@ function resolveBundledPluginDir(pluginId: string): string | null { /** * Resolve the actual loadable entry FILE path for a plugin directory. Node ESM * does not allow directory imports, so we must register the explicit file the - * loader will dynamic-import. Preference order: - * 1. ./bundled.js (esbuild-bundled, shipped in npm tarball) - * 2. ./dist/index.js (legacy prebuilt fallback) - * 3. ./src/index.ts (workspace/dev fallback when no bundle exists) + * loader will dynamic-import. Resolution keeps ./bundled.js unconditional + * because production npm tarballs ship that esbuild-bundled entry. In + * dev/worktree contexts where no bundle exists, ./dist/index.js remains the + * prebuilt fallback unless any file under ./src/ is newer than dist/index.js; + * then ./src/index.ts wins so stale gitignored dist output cannot mask a source + * fix (FN-6615/FN-6596). + * + * FNXC:PluginLoader 2026-06-17-19:20: + * Prefer fresher src over stale dist only when bundled.js is absent. This keeps + * production tarballs on their bundled entry while preventing dev/worktree runs + * from silently loading old gitignored build output after a source fix. * * Returns null when the directory exists but none of the loadable entry files * are present. Callers must treat that as a missing bundle rather than @@ -82,16 +89,74 @@ function resolveBundledPluginDir(pluginId: string): string | null { * Keep in sync with resolvePluginEntryPath in @fusion/core (plugin-loader.ts), * which the dashboard install/enable routes use for the same contract. */ -export function resolvePluginEntryPath(pluginDir: string): string | null { - const candidates = [ - join(pluginDir, "bundled.js"), - join(pluginDir, "dist", "index.js"), - join(pluginDir, "src", "index.ts"), - ]; - for (const candidate of candidates) { - if (existsSync(candidate)) { - return candidate; +function newestSourceMtimeMs(srcDir: string): number | null { + let newest = Number.NEGATIVE_INFINITY; + + function visit(dir: string): boolean { + const entries = (() => { + try { + return readdirSync(dir, { withFileTypes: true, encoding: "utf8" }); + } catch { + return null; + } + })(); + if (!entries) return false; + + for (const entry of entries) { + const entryPath = join(dir, entry.name); + let entryStat: ReturnType; + try { + entryStat = statSync(entryPath); + } catch { + return false; + } + + if (entryStat.isDirectory()) { + if (!visit(entryPath)) return false; + continue; + } + + if (entryStat.mtimeMs > newest) { + newest = entryStat.mtimeMs; + } } + + return true; + } + + return visit(srcDir) && newest !== Number.NEGATIVE_INFINITY ? newest : null; +} + +function isSourceNewerThanDist(srcDir: string, distIndexPath: string): boolean { + try { + const distMtimeMs = statSync(distIndexPath).mtimeMs; + const srcMtimeMs = newestSourceMtimeMs(srcDir); + return srcMtimeMs !== null && srcMtimeMs > distMtimeMs; + } catch { + return false; + } +} + +export function resolvePluginEntryPath(pluginDir: string): string | null { + const bundledPath = join(pluginDir, "bundled.js"); + if (existsSync(bundledPath)) { + return bundledPath; + } + + const distIndexPath = join(pluginDir, "dist", "index.js"); + const srcDir = join(pluginDir, "src"); + const srcIndexPath = join(srcDir, "index.ts"); + const hasDist = existsSync(distIndexPath); + const hasSrc = existsSync(srcIndexPath); + + if (hasDist && hasSrc) { + return isSourceNewerThanDist(srcDir, distIndexPath) ? srcIndexPath : distIndexPath; + } + if (hasDist) { + return distIndexPath; + } + if (hasSrc) { + return srcIndexPath; } return null; } diff --git a/packages/core/src/__tests__/plugin-loader.test.ts b/packages/core/src/__tests__/plugin-loader.test.ts index bb011e0a1b..0bf99618e1 100644 --- a/packages/core/src/__tests__/plugin-loader.test.ts +++ b/packages/core/src/__tests__/plugin-loader.test.ts @@ -2,9 +2,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import { writeFile, mkdir } from "node:fs/promises"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; -import { mkdtempSync, existsSync } from "node:fs"; +import { mkdtempSync, existsSync, rmSync, utimesSync } from "node:fs"; import { tmpdir } from "node:os"; -import { PluginLoader } from "../plugin-loader.js"; +import { PluginLoader, resolvePluginEntryPath } from "../plugin-loader.js"; import * as loggerModule from "../logger.js"; const scanPluginSecurityMock = vi.fn(); @@ -126,6 +126,73 @@ function droidPluginModulePath(): string { ); } +describe("resolvePluginEntryPath", () => { + let pluginDir: string; + + beforeEach(() => { + pluginDir = makeTmpDir(); + }); + + afterEach(() => { + rmSync(pluginDir, { recursive: true, force: true }); + }); + + async function writeEntry(relative: string): Promise { + const path = join(pluginDir, relative); + await mkdir(join(path, ".."), { recursive: true }); + await writeFile(path, "// entry\n"); + return path; + } + + it("prefers fresher src/index.ts over stale dist when no bundle exists", async () => { + const dist = await writeEntry("dist/index.js"); + const src = await writeEntry("src/index.ts"); + const older = new Date("2026-01-01T00:00:00.000Z"); + const newer = new Date("2026-01-01T00:01:00.000Z"); + utimesSync(dist, older, older); + utimesSync(src, newer, newer); + + expect(resolvePluginEntryPath(pluginDir)).toBe(src); + }); + + it("keeps dist/index.js when dist is newer than src", async () => { + const dist = await writeEntry("dist/index.js"); + const src = await writeEntry("src/index.ts"); + const older = new Date("2026-01-01T00:00:00.000Z"); + const newer = new Date("2026-01-01T00:01:00.000Z"); + utimesSync(dist, newer, newer); + utimesSync(src, older, older); + + expect(resolvePluginEntryPath(pluginDir)).toBe(dist); + }); + + it("uses newest non-index src file for freshness and still returns src/index.ts", async () => { + const dist = await writeEntry("dist/index.js"); + const src = await writeEntry("src/index.ts"); + const settings = await writeEntry("src/settings.ts"); + const older = new Date("2026-01-01T00:00:00.000Z"); + const newer = new Date("2026-01-01T00:01:00.000Z"); + utimesSync(dist, older, older); + utimesSync(src, older, older); + utimesSync(settings, newer, newer); + + expect(resolvePluginEntryPath(pluginDir)).toBe(src); + }); + + it("always keeps bundled.js first regardless of dist or src freshness", async () => { + const bundled = await writeEntry("bundled.js"); + const dist = await writeEntry("dist/index.js"); + const src = await writeEntry("src/index.ts"); + const older = new Date("2026-01-01T00:00:00.000Z"); + const newer = new Date("2026-01-01T00:01:00.000Z"); + utimesSync(bundled, older, older); + utimesSync(dist, older, older); + utimesSync(src, newer, newer); + + expect(resolvePluginEntryPath(pluginDir)).toBe(bundled); + }); +}); + // Mock TaskStore for testing const mockTaskStore = { logActivity: vi.fn(), diff --git a/packages/core/src/plugin-loader.ts b/packages/core/src/plugin-loader.ts index d6f1f5adb1..8e8e6a1f7e 100644 --- a/packages/core/src/plugin-loader.ts +++ b/packages/core/src/plugin-loader.ts @@ -10,7 +10,7 @@ */ import { basename, dirname, extname, isAbsolute, join, resolve } from "node:path"; -import { existsSync } from "node:fs"; +import { existsSync, readdirSync, statSync } from "node:fs"; import { stat } from "node:fs/promises"; import { copyFile } from "node:fs/promises"; import { pathToFileURL } from "node:url"; @@ -53,10 +53,17 @@ let moduleImportVersion = 0; /** * Resolve the actual loadable entry FILE path for a plugin directory. Node ESM * does not allow directory imports, so the registered plugin path must be the - * explicit file the loader will dynamic-import. Preference order: - * 1. ./bundled.js (esbuild-bundled, shipped in npm tarball) - * 2. ./dist/index.js (legacy prebuilt fallback) - * 3. ./src/index.ts (workspace/dev fallback when no bundle exists) + * explicit file the loader will dynamic-import. Resolution keeps ./bundled.js + * unconditional because production npm tarballs ship that esbuild-bundled entry. + * In dev/worktree contexts where no bundle exists, ./dist/index.js remains the + * prebuilt fallback unless any file under ./src/ is newer than dist/index.js; + * then ./src/index.ts wins so stale gitignored dist output cannot mask a source + * fix (FN-6615/FN-6596). + * + * FNXC:PluginLoader 2026-06-17-19:20: + * Prefer fresher src over stale dist only when bundled.js is absent. This keeps + * production tarballs on their bundled entry while preventing dev/worktree runs + * from silently loading old gitignored build output after a source fix. * * Returns null when the directory exists but none of the loadable entry files * are present. Callers must treat that as a missing/unloadable plugin rather @@ -65,16 +72,74 @@ let moduleImportVersion = 0; * Keep in sync with resolvePluginEntryPath in the CLI's * bundled-plugin-install.ts, which keeps a local copy so its fs mocks work. */ -export function resolvePluginEntryPath(pluginDir: string): string | null { - const candidates = [ - join(pluginDir, "bundled.js"), - join(pluginDir, "dist", "index.js"), - join(pluginDir, "src", "index.ts"), - ]; - for (const candidate of candidates) { - if (existsSync(candidate)) { - return candidate; +function newestSourceMtimeMs(srcDir: string): number | null { + let newest = Number.NEGATIVE_INFINITY; + + function visit(dir: string): boolean { + const entries = (() => { + try { + return readdirSync(dir, { withFileTypes: true, encoding: "utf8" }); + } catch { + return null; + } + })(); + if (!entries) return false; + + for (const entry of entries) { + const entryPath = join(dir, entry.name); + let entryStat: ReturnType; + try { + entryStat = statSync(entryPath); + } catch { + return false; + } + + if (entryStat.isDirectory()) { + if (!visit(entryPath)) return false; + continue; + } + + if (entryStat.mtimeMs > newest) { + newest = entryStat.mtimeMs; + } } + + return true; + } + + return visit(srcDir) && newest !== Number.NEGATIVE_INFINITY ? newest : null; +} + +function isSourceNewerThanDist(srcDir: string, distIndexPath: string): boolean { + try { + const distMtimeMs = statSync(distIndexPath).mtimeMs; + const srcMtimeMs = newestSourceMtimeMs(srcDir); + return srcMtimeMs !== null && srcMtimeMs > distMtimeMs; + } catch { + return false; + } +} + +export function resolvePluginEntryPath(pluginDir: string): string | null { + const bundledPath = join(pluginDir, "bundled.js"); + if (existsSync(bundledPath)) { + return bundledPath; + } + + const distIndexPath = join(pluginDir, "dist", "index.js"); + const srcDir = join(pluginDir, "src"); + const srcIndexPath = join(srcDir, "index.ts"); + const hasDist = existsSync(distIndexPath); + const hasSrc = existsSync(srcIndexPath); + + if (hasDist && hasSrc) { + return isSourceNewerThanDist(srcDir, distIndexPath) ? srcIndexPath : distIndexPath; + } + if (hasDist) { + return distIndexPath; + } + if (hasSrc) { + return srcIndexPath; } return null; }