Uh oh!
There was an error while loading. Please reload this page.
[APPS-2792] Add: wire local execution into the real dev server - #481
Conversation
This comment has been minimized.
This comment has been minimized.
6e85225 to
ae53df1Compare…function body server.ssrLoadModule(func.absolutePath) goes through the same transform hook (vite/index.ts) that rewrites *.backend.ts into the client-side RPC-proxy stub — so local execution's "real" import can actually still be the proxy stub, which crashes since globalThis.DD_APPS_RUNTIME doesn't exist server-side. Every existing test here mocks loadModule directly, so none of them exercise the real transform pipeline and would catch this. Append the same query-suffix marker introduced in #481 (matching Vite's own ?raw/?url convention) so the shared transform hook can recognize this specific request and skip proxy generation for it. The transform-hook side of this fix lives in #481, since that's where local execution is actually wired to a real, plugin-registered dev server — this PR only needs its own call site and mocks to stay consistent with that contract so the two branches reconcile cleanly whichever merges first.
7cbbeec to
d976f85Compare…function body server.ssrLoadModule(func.absolutePath) goes through the same transform hook (vite/index.ts) that rewrites *.backend.ts into the client-side RPC-proxy stub — so local execution's "real" import can actually still be the proxy stub, which crashes since globalThis.DD_APPS_RUNTIME doesn't exist server-side. Every existing test here mocks loadModule directly, so none of them exercise the real transform pipeline and would catch this. Append the same query-suffix marker introduced in #481 (matching Vite's own ?raw/?url convention) so the shared transform hook can recognize this specific request and skip proxy generation for it. The transform-hook side of this fix lives in #481, since that's where local execution is actually wired to a real, plugin-registered dev server — this PR only needs its own call site and mocks to stay consistent with that contract so the two branches reconcile cleanly whichever merges first.
d976f85 to
1900a78Compare1900a78 to
a0bcc4fCompareWires the new direct-import local-execution path (local-execution.ts) into the real Vite dev server: threads server.ssrLoadModule through as the loadModule dependency, drops the bundling step from /__dd/executeAction entirely (debugBundle and executeActionViaCloud still bundle, unchanged), and forwards connectionId end-to-end through makeExecuteActionRemotely so a $.Actions call naming a specific connection actually reaches it instead of being silently dropped. Also forces @datadog/apps-backend and @datadog/action-catalog through Vite's SSR transform pipeline (ssr.noExternal) rather than letting the dev server's default node_modules externalization `require()` them directly -- both ship ESM-only, so an externalized `require()` throws "Cannot use import statement outside a module".
dev-server.test.ts and local-execution.test.ts (build-plugins#480/#484) each defined their own near-identical LoadModule resolver double. Factor the common resolve-or-throw logic into moduleResolverFor in the shared mocks helper so both can build on it instead of duplicating it.
…C-proxy transform server.ssrLoadModule(func.absolutePath) went through the same transform hook that rewrites *.backend.ts into the client-side RPC-proxy stub (globalThis.DD_APPS_RUNTIME.executeBackendFunction(...)) — so local execution's "real" import was actually still the proxy stub, which crashes immediately since that global doesn't exist server-side. Every existing test mocked loadModule directly, so none of them exercised the real transform pipeline and caught this. Mark local execution's own load with a query suffix (matching Vite's own ?raw/?url convention) and have the transform hook skip proxy generation for that specific marked request, deferring to Vite's normal TS/esbuild transform instead. Checking the marker rather than the generic Vite-supplied options.ssr flag keeps this from also affecting any other, unrelated future SSR-context load of the same file.
…return The transform handler's this.parse(code) call ran after the LOCAL_EXECUTION_LOAD_SUFFIX early-return, so any static safety check inserted at that point (e.g. rejecting Node builtin imports) would silently never run for the one path that actually executes the code. Moving the parse ahead of the early-return closes that gap and makes a future merge with such a check conflict loudly instead of merging clean. Also corrects dev-server.integration.test.ts's doc comment, which claimed to exercise the transform hook's production wiring even though its createServer() call never registers getVitePlugin()'s plugin.
…tch) submitQuery's doc comment referenced executeSingleActionRemotely, a function that never existed under that name — the real one is makeExecuteActionRemotely, already correctly named elsewhere in this file. mockLoadModuleReturning's fn parameter used any[] as an escape hatch; (...args: never[]) => unknown accepts the same range of test callback signatures without it, since never is assignable to any parameter type a caller's own lambda declares.
a0bcc4f to
dc33400Comparetyffical
commented
Aug 21, 2026
There was a problem hiding this comment.
Pull request overview
Friend, this PR wires backend functions into Vite’s in-process local execution path while retaining cloud execution separately.
Changes:
- Routes
/__dd/executeActionlocally and adds the cloud-specific endpoint. - Preserves real backend source during local Vite loading.
- Adds action connection forwarding and regression coverage.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/tests/src/_jest/helpers/mocks.ts | Adds a shared module resolver mock. |
packages/plugins/apps/src/constants.ts | Defines the local-load marker. |
packages/plugins/apps/src/vite/local-execution.ts | Loads marked backend modules. |
packages/plugins/apps/src/vite/local-execution.test.ts | Updates module-loading tests. |
packages/plugins/apps/src/vite/index.ts | Configures SSR loading and middleware. |
packages/plugins/apps/src/vite/index.test.ts | Tests transforms and SSR configuration. |
packages/plugins/apps/src/vite/dev-server.ts | Splits local and cloud execution. |
packages/plugins/apps/src/vite/dev-server.test.ts | Tests both execution routes. |
packages/plugins/apps/src/vite/dev-server.integration.test.ts | Exercises real Vite module loading. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:dc334004da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Uh oh!
There was an error while loading. Please reload this page.
…-import gap executeScriptViaDatadog's outputs return no longer needs an `as BackendOutputs` cast — TypeScript already structurally narrows outputs to the right shape after the preceding object/null/'data' in outputs guard; returning it directly keeps the compiler checking the contract instead of asserting past it. Also documents (doesn't yet fix) a real gap found in review: the LOCAL_EXECUTION_LOAD_SUFFIX marker only reaches the entry module ssrLoadModule requests — a *.backend.ts file statically importing another one would have that nested import replaced with the frontend RPC-proxy stub instead of running for real. Deferred until backend files importing each other is an actual pattern in use.
tyffical
commented
Aug 21, 2026
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/plugins/apps/src/vite/index.test.ts:231
- The added lint suppression and
anycast remove type checking from the config-hook contract this test is intended to verify. Narrow the plugin and hook at runtime instead so an unexpected plugin shape fails clearly without bypassing TypeScript.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const config = (plugin as any).config();
| filter: { | ||
| id: { | ||
| include: [BACKEND_FILE_RE], | ||
| include: [BACKEND_FILE_RE, LOCAL_EXECUTION_LOAD_RE], |
| server.middlewares.use( | ||
| createDevServerMiddleware( | ||
| bundler.build, | ||
| server.ssrLoadModule.bind(server), |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:b9470c1614
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| log.debug(`Executing action locally: ${displayName} with args`); | ||
| const executeAction = makeExecuteActionRemotely(auth, doAuthenticatedRequest, log); | ||
| const result = await executeScriptLocally(func, args, executeAction, loadModule, log); |
There was a problem hiding this comment.
Allow remote action polling past ten seconds
When a locally executed function awaits $.Actions, makeExecuteActionRemotely may spend up to roughly 30 seconds in a single long-poll request, but this call invokes executeScriptLocally with its fixed 10-second default timeout. Any otherwise-valid action taking longer than ten seconds therefore returns a 500 timeout from /__dd/executeAction while its API request continues in the background; pass an action-aware timeout or reconcile the local timeout with the polling lifecycle.
Useful? React with 👍 / 👎.
…ports LOCAL_EXECUTION_LOAD_SUFFIX only reached the one entry module ssrLoadModule was called with. A .backend.ts file statically importing another .backend.ts file resolved that nested import unsuffixed, hitting the transform hook's "not suffixed" branch and getting replaced with the frontend RPC-proxy stub — calling globalThis.DD_APPS_RUNTIME, which doesn't exist server-side, instead of running the real code. Adds a resolveId hook that propagates the suffix onto a nested .backend.ts import when its importer was itself suffixed. The hook has to run with order: 'pre' — a plain relative specifier is fully resolvable by Vite's own built-in resolver at the default order, which would otherwise resolve and short-circuit the plugin hook chain before this one ever saw it. Verified against a real Vite dev server with the actual plugin registered (not a mocked resolve), since the bug is specifically about Vite's own resolution behavior — a loadModule test double can't reproduce it.
Motivation
/__dd/executeActionstill bundles and calls the full cloud round trip. This PR is what actually makesnpm run devfast: it swaps the customer-facing endpoint over to the in-process path and drops the bundling step from it entirely.$.Actionscalls droppedconnectionIdend-to-end, so an action naming a specific connection had no way to reach it. Now threaded throughExecuteAction→makeExecuteActionRemotely→ the single-actionpreview-asyncquery spec.server.ssrLoadModuleruns through the same transform hook that rewrites.backend.tsinto the client-side RPC-proxy stub for frontend imports — so local execution's "real" import was actually still that stub, which crashes since the stub callsglobalThis.DD_APPS_RUNTIME, a browser-only global that doesn't exist server-side. Invisible to every test in this stack, since they all inject a mockedloadModulethat never touches the real transform pipeline.Architecture
createDevServerMiddlewarenow routes the two execution endpoints down genuinely different paths — one bundle-free and in-process, one bundling and cloud-bound — that only reconverge at the sharedsubmitQuery/pollQueryExecutionhelpers once an$.Actionscall needs to reach the real Datadog API:The
executeActionpath never bundles at all —executeScriptLocallyimports the customer's real file directly vialoadModule(Vite's ownssrLoadModule, so it gets the same TS-transform/resolve rules and HMR-aware module cache a real request gets) and runs the exported function in this process. No auth check happens until the function actually calls$.Actions; that call becomes its own direct single-actionpreview-asyncquery viamakeExecuteActionRemotely, rather than being wrapped in a whole-script query. TheexecuteActionViaCloudpath is the unchanged production round trip: bundle the whole function with Rollup, wrap it as ajsFunctionWithActionsquery, and submit/poll it the same way. See the RFC's Proposed Solution for the design-level version of this split.server.ssrLoadModuleshares the same transform pipeline as every other module Vite serves — includingvite/index.ts's own.backend.ts→ RPC-proxy transform, which exists for frontend imports of the same file. Local execution'sloadModulecall marks its own request with a query suffix (LOCAL_EXECUTION_LOAD_SUFFIX, matching Vite's own?raw/?urlconvention) so the transform hook can skip proxy generation specifically for that request, rather than for every SSR-context load of a.backend.tsfile (which would also silently affect any unrelated future feature hitting the same hook).Changes
/__dd/executeActionnow looks up the requested function and runs it directly viaexecuteScriptLocally— no bundling on this path at all./__dd/debugBundleand the cloud round trip (/__dd/executeActionViaCloud) are unchanged and still bundle.makeExecuteActionRemotelynow forwardsconnectionIdinto the single-actionpreview-asyncquery spec ({fqn, inputs, connectionId}) instead of silently dropping it.createDevServerMiddlewaretakes a newloadModule: LoadModuleparameter, threaded fromvite/index.ts'sconfigureServer(server)asserver.ssrLoadModule.bind(server)— the real Vite dev server's own module loader, giving the local path the same TS-transform/resolve rules and HMR-aware module cache a real request gets.config()hook returningssr: { noExternal: [...] }for@datadog/apps-backend/@datadog/action-catalog. Found while testing: both ship ESM-only, and Vite's dev server externalizesnode_modulesby default (a plainrequire(), for speed) — which throwsCannot use import statement outside a modulethe first time a customer's function actually uses either SDK locally.noExternalforces Vite's SSR transform pipeline to handle them instead, matching how the production bundling path already inlines every dependency.LOCAL_EXECUTION_LOAD_SUFFIX/LOCAL_EXECUTION_LOAD_RE: local execution'sloadModulecall now marks its request with this suffix so the transform hook can tell it apart from a normal frontend import of the same file and skip generating the RPC-proxy stub — fixes the bug described in Motivation.loadModuletest double — previously hand-rolled separately in this file and in #480/#484'slocal-execution.test.ts— into a sharedmoduleResolverForhelper.createServer, middleware mode, no port bound) rooted at the sameapps_backend_projectfixture, and lets its realssrLoadModuleimport a real.backend.tsfile directly — no mocked bundler, no mockedloadModule. Confirms a real@datadog/apps-backendtyped import resolves$.Sourcecorrectly through this exact path.$.Actions), a clear error when a function does call$.Actionswith no auth configured, the single-actionpreview-asyncrequest-body shape now includingconnectionId, and the newconfig()hook'sssr.noExternalcontract. Existing cloud-path tests unchanged aside from the newloadModuleparameter threaded through everycreateDevServerMiddlewarecall.QA Instructions
yarn test:unit packages/plugins/apps # Expected: Test Suites: 24 passed / Tests: 316 passed ✅ VERIFIEDyarn workspace @dd/apps-plugin run typecheck # Expected: no output, clean exit ✅ VERIFIEDnpx eslint packages/plugins/apps/src/vite/dev-server.ts packages/plugins/apps/src/vite/dev-server.test.ts packages/plugins/apps/src/vite/dev-server.integration.test.ts packages/plugins/apps/src/vite/index.ts packages/plugins/apps/src/vite/index.test.ts packages/plugins/apps/src/vite/local-execution.ts packages/plugins/apps/src/vite/local-execution.test.ts packages/plugins/apps/src/constants.ts packages/tests/src/_jest/helpers/mocks.ts --quiet # Expected: no output, clean exit ✅ VERIFIEDManual QA — real scaffolded app, real dev server (both local and staging)
Built and
npm link'd this branch's@datadog/vite-plugin, scaffolded a fresh app (npm create @datadog/apps@latest), linked the local build in, added real backend functions (a basic doubling function, aconsole.log-emitting function, a deliberate-throw function):Local (
npm run dev,POST /__dd/executeAction):{"success":true,"result":{"data":{"doubled":42}}}Confirmed sub-millisecond timing (no cloud round trip), real-time
console.logstreaming to the terminal (not the browser console, not delayed), and a deliberatethrowreturning a clean{"success":false,"error":"..."}rather than a crash — all against the fixed code. Against the pre-fix code, every one of these failed identically withCannot read properties of undefined (reading 'executeBackendFunction'), which is what led to the fix in this PR. ✅ VERIFIEDStaging (
dd-auth --domain dd.datad0g.com -- npm run dev, realpreview-asyncround trip via/__dd/executeActionViaCloud): this combination is confirmed working — see #473's manual QA section for a real, verified run with actual staging responses.A durable writeup of this QA flow (including the local↔staging↔app-builder-code architecture) is being consolidated into a Confluence guide, linked from here once published.
Blast Radius
npm run dev's/__dd/executeActionnow executes locally by direct import, with no bundling step, instead of round-tripping to the cloud. Still gated behind this whole stack not being released yet (no version bump, nobump.yamltrigger in this PR)./__dd/executeActionViaCloud) — nothing currently calling/__dd/executeActionin production exists yet (this endpoint isn't released), so there's no live caller to break.ssr.noExternalconfig change affects every Vite dev-server session this plugin runs in, not just the local-execution path — low risk in practice (it only forces two specific, already-known-to-this-plugin packages through the transform pipeline instead of externalizing them), but worth noting as a config-surface change.LOCAL_EXECUTION_LOAD_SUFFIXtransform-hook change only special-cases requests carrying that exact marker — no behavior change for any existing frontend import of a.backend.tsfile.LOCAL_EXECUTION_LOAD_SUFFIXcall-site change, so stacking lets that shared history reconcile once via rebase instead of as a merge conflict whichever PR landed second.Out of Scope / Follow-ups
npm run dev:verifyCLI (mode-aware routing to/__dd/executeActionViaCloud, web-ui template changes)@datadog/action-catalogfixture package for a typed-import e2e test$.Actionsrouting8292a6aa, added after this branch diverged), which was failing CI's auto-merge checkDocumentation