chore(mitm): fix stub comment accuracy + broaden stub-drift guard (PR review)

Addresses findings from the multi-agent PR review of the #3066 fix:

- manager.stub.ts comments: the previous inline comment claimed the throwing ops
  (getMitmStatus/startMitm/stopMitm) are "dynamic-import paths that should never hit
  the stub at runtime" — factually wrong: those are static imports too, baked into the
  bundled build just like getAllAgentsStatus. Rewrote the file header to describe the
  real split — exports with a safe degraded value return it (getCachedPassword/
  setCachedPassword/clearCachedPassword → null/no-op, getAllAgentsStatus → []) while
  getMitmStatus/startMitm/stopMitm throw STUB_ERROR — and trimmed the inline comment.
  Comment-only; no runtime/build change (the export still exists).

- stub-drift guard test: now scans ALL of src/ instead of only src/app —
  src/lib/tailscaleTunnel.ts statically imports getCachedPassword/setCachedPassword
  from @/mitm/manager and is pulled into routes transitively, so the src/app-only scan
  had a false-negative blind spot. Also skips inline `type` imports (erased at build,
  need no runtime export) and detects stub exports from declaration AND `export { … }`
  forms (no false-positive if the stub later uses class/re-export).

Verified: next-config suite 4/4, typecheck:core / lint clean.
This commit is contained in:
diegosouzapw
2026-06-02 13:44:25 -03:00
parent 7c2dc1cde6
commit f5acb60cac
2 changed files with 37 additions and 22 deletions

View File

@@ -1,7 +1,12 @@
// Build-time stub for @/mitm/manager
// Used by Turbopack during next build to avoid native module resolution errors.
// Dynamic import() in route handlers should load the REAL manager at runtime.
// If this stub is reached at runtime, the build alias is incorrectly applied.
// Build-time stub for @/mitm/manager, aliased in by Turbopack during `next build`
// (the Docker image build) so native MITM modules aren't bundled. Routes that
// *statically* import @/mitm/manager get this stub baked in and may reach it at
// runtime in the bundled/container build. Exports that have a safe degraded value
// return it (getCachedPassword/setCachedPassword/clearCachedPassword → null/no-op,
// getAllAgentsStatus → empty list) because MITM needs host access the container
// lacks; getMitmStatus/startMitm/stopMitm throw STUB_ERROR since they can't return
// anything meaningful without the real MITM process. Routes that need real MITM at
// runtime dynamic-import @/mitm/manager.runtime (the real module) instead.
const STUB_ERROR =
"MITM manager stub reached at runtime — build alias applied incorrectly. " +
@@ -13,13 +18,9 @@ export const clearCachedPassword = () => {};
export const getMitmStatus = async () => {
throw new Error(STUB_ERROR);
};
// Statically imported by /api/tools/agent-bridge/state, so the stub MUST export it or
// the Turbopack build fails ("Export getAllAgentsStatus doesn't exist"). Unlike the
// heavy ops above (dynamic-import paths that should never hit the stub at runtime), a
// static import IS baked into the bundled build, so this is legitimately reached at
// runtime there — it returns the truthful "no agents" state (an empty list) instead of
// throwing. MITM/agent bridge needs host-level access and is otherwise non-functional
// in a bundled/container build. See issue #3066.
// Must be exported or the Turbopack build fails ("Export getAllAgentsStatus doesn't
// exist") — /api/tools/agent-bridge/state imports it statically. Returns the truthful
// empty agent list in the bundled build rather than throwing (see file header). See #3066.
export const getAllAgentsStatus = (): never[] => [];
export const startMitm = async (
_apiKey: string,

View File

@@ -109,10 +109,13 @@ test("next config declares Turbopack aliases, runtime assets and server external
test("manager.stub.ts exports every name statically imported from @/mitm/manager", async () => {
const fs = await import("node:fs");
const appDir = path.join(process.cwd(), "src", "app");
const srcDir = path.join(process.cwd(), "src");
// Collect named imports from `... from "@/mitm/manager"` (NOT manager.runtime, which
// is loaded via dynamic import() and resolves to the real module at runtime).
// Collect value names imported via `... from "@/mitm/manager"` across ALL of src/ —
// not just src/app: src/lib/tailscaleTunnel.ts imports from it and is pulled into
// routes transitively, so a src/app-only scan would miss that surface. NOT
// manager.runtime (loaded via dynamic import(), resolves to the real module at
// runtime). Inline `type` imports are erased at build time and need no stub export.
const collectImports = (dir: string, acc: Set<string>): void => {
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
const full = path.join(dir, entry.name);
@@ -126,7 +129,9 @@ test("manager.stub.ts exports every name statically imported from @/mitm/manager
let m: RegExpExecArray | null;
while ((m = re.exec(src)) !== null) {
for (const raw of m[1].split(",")) {
const name = raw.trim().split(/\s+as\s+/)[0].trim();
const token = raw.trim();
if (!token || /^type\s/.test(token)) continue; // type-only import: no runtime export needed
const name = token.split(/\s+as\s+/)[0].trim();
if (name) acc.add(name);
}
}
@@ -134,22 +139,31 @@ test("manager.stub.ts exports every name statically imported from @/mitm/manager
};
const imported = new Set<string>();
collectImports(appDir, imported);
collectImports(srcDir, imported);
// Sanity: the guard is meaningless if the scan finds nothing to check. Kept generic
// (>= 1 import) rather than asserting a specific symbol, so the test stays valid if any
// single agent-bridge/traffic-inspector route is later renamed or removed.
assert.ok(imported.size > 0, "expected at least one static @/mitm/manager import in src/app");
assert.ok(imported.size > 0, "expected at least one static @/mitm/manager import in src/");
const stubSrc = fs.readFileSync(
path.join(process.cwd(), "src", "mitm", "manager.stub.ts"),
"utf-8"
);
const stubExports = new Set(
[...stubSrc.matchAll(/export\s+(?:const|function|async\s+function)\s+([A-Za-z0-9_]+)/g)].map(
(m) => m[1]
)
);
// Collect stub exports from both declaration forms and named re-export blocks so the
// guard doesn't false-positive if the stub later uses `export class` / `export { … }`.
const stubExports = new Set<string>();
for (const m of stubSrc.matchAll(
/export\s+(?:const|let|var|class|function|async\s+function)\s+([A-Za-z0-9_]+)/g
)) {
stubExports.add(m[1]);
}
for (const m of stubSrc.matchAll(/export\s*\{([^}]*)\}/g)) {
for (const part of m[1].split(",")) {
const exported = part.trim().split(/\s+as\s+/).pop()?.trim(); // `x as y` exports y
if (exported) stubExports.add(exported);
}
}
const missing = [...imported].filter((name) => !stubExports.has(name));
assert.deepEqual(