fix(api): dynamic import for MITM + fix Turbopack over-bundling warnings - #6366
diegosouzapw merged 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the server route for the agent bridge to dynamically import startMitm and stopMitm from the runtime manager during action execution. The review feedback correctly identifies that getMitmStatus should also be dynamically imported to avoid Turbopack stubbing issues, and notes a potential bug where the cached password is lost during a restart. Additionally, the feedback points out that tests must be included when modifying production code, in accordance with the repository style guide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| if (action === "restart") { | ||
| const pwd = sudoPassword || getCachedPassword() || ""; | ||
| const { startMitm, stopMitm } = await import("@/mitm/manager.runtime"); |
There was a problem hiding this comment.
There are two correctness issues in this restart block:\n\n1. Stubbed getMitmStatus: The statically imported getMitmStatus will resolve to the stubbed version (manager.stub.ts) due to the Turbopack alias. Since the stub cannot access native APIs, it will return a fallback status (e.g., running: false), causing the restart action to skip stopping the active MITM server and then fail when attempting to start it again.\n * Fix: Dynamically import getMitmStatus from @/mitm/manager.runtime along with startMitm and stopMitm.\n\n2. Cached Password Cleared: Calling stopMitm(pwd) internally calls clearCachedPassword(). If the user triggered restart without providing a sudoPassword in the request body (relying on the cached password), the cached password will be lost after stopMitm runs. The subsequent startMitm will succeed using the local pwd variable, but any future actions requiring the cached password will fail.\n * Fix: Re-cache the password after stopping by changing if (sudoPassword) setCachedPassword(sudoPassword); to if (sudoPassword || pwd) setCachedPassword(sudoPassword || pwd);.
| const { startMitm, stopMitm } = await import("@/mitm/manager.runtime"); | |
| const { startMitm, stopMitm, getMitmStatus } = await import("@/mitm/manager.runtime"); |
| */ | ||
| import { AgentBridgeServerActionSchema } from "@/shared/schemas/agentBridge"; | ||
| import { startMitm, stopMitm, getMitmStatus, setCachedPassword, getCachedPassword } from "@/mitm/manager"; | ||
| import { getCachedPassword, setCachedPassword, getMitmStatus } from "@/mitm/manager"; |
There was a problem hiding this comment.
Since getMitmStatus is only used in the restart action and must be dynamically imported from @/mitm/manager.runtime to avoid the Turbopack stub, it should be removed from the static imports here.
| import { getCachedPassword, setCachedPassword, getMitmStatus } from "@/mitm/manager"; | |
| import { getCachedPassword, setCachedPassword } from "@/mitm/manager"; |
| try { | ||
| if (action === "start") { | ||
| if (sudoPassword) setCachedPassword(sudoPassword); | ||
| const { startMitm } = await import("@/mitm/manager.runtime"); |
There was a problem hiding this comment.
According to the Repository Style Guide (Rule 9), changing production code under src/ requires including tests. Please ensure that appropriate unit or integration tests are added or updated in the tests/ directory to cover these changes (e.g., verifying the dynamic import behavior and the restart action).
References
- Always include tests when changing production code (src/, open-sse/, electron/, bin/). (link)
560c8da to
bdc038f
Compare
…ling warnings MITM stub error (main blocker): The agent-bridge/server route statically imported startMitm, stopMitm, and getMitmStatus from @/mitm/manager, which Turbopack aliases to manager.stub.ts during build. The stub throws STUB_ERROR on start/stop and returns stubbed status, causing the AgentBridge dashboard to crash. Switch to dynamic import of @/mitm/manager.runtime (bypasses the alias) for start, stop, and restart actions. Static imports for getCachedPassword and setCachedPassword remain on the stub (safe degraded behavior). Also re-cache password after stopMitm in restart action, since stopMitm calls clearCachedPassword() internally. Turbopack warnings (379 warnings about 33,530 files): generator.ts used path.resolve(process.cwd(), outputDir) which Turbopack cannot resolve statically, causing it to trace the entire project root. Switch to path.join() for explicit path construction. Added unit tests verifying the dynamic import path resolves the real MITM module (not the stub). Closes diegosouzapw#6329
bdc038f to
a61ff1b
Compare
…k anchor bullet Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
|
Merged — thank you, @Iammilansoni! The agent-bridge route now dynamically imports |
…ssion) #6366 switched the output base from path.resolve to path.join(process.cwd(), outputDir) to keep Turbopack's static analyzer from tracing the project root. path.join mangles an absolute outputDir (a tmp dir in the generator tests) into cwd/tmp/…, so apply mode reported success while writing nothing at the expected path. Guard with path.isAbsolute — absolute paths pass through, the relative production case ('skills') keeps the Turbopack-friendly join form. The existing agentSkills-generator suite is the regression guard (now 21/21).
…auto-resolve, add captain pre-flight fixes, Contributors hall (Phase 0a.1/0a.3a) Repairs ~29 [3.8.46] bullets whose (#N) PR links were stripped by a prior merge conflict auto-resolve, adds bullets for the captain's own pre-flight base-red fixes (#6408 catalog cache, #6366 agentSkills, doubao CodeQL), folds the ~53 v3.8.45 sync-back carryover commits (already documented under [3.8.45]) into a coverage-satisfying Maintenance note, consolidates the duplicate New Features block + normalizes the Bug Fixes heading, and injects the mandatory ### 🙌 Contributors table (45 external contributors). Coverage 90→13 uncovered (the 13 are zero-ref release plumbing). Syncs all 42 i18n mirrors.
…ngs (diegosouzapw#6366) Dynamic MITM manager import on the agent-bridge route + Turbopack static-analyzer anchor in the skills generator (diegosouzapw#6329). Integrated into release/v3.8.46.
…w#6366 regression) diegosouzapw#6366 switched the output base from path.resolve to path.join(process.cwd(), outputDir) to keep Turbopack's static analyzer from tracing the project root. path.join mangles an absolute outputDir (a tmp dir in the generator tests) into cwd/tmp/…, so apply mode reported success while writing nothing at the expected path. Guard with path.isAbsolute — absolute paths pass through, the relative production case ('skills') keeps the Turbopack-friendly join form. The existing agentSkills-generator suite is the regression guard (now 21/21).
…auto-resolve, add captain pre-flight fixes, Contributors hall (Phase 0a.1/0a.3a) Repairs ~29 [3.8.46] bullets whose (#N) PR links were stripped by a prior merge conflict auto-resolve, adds bullets for the captain's own pre-flight base-red fixes (diegosouzapw#6408 catalog cache, diegosouzapw#6366 agentSkills, doubao CodeQL), folds the ~53 v3.8.45 sync-back carryover commits (already documented under [3.8.45]) into a coverage-satisfying Maintenance note, consolidates the duplicate New Features block + normalizes the Bug Fixes heading, and injects the mandatory ### 🙌 Contributors table (45 external contributors). Coverage 90→13 uncovered (the 13 are zero-ref release plumbing). Syncs all 42 i18n mirrors.
…ngs (diegosouzapw#6366) Dynamic MITM manager import on the agent-bridge route + Turbopack static-analyzer anchor in the skills generator (diegosouzapw#6329). Integrated into release/v3.8.46.
…w#6366 regression) diegosouzapw#6366 switched the output base from path.resolve to path.join(process.cwd(), outputDir) to keep Turbopack's static analyzer from tracing the project root. path.join mangles an absolute outputDir (a tmp dir in the generator tests) into cwd/tmp/…, so apply mode reported success while writing nothing at the expected path. Guard with path.isAbsolute — absolute paths pass through, the relative production case ('skills') keeps the Turbopack-friendly join form. The existing agentSkills-generator suite is the regression guard (now 21/21).
Summary
Changes
1. MITM stub error (main blocker)
src/app/api/tools/agent-bridge/server/route.ts: SwitchstartMitm/stopMitmto dynamicawait import("@/mitm/manager.runtime")forstart,stop, andrestartactions. Static imports forgetCachedPassword,setCachedPassword,getMitmStatusremain on the stub (safe degraded behavior).2. Turbopack over-bundling warnings
src/lib/agentSkills/generator.ts: Changedpath.resolve(process.cwd(), outputDir)topath.join(process.cwd(), outputDir). Turbopack's static analyzer cannot resolvepath.resolve()withprocess.cwd(), causing it to fall back to tracing the entire project root (33,530 files).path.join()produces a literal path the analyzer can resolve.Context
The
@/mitm/manager->manager.stub.tsalias innext.config.mjsis intentional — it prevents Turbopack from bundling nativechild_process/fsimports duringnext build. The stub provides safe fallbacks for status/password functions but correctly throws forstartMitm/stopMitm. Routes that need the real implementation (likesettings/mitm/route.ts) already use dynamic imports of@/mitm/manager.runtimeto bypass the alias. This PR applies the same pattern to the agent-bridge server route.For the Turbopack warnings:
generator.tsis dynamically imported byagent-skills/generate/route.ts, so it IS part of the build. Thepath.resolve(process.cwd(), ...)pattern produces a path that Turbopack cannot statically analyze, triggering the broad pattern warning.Closes #6329