From 949eabb82c3873aeb637dc07796e9f3c5e0714bd Mon Sep 17 00:00:00 2001 From: Casey Gowrie Date: Sun, 12 Jul 2026 16:22:22 -0400 Subject: [PATCH] fix(eve): stabilize dev instrumentation entrypoint Signed-off-by: Casey Gowrie --- .../stable-dev-instrumentation-entrypoint.md | 5 ++ .../application/compiled-artifacts.ts | 79 ++++++------------- .../application/instrumentation-module.ts | 21 +++++ .../host/build-application.scenario.test.ts | 6 ++ .../create-application-nitro.scenario.test.ts | 41 ++++++++++ .../nitro/host/create-application-nitro.ts | 23 ++---- ...v-authored-source-watcher.scenario.test.ts | 24 ++++++ .../nitro/host/dev-authored-source-watcher.ts | 7 +- .../prepare-application-host.scenario.test.ts | 3 + ...piled-artifacts-bootstrap.scenario.test.ts | 20 ++++- .../scenarios/dev-server.scenario.test.ts | 57 ++++++++++++- 11 files changed, 212 insertions(+), 74 deletions(-) create mode 100644 .changeset/stable-dev-instrumentation-entrypoint.md create mode 100644 packages/eve/src/internal/application/instrumentation-module.ts diff --git a/.changeset/stable-dev-instrumentation-entrypoint.md b/.changeset/stable-dev-instrumentation-entrypoint.md new file mode 100644 index 000000000..527f878bc --- /dev/null +++ b/.changeset/stable-dev-instrumentation-entrypoint.md @@ -0,0 +1,5 @@ +--- +"eve": patch +--- + +`eve dev` now keeps its Nitro instrumentation entrypoint stable when authored instrumentation is added or removed. Instrumentation changes trigger a structural reload without leaving retained plugin paths that import deleted files. diff --git a/packages/eve/src/internal/application/compiled-artifacts.ts b/packages/eve/src/internal/application/compiled-artifacts.ts index bce21b0be..d0d36e2e5 100644 --- a/packages/eve/src/internal/application/compiled-artifacts.ts +++ b/packages/eve/src/internal/application/compiled-artifacts.ts @@ -1,4 +1,3 @@ -import { existsSync } from "node:fs"; import { mkdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -7,6 +6,7 @@ import type { CompileAgentResult } from "#compiler/compile-agent.js"; import { createCompiledModuleMapSource } from "#compiler/module-map.js"; import { getWorldImport } from "@workflow/utils"; import { stringifyEsmImportSpecifier } from "#internal/application/import-specifier.js"; +import { resolveInstrumentationModule } from "#internal/application/instrumentation-module.js"; import { resolvePackageCompiledFilePath, resolvePackageSourceFilePath, @@ -25,16 +25,8 @@ export interface GeneratedCompiledArtifactsFiles { bootstrapPath: string; /** Nitro plugin that installs the selected vendored Workflow world. */ workflowWorldPluginPath: string; - /** - * Optional Nitro plugin that imports the authored instrumentation module - * from the application when present. - */ - instrumentationPluginPath?: string; - /** - * Absolute path to the authored instrumentation module when present. - * Nitro uses this to preserve the module's side effects during bundling. - */ - instrumentationSourcePath?: string; + /** Nitro plugin that imports authored instrumentation when present. */ + instrumentationPluginPath: string; } /** @@ -71,45 +63,20 @@ export async function writeCompiledArtifactsFiles(input: { ), ); - if (instrumentationPath !== undefined) { - await writeFile( - instrumentationPluginPath, - createInstrumentationPluginSource({ - agentName: input.compileResult.manifest.config.name, - instrumentationPath, - registerConfigPath: resolvePackageSourceFilePath("src/harness/instrumentation-config.ts"), - }), - ); - } + await writeFile( + instrumentationPluginPath, + createInstrumentationPluginSource({ + agentName: input.compileResult.manifest.config.name, + instrumentationPath, + registerConfigPath: resolvePackageSourceFilePath("src/harness/instrumentation-config.ts"), + }), + ); - const generatedArtifacts: GeneratedCompiledArtifactsFiles = { + return { bootstrapPath, + instrumentationPluginPath, workflowWorldPluginPath, }; - - if (instrumentationPath !== undefined) { - generatedArtifacts.instrumentationPluginPath = instrumentationPluginPath; - generatedArtifacts.instrumentationSourcePath = instrumentationPath; - } - - return generatedArtifacts; -} - -const INSTRUMENTATION_EXTENSIONS = [".ts", ".mts", ".js", ".mjs"]; - -/** - * Resolves the optional `agent/instrumentation` module from the agent root - * directory. Returns the absolute path if found, `undefined` otherwise. - */ -function resolveInstrumentationModule(agentRoot: string): string | undefined { - for (const ext of INSTRUMENTATION_EXTENSIONS) { - const candidate = join(agentRoot, `instrumentation${ext}`); - if (existsSync(candidate)) { - return candidate; - } - } - - return undefined; } function stripCompiledModuleMapExports(source: string): string { @@ -212,18 +179,22 @@ export function createWorkflowWorldPluginSource( function createInstrumentationPluginSource(input: { agentName: string; - instrumentationPath: string; + instrumentationPath: string | undefined; registerConfigPath: string; }): string { return [ "// Generated by eve. Do not edit by hand.", - `import * as instrumentationModule from ${stringifyEsmImportSpecifier(input.instrumentationPath)};`, - `import { registerInstrumentationConfig } from ${stringifyEsmImportSpecifier(input.registerConfigPath)};`, - "", - "if (instrumentationModule.default != null) {", - ` registerInstrumentationConfig(instrumentationModule.default, { agentName: ${JSON.stringify(input.agentName)} });`, - "}", - "", + ...(input.instrumentationPath === undefined + ? [] + : [ + `import * as instrumentationModule from ${stringifyEsmImportSpecifier(input.instrumentationPath)};`, + `import { registerInstrumentationConfig } from ${stringifyEsmImportSpecifier(input.registerConfigPath)};`, + "", + "if (instrumentationModule.default != null) {", + ` registerInstrumentationConfig(instrumentationModule.default, { agentName: ${JSON.stringify(input.agentName)} });`, + "}", + "", + ]), "// Default export satisfies the Nitro plugin contract so this file", "// can be used directly as a Nitro plugin without a separate wrapper.", "export default function installInstrumentationPlugin() {}", diff --git a/packages/eve/src/internal/application/instrumentation-module.ts b/packages/eve/src/internal/application/instrumentation-module.ts new file mode 100644 index 000000000..9b50643f5 --- /dev/null +++ b/packages/eve/src/internal/application/instrumentation-module.ts @@ -0,0 +1,21 @@ +import { existsSync } from "node:fs"; +import { join, resolve } from "node:path"; + +const INSTRUMENTATION_EXTENSIONS = [".ts", ".mts", ".js", ".mjs"] as const; + +export function resolveInstrumentationModulePaths(agentRoot: string): string[] { + return INSTRUMENTATION_EXTENSIONS.map((extension) => + join(agentRoot, `instrumentation${extension}`), + ); +} + +export function resolveInstrumentationModule(agentRoot: string): string | undefined { + return resolveInstrumentationModulePaths(agentRoot).find((path) => existsSync(path)); +} + +export function isInstrumentationModulePath(agentRoot: string, path: string): boolean { + const resolvedPath = resolve(path); + return resolveInstrumentationModulePaths(agentRoot).some( + (candidate) => resolve(candidate) === resolvedPath, + ); +} diff --git a/packages/eve/src/internal/nitro/host/build-application.scenario.test.ts b/packages/eve/src/internal/nitro/host/build-application.scenario.test.ts index c0352827f..634fa83d2 100644 --- a/packages/eve/src/internal/nitro/host/build-application.scenario.test.ts +++ b/packages/eve/src/internal/nitro/host/build-application.scenario.test.ts @@ -116,6 +116,12 @@ function createPreparedHost(appRoot: string): PreparedApplicationHost { } as unknown as PreparedApplicationHost["compileResult"], compiledArtifacts: { bootstrapPath: join(appRoot, ".eve", "compile", "compiled-artifacts-bootstrap.mjs"), + instrumentationPluginPath: join( + appRoot, + ".eve", + "compile", + "compiled-artifacts-instrumentation.mjs", + ), workflowWorldPluginPath: join( appRoot, ".eve", diff --git a/packages/eve/src/internal/nitro/host/create-application-nitro.scenario.test.ts b/packages/eve/src/internal/nitro/host/create-application-nitro.scenario.test.ts index a0ad56dc7..ad0c11fbf 100644 --- a/packages/eve/src/internal/nitro/host/create-application-nitro.scenario.test.ts +++ b/packages/eve/src/internal/nitro/host/create-application-nitro.scenario.test.ts @@ -136,6 +136,7 @@ function createPreparedHost(): PreparedApplicationHost { } as unknown as PreparedApplicationHost["compileResult"], compiledArtifacts: { bootstrapPath: `${appRoot}/.eve/bootstrap.mjs`, + instrumentationPluginPath: `${appRoot}/.eve/instrumentation.mjs`, workflowWorldPluginPath: `${appRoot}/.eve/workflow-world.mjs`, } as PreparedApplicationHost["compiledArtifacts"], scheduleRegistrations: [], @@ -168,6 +169,46 @@ describe("createApplicationNitro", () => { expect(plugins.indexOf(preparedHost.compiledArtifacts.bootstrapPath)).toBeLessThan( plugins.indexOf(preparedHost.compiledArtifacts.workflowWorldPluginPath), ); + expect(plugins).toContain(preparedHost.compiledArtifacts.instrumentationPluginPath); + }); + + it("preserves side effects for every authored instrumentation module extension", async () => { + const nitroStub = createNitroStub(); + createNitroMock.mockResolvedValueOnce(nitroStub.nitro); + + const { createApplicationNitro } = + await import("#internal/nitro/host/create-application-nitro.js"); + const preparedHost = createPreparedHost(); + await createApplicationNitro(preparedHost, true); + + const rollupBeforeHooks = nitroStub.hookHandlers.get("rollup:before") ?? []; + const config = { plugins: [] as unknown[] }; + for (const hook of rollupBeforeHooks) { + await hook(nitroStub.nitro, config); + } + const sideEffectsPlugin = ( + config.plugins as Array<{ + name?: string; + resolveId?: (source: string) => unknown; + }> + ).find((plugin) => plugin.name === "eve:instrumentation-module-side-effects"); + + if (sideEffectsPlugin?.resolveId === undefined) { + throw new Error("Expected instrumentation side-effects plugin to be registered."); + } + + for (const extension of [".ts", ".mts", ".js", ".mjs"]) { + expect( + sideEffectsPlugin.resolveId( + join(preparedHost.compileResult.project.agentRoot, `instrumentation${extension}`), + ), + ).toMatchObject({ moduleSideEffects: "no-treeshake" }); + } + expect( + sideEffectsPlugin.resolveId( + join(preparedHost.compileResult.project.agentRoot, "instructions.md"), + ), + ).toBeNull(); }); it("preserves workflow bundle side effects and skips workflow transform for cached bundles", async () => { diff --git a/packages/eve/src/internal/nitro/host/create-application-nitro.ts b/packages/eve/src/internal/nitro/host/create-application-nitro.ts index 5a1cc2c52..91c8c2d79 100644 --- a/packages/eve/src/internal/nitro/host/create-application-nitro.ts +++ b/packages/eve/src/internal/nitro/host/create-application-nitro.ts @@ -16,6 +16,7 @@ import { writeEveVersionedCacheMetadata, } from "#internal/application/cache-metadata.js"; import { resolveNitroBuildDirectory } from "#internal/application/paths.js"; +import { resolveInstrumentationModulePaths } from "#internal/application/instrumentation-module.js"; import { createNitroArtifactsConfig, type NitroArtifactsConfigInput, @@ -564,11 +565,10 @@ function addDynamicToolTransformPlugin(nitro: Nitro): void { * Rollup/Rolldown pass preserves its eager evaluation from the generated * instrumentation plugin. */ -function addInstrumentationModuleSideEffectsPlugin( - nitro: Nitro, - instrumentationModulePath: string, -): void { - const normalizedInstrumentationModulePath = normalizePath(instrumentationModulePath); +function addInstrumentationModuleSideEffectsPlugin(nitro: Nitro, agentRoot: string): void { + const normalizedInstrumentationModulePaths = new Set( + resolveInstrumentationModulePaths(agentRoot).map((path) => normalizePath(path)), + ); nitro.hooks.hook("rollup:before", (_nitro, config) => { if (!Array.isArray(config.plugins)) { @@ -578,7 +578,7 @@ function addInstrumentationModuleSideEffectsPlugin( config.plugins.unshift({ name: "eve:instrumentation-module-side-effects", resolveId(source: string) { - if (normalizePath(source) !== normalizedInstrumentationModulePath) { + if (!normalizedInstrumentationModulePaths.has(normalizePath(source))) { return null; } @@ -712,6 +712,7 @@ export async function createApplicationNitro( const nitroPlugins: string[] = []; nitroPlugins.push(preparedHost.compiledArtifacts.bootstrapPath); nitroPlugins.push(preparedHost.compiledArtifacts.workflowWorldPluginPath); + nitroPlugins.push(preparedHost.compiledArtifacts.instrumentationPluginPath); if (!dev) { // Stops all tracked sandboxes when the production server shuts // down. Dev servers are excluded: the dev CLI parent already stops @@ -725,9 +726,6 @@ export async function createApplicationNitro( resolvePackageSourceFilePath("src/internal/nitro/host/workflow-sandbox-runtime-plugin.ts"), ); } - if (preparedHost.compiledArtifacts.instrumentationPluginPath !== undefined) { - nitroPlugins.push(preparedHost.compiledArtifacts.instrumentationPluginPath); - } await prepareEveVersionedCacheDirectory(nitroBuildDir); const nitro = await createNitro( { @@ -798,12 +796,7 @@ export async function createApplicationNitro( // execute functions for all tool files, not just workflow step targets. addDynamicToolTransformPlugin(nitro); - if (preparedHost.compiledArtifacts.instrumentationSourcePath !== undefined) { - addInstrumentationModuleSideEffectsPlugin( - nitro, - preparedHost.compiledArtifacts.instrumentationSourcePath, - ); - } + addInstrumentationModuleSideEffectsPlugin(nitro, preparedHost.compileResult.project.agentRoot); // Prevent Nitro from re-bundling the pre-built workflow bundle in dev // mode. `steps.mjs` is now a source entry that Nitro must still bundle diff --git a/packages/eve/src/internal/nitro/host/dev-authored-source-watcher.scenario.test.ts b/packages/eve/src/internal/nitro/host/dev-authored-source-watcher.scenario.test.ts index a6e699da8..f16b5c8e9 100644 --- a/packages/eve/src/internal/nitro/host/dev-authored-source-watcher.scenario.test.ts +++ b/packages/eve/src/internal/nitro/host/dev-authored-source-watcher.scenario.test.ts @@ -447,6 +447,30 @@ describe("startAuthoredSourceWatcher", () => { } }); + it("reloads Nitro when authored instrumentation changes", async () => { + const previousHost = createPreparedHost(); + const nextHost = createPreparedHost(); + const nitroStub = createNitroStub(); + + prepareApplicationHostMock.mockResolvedValueOnce(nextHost); + + const watcher = await startAuthoredSourceWatcher({ + nitro: nitroStub.nitro, + preparedHost: previousHost, + }); + + try { + await triggerChangeEvent( + join(previousHost.compileResult.project.agentRoot, "instrumentation.ts"), + ); + + expect(nitroStub.callHook).toHaveBeenCalledTimes(1); + expect(nitroStub.callHook).toHaveBeenCalledWith("rollup:reload"); + } finally { + await watcher.close(); + } + }); + it("never registers authored schedules in dev when registrations change", async () => { // `eve dev` intentionally does not register Nitro scheduled tasks for // authored schedules — production cron firing in dev would invoke every diff --git a/packages/eve/src/internal/nitro/host/dev-authored-source-watcher.ts b/packages/eve/src/internal/nitro/host/dev-authored-source-watcher.ts index 4d27dda11..5c2af5f6e 100644 --- a/packages/eve/src/internal/nitro/host/dev-authored-source-watcher.ts +++ b/packages/eve/src/internal/nitro/host/dev-authored-source-watcher.ts @@ -5,6 +5,7 @@ import type { Nitro } from "nitro/types"; import { clearCompiledRuntimeAgentBundleCache } from "#runtime/sessions/compiled-agent-cache.js"; import { toErrorMessage } from "#shared/errors.js"; import { resolveTsConfigDependencyPaths } from "#internal/application/tsconfig-dependencies.js"; +import { isInstrumentationModulePath } from "#internal/application/instrumentation-module.js"; import { createNitroArtifactsConfig } from "#internal/nitro/host/artifacts-config.js"; import { resolveDevelopmentSourceSnapshotWatchPaths } from "#internal/nitro/dev-runtime-source-snapshot.js"; import { pruneDevelopmentRuntimeArtifactsSnapshotsInBackground } from "#internal/nitro/dev-runtime-artifacts.js"; @@ -133,6 +134,9 @@ export async function startAuthoredSourceWatcher(input: { previousHost.appRoot, changedPaths, ); + const hasInstrumentationChange = changedPaths.some((path) => + isInstrumentationModulePath(previousHost.compileResult.project.agentRoot, path), + ); if (changeEvents.length > 0) { console.log(formatChangeDetectedLogLine(previousHost.appRoot, changeEvents)); } @@ -173,7 +177,8 @@ export async function startAuthoredSourceWatcher(input: { // `POST /eve/v1/dev/schedules/:scheduleId` route, which reads // compiled registrations from disk on every request without // needing Nitro wiring. - const hasStructuralChange = hasChannelRouteChanged || hasEnvironmentChange; + const hasStructuralChange = + hasChannelRouteChanged || hasEnvironmentChange || hasInstrumentationChange; if (hasStructuralChange) { console.log(STRUCTURAL_RELOAD_LOG_LINE); diff --git a/packages/eve/src/internal/nitro/host/prepare-application-host.scenario.test.ts b/packages/eve/src/internal/nitro/host/prepare-application-host.scenario.test.ts index fc762b5af..7dba00dfb 100644 --- a/packages/eve/src/internal/nitro/host/prepare-application-host.scenario.test.ts +++ b/packages/eve/src/internal/nitro/host/prepare-application-host.scenario.test.ts @@ -51,6 +51,9 @@ describe("prepareApplicationHost", () => { expect(firstHost.compiledArtifacts.workflowWorldPluginPath).toBe( join(stableHostDirectory, "compiled-artifacts-workflow-world.mjs"), ); + expect(firstHost.compiledArtifacts.instrumentationPluginPath).toBe( + join(stableHostDirectory, "compiled-artifacts-instrumentation.mjs"), + ); expect(firstHost.compiledArtifacts.bootstrapPath).not.toContain("/.eve/dev-runtime/snapshots/"); expect(await readFile(stableBootstrapPath, "utf8")).toContain( normalizeEsmImportSpecifier(agentModulePath), diff --git a/packages/eve/test/scenarios/compiled-artifacts-bootstrap.scenario.test.ts b/packages/eve/test/scenarios/compiled-artifacts-bootstrap.scenario.test.ts index d02c82e12..4e31ffe51 100644 --- a/packages/eve/test/scenarios/compiled-artifacts-bootstrap.scenario.test.ts +++ b/packages/eve/test/scenarios/compiled-artifacts-bootstrap.scenario.test.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile, writeFile } from "node:fs/promises"; +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { pathToFileURL } from "node:url"; @@ -108,6 +108,24 @@ describe("writeCompiledArtifactsFiles", () => { expect((globalThis as Record).__eveInstrumentationLoaded).toBe("yes"); expect(instrumentationPluginModule.default()).toBeUndefined(); + + await rm(join(agentRoot, "instrumentation.ts")); + await writeCompiledArtifactsFiles({ + compileResult, + outDir, + }); + + const instrumentationPluginWithoutAuthoredModule = await readFile( + instrumentationPluginPath, + "utf8", + ); + expect(instrumentationPluginWithoutAuthoredModule).not.toContain("instrumentation.ts"); + expect(instrumentationPluginWithoutAuthoredModule).not.toContain( + "registerInstrumentationConfig", + ); + await expect( + import(`${pathToFileURL(instrumentationPluginPath).href}?case=instrumentation-removed`), + ).resolves.toMatchObject({ default: expect.any(Function) }); }); it("surfaces instrumentation import failures when the Nitro plugin module loads", async () => { diff --git a/packages/eve/test/scenarios/dev-server.scenario.test.ts b/packages/eve/test/scenarios/dev-server.scenario.test.ts index e207c02e6..c8229626b 100644 --- a/packages/eve/test/scenarios/dev-server.scenario.test.ts +++ b/packages/eve/test/scenarios/dev-server.scenario.test.ts @@ -1,6 +1,6 @@ import { spawn, type ChildProcessByStdio } from "node:child_process"; import { existsSync } from "node:fs"; -import { writeFile } from "node:fs/promises"; +import { readFile, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import type { Readable } from "node:stream"; @@ -35,6 +35,18 @@ const DEV_SERVER_AGENT_DESCRIPTOR: ScenarioAppDescriptor = { ), ), }; +const DEV_SERVER_WITH_INSTRUMENTATION_DESCRIPTOR: ScenarioAppDescriptor = { + ...DEV_SERVER_AGENT_DESCRIPTOR, + files: { + ...DEV_SERVER_AGENT_DESCRIPTOR.files, + "agent/instrumentation.ts": [ + 'import { defineInstrumentation } from "eve/instrumentation";', + "", + "export default defineInstrumentation({ events: {} });", + "", + ].join("\n"), + }, +}; interface RunningEveDev { readonly stderr: () => string; @@ -260,7 +272,7 @@ describe("eve dev server", () => { it( "rebuilds after pruning its startup runtime snapshot and completes a streamed turn", async () => { - const app = await scenarioApp(DEV_SERVER_AGENT_DESCRIPTOR); + const app = await scenarioApp(DEV_SERVER_WITH_INSTRUMENTATION_DESCRIPTOR); const server = await startEveDev(app.appRoot); try { @@ -309,7 +321,7 @@ describe("eve dev server", () => { }); expect(existsSync(startupRuntimeRoot)).toBe(false); - await writeFile(join(app.appRoot, ".env.local"), "EVE_SCENARIO_RELOAD=1\n"); + await rm(join(app.appRoot, "agent", "instrumentation.ts")); await waitForCondition( () => server.stdout().includes(STRUCTURAL_RELOAD_LOG_LINE), `Timed out waiting for a structural Nitro reload.\n\nstdout:\n${server.stdout()}\n\nstderr:\n${server.stderr()}`, @@ -357,4 +369,43 @@ describe("eve dev server", () => { }, DEV_SERVER_SCENARIO_TIMEOUT_MS, ); + + it( + "loads instrumentation added after startup", + async () => { + const app = await scenarioApp(DEV_SERVER_AGENT_DESCRIPTOR); + const markerPath = join(app.appRoot, ".instrumentation-loaded"); + const server = await startEveDev(app.appRoot); + + try { + expect(existsSync(markerPath)).toBe(false); + + await writeFile( + join(app.appRoot, "agent", "instrumentation.ts"), + [ + 'import { writeFileSync } from "node:fs";', + 'import { defineInstrumentation } from "eve/instrumentation";', + "", + "export default defineInstrumentation({", + " setup() {", + ` writeFileSync(${JSON.stringify(markerPath)}, "loaded");`, + " },", + "});", + "", + ].join("\n"), + ); + + await waitForCondition( + () => existsSync(markerPath), + `Timed out waiting for added instrumentation to load.\n\nstdout:\n${server.stdout()}\n\nstderr:\n${server.stderr()}`, + ); + + expect(await readFile(markerPath, "utf8")).toBe("loaded"); + expect(server.stdout()).toContain(STRUCTURAL_RELOAD_LOG_LINE); + } finally { + await server.stop(); + } + }, + DEV_SERVER_SCENARIO_TIMEOUT_MS, + ); });