Skip to content

Commit a4467ef

Browse files
committed
fix(integrations): pick up changed backend code on reload without an app-api restart
reloadIntegrations() fully tears down and re-imports every integration, but backendEntryImportSpecifier only cache-busted the dynamic import in dev — in prod it returned the process-lifetime-cached ESM module, so a store update of an integration whose bundle path is unchanged silently kept running the old code until app-api restarted (the same silent-staleness class as the GHCR pull bug). Cache-bust on the entry file's mtime+size in prod too. Keying on mtime+size means an unchanged file reuses its URL (cached module, no leak); only a changed file mints a new URL, so the unavoidable (ESM modules are never GC'd) per-import leak is bounded to one tiny module per actual code change — not per reload — and credential-rotation reloads (no code change) leak nothing. The dev-only /api/integrations/reload endpoint stays dev-gated. Adds a NODE_ENV=production regression test (mutates a community bundle between init and reload, asserts the new code loads).
1 parent c494ed6 commit a4467ef

2 files changed

Lines changed: 78 additions & 13 deletions

File tree

apps/api/src/integration-host.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
* All tests exercise only the public API; no internals are imported or patched.
77
*/
88

9+
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
10+
import { tmpdir } from "node:os";
911
import { join } from "node:path";
1012
import { fileURLToPath } from "node:url";
1113
import Fastify, { type FastifyInstance } from "fastify";
@@ -339,3 +341,58 @@ describe("shutdownIntegrations", () => {
339341
expect(getAllIntegrations()).toHaveLength(0);
340342
});
341343
});
344+
345+
// Regression guard for the prod cache-bust: a store update of a community
346+
// integration whose bundle path is unchanged must take effect on reload WITHOUT
347+
// restarting app-api. Runs under NODE_ENV=production specifically — the old
348+
// behavior skipped cache-busting in prod, so the re-import returned the stale
349+
// module and this test would fail.
350+
describe("reloadIntegrations — re-imports changed backend code in production", () => {
351+
it("picks up a rewritten community bundle on reload (no restart)", async () => {
352+
const prevEnv = process.env.NODE_ENV;
353+
const parent = mkdtempSync(join(tmpdir(), "omx-reload-probe-"));
354+
const intgDir = join(parent, "reload-probe");
355+
const bundleDir = join(intgDir, "dist", "backend");
356+
mkdirSync(bundleDir, { recursive: true });
357+
writeFileSync(
358+
join(intgDir, "manifest.json"),
359+
JSON.stringify({
360+
id: "reload-probe",
361+
version: "1.0.0",
362+
license: "MIT",
363+
domains: ["knowledge"],
364+
backend: { routes: false },
365+
quality: "community-verified",
366+
}),
367+
);
368+
const bundleFile = join(bundleDir, "index.mjs");
369+
// Marker is set at MODULE EVALUATION time, so it only changes if the module
370+
// is actually re-imported (a fresh URL) — not merely re-`setup()`-ed.
371+
const writeBundle = (marker: string) =>
372+
writeFileSync(
373+
bundleFile,
374+
`globalThis.__reloadProbeVersion = ${JSON.stringify(marker)};\nexport function setup() {}\n`,
375+
);
376+
377+
const app = makeApp();
378+
const g = globalThis as Record<string, unknown>;
379+
try {
380+
process.env.NODE_ENV = "production";
381+
382+
writeBundle("v1");
383+
await initIntegrations(app, [{ directory: parent, isBuiltIn: false }]);
384+
expect(g.__reloadProbeVersion).toBe("v1");
385+
386+
// Rewrite the bundle. Different content AND length so the mtime+size key
387+
// changes even on a coarse-mtime filesystem.
388+
writeBundle("v2-rewritten-and-longer");
389+
await reloadIntegrations();
390+
expect(g.__reloadProbeVersion).toBe("v2-rewritten-and-longer");
391+
} finally {
392+
if (prevEnv === undefined) delete process.env.NODE_ENV;
393+
else process.env.NODE_ENV = prevEnv;
394+
rmSync(parent, { recursive: true, force: true });
395+
delete g.__reloadProbeVersion;
396+
}
397+
});
398+
});

apps/api/src/integration-host.ts

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -228,21 +228,29 @@ function resolveBackendEntryPoint(directory: string, isBuiltIn: boolean): string
228228
return existsSync(jsModulePath) ? jsModulePath : null;
229229
}
230230

231-
// In dev we mtime-bust so editing `index.ts` and hitting `/api/integrations/reload`
232-
// picks up the change. In prod we never cache-bust — ESM `import()` is
233-
// process-lifetime cached, so re-importing the same file URL would return the
234-
// old module *and* leak nothing. Production updates require restarting app-api
235-
// to load fresh backend code; the install/update job handlers say so in their
236-
// logs.
231+
// Cache-bust the dynamic import by the entry file's mtime+size so a reload
232+
// (`reloadIntegrations` — fired by extension install/update and credential
233+
// set/delete) picks up CHANGED backend code WITHOUT restarting app-api. ESM
234+
// `import()` is keyed by URL and process-lifetime cached, so re-importing the
235+
// same file URL returns the stale module — which is why a store update of an
236+
// integration whose bundle path is unchanged otherwise silently keeps running
237+
// the old code until a restart.
238+
//
239+
// Keying the bust on mtime+size (in dev AND prod) means an UNCHANGED file
240+
// reuses the same URL → the cached module is returned, no new module is created;
241+
// only a CHANGED file mints a new URL → a fresh import. The cost is that ESM
242+
// modules are never garbage-collected, so each new URL leaks its (tiny,
243+
// self-contained) module graph for the process lifetime — but the leak is now
244+
// bounded to one module per actual code change (reloads with no code change,
245+
// e.g. a credential rotation, reuse the cached URL and leak nothing), which over
246+
// a process lifetime is negligible and well worth not requiring a restart.
237247
function backendEntryImportSpecifier(entryPoint: string): string {
238248
const url = pathToFileURL(entryPoint);
239-
if (process.env.NODE_ENV !== "production") {
240-
try {
241-
const stats = statSync(entryPoint);
242-
url.searchParams.set("v", `${stats.mtimeMs}-${stats.size}`);
243-
} catch {
244-
url.searchParams.set("v", Date.now().toString());
245-
}
249+
try {
250+
const stats = statSync(entryPoint);
251+
url.searchParams.set("v", `${stats.mtimeMs}-${stats.size}`);
252+
} catch {
253+
url.searchParams.set("v", Date.now().toString());
246254
}
247255
return url.href;
248256
}

0 commit comments

Comments
 (0)