From 20f63432e189f3f2ae8a381a4807ec404eaf9a7d Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 27 Aug 2026 01:37:38 -0400 Subject: [PATCH 01/15] feat(apps): in-process local execution for backend functions Executes a backend function's file directly in-process inside the Vite dev server, mirroring executeScriptViaDatadog's BackendOutputs contract as a drop-in alternate implementation for local dev. --- packages/plugins/apps/src/backend/types.ts | 3 + packages/plugins/apps/src/constants.ts | 4 + packages/plugins/apps/src/vite/dev-server.ts | 7 +- packages/plugins/apps/src/vite/index.test.ts | 43 ++ packages/plugins/apps/src/vite/index.ts | 14 +- .../apps/src/vite/local-execution.test.ts | 563 ++++++++++++++++++ .../plugins/apps/src/vite/local-execution.ts | 210 +++++++ 7 files changed, 836 insertions(+), 8 deletions(-) create mode 100644 packages/plugins/apps/src/vite/local-execution.test.ts create mode 100644 packages/plugins/apps/src/vite/local-execution.ts diff --git a/packages/plugins/apps/src/backend/types.ts b/packages/plugins/apps/src/backend/types.ts index edce3b5f6..e528255d5 100644 --- a/packages/plugins/apps/src/backend/types.ts +++ b/packages/plugins/apps/src/backend/types.ts @@ -12,3 +12,6 @@ export interface BackendFunction { /** Connection IDs this backend function is allowed to use. */ allowedConnectionIds: string[]; } + +/** Shared result shape for both the remote (dev-server.ts) and in-process (local-execution.ts) execution paths. */ +export type BackendOutputs = { data: unknown }; diff --git a/packages/plugins/apps/src/constants.ts b/packages/plugins/apps/src/constants.ts index db612df45..3d5866c72 100644 --- a/packages/plugins/apps/src/constants.ts +++ b/packages/plugins/apps/src/constants.ts @@ -10,6 +10,10 @@ export const PLUGIN_NAME: PluginName = 'datadog-apps-plugin' as const; export const APPS_API_PATH = 'api/unstable/app-builder-code/apps'; export const ARCHIVE_FILENAME = 'datadog-apps-assets.zip'; export const BACKEND_FILE_RE = /\.backend\.(ts|tsx|js|jsx)$/; + +/** Query suffix marking a local-execution load, so the transform hook below can skip proxy generation for it instead of matching via the broader `options.ssr` flag. */ +export const LOCAL_EXECUTION_LOAD_SUFFIX = '?dd-local-exec'; +export const LOCAL_EXECUTION_LOAD_RE = /\.backend\.(ts|tsx|js|jsx)\?dd-local-exec$/; export const BACKEND_CODE_EXTENSIONS = [ '.ts', '.tsx', diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index 2cd818b74..5c9634a92 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -13,7 +13,7 @@ import { AUTH_GUIDANCE } from '../auth'; import type { DoAuthenticatedRequest } from '../auth'; import { encodeQueryName } from '../backend/encodeQueryName'; import type { ExecuteActionRequest, ExecuteActionResponse } from '../backend/protocol'; -import type { BackendFunction } from '../backend/types'; +import type { BackendFunction, BackendOutputs } from '../backend/types'; import { generateDevVirtualEntryContent } from '../backend/virtual-entry'; import { createBackendConnectionIdCollector } from './backend-connection-id-collector'; @@ -31,11 +31,6 @@ const DEV_VIRTUAL_PREFIX = 'virtual:dd-backend-dev:'; type AuthConfig = AuthOptionsWithDefaults; -/** Shape of the `outputs` field in a Datadog app-builder query response — - * the API wraps a JS action's return value as `{ data: }`. - */ -type BackendOutputs = { data: unknown }; - /** * Format a BackendFunction for display in log/error messages. */ diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index d3965f072..6ba1b839f 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -12,6 +12,7 @@ import { parseAst } from 'rollup/parseAst'; import { encodeQueryName } from '../backend/encodeQueryName'; import type { BackendFunction } from '../backend/types'; +import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; type TransformHandler = (code: string, id: string) => unknown; @@ -235,6 +236,48 @@ describe('Backend Functions - getVitePlugin', () => { expect(mockLogFn).toHaveBeenCalledWith(expect.stringContaining('Intl'), 'warn'); }); + // Regression test: without the suffix check, ssrLoadModule() would get the RPC-proxy stub instead of the real function body. + test('Should skip proxy generation for a suffixed local-execution load, returning the real source untouched', async () => { + const plugin = getVitePlugin(defaultOptions); + const handler = getTransformHandler(plugin); + + const realSource = 'export function myHandler() { return 42; }'; + const result = await handler.call( + { + parse: parseAst, + resolve: jest.fn(async () => null), + load: jest.fn(async () => null), + addWatchFile: jest.fn(), + }, + realSource, + `/build/src/backend/myHandler.backend.ts${LOCAL_EXECUTION_LOAD_SUFFIX}`, + ); + + expect(result).toBeNull(); + }); + + test('Should still generate the frontend RPC-proxy for a normal (unsuffixed) import of the same file', async () => { + const plugin = getVitePlugin(defaultOptions); + const handler = getTransformHandler(plugin); + + const result = await handler.call( + { + parse: parseAst, + resolve: jest.fn(async () => null), + load: jest.fn(async () => null), + addWatchFile: jest.fn(), + }, + 'export function myHandler() { return 42; }', + '/build/src/backend/myHandler.backend.ts', + ); + + expect( + typeof result === 'object' && result !== null && 'code' in result + ? result.code + : undefined, + ).toEqual(expect.stringContaining('executeBackendFunction')); + }); + test('Should inject the apps runtime', () => { getVitePlugin(defaultOptions); diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index c31da9b63..8b26d13bc 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -20,7 +20,12 @@ import { ensureProgram } from '../backend/ast-parsing/type-guards'; import { encodeQueryName } from '../backend/encodeQueryName'; import { generateProxyModule } from '../backend/proxy-codegen'; import type { BackendFunction } from '../backend/types'; -import { BACKEND_FILE_RE, PLUGIN_NAME } from '../constants'; +import { + BACKEND_FILE_RE, + LOCAL_EXECUTION_LOAD_RE, + LOCAL_EXECUTION_LOAD_SUFFIX, + PLUGIN_NAME, +} from '../constants'; import type { AppsOptionsWithDefaults } from '../types'; import { buildBackendFunctions } from './build-backend-functions'; @@ -124,7 +129,7 @@ export const getVitePlugin = ({ transform: { filter: { id: { - include: [BACKEND_FILE_RE], + include: [BACKEND_FILE_RE, LOCAL_EXECUTION_LOAD_RE], exclude: [/node_modules/, /[/\\]dist[/\\]/], }, }, @@ -132,6 +137,11 @@ export const getVitePlugin = ({ // them as backend functions, and replace the module with a // frontend proxy that calls executeBackendFunction at runtime. handler(code, id) { + if (id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX)) { + // Local execution needs the real function body, not the RPC-proxy stub generated below. + return null; + } + const ast = this.parse(code); const program = ensureProgram(ast, id); // Shared so the checks below don't each independently re-walk the same AST to build the same scope graph. diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts new file mode 100644 index 000000000..ab54b3ed9 --- /dev/null +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -0,0 +1,563 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +/* global globalThis, NodeJS */ + +import { mockLogger } from '@dd/tests/_jest/helpers/mocks'; + +import * as shared from '../backend/shared'; +import type { BackendFunction } from '../backend/types'; +import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; + +import type { ExecuteAction, LoadModule } from './local-execution'; +import { executeScriptLocally } from './local-execution'; + +const func: BackendFunction = { + relativePath: 'src/example', + name: 'example', + absolutePath: '/src/example.backend.ts', + allowedConnectionIds: [], +}; + +const funcWithConnection: BackendFunction = { ...func, allowedConnectionIds: ['conn-1'] }; + +const TEST_PROJECT_ROOT = '/project'; + +beforeEach(() => { + // Neither optional SDK is installed by default; tests exercising the "installed" path override this. + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(false); + jest.spyOn(shared, 'isDatadogAppsBackendInstalled').mockReturnValue(false); +}); + +const stubExecuteAction: ExecuteAction = async (fqn) => ({ data: null, stub: true, fqn }); + +/** A `loadModule` double that resolves the customer's function from a map and rejects anything else with a module-not-found error, matching the common case where neither optional package is installed. */ +function loadModuleReturning(exports: Record): LoadModule { + return async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return exports; + } + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }; +} + +describe('local-execution — executeScriptLocally', () => { + test('Should run a simple function in-process and return its result', async () => { + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [21], + stubExecuteAction, + loadModuleReturning({ example: (n: number) => n * 2 }), + mockLogger, + ); + expect(result).toEqual({ data: 42 }); + }); + + test('Should pick up a changed loadModule result on a subsequent call, not a stale cached result', async () => { + const first = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => 1 }), + mockLogger, + ); + const second = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => 2 }), + mockLogger, + ); + expect(first).toEqual({ data: 1 }); + expect(second).toEqual({ data: 2 }); + }); + + test('Should reject with a clear error when the named export is missing from the loaded module', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ somethingElse: () => 1 }), + mockLogger, + ), + ).rejects.toThrow(`"example" is not a function exported from ${func.absolutePath}`); + }); + + test('Should resolve a $.Actions.foo.bar(...) call through the injected executeAction, including connectionId', async () => { + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + const result = await executeScriptLocally( + funcWithConnection, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: () => + (globalThis as Record).$.Actions.slack.chat.postMessage({ + inputs: { text: 'hi' }, + connectionId: 'conn-1', + }), + }), + mockLogger, + ); + expect(result).toEqual({ data: { ok: true } }); + expect(executeAction).toHaveBeenCalledWith( + 'com.datadoghq.slack.chat.postMessage', + { text: 'hi' }, + 'conn-1', + ); + }); + + test("Should reject a $.Actions call whose connectionId isn't in the function's allowedConnectionIds", async () => { + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: () => + (globalThis as Record).$.Actions.slack.chat.postMessage({ + inputs: { text: 'hi' }, + connectionId: 'conn-not-allowed', + }), + }), + mockLogger, + ), + ).rejects.toThrow(/not in this function's allowed connections/); + expect(executeAction).not.toHaveBeenCalled(); + }); + + test('Should allow a $.Actions call with no connectionId regardless of allowedConnectionIds', async () => { + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: () => + (globalThis as Record).$.Actions.slack.chat.postMessage({ + inputs: { text: 'hi' }, + }), + }), + mockLogger, + ); + expect(result).toEqual({ data: { ok: true } }); + }); + + test('Should reject when the action call is missing an inputs field', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => + (globalThis as Record).$.Actions.slack.chat.postMessage({}), + }), + mockLogger, + ), + ).rejects.toThrow(/must have an inputs field/); + }); + + test('Should reject with the thrown message when the customer function throws synchronously', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + throw new Error('boom'); + }, + }), + mockLogger, + ), + ).rejects.toThrow('boom'); + }); + + test('Should reject with the rejection reason when the customer function rejects asynchronously', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => Promise.reject(new Error('async boom')) }), + mockLogger, + ), + ).rejects.toThrow('async boom'); + }); + + test('Should time out a hung async function with an explicit, attributed error', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => new Promise(() => {}) }), + mockLogger, + 50, + ), + ).rejects.toThrow(/timed out after 50ms/); + }); + + // Asserts $'s exact key set, since a token added inside globalThis.$ wouldn't be caught by the weaker top-level check below. + test('Should never expose an auth token to the customer module — only backendFunctionArgs, Actions, and Source are visible on globalThis.$', async () => { + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => Object.keys((globalThis as Record).$).sort(), + }), + mockLogger, + ); + expect(result).toEqual({ data: ['Actions', 'Source', 'backendFunctionArgs'] }); + }); + + test('Should never expose an auth token via globalThis, including nested inside $.Source', async () => { + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + // Recurses into $.Source (a plain data object) but not $.Actions (a Proxy dispatch mechanism, not a data container we'd leak a token into). + const containsTokenKey = (value: unknown): boolean => + typeof value === 'object' && + value !== null && + Object.entries(value).some( + ([key, nested]) => + key.toLowerCase().includes('token') || containsTokenKey(nested), + ); + const dollar = (globalThis as Record).$; + return ( + Object.keys(globalThis).some((k) => k.toLowerCase().includes('token')) || + containsTokenKey(dollar.Source) + ); + }, + }), + mockLogger, + ); + expect(result).toEqual({ data: false }); + }); + + test('Should populate $.Source with a synthetic local-dev identity, reachable via globalThis.$', async () => { + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => (globalThis as Record).$.Source }), + mockLogger, + ); + expect(result).toEqual({ + data: { + initiator: { id: 'local-dev', orgId: 'local-dev-org' }, + runAsUser: { id: 'local-dev', orgId: 'local-dev-org' }, + }, + }); + }); + + test("Should give each execution its own $.Source object, so one execution mutating it can't corrupt a later execution's identity", async () => { + const first = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + (globalThis as Record).$.Source.initiator.id = 'hacked'; + return 'first done'; + }, + }), + mockLogger, + ); + expect(first).toEqual({ data: 'first done' }); + + const second = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => (globalThis as Record).$.Source }), + mockLogger, + ); + expect(second).toEqual({ + data: { + initiator: { id: 'local-dev', orgId: 'local-dev-org' }, + runAsUser: { id: 'local-dev', orgId: 'local-dev-org' }, + }, + }); + }); + + test('Should restore a pre-existing globalThis.$ (e.g. from zx/globals) once the execution completes, not leave the execution context in place permanently', async () => { + const preExisting = { notOurs: true }; + (globalThis as Record).$ = preExisting; + try { + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => (globalThis as Record).$.backendFunctionArgs, + }), + mockLogger, + ); + expect(result).toEqual({ data: [] }); + // Compares via a plain boolean, not a direct .toBe() on the value — $.Actions is a Proxy whose get trap returns another Proxy for every property (including well-known symbols), which crashes Jest's diff formatting if this assertion ever fails and needs to pretty-print it. + expect(Object.is((globalThis as Record).$, preExisting)).toBe(true); + } finally { + delete (globalThis as Record).$; + } + }); + + test("Should restore a pre-existing globalThis.$ even when the customer function throws, not leave the execution's context behind", async () => { + const preExisting = { notOurs: true }; + (globalThis as Record).$ = preExisting; + try { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + throw new Error('customer function failed'); + }, + }), + mockLogger, + ), + ).rejects.toThrow('customer function failed'); + expect(Object.is((globalThis as Record).$, preExisting)).toBe(true); + } finally { + delete (globalThis as Record).$; + } + }); + + test('Should remove globalThis.$ once the execution completes when nothing was previously defined there', async () => { + delete (globalThis as Record).$; + await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => 'done' }), + mockLogger, + ); + expect(Object.prototype.hasOwnProperty.call(globalThis, '$')).toBe(false); + }); + + describe('action-catalog / apps-backend registration', () => { + test('Should silently skip registration when neither package is installed', async () => { + // Confirms loadModuleReturning's rejection of other specifiers doesn't surface as an execution failure. + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: () => 'fine' }), + mockLogger, + ); + expect(result).toEqual({ data: 'fine' }); + }); + + test('Should propagate a real load failure from an installed action-catalog package, not treat it as absent', async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { example: () => 'unreachable' }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + // A real transform/evaluation failure, not a module-not-found error — must not be swallowed as "not installed". + throw new Error('Unexpected token in action-catalog/action-execution'); + } + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }; + + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + ), + ).rejects.toThrow('Unexpected token in action-catalog/action-execution'); + }); + + test('Should route an action-catalog typed-wrapper call through the same injected executeAction', async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { + example: async () => + registeredImpl?.('com.datadoghq.slack.chat.postMessage', { + inputs: { text: 'hi' }, + connectionId: 'conn-1', + }), + }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }; + + const result = await executeScriptLocally( + funcWithConnection, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModule, + mockLogger, + ); + expect(result).toEqual({ data: { ok: true } }); + expect(executeAction).toHaveBeenCalledWith( + 'com.datadoghq.slack.chat.postMessage', + { text: 'hi' }, + 'conn-1', + ); + }); + + test("Should reject an action-catalog typed-wrapper call whose connectionId isn't in the function's allowedConnectionIds", async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { + example: async () => + registeredImpl?.('com.datadoghq.slack.chat.postMessage', { + inputs: { text: 'hi' }, + connectionId: 'conn-not-allowed', + }), + }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }; + + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModule, + mockLogger, + ), + ).rejects.toThrow(/not in this function's allowed connections/); + expect(executeAction).not.toHaveBeenCalled(); + }); + }); + + describe('serialization of concurrent executions', () => { + function delayedResult(label: T, delayMs: number): () => Promise { + return () => new Promise((resolve) => setTimeout(() => resolve(label), delayMs)); + } + + test("Should allow two independent calls to run without cross-contaminating each other's result", async () => { + const [resultA, resultB] = await Promise.all([ + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: delayedResult('A', 20) }), + mockLogger, + ), + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ example: delayedResult('B', 0) }), + mockLogger, + ), + ]); + expect([resultA, resultB]).toEqual([{ data: 'A' }, { data: 'B' }]); + }); + + // Reads $.backendFunctionArgs after a delay, which is what would surface cross-contamination between concurrent calls' globalThis.$. + function readOwnArgsAfterDelay(delayMs: number): () => Promise { + return () => + new Promise((resolve) => + setTimeout( + () => resolve((globalThis as Record).$.backendFunctionArgs), + delayMs, + ), + ); + } + + // Known race: two concurrent calls both write globalThis.$ synchronously, so the second write wins for both calls' duration. Skip until calls are serialized through an execution queue. + test.skip("Should let each concurrent call see its OWN backendFunctionArgs via globalThis.$, not the other call's", async () => { + const [resultA, resultB] = await Promise.all([ + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + ['A-arg'], + stubExecuteAction, + loadModuleReturning({ example: readOwnArgsAfterDelay(20) }), + mockLogger, + ), + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + ['B-arg'], + stubExecuteAction, + loadModuleReturning({ example: readOwnArgsAfterDelay(0) }), + mockLogger, + ), + ]); + expect(resultA).toEqual({ data: ['A-arg'] }); + expect(resultB).toEqual({ data: ['B-arg'] }); + }); + }); +}); diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts new file mode 100644 index 000000000..a214abd26 --- /dev/null +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -0,0 +1,210 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the MIT License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +/* global Proxy, globalThis */ + +/** Executes a backend function's file directly in-process inside the Vite dev server, mirroring executeScriptViaDatadog's `BackendOutputs` contract in dev-server.ts as a drop-in alternate implementation. */ + +import type { Logger } from '@dd/core/types'; + +import { isActionCatalogInstalled, isDatadogAppsBackendInstalled } from '../backend/shared'; +import type { BackendFunction, BackendOutputs } from '../backend/types'; +import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; + +interface ActionCallArgs { + inputs: Record; + connectionId?: string; +} + +const DEFAULT_TIMEOUT_MS = 10_000; + +/** Loads a module by specifier, resolved against the customer's own project rather than build-plugins' dependency tree — the dev server passes its Vite instance's `ssrLoadModule` here. */ +export type LoadModule = (specifier: string) => Promise>; + +/** Executes a real `$.Actions.foo.bar(...)` call; the dev server supplies the implementation using its own auth, so this module never holds or sees a credential itself. */ +export type ExecuteAction = ( + fqn: string, + inputs: unknown, + connectionId: string | undefined, +) => Promise; + +/** Synthetic local-dev identity for `$.Source` — a fresh object per call, since customer code could otherwise mutate a shared singleton and corrupt every later execution's identity. */ +function makeLocalDevSource() { + return { + initiator: { id: 'local-dev', orgId: 'local-dev-org' }, + runAsUser: { id: 'local-dev', orgId: 'local-dev-org' }, + }; +} + +/** Mirrors the cloud path's server-side allowedConnectionIds restriction, so local dev enforces the same connection scoping as production. */ +function assertConnectionIdAllowed( + connectionId: string | undefined, + allowedConnectionIds: string[], + actionDescription: string, +): void { + if (connectionId !== undefined && !allowedConnectionIds.includes(connectionId)) { + throw new Error( + `Action ${actionDescription} used connection "${connectionId}", which is not in this function's allowed connections: [${allowedConnectionIds.join(', ')}]`, + ); + } +} + +/** Resolves a nested property path (e.g. $.Actions.slack.chat.postMessage) to a callable that invokes `executeAction` directly — no IPC needed since there's no separate process to cross. */ +function makeActionsProxy( + executeAction: ExecuteAction, + allowedConnectionIds: string[], + pathParts: string[] = [], +): unknown { + return new Proxy(function () {}, { + get(_target, prop) { + return makeActionsProxy( + executeAction, + allowedConnectionIds, + pathParts.concat(String(prop)), + ); + }, + apply(_target, _thisArg, args: unknown[]) { + if (args.length === 0) { + return Promise.reject( + new Error(`No arguments provided to action $.Actions.${pathParts.join('.')}`), + ); + } + const { inputs, connectionId } = (args[0] ?? {}) as Partial; + if (typeof inputs !== 'object' || !inputs) { + return Promise.reject( + new Error( + `First argument to action $.Actions.${pathParts.join('.')} must have an inputs field`, + ), + ); + } + try { + assertConnectionIdAllowed( + connectionId, + allowedConnectionIds, + `$.Actions.${pathParts.join('.')}`, + ); + } catch (error) { + return Promise.reject(error); + } + const fqn = `com.datadoghq.${pathParts.join('.')}`; + return executeAction(fqn, inputs, connectionId); + }, + }); +} + +/** No-ops if @datadog/action-catalog isn't installed; checks `isActionCatalogInstalled` up front rather than catching a load failure, since `loadModule` doesn't guarantee an error code for a missing bare specifier. */ +async function registerActionCatalogIfInstalled( + loadModule: LoadModule, + projectRoot: string, + executeAction: ExecuteAction, + allowedConnectionIds: string[], +): Promise { + if (!isActionCatalogInstalled(projectRoot)) { + return; + } + const mod = await loadModule('@datadog/action-catalog/action-execution'); + const setExecuteActionImplementation = mod.setExecuteActionImplementation; + if (typeof setExecuteActionImplementation !== 'function') { + return; + } + setExecuteActionImplementation(async (actionId: string, request: unknown) => { + const { inputs, connectionId } = (request ?? {}) as Partial; + assertConnectionIdAllowed(connectionId, allowedConnectionIds, `"${actionId}"`); + return executeAction(actionId, inputs, connectionId); + }); +} + +/** No-ops if @datadog/apps-backend isn't installed; see `registerActionCatalogIfInstalled` for why this checks installedness up front rather than catching a load failure. */ +async function registerBackendRuntimeIfInstalled( + loadModule: LoadModule, + projectRoot: string, + $: unknown, +): Promise { + if (!isDatadogAppsBackendInstalled(projectRoot)) { + return; + } + const [jsFunctionWithActionsModule, runtimeModule] = await Promise.all([ + loadModule('@datadog/apps-backend/runtime/jsFunctionWithActions'), + loadModule('@datadog/apps-backend/runtime'), + ]); + const buildRuntimeFromJsFunctionWithActions = + jsFunctionWithActionsModule.buildRuntimeFromJsFunctionWithActions; + const setBackend = runtimeModule.setBackend; + if ( + typeof buildRuntimeFromJsFunctionWithActions !== 'function' || + typeof setBackend !== 'function' + ) { + return; + } + setBackend(buildRuntimeFromJsFunctionWithActions($)); +} + +/** `globalThis.$` and the registrations above provide the same customer-visible bindings production's generated wrapper module sets up via text injection. */ +export async function executeScriptLocally( + func: BackendFunction, + projectRoot: string, + args: unknown[], + executeAction: ExecuteAction, + loadModule: LoadModule, + log: Logger, + timeoutMs: number = DEFAULT_TIMEOUT_MS, +): Promise { + // Never log the args themselves — they may carry secrets/PII, matching dev-server.ts's cloud path. + log.debug(`Executing "${func.name}" in-process with args`); + + const $ = { + backendFunctionArgs: args, + Actions: makeActionsProxy(executeAction, func.allowedConnectionIds), + Source: makeLocalDevSource(), + }; + + const run = async (): Promise => { + // Restores whatever globalThis.$ held before this call (or removes it entirely if nothing did) once the execution settles, so a pre-existing global (e.g. from zx/globals) isn't permanently clobbered and a completed execution's own context isn't left reachable by unrelated process code. + const hadPreviousDollar = Object.prototype.hasOwnProperty.call(globalThis, '$'); + const previousDollar = (globalThis as Record).$; + (globalThis as Record).$ = $; + try { + await Promise.all([ + registerActionCatalogIfInstalled( + loadModule, + projectRoot, + executeAction, + func.allowedConnectionIds, + ), + registerBackendRuntimeIfInstalled(loadModule, projectRoot, $), + ]); + + const mod = await loadModule(func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX); + const fn = mod[func.name]; + if (typeof fn !== 'function') { + throw new Error( + `"${func.name}" is not a function exported from ${func.absolutePath}`, + ); + } + + const result = await fn(...args); + return { data: result }; + } finally { + if (hadPreviousDollar) { + (globalThis as Record).$ = previousDollar; + } else { + delete (globalThis as Record).$; + } + } + }; + + let timer: ReturnType | undefined; + const timeout = new Promise((_resolve, reject) => { + timer = setTimeout(() => { + reject(new Error(`Local execution of "${func.name}" timed out after ${timeoutMs}ms`)); + }, timeoutMs); + }); + + try { + // Racing against the timeout only stops the caller from waiting — run() keeps executing in-process afterward, so a customer function that resumes post-timeout can still fire real $.Actions side effects. True cancellation requires terminating a Worker thread, not possible for in-process execution. + return await Promise.race([run(), timeout]); + } finally { + clearTimeout(timer); + } +} From 46831a9cf53fe34e6b7d95737c05d8c421c32ae8 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Wed, 26 Aug 2026 20:49:30 -0400 Subject: [PATCH 02/15] fix(apps): stop $.Actions proxy hangs on an un-invoked reference A customer function returning $.Actions.foo.bar without calling it made the outer await treat the callable Proxy as a thenable (its get trap returned another callable Proxy for .then too), hanging until the timeout instead of just returning the value. Also converts the apply trap to async, since the manual Promise.reject/try-catch wrapping was only there to turn a synchronous throw into a rejection. --- .../apps/src/vite/local-execution.test.ts | 16 ++++++++++ .../plugins/apps/src/vite/local-execution.ts | 30 ++++++++----------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index ab54b3ed9..039118fbb 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -115,6 +115,22 @@ describe('local-execution — executeScriptLocally', () => { ); }); + test('Should not hang when a customer function returns an un-invoked $.Actions reference instead of calling it', async () => { + // $.Actions.slack.chat is itself a callable Proxy; forgetting the trailing .postMessage(...) call and just returning it must not make `await fn(...args)` treat it as a thenable and hang until the timeout. + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => (globalThis as Record).$.Actions.slack.chat, + }), + mockLogger, + 20, + ); + expect(result.data).toBeDefined(); + }); + test("Should reject a $.Actions call whose connectionId isn't in the function's allowedConnectionIds", async () => { const executeAction = jest.fn().mockResolvedValue({ ok: true }); await expect( diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index a214abd26..6cae49a53 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -58,35 +58,31 @@ function makeActionsProxy( ): unknown { return new Proxy(function () {}, { get(_target, prop) { + // A customer function that returns an un-invoked reference (e.g. $.Actions.foo.bar without the trailing call) must not be treated as a thenable — Promise's resolution protocol would call .then() on it and hang until the timeout, since apply() below never settles it. + if (prop === 'then') { + return undefined; + } return makeActionsProxy( executeAction, allowedConnectionIds, pathParts.concat(String(prop)), ); }, - apply(_target, _thisArg, args: unknown[]) { + async apply(_target, _thisArg, args: unknown[]) { if (args.length === 0) { - return Promise.reject( - new Error(`No arguments provided to action $.Actions.${pathParts.join('.')}`), - ); + throw new Error(`No arguments provided to action $.Actions.${pathParts.join('.')}`); } const { inputs, connectionId } = (args[0] ?? {}) as Partial; if (typeof inputs !== 'object' || !inputs) { - return Promise.reject( - new Error( - `First argument to action $.Actions.${pathParts.join('.')} must have an inputs field`, - ), - ); - } - try { - assertConnectionIdAllowed( - connectionId, - allowedConnectionIds, - `$.Actions.${pathParts.join('.')}`, + throw new Error( + `First argument to action $.Actions.${pathParts.join('.')} must have an inputs field`, ); - } catch (error) { - return Promise.reject(error); } + assertConnectionIdAllowed( + connectionId, + allowedConnectionIds, + `$.Actions.${pathParts.join('.')}`, + ); const fqn = `com.datadoghq.${pathParts.join('.')}`; return executeAction(fqn, inputs, connectionId); }, From 0bc0151a08977dd751d3e854e985fc2afdb33dd4 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Wed, 26 Aug 2026 20:49:37 -0400 Subject: [PATCH 03/15] refactor(apps): derive LOCAL_EXECUTION_LOAD_RE instead of hand-spelling it The regex re-typed BACKEND_FILE_RE's extension list and the literal suffix as an independent pattern, so a change to either one could silently stop matching real backend files without any compiler or lint signal. --- packages/plugins/apps/src/constants.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/plugins/apps/src/constants.ts b/packages/plugins/apps/src/constants.ts index 3d5866c72..53732b39c 100644 --- a/packages/plugins/apps/src/constants.ts +++ b/packages/plugins/apps/src/constants.ts @@ -13,7 +13,10 @@ export const BACKEND_FILE_RE = /\.backend\.(ts|tsx|js|jsx)$/; /** Query suffix marking a local-execution load, so the transform hook below can skip proxy generation for it instead of matching via the broader `options.ssr` flag. */ export const LOCAL_EXECUTION_LOAD_SUFFIX = '?dd-local-exec'; -export const LOCAL_EXECUTION_LOAD_RE = /\.backend\.(ts|tsx|js|jsx)\?dd-local-exec$/; +// Derived from BACKEND_FILE_RE plus the escaped suffix (its only regex-special character is the leading `?`), so the two can't drift apart if either the extension list or the suffix ever changes. +export const LOCAL_EXECUTION_LOAD_RE = new RegExp( + `${BACKEND_FILE_RE.source.slice(0, -1)}\\${LOCAL_EXECUTION_LOAD_SUFFIX}$`, +); export const BACKEND_CODE_EXTENSIONS = [ '.ts', '.tsx', From b038dd8b746f74196e3cf9c058d84df72cba5394 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 27 Aug 2026 01:38:10 -0400 Subject: [PATCH 04/15] refactor(apps): eliminate remaining as-casts flagged by review bots Replace direct globalThis.$ assignment/deletion and Partial casts with helper functions and a type guard, since TypeScript can narrow these without an assertion. --- .../apps/src/vite/local-execution.test.ts | 40 +++++++++++-------- .../plugins/apps/src/vite/local-execution.ts | 30 +++++++++++--- 2 files changed, 49 insertions(+), 21 deletions(-) diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index 039118fbb..ce857ae1f 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -24,6 +24,18 @@ const funcWithConnection: BackendFunction = { ...func, allowedConnectionIds: ['c const TEST_PROJECT_ROOT = '/project'; +interface TestGlobalDollar { + backendFunctionArgs: unknown[]; + // Left untyped: $.Actions is a Proxy of unbounded, dynamic depth ($.Actions.....(...)), the same shape a real customer's untyped code sees. + Actions: any; + Source: { initiator: { id: string; orgId: string }; runAsUser: { id: string; orgId: string } }; +} + +/** Reads the `$` this module installs onto `globalThis` during an execution, from the customer-code perspective these tests simulate — genuinely untyped from TypeScript's static perspective since it's a runtime-only property (see local-execution.ts's `Object.defineProperty`). Centralized here instead of repeating the same cast at each call site. */ +function testDollar(): TestGlobalDollar { + return (globalThis as unknown as { $: TestGlobalDollar }).$; +} + beforeEach(() => { // Neither optional SDK is installed by default; tests exercising the "installed" path override this. jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(false); @@ -100,7 +112,7 @@ describe('local-execution — executeScriptLocally', () => { executeAction, loadModuleReturning({ example: () => - (globalThis as Record).$.Actions.slack.chat.postMessage({ + testDollar().Actions.slack.chat.postMessage({ inputs: { text: 'hi' }, connectionId: 'conn-1', }), @@ -123,7 +135,7 @@ describe('local-execution — executeScriptLocally', () => { [], stubExecuteAction, loadModuleReturning({ - example: () => (globalThis as Record).$.Actions.slack.chat, + example: () => testDollar().Actions.slack.chat, }), mockLogger, 20, @@ -141,7 +153,7 @@ describe('local-execution — executeScriptLocally', () => { executeAction, loadModuleReturning({ example: () => - (globalThis as Record).$.Actions.slack.chat.postMessage({ + testDollar().Actions.slack.chat.postMessage({ inputs: { text: 'hi' }, connectionId: 'conn-not-allowed', }), @@ -161,7 +173,7 @@ describe('local-execution — executeScriptLocally', () => { executeAction, loadModuleReturning({ example: () => - (globalThis as Record).$.Actions.slack.chat.postMessage({ + testDollar().Actions.slack.chat.postMessage({ inputs: { text: 'hi' }, }), }), @@ -178,8 +190,7 @@ describe('local-execution — executeScriptLocally', () => { [], stubExecuteAction, loadModuleReturning({ - example: () => - (globalThis as Record).$.Actions.slack.chat.postMessage({}), + example: () => testDollar().Actions.slack.chat.postMessage({}), }), mockLogger, ), @@ -238,7 +249,7 @@ describe('local-execution — executeScriptLocally', () => { [], stubExecuteAction, loadModuleReturning({ - example: () => Object.keys((globalThis as Record).$).sort(), + example: () => Object.keys(testDollar()).sort(), }), mockLogger, ); @@ -261,7 +272,7 @@ describe('local-execution — executeScriptLocally', () => { ([key, nested]) => key.toLowerCase().includes('token') || containsTokenKey(nested), ); - const dollar = (globalThis as Record).$; + const dollar = testDollar(); return ( Object.keys(globalThis).some((k) => k.toLowerCase().includes('token')) || containsTokenKey(dollar.Source) @@ -279,7 +290,7 @@ describe('local-execution — executeScriptLocally', () => { TEST_PROJECT_ROOT, [], stubExecuteAction, - loadModuleReturning({ example: () => (globalThis as Record).$.Source }), + loadModuleReturning({ example: () => testDollar().Source }), mockLogger, ); expect(result).toEqual({ @@ -298,7 +309,7 @@ describe('local-execution — executeScriptLocally', () => { stubExecuteAction, loadModuleReturning({ example: () => { - (globalThis as Record).$.Source.initiator.id = 'hacked'; + testDollar().Source.initiator.id = 'hacked'; return 'first done'; }, }), @@ -311,7 +322,7 @@ describe('local-execution — executeScriptLocally', () => { TEST_PROJECT_ROOT, [], stubExecuteAction, - loadModuleReturning({ example: () => (globalThis as Record).$.Source }), + loadModuleReturning({ example: () => testDollar().Source }), mockLogger, ); expect(second).toEqual({ @@ -332,7 +343,7 @@ describe('local-execution — executeScriptLocally', () => { [], stubExecuteAction, loadModuleReturning({ - example: () => (globalThis as Record).$.backendFunctionArgs, + example: () => testDollar().backendFunctionArgs, }), mockLogger, ); @@ -545,10 +556,7 @@ describe('local-execution — executeScriptLocally', () => { function readOwnArgsAfterDelay(delayMs: number): () => Promise { return () => new Promise((resolve) => - setTimeout( - () => resolve((globalThis as Record).$.backendFunctionArgs), - delayMs, - ), + setTimeout(() => resolve(testDollar().backendFunctionArgs), delayMs), ); } diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index 6cae49a53..ee1e3c64d 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -17,6 +17,25 @@ interface ActionCallArgs { connectionId?: string; } +/** Narrows an unknown value enough to read named properties off it by key. */ +function isIndexableRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} + +/** `globalThis.$` is a runtime-only property TypeScript's built-in `typeof globalThis` has no way to know about — reading it needs one assertion, centralized here instead of repeated inline at each call site. */ +function getGlobalDollar(): unknown { + return (globalThis as Record).$; +} + +/** `Object.assign`'s signature doesn't require its source object's keys to already exist on the target, so this installs `$` without asserting `globalThis`'s type. */ +function setGlobalDollar(value: unknown): void { + Object.assign(globalThis, { $: value }); +} + +function deleteGlobalDollar(): void { + Reflect.deleteProperty(globalThis, '$'); +} + const DEFAULT_TIMEOUT_MS = 10_000; /** Loads a module by specifier, resolved against the customer's own project rather than build-plugins' dependency tree — the dev server passes its Vite instance's `ssrLoadModule` here. */ @@ -72,7 +91,8 @@ function makeActionsProxy( if (args.length === 0) { throw new Error(`No arguments provided to action $.Actions.${pathParts.join('.')}`); } - const { inputs, connectionId } = (args[0] ?? {}) as Partial; + const call: Partial = isIndexableRecord(args[0]) ? args[0] : {}; + const { inputs, connectionId } = call; if (typeof inputs !== 'object' || !inputs) { throw new Error( `First argument to action $.Actions.${pathParts.join('.')} must have an inputs field`, @@ -158,8 +178,8 @@ export async function executeScriptLocally( const run = async (): Promise => { // Restores whatever globalThis.$ held before this call (or removes it entirely if nothing did) once the execution settles, so a pre-existing global (e.g. from zx/globals) isn't permanently clobbered and a completed execution's own context isn't left reachable by unrelated process code. const hadPreviousDollar = Object.prototype.hasOwnProperty.call(globalThis, '$'); - const previousDollar = (globalThis as Record).$; - (globalThis as Record).$ = $; + const previousDollar = getGlobalDollar(); + setGlobalDollar($); try { await Promise.all([ registerActionCatalogIfInstalled( @@ -183,9 +203,9 @@ export async function executeScriptLocally( return { data: result }; } finally { if (hadPreviousDollar) { - (globalThis as Record).$ = previousDollar; + setGlobalDollar(previousDollar); } else { - delete (globalThis as Record).$; + deleteGlobalDollar(); } } }; From 684aa8853707b093c7b11d044aebe24aa23ccb14 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 27 Aug 2026 02:39:17 -0400 Subject: [PATCH 05/15] fix(apps): require SSR context for local-execution loads, evaluate module before installing $ The suffix alone was spoofable from frontend source (e.g. a literal ./secrets.backend.ts?dd-local-exec import); requiring SSR context too means a spoofed client-side import still gets the safe RPC-proxy stub instead of the real backend module body. Also loads and evaluates the customer module before installing $ and the SDK bridges, matching production's own ordering, so code that reaches for $ during its own top-level evaluation fails the same way locally as it would in Datadog. --- packages/plugins/apps/src/vite/index.test.ts | 25 ++++++++++++++++- packages/plugins/apps/src/vite/index.ts | 6 ++-- .../apps/src/vite/local-execution.test.ts | 28 +++++++++++++++++++ .../plugins/apps/src/vite/local-execution.ts | 15 +++++----- 4 files changed, 62 insertions(+), 12 deletions(-) diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 6ba1b839f..6dd709ba0 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -237,7 +237,7 @@ describe('Backend Functions - getVitePlugin', () => { }); // Regression test: without the suffix check, ssrLoadModule() would get the RPC-proxy stub instead of the real function body. - test('Should skip proxy generation for a suffixed local-execution load, returning the real source untouched', async () => { + test('Should skip proxy generation for a suffixed local-execution load made from SSR context, returning the real source untouched', async () => { const plugin = getVitePlugin(defaultOptions); const handler = getTransformHandler(plugin); @@ -251,11 +251,34 @@ describe('Backend Functions - getVitePlugin', () => { }, realSource, `/build/src/backend/myHandler.backend.ts${LOCAL_EXECUTION_LOAD_SUFFIX}`, + { ssr: true }, ); expect(result).toBeNull(); }); + // Regression test: the suffix alone must not bypass proxy generation — only real local-execution + // loads (via ssrLoadModule, always SSR context) get the real source; a spoofed client-side import + // using the same suffix (e.g. `./secrets.backend.ts?dd-local-exec`) still gets the safe RPC-proxy + // stub, never the real backend module body. + test('Should still generate the frontend RPC-proxy for a suffixed import made outside SSR context', async () => { + const plugin = getVitePlugin(defaultOptions); + const transformHandler = getTransformHandler(plugin); + + const result = (await transformHandler.call( + { + parse: parseAst, + resolve: jest.fn(async () => null), + load: jest.fn(async () => null), + addWatchFile: jest.fn(), + }, + 'export function myHandler() { return 42; }', + `/build/src/backend/myHandler.backend.ts${LOCAL_EXECUTION_LOAD_SUFFIX}`, + )) as { code: string } | null; + + expect(result?.code).toEqual(expect.stringContaining('executeBackendFunction')); + }); + test('Should still generate the frontend RPC-proxy for a normal (unsuffixed) import of the same file', async () => { const plugin = getVitePlugin(defaultOptions); const handler = getTransformHandler(plugin); diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 8b26d13bc..32977394d 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -136,9 +136,9 @@ export const getVitePlugin = ({ // For each .backend.* file, parse its named exports, register // them as backend functions, and replace the module with a // frontend proxy that calls executeBackendFunction at runtime. - handler(code, id) { - if (id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX)) { - // Local execution needs the real function body, not the RPC-proxy stub generated below. + handler(code, id, transformOptions) { + if (id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX) && transformOptions?.ssr) { + // Local execution needs the real function body, not the RPC-proxy stub generated below. Requiring SSR context too (not just the suffix) means a spoofed client-side import like `./secrets.backend.ts?dd-local-exec` still falls through to the same safe proxy-stub generation as any other backend file — real local-execution loads always go through ssrLoadModule, which runs in SSR context. return null; } diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index ce857ae1f..b18b0546a 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -103,6 +103,34 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow(`"example" is not a function exported from ${func.absolutePath}`); }); + test('Should load and evaluate the customer module before installing globalThis.$, matching production module-evaluation order', async () => { + let dollarDuringModuleLoad: unknown = 'not captured'; + const loadModule: LoadModule = async (specifier) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + // Captures globalThis.$ at module-evaluation time — production's static customer-module import runs before its wrapper installs $, so a customer module reaching for $ during its own top-level evaluation must see the same absence locally, not this execution's own $ installed early. + dollarDuringModuleLoad = (globalThis as Record).$; + return { example: () => 'done' }; + } + const notFoundError: NodeJS.ErrnoException = new Error( + `Cannot find module '${specifier}'`, + ); + notFoundError.code = 'MODULE_NOT_FOUND'; + throw notFoundError; + }; + + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModule, + mockLogger, + ); + + expect(result).toEqual({ data: 'done' }); + expect(dollarDuringModuleLoad).toBeUndefined(); + }); + test('Should resolve a $.Actions.foo.bar(...) call through the injected executeAction, including connectionId', async () => { const executeAction = jest.fn().mockResolvedValue({ ok: true }); const result = await executeScriptLocally( diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index ee1e3c64d..4cffdf061 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -176,6 +176,13 @@ export async function executeScriptLocally( }; const run = async (): Promise => { + // Loads and evaluates the customer's module BEFORE installing $ and the SDK bridges below, matching production's own ordering (backend/virtual-entry.ts statically imports the customer module before its wrapper installs $ and the SDK bridges) — code that reaches for $ or a typed action during its own top-level evaluation fails the same way locally as it would in Datadog, instead of silently succeeding against bindings production wouldn't have installed yet. + const mod = await loadModule(func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX); + const fn = mod[func.name]; + if (typeof fn !== 'function') { + throw new Error(`"${func.name}" is not a function exported from ${func.absolutePath}`); + } + // Restores whatever globalThis.$ held before this call (or removes it entirely if nothing did) once the execution settles, so a pre-existing global (e.g. from zx/globals) isn't permanently clobbered and a completed execution's own context isn't left reachable by unrelated process code. const hadPreviousDollar = Object.prototype.hasOwnProperty.call(globalThis, '$'); const previousDollar = getGlobalDollar(); @@ -191,14 +198,6 @@ export async function executeScriptLocally( registerBackendRuntimeIfInstalled(loadModule, projectRoot, $), ]); - const mod = await loadModule(func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX); - const fn = mod[func.name]; - if (typeof fn !== 'function') { - throw new Error( - `"${func.name}" is not a function exported from ${func.absolutePath}`, - ); - } - const result = await fn(...args); return { data: result }; } finally { From 7097440ccad0007afa690caefff5c7f66bcb5c32 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 27 Aug 2026 02:59:31 -0400 Subject: [PATCH 06/15] fix(apps): normalize the local-exec suffix before proxy-gen fallback, avoid unhandled rejection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A spoofed client-side import falling through to buildProxyModule still carried the ?dd-local-exec suffix in its id, so BACKEND_FILE_RE (anchored to end-of-string) never stripped it — the function registered under a corrupted relativePath/query-name distinct from the file's real registration. Strips the suffix first so it dedupes onto the same entry. Also attaches a no-op catch to run()'s promise once the timeout has already settled the race, since nothing else awaits it — a customer function that rejects after its own timeout would otherwise be an unhandled rejection that crashes the whole dev server. Replaces a remaining raw as-cast in registerActionCatalogIfInstalled with the file's existing isIndexableRecord guard. --- packages/plugins/apps/src/vite/index.ts | 27 +++++++++++-------- .../plugins/apps/src/vite/local-execution.ts | 11 +++++--- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 32977394d..9c8037bc1 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -137,36 +137,41 @@ export const getVitePlugin = ({ // them as backend functions, and replace the module with a // frontend proxy that calls executeBackendFunction at runtime. handler(code, id, transformOptions) { - if (id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX) && transformOptions?.ssr) { - // Local execution needs the real function body, not the RPC-proxy stub generated below. Requiring SSR context too (not just the suffix) means a spoofed client-side import like `./secrets.backend.ts?dd-local-exec` still falls through to the same safe proxy-stub generation as any other backend file — real local-execution loads always go through ssrLoadModule, which runs in SSR context. - return null; + let normalizedId = id; + if (id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX)) { + if (transformOptions?.ssr) { + // Local execution needs the real function body, not the RPC-proxy stub generated below. + return null; + } + // A spoofed client-side import like `./secrets.backend.ts?dd-local-exec` falls through to the same safe proxy-stub generation as any other backend file instead — real local-execution loads always go through ssrLoadModule, which runs in SSR context. Strips the suffix first so this registers under the same relativePath/query-name as the file's real (unsuffixed) import, not a second, corrupted entry. + normalizedId = id.slice(0, -LOCAL_EXECUTION_LOAD_SUFFIX.length); } const ast = this.parse(code); - const program = ensureProgram(ast, id); + const program = ensureProgram(ast, normalizedId); // Shared so the checks below don't each independently re-walk the same AST to build the same scope graph. const scopeAnalysis = analyzeModuleScope(program); // Runs even for a file with zero exports, to catch a banned import/global as soon as it's written. - runBackendStaticChecks(ast, id, log, scopeAnalysis); - const exportNames = extractExportedFunctions(ast, id); + runBackendStaticChecks(ast, normalizedId, log, scopeAnalysis); + const exportNames = extractExportedFunctions(ast, normalizedId); if (exportNames.length === 0) { log.warn( - `Backend file ${id} has no exported functions. ` + + `Backend file ${normalizedId} has no exported functions. ` + `Did you forget to add a named export?`, ); // Clear any previously registered functions for this file // so stale entries don't persist across HMR re-transforms. - setBackendFunctions(id, []); + setBackendFunctions(normalizedId, []); return { code: '', map: null }; } const { functions, proxyCode } = buildProxyModule( exportNames, - id, + normalizedId, context.buildRoot, ); - setBackendFunctions(id, functions); - log.debug(`Generated proxy for ${id} with ${functions.length} export(s)`); + setBackendFunctions(normalizedId, functions); + log.debug(`Generated proxy for ${normalizedId} with ${functions.length} export(s)`); return { code: proxyCode, map: null }; }, diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index 4cffdf061..d0eec9a0a 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -125,7 +125,8 @@ async function registerActionCatalogIfInstalled( return; } setExecuteActionImplementation(async (actionId: string, request: unknown) => { - const { inputs, connectionId } = (request ?? {}) as Partial; + const call: Partial = isIndexableRecord(request) ? request : {}; + const { inputs, connectionId } = call; assertConnectionIdAllowed(connectionId, allowedConnectionIds, `"${actionId}"`); return executeAction(actionId, inputs, connectionId); }); @@ -216,9 +217,13 @@ export async function executeScriptLocally( }, timeoutMs); }); + // Racing against the timeout only stops the caller from waiting — run() keeps executing in-process afterward, so a customer function that resumes post-timeout can still fire real $.Actions side effects. True cancellation requires terminating a Worker thread, not possible for in-process execution. + const runPromise = run(); + // Nothing awaits runPromise once the timeout has already settled the race — an unhandled rejection from it later would otherwise crash the whole dev server process. + runPromise.catch(() => {}); + try { - // Racing against the timeout only stops the caller from waiting — run() keeps executing in-process afterward, so a customer function that resumes post-timeout can still fire real $.Actions side effects. True cancellation requires terminating a Worker thread, not possible for in-process execution. - return await Promise.race([run(), timeout]); + return await Promise.race([runPromise, timeout]); } finally { clearTimeout(timer); } From 4c0cb417072d8c82dffe02d5d0ac4b2c5b8be8d7 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 27 Aug 2026 14:01:24 -0400 Subject: [PATCH 07/15] fix(apps): log an abandoned execution's late failure instead of swallowing it runPromise.catch(() => {}) discarded the rejection reason from a hung customer function once the outer timeout race already settled, leaving the real cause of a slow failure undiagnosable. --- .../apps/src/vite/local-execution.test.ts | 31 ++++++++++++++++++- .../plugins/apps/src/vite/local-execution.ts | 7 +++-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index b18b0546a..3116dfa5e 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -4,7 +4,7 @@ /* global globalThis, NodeJS */ -import { mockLogger } from '@dd/tests/_jest/helpers/mocks'; +import { mockLogFn, mockLogger } from '@dd/tests/_jest/helpers/mocks'; import * as shared from '../backend/shared'; import type { BackendFunction } from '../backend/types'; @@ -269,6 +269,35 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow(/timed out after 50ms/); }); + // The caller already moved on after the timeout rejection above; this covers the abandoned execution's own eventual failure, which has no caller left to report it to. + test('Should log a late failure from an abandoned execution instead of swallowing it silently', async () => { + let rejectHung: ((error: Error) => void) | undefined; + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => + new Promise((_resolve, reject) => { + rejectHung = reject; + }), + }), + mockLogger, + 50, + ), + ).rejects.toThrow(/timed out after 50ms/); + + rejectHung?.(new Error('late failure after caller stopped waiting')); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining('late failure after caller stopped waiting'), + 'debug', + ); + }); + // Asserts $'s exact key set, since a token added inside globalThis.$ wouldn't be caught by the weaker top-level check below. test('Should never expose an auth token to the customer module — only backendFunctionArgs, Actions, and Source are visible on globalThis.$', async () => { const result = await executeScriptLocally( diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index d0eec9a0a..eda5ef901 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -219,8 +219,11 @@ export async function executeScriptLocally( // Racing against the timeout only stops the caller from waiting — run() keeps executing in-process afterward, so a customer function that resumes post-timeout can still fire real $.Actions side effects. True cancellation requires terminating a Worker thread, not possible for in-process execution. const runPromise = run(); - // Nothing awaits runPromise once the timeout has already settled the race — an unhandled rejection from it later would otherwise crash the whole dev server process. - runPromise.catch(() => {}); + // Nothing awaits runPromise once the timeout has already settled the race — an unhandled rejection from it later would otherwise crash the whole dev server process. Logged (not swallowed silently) so a slow real failure is still diagnosable after the caller has already moved on. + runPromise.catch((error: unknown) => { + const message = error instanceof Error ? error.message : String(error); + log.debug(`"${func.name}" failed after its caller had already stopped waiting: ${message}`); + }); try { return await Promise.race([runPromise, timeout]); From 3b90cea2ebc6a729e56cb94b91df96caf6ac8e4a Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 27 Aug 2026 14:49:24 -0400 Subject: [PATCH 08/15] fix(apps): validate inputs on the action-catalog typed-wrapper call path The raw $.Actions proxy already rejects a call missing an inputs field; the action-catalog dispatcher funnels into the same executeAction but skipped this check, silently forwarding inputs: undefined instead. --- .../apps/src/vite/local-execution.test.ts | 43 +++++++++++++++++++ .../plugins/apps/src/vite/local-execution.ts | 6 +++ 2 files changed, 49 insertions(+) diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index 3116dfa5e..10724e32c 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -580,6 +580,49 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow(/not in this function's allowed connections/); expect(executeAction).not.toHaveBeenCalled(); }); + + test('Should reject an action-catalog typed-wrapper call missing an inputs field, same as a raw $.Actions call', async () => { + jest.spyOn(shared, 'isActionCatalogInstalled').mockReturnValue(true); + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + let registeredImpl: + | ((actionId: string, request: unknown) => Promise) + | undefined; + + const loadModule: LoadModule = async (specifier: string) => { + if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { + return { + example: async () => + registeredImpl?.('com.datadoghq.slack.chat.postMessage', { + connectionId: 'conn-1', + }), + }; + } + if (specifier === '@datadog/action-catalog/action-execution') { + return { + setExecuteActionImplementation: ( + impl: (actionId: string, request: unknown) => Promise, + ) => { + registeredImpl = impl; + }, + }; + } + const error: NodeJS.ErrnoException = new Error(`Cannot find module '${specifier}'`); + error.code = 'MODULE_NOT_FOUND'; + throw error; + }; + + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModule, + mockLogger, + ), + ).rejects.toThrow(/must have an inputs field/); + expect(executeAction).not.toHaveBeenCalled(); + }); }); describe('serialization of concurrent executions', () => { diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index eda5ef901..05069e355 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -127,6 +127,12 @@ async function registerActionCatalogIfInstalled( setExecuteActionImplementation(async (actionId: string, request: unknown) => { const call: Partial = isIndexableRecord(request) ? request : {}; const { inputs, connectionId } = call; + // Same invariant makeActionsProxy's apply trap enforces for a raw $.Actions call — + // both entry points funnel into the same executeAction, so both must reject the + // same malformed shape instead of silently forwarding inputs: undefined. + if (typeof inputs !== 'object' || !inputs) { + throw new Error(`Action "${actionId}" must have an inputs field`); + } assertConnectionIdAllowed(connectionId, allowedConnectionIds, `"${actionId}"`); return executeAction(actionId, inputs, connectionId); }); From 64536aa8f8fbe28992a2b0092d8deae391c5eafa Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 27 Aug 2026 16:27:04 -0400 Subject: [PATCH 09/15] refactor(apps): share $.Actions call validation between both entry points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The raw proxy and the action-catalog dispatcher each re-implemented the same inputs/connectionId validation by hand — the exact duplication that let the action-catalog path drift out of sync and skip the inputs check in the first place. Extracted into one validateActionCall helper both entry points now call, so the two can no longer diverge. Also corrects a test comment referencing Object.defineProperty, which this file doesn't use — globalThis.$ is set via setGlobalDollar's Object.assign. --- .../apps/src/vite/local-execution.test.ts | 2 +- .../plugins/apps/src/vite/local-execution.ts | 37 +++++++++++-------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index 10724e32c..bdd5f338c 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -31,7 +31,7 @@ interface TestGlobalDollar { Source: { initiator: { id: string; orgId: string }; runAsUser: { id: string; orgId: string } }; } -/** Reads the `$` this module installs onto `globalThis` during an execution, from the customer-code perspective these tests simulate — genuinely untyped from TypeScript's static perspective since it's a runtime-only property (see local-execution.ts's `Object.defineProperty`). Centralized here instead of repeating the same cast at each call site. */ +/** Reads the `$` this module installs onto `globalThis` during an execution, from the customer-code perspective these tests simulate — genuinely untyped from TypeScript's static perspective since it's a runtime-only property (see local-execution.ts's `setGlobalDollar`). Centralized here instead of repeating the same cast at each call site. */ function testDollar(): TestGlobalDollar { return (globalThis as unknown as { $: TestGlobalDollar }).$; } diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index 05069e355..738717926 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -69,6 +69,20 @@ function assertConnectionIdAllowed( } } +/** Shared validation for both $.Actions entry points (the raw proxy and the action-catalog typed-wrapper dispatcher) — extracted so a future change to this contract can't be applied to one and missed on the other, the exact gap that let the action-catalog path silently forward `inputs: undefined`. */ +function validateActionCall( + call: Partial, + allowedConnectionIds: string[], + actionDescription: string, +): { inputs: Record; connectionId: string | undefined } { + const { inputs, connectionId } = call; + if (typeof inputs !== 'object' || !inputs) { + throw new Error(`Action ${actionDescription} must have an inputs field`); + } + assertConnectionIdAllowed(connectionId, allowedConnectionIds, actionDescription); + return { inputs, connectionId }; +} + /** Resolves a nested property path (e.g. $.Actions.slack.chat.postMessage) to a callable that invokes `executeAction` directly — no IPC needed since there's no separate process to cross. */ function makeActionsProxy( executeAction: ExecuteAction, @@ -92,14 +106,8 @@ function makeActionsProxy( throw new Error(`No arguments provided to action $.Actions.${pathParts.join('.')}`); } const call: Partial = isIndexableRecord(args[0]) ? args[0] : {}; - const { inputs, connectionId } = call; - if (typeof inputs !== 'object' || !inputs) { - throw new Error( - `First argument to action $.Actions.${pathParts.join('.')} must have an inputs field`, - ); - } - assertConnectionIdAllowed( - connectionId, + const { inputs, connectionId } = validateActionCall( + call, allowedConnectionIds, `$.Actions.${pathParts.join('.')}`, ); @@ -126,14 +134,11 @@ async function registerActionCatalogIfInstalled( } setExecuteActionImplementation(async (actionId: string, request: unknown) => { const call: Partial = isIndexableRecord(request) ? request : {}; - const { inputs, connectionId } = call; - // Same invariant makeActionsProxy's apply trap enforces for a raw $.Actions call — - // both entry points funnel into the same executeAction, so both must reject the - // same malformed shape instead of silently forwarding inputs: undefined. - if (typeof inputs !== 'object' || !inputs) { - throw new Error(`Action "${actionId}" must have an inputs field`); - } - assertConnectionIdAllowed(connectionId, allowedConnectionIds, `"${actionId}"`); + const { inputs, connectionId } = validateActionCall( + call, + allowedConnectionIds, + `"${actionId}"`, + ); return executeAction(actionId, inputs, connectionId); }); } From 5684eddab9b4e229e7adad31bad95e52e2a78d0e Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 27 Aug 2026 17:55:02 -0400 Subject: [PATCH 10/15] fix(apps): drop the last as-cast and stop mislogging ordinary rejections getGlobalDollar read globalThis.$ via an `as Record` cast; Reflect.get reads it without one, matching the pattern deleteGlobalDollar already used for the same property. The "caller had already stopped waiting" debug log fired on every run() rejection, not just ones abandoned after the timeout race already settled, since the .catch handler had no way to tell the two cases apart. Gates it on whether the race has settled yet. Also restores the BackendOutputs doc comment's explanation of why the shape is `{ data: unknown }` (mirrors the app-builder query response), dropped when the type was consolidated into backend/types.ts. --- packages/plugins/apps/src/backend/types.ts | 2 +- .../apps/src/vite/local-execution.test.ts | 25 +++++++++++++++++++ .../plugins/apps/src/vite/local-execution.ts | 10 ++++++-- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/packages/plugins/apps/src/backend/types.ts b/packages/plugins/apps/src/backend/types.ts index e528255d5..95f228d33 100644 --- a/packages/plugins/apps/src/backend/types.ts +++ b/packages/plugins/apps/src/backend/types.ts @@ -13,5 +13,5 @@ export interface BackendFunction { allowedConnectionIds: string[]; } -/** Shared result shape for both the remote (dev-server.ts) and in-process (local-execution.ts) execution paths. */ +/** Shape of a backend function's result, shared by the remote (dev-server.ts) and in-process (local-execution.ts) execution paths — mirrors the Datadog app-builder query response, which wraps a JS action's return value as `{ data: }`. */ export type BackendOutputs = { data: unknown }; diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index bdd5f338c..fac1fce34 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -242,6 +242,31 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow('boom'); }); + // Regression test: the "late failure" log is meant for an execution abandoned after the caller's own + // await already gave up (see the test below), not every rejection — this one's caller is still waiting + // and receives the same error normally via its own `rejects.toThrow` above. + test('Should not log a "caller had already stopped waiting" message for an ordinary, timely rejection', async () => { + await expect( + executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + stubExecuteAction, + loadModuleReturning({ + example: () => { + throw new Error('boom'); + }, + }), + mockLogger, + ), + ).rejects.toThrow('boom'); + + expect(mockLogFn).not.toHaveBeenCalledWith( + expect.stringContaining('already stopped waiting'), + 'debug', + ); + }); + test('Should reject with the rejection reason when the customer function rejects asynchronously', async () => { await expect( executeScriptLocally( diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index 738717926..d23580de7 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -22,9 +22,9 @@ function isIndexableRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null; } -/** `globalThis.$` is a runtime-only property TypeScript's built-in `typeof globalThis` has no way to know about — reading it needs one assertion, centralized here instead of repeated inline at each call site. */ +/** `globalThis.$` is a runtime-only property TypeScript's built-in `typeof globalThis` has no way to know about — `Reflect.get` reads it without a type assertion, the same way `deleteGlobalDollar` below already avoids one for deletion. */ function getGlobalDollar(): unknown { - return (globalThis as Record).$; + return Reflect.get(globalThis, '$'); } /** `Object.assign`'s signature doesn't require its source object's keys to already exist on the target, so this installs `$` without asserting `globalThis`'s type. */ @@ -230,8 +230,13 @@ export async function executeScriptLocally( // Racing against the timeout only stops the caller from waiting — run() keeps executing in-process afterward, so a customer function that resumes post-timeout can still fire real $.Actions side effects. True cancellation requires terminating a Worker thread, not possible for in-process execution. const runPromise = run(); + // Set once the race below has settled, so the handler right after can tell a genuinely abandoned rejection (caller already gone) from an ordinary one the caller's own `await Promise.race` is about to receive normally. + let raceSettled = false; // Nothing awaits runPromise once the timeout has already settled the race — an unhandled rejection from it later would otherwise crash the whole dev server process. Logged (not swallowed silently) so a slow real failure is still diagnosable after the caller has already moved on. runPromise.catch((error: unknown) => { + if (!raceSettled) { + return; + } const message = error instanceof Error ? error.message : String(error); log.debug(`"${func.name}" failed after its caller had already stopped waiting: ${message}`); }); @@ -239,6 +244,7 @@ export async function executeScriptLocally( try { return await Promise.race([runPromise, timeout]); } finally { + raceSettled = true; clearTimeout(timer); } } From f6136313715fe0988535f6005ffcdc3cf1ea724e Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Thu, 27 Aug 2026 19:23:44 -0400 Subject: [PATCH 11/15] fix(apps): match backend files with any query string in the transform filter An include filter scoped only to no-query and the exact `?dd-local-exec` suffix lets an unrecognized query (e.g. `?x`, or a malformed `?dd-local-exec&x`) bypass the transform filter entirely, so Vite falls back to its default loader instead of the safe RPC-proxy stub. Matching every query on a backend file and deciding safety in the handler closes that gap. --- packages/plugins/apps/src/backend/types.ts | 2 +- packages/plugins/apps/src/constants.ts | 8 ++-- packages/plugins/apps/src/vite/index.test.ts | 44 ++++++++++++++++--- packages/plugins/apps/src/vite/index.ts | 18 ++++---- .../apps/src/vite/local-execution.test.ts | 14 +++--- .../plugins/apps/src/vite/local-execution.ts | 16 +++---- 6 files changed, 66 insertions(+), 36 deletions(-) diff --git a/packages/plugins/apps/src/backend/types.ts b/packages/plugins/apps/src/backend/types.ts index 95f228d33..2022e8faa 100644 --- a/packages/plugins/apps/src/backend/types.ts +++ b/packages/plugins/apps/src/backend/types.ts @@ -13,5 +13,5 @@ export interface BackendFunction { allowedConnectionIds: string[]; } -/** Shape of a backend function's result, shared by the remote (dev-server.ts) and in-process (local-execution.ts) execution paths — mirrors the Datadog app-builder query response, which wraps a JS action's return value as `{ data: }`. */ +/** Shape of a backend function's result, shared by the remote (dev-server.ts) and in-process (local-execution.ts) paths — mirrors the app-builder query response's `{ data: }` wrapper. */ export type BackendOutputs = { data: unknown }; diff --git a/packages/plugins/apps/src/constants.ts b/packages/plugins/apps/src/constants.ts index 53732b39c..0c16433d8 100644 --- a/packages/plugins/apps/src/constants.ts +++ b/packages/plugins/apps/src/constants.ts @@ -11,11 +11,11 @@ export const APPS_API_PATH = 'api/unstable/app-builder-code/apps'; export const ARCHIVE_FILENAME = 'datadog-apps-assets.zip'; export const BACKEND_FILE_RE = /\.backend\.(ts|tsx|js|jsx)$/; -/** Query suffix marking a local-execution load, so the transform hook below can skip proxy generation for it instead of matching via the broader `options.ssr` flag. */ +/** Query suffix marking a local-execution load, so the transform hook can target it directly instead of matching on the broader `options.ssr` flag. */ export const LOCAL_EXECUTION_LOAD_SUFFIX = '?dd-local-exec'; -// Derived from BACKEND_FILE_RE plus the escaped suffix (its only regex-special character is the leading `?`), so the two can't drift apart if either the extension list or the suffix ever changes. -export const LOCAL_EXECUTION_LOAD_RE = new RegExp( - `${BACKEND_FILE_RE.source.slice(0, -1)}\\${LOCAL_EXECUTION_LOAD_SUFFIX}$`, +// Matches a backend file with any (or no) trailing query string — scoping only to the exact local-execution suffix would let an unrecognized query slip past this filter and leak the real backend source instead of the safe proxy stub; the handler decides safety per case. +export const BACKEND_FILE_WITH_QUERY_RE = new RegExp( + `${BACKEND_FILE_RE.source.slice(0, -1)}(\\?.*)?$`, ); export const BACKEND_CODE_EXTENSIONS = [ '.ts', diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 6dd709ba0..5010b8418 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -236,7 +236,7 @@ describe('Backend Functions - getVitePlugin', () => { expect(mockLogFn).toHaveBeenCalledWith(expect.stringContaining('Intl'), 'warn'); }); - // Regression test: without the suffix check, ssrLoadModule() would get the RPC-proxy stub instead of the real function body. + // Regression test: without the suffix check, ssrLoadModule() would get the proxy stub instead of the real function body. test('Should skip proxy generation for a suffixed local-execution load made from SSR context, returning the real source untouched', async () => { const plugin = getVitePlugin(defaultOptions); const handler = getTransformHandler(plugin); @@ -257,10 +257,7 @@ describe('Backend Functions - getVitePlugin', () => { expect(result).toBeNull(); }); - // Regression test: the suffix alone must not bypass proxy generation — only real local-execution - // loads (via ssrLoadModule, always SSR context) get the real source; a spoofed client-side import - // using the same suffix (e.g. `./secrets.backend.ts?dd-local-exec`) still gets the safe RPC-proxy - // stub, never the real backend module body. + // Regression test: the suffix alone must not bypass proxy generation — a spoofed client-side import reusing it still gets the safe proxy stub, never the real backend module body. test('Should still generate the frontend RPC-proxy for a suffixed import made outside SSR context', async () => { const plugin = getVitePlugin(defaultOptions); const transformHandler = getTransformHandler(plugin); @@ -301,6 +298,43 @@ describe('Backend Functions - getVitePlugin', () => { ).toEqual(expect.stringContaining('executeBackendFunction')); }); + // Regression test: an unrecognized query string must still be caught by the transform filter, or Vite falls back to its default loader and leaks the real backend source. + test('Transform filter should match a backend file carrying an unrecognized query string', () => { + const plugin = getVitePlugin(defaultOptions); + const filter = (plugin!.transform as { filter?: { id?: { include?: RegExp[] } } }).filter; + const includePatterns = filter?.id?.include ?? []; + + const idsThatMustMatch = [ + '/build/src/backend/myHandler.backend.ts', + `/build/src/backend/myHandler.backend.ts${LOCAL_EXECUTION_LOAD_SUFFIX}`, + '/build/src/backend/myHandler.backend.ts?x', + `/build/src/backend/myHandler.backend.ts${LOCAL_EXECUTION_LOAD_SUFFIX}&x`, + ]; + + for (const id of idsThatMustMatch) { + expect(includePatterns.some((pattern) => pattern.test(id))).toBe(true); + } + }); + + // Regression test: an unrecognized query must still default to the safe proxy stub, not the real backend source. + test('Should still generate the frontend RPC-proxy for an import with an unrecognized query string', async () => { + const plugin = getVitePlugin(defaultOptions); + const transformHandler = getTransformHandler(plugin); + + const result = (await transformHandler.call( + { + parse: parseAst, + resolve: jest.fn(async () => null), + load: jest.fn(async () => null), + addWatchFile: jest.fn(), + }, + 'export function myHandler() { return 42; }', + '/build/src/backend/myHandler.backend.ts?x', + )) as { code: string } | null; + + expect(result?.code).toEqual(expect.stringContaining('executeBackendFunction')); + }); + test('Should inject the apps runtime', () => { getVitePlugin(defaultOptions); diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 9c8037bc1..a2b898fd7 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -22,7 +22,7 @@ import { generateProxyModule } from '../backend/proxy-codegen'; import type { BackendFunction } from '../backend/types'; import { BACKEND_FILE_RE, - LOCAL_EXECUTION_LOAD_RE, + BACKEND_FILE_WITH_QUERY_RE, LOCAL_EXECUTION_LOAD_SUFFIX, PLUGIN_NAME, } from '../constants'; @@ -129,7 +129,7 @@ export const getVitePlugin = ({ transform: { filter: { id: { - include: [BACKEND_FILE_RE, LOCAL_EXECUTION_LOAD_RE], + include: [BACKEND_FILE_WITH_QUERY_RE], exclude: [/node_modules/, /[/\\]dist[/\\]/], }, }, @@ -137,15 +137,13 @@ export const getVitePlugin = ({ // them as backend functions, and replace the module with a // frontend proxy that calls executeBackendFunction at runtime. handler(code, id, transformOptions) { - let normalizedId = id; - if (id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX)) { - if (transformOptions?.ssr) { - // Local execution needs the real function body, not the RPC-proxy stub generated below. - return null; - } - // A spoofed client-side import like `./secrets.backend.ts?dd-local-exec` falls through to the same safe proxy-stub generation as any other backend file instead — real local-execution loads always go through ssrLoadModule, which runs in SSR context. Strips the suffix first so this registers under the same relativePath/query-name as the file's real (unsuffixed) import, not a second, corrupted entry. - normalizedId = id.slice(0, -LOCAL_EXECUTION_LOAD_SUFFIX.length); + if (id.endsWith(LOCAL_EXECUTION_LOAD_SUFFIX) && transformOptions?.ssr) { + // Local execution needs the real function body, not the proxy stub below — real loads always go through ssrLoadModule, which runs in SSR, so this only fires for that legitimate path. + return null; } + // Any other case (no query, a spoofed client-side import reusing the suffix, or an unrecognized query) falls through to the safe proxy-stub generation below. Strip the query first so it registers under the file's real (unsuffixed) relativePath/query-name, not a duplicate. + const queryIndex = id.indexOf('?'); + const normalizedId = queryIndex === -1 ? id : id.slice(0, queryIndex); const ast = this.parse(code); const program = ensureProgram(ast, normalizedId); diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index fac1fce34..c445397fb 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -31,7 +31,7 @@ interface TestGlobalDollar { Source: { initiator: { id: string; orgId: string }; runAsUser: { id: string; orgId: string } }; } -/** Reads the `$` this module installs onto `globalThis` during an execution, from the customer-code perspective these tests simulate — genuinely untyped from TypeScript's static perspective since it's a runtime-only property (see local-execution.ts's `setGlobalDollar`). Centralized here instead of repeating the same cast at each call site. */ +/** Reads the `$` this module installs on `globalThis`, from the customer-code perspective these tests simulate — untyped since it's a runtime-only property (see `setGlobalDollar`). Centralized here instead of repeating the cast at each call site. */ function testDollar(): TestGlobalDollar { return (globalThis as unknown as { $: TestGlobalDollar }).$; } @@ -107,7 +107,7 @@ describe('local-execution — executeScriptLocally', () => { let dollarDuringModuleLoad: unknown = 'not captured'; const loadModule: LoadModule = async (specifier) => { if (specifier === func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX) { - // Captures globalThis.$ at module-evaluation time — production's static customer-module import runs before its wrapper installs $, so a customer module reaching for $ during its own top-level evaluation must see the same absence locally, not this execution's own $ installed early. + // Captures globalThis.$ at module-evaluation time — production's static import runs before its wrapper installs $, so code reaching for $ during top-level evaluation must see the same absence locally. dollarDuringModuleLoad = (globalThis as Record).$; return { example: () => 'done' }; } @@ -156,7 +156,7 @@ describe('local-execution — executeScriptLocally', () => { }); test('Should not hang when a customer function returns an un-invoked $.Actions reference instead of calling it', async () => { - // $.Actions.slack.chat is itself a callable Proxy; forgetting the trailing .postMessage(...) call and just returning it must not make `await fn(...args)` treat it as a thenable and hang until the timeout. + // $.Actions.slack.chat is itself a callable Proxy; returning it without the trailing .postMessage(...) call must not make `await fn(...args)` treat it as a thenable and hang. const result = await executeScriptLocally( func, TEST_PROJECT_ROOT, @@ -242,9 +242,7 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow('boom'); }); - // Regression test: the "late failure" log is meant for an execution abandoned after the caller's own - // await already gave up (see the test below), not every rejection — this one's caller is still waiting - // and receives the same error normally via its own `rejects.toThrow` above. + // Regression test: the "late failure" log fires only for an execution abandoned after the caller stopped waiting (see the test below) — here the caller is still waiting and gets the error via `rejects.toThrow` above. test('Should not log a "caller had already stopped waiting" message for an ordinary, timely rejection', async () => { await expect( executeScriptLocally( @@ -430,7 +428,7 @@ describe('local-execution — executeScriptLocally', () => { mockLogger, ); expect(result).toEqual({ data: [] }); - // Compares via a plain boolean, not a direct .toBe() on the value — $.Actions is a Proxy whose get trap returns another Proxy for every property (including well-known symbols), which crashes Jest's diff formatting if this assertion ever fails and needs to pretty-print it. + // Compares via a plain boolean, not .toBe() directly — $.Actions's get trap returns a Proxy for every property, which crashes Jest's diff formatting if this assertion ever fails. expect(Object.is((globalThis as Record).$, preExisting)).toBe(true); } finally { delete (globalThis as Record).$; @@ -685,7 +683,7 @@ describe('local-execution — executeScriptLocally', () => { ); } - // Known race: two concurrent calls both write globalThis.$ synchronously, so the second write wins for both calls' duration. Skip until calls are serialized through an execution queue. + // Known race: two concurrent calls both write globalThis.$ synchronously, so the second write wins for both — skip until calls are serialized through an execution queue. test.skip("Should let each concurrent call see its OWN backendFunctionArgs via globalThis.$, not the other call's", async () => { const [resultA, resultB] = await Promise.all([ executeScriptLocally( diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index d23580de7..cc094e58a 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -22,7 +22,7 @@ function isIndexableRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null; } -/** `globalThis.$` is a runtime-only property TypeScript's built-in `typeof globalThis` has no way to know about — `Reflect.get` reads it without a type assertion, the same way `deleteGlobalDollar` below already avoids one for deletion. */ +/** `globalThis.$` is a runtime-only property `typeof globalThis` doesn't know about; `Reflect.get` reads it without a type assertion, like `deleteGlobalDollar` does for deletion. */ function getGlobalDollar(): unknown { return Reflect.get(globalThis, '$'); } @@ -69,7 +69,7 @@ function assertConnectionIdAllowed( } } -/** Shared validation for both $.Actions entry points (the raw proxy and the action-catalog typed-wrapper dispatcher) — extracted so a future change to this contract can't be applied to one and missed on the other, the exact gap that let the action-catalog path silently forward `inputs: undefined`. */ +/** Shared validation for both $.Actions entry points (raw proxy and action-catalog typed wrapper) — extracted so a contract change can't be applied to one and missed on the other, as happened when the action-catalog path silently forwarded `inputs: undefined`. */ function validateActionCall( call: Partial, allowedConnectionIds: string[], @@ -91,7 +91,7 @@ function makeActionsProxy( ): unknown { return new Proxy(function () {}, { get(_target, prop) { - // A customer function that returns an un-invoked reference (e.g. $.Actions.foo.bar without the trailing call) must not be treated as a thenable — Promise's resolution protocol would call .then() on it and hang until the timeout, since apply() below never settles it. + // An un-invoked reference (e.g. $.Actions.foo.bar with no call) must not look like a thenable, or Promise's resolution protocol calls .then() on it and hangs until timeout. if (prop === 'then') { return undefined; } @@ -188,14 +188,14 @@ export async function executeScriptLocally( }; const run = async (): Promise => { - // Loads and evaluates the customer's module BEFORE installing $ and the SDK bridges below, matching production's own ordering (backend/virtual-entry.ts statically imports the customer module before its wrapper installs $ and the SDK bridges) — code that reaches for $ or a typed action during its own top-level evaluation fails the same way locally as it would in Datadog, instead of silently succeeding against bindings production wouldn't have installed yet. + // Loads the customer module before installing $ and the SDK bridges, matching production's import order (backend/virtual-entry.ts) — code reaching for $ during top-level evaluation fails the same way locally as in Datadog, instead of succeeding early. const mod = await loadModule(func.absolutePath + LOCAL_EXECUTION_LOAD_SUFFIX); const fn = mod[func.name]; if (typeof fn !== 'function') { throw new Error(`"${func.name}" is not a function exported from ${func.absolutePath}`); } - // Restores whatever globalThis.$ held before this call (or removes it entirely if nothing did) once the execution settles, so a pre-existing global (e.g. from zx/globals) isn't permanently clobbered and a completed execution's own context isn't left reachable by unrelated process code. + // Restores whatever globalThis.$ held before this call (or removes it) once the execution settles, so a pre-existing global (e.g. zx/globals) isn't clobbered and this execution's context isn't left reachable afterward. const hadPreviousDollar = Object.prototype.hasOwnProperty.call(globalThis, '$'); const previousDollar = getGlobalDollar(); setGlobalDollar($); @@ -228,11 +228,11 @@ export async function executeScriptLocally( }, timeoutMs); }); - // Racing against the timeout only stops the caller from waiting — run() keeps executing in-process afterward, so a customer function that resumes post-timeout can still fire real $.Actions side effects. True cancellation requires terminating a Worker thread, not possible for in-process execution. + // Racing the timeout only stops the caller from waiting — run() keeps executing afterward, so a resumed customer function can still fire real $.Actions side effects; true cancellation would need a Worker thread, not possible in-process. const runPromise = run(); - // Set once the race below has settled, so the handler right after can tell a genuinely abandoned rejection (caller already gone) from an ordinary one the caller's own `await Promise.race` is about to receive normally. + // Set once the race settles, so the handler below can tell an abandoned rejection (caller already gone) from an ordinary one the caller is about to receive normally. let raceSettled = false; - // Nothing awaits runPromise once the timeout has already settled the race — an unhandled rejection from it later would otherwise crash the whole dev server process. Logged (not swallowed silently) so a slow real failure is still diagnosable after the caller has already moved on. + // Nothing awaits runPromise once the timeout wins the race, so a later rejection would otherwise crash the dev server as unhandled — logged instead so a slow real failure stays diagnosable. runPromise.catch((error: unknown) => { if (!raceSettled) { return; From 454559314f11ceeba2f42dcc3d56031ef0c90877 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Fri, 28 Aug 2026 16:56:49 -0400 Subject: [PATCH 12/15] test(apps): cover makeActionsProxy depth edge cases and array inputs Locks in current behavior for two coverage gaps: depth-1 and zero-depth $.Actions proxy calls (the latter yields a trailing-dot fqn), a 5-segment deep chain, and validateActionCall accepting an array as inputs since typeof [] === 'object'. --- .../apps/src/vite/local-execution.test.ts | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/packages/plugins/apps/src/vite/local-execution.test.ts b/packages/plugins/apps/src/vite/local-execution.test.ts index c445397fb..ce411dd31 100644 --- a/packages/plugins/apps/src/vite/local-execution.test.ts +++ b/packages/plugins/apps/src/vite/local-execution.test.ts @@ -171,6 +171,59 @@ describe('local-execution — executeScriptLocally', () => { expect(result.data).toBeDefined(); }); + test('Should resolve a single-segment $.Actions.foo(...) call to a single-segment fqn', async () => { + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: () => testDollar().Actions.foo({ inputs: { text: 'hi' } }), + }), + mockLogger, + ); + expect(result).toEqual({ data: { ok: true } }); + expect(executeAction).toHaveBeenCalledWith('com.datadoghq.foo', { text: 'hi' }, undefined); + }); + + test('Should resolve a $.Actions(...) call with no property access to a trailing-dot fqn with no action name segment', async () => { + // Documents current behavior: pathParts is empty at this call site, so `com.datadoghq.${pathParts.join('.')}` yields a malformed fqn rather than being rejected. + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: () => testDollar().Actions({ inputs: { text: 'hi' } }), + }), + mockLogger, + ); + expect(result).toEqual({ data: { ok: true } }); + expect(executeAction).toHaveBeenCalledWith('com.datadoghq.', { text: 'hi' }, undefined); + }); + + test('Should resolve a deeply nested $.Actions.a.b.c.d(...) call to its full dotted fqn', async () => { + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: () => testDollar().Actions.a.b.c.d({ inputs: { text: 'hi' } }), + }), + mockLogger, + ); + expect(result).toEqual({ data: { ok: true } }); + expect(executeAction).toHaveBeenCalledWith( + 'com.datadoghq.a.b.c.d', + { text: 'hi' }, + undefined, + ); + }); + test("Should reject a $.Actions call whose connectionId isn't in the function's allowedConnectionIds", async () => { const executeAction = jest.fn().mockResolvedValue({ ok: true }); await expect( @@ -225,6 +278,27 @@ describe('local-execution — executeScriptLocally', () => { ).rejects.toThrow(/must have an inputs field/); }); + test('Should currently accept an array as inputs without a validation error, since typeof [] === "object"', async () => { + // Documents a known, accepted gap: inputs is semantically a plain object of named parameters, but validateActionCall's `typeof inputs !== 'object'` check also passes an array through unchanged. + const executeAction = jest.fn().mockResolvedValue({ ok: true }); + const result = await executeScriptLocally( + func, + TEST_PROJECT_ROOT, + [], + executeAction, + loadModuleReturning({ + example: () => testDollar().Actions.slack.chat.postMessage({ inputs: ['a', 'b'] }), + }), + mockLogger, + ); + expect(result).toEqual({ data: { ok: true } }); + expect(executeAction).toHaveBeenCalledWith( + 'com.datadoghq.slack.chat.postMessage', + ['a', 'b'], + undefined, + ); + }); + test('Should reject with the thrown message when the customer function throws synchronously', async () => { await expect( executeScriptLocally( From 198e4fb4de37beb0c7c5a11163f1c94b9f595b6e Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Fri, 28 Aug 2026 18:01:56 -0400 Subject: [PATCH 13/15] fix(apps): extract inlined function-call arguments into named locals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches this file's own no-inlined-call-argument convention at every other call site — three spots (a nested makeActionsProxy call, a buildRuntimeFromJsFunctionWithActions/setBackend pair, and the Promise.all registration array) still inlined a call directly as another call's argument. Also tightens one comment to state the current invariant rather than narrating a past bug. --- .../plugins/apps/src/vite/local-execution.ts | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/packages/plugins/apps/src/vite/local-execution.ts b/packages/plugins/apps/src/vite/local-execution.ts index cc094e58a..4e315b08e 100644 --- a/packages/plugins/apps/src/vite/local-execution.ts +++ b/packages/plugins/apps/src/vite/local-execution.ts @@ -69,7 +69,7 @@ function assertConnectionIdAllowed( } } -/** Shared validation for both $.Actions entry points (raw proxy and action-catalog typed wrapper) — extracted so a contract change can't be applied to one and missed on the other, as happened when the action-catalog path silently forwarded `inputs: undefined`. */ +/** Shared validation for both $.Actions entry points (raw proxy and action-catalog typed wrapper) — extracted so a contract change can't be applied to one path and missed on the other. */ function validateActionCall( call: Partial, allowedConnectionIds: string[], @@ -95,11 +95,8 @@ function makeActionsProxy( if (prop === 'then') { return undefined; } - return makeActionsProxy( - executeAction, - allowedConnectionIds, - pathParts.concat(String(prop)), - ); + const nestedPathParts = pathParts.concat(String(prop)); + return makeActionsProxy(executeAction, allowedConnectionIds, nestedPathParts); }, async apply(_target, _thisArg, args: unknown[]) { if (args.length === 0) { @@ -165,7 +162,8 @@ async function registerBackendRuntimeIfInstalled( ) { return; } - setBackend(buildRuntimeFromJsFunctionWithActions($)); + const backendRuntime = buildRuntimeFromJsFunctionWithActions($); + setBackend(backendRuntime); } /** `globalThis.$` and the registrations above provide the same customer-visible bindings production's generated wrapper module sets up via text injection. */ @@ -200,15 +198,18 @@ export async function executeScriptLocally( const previousDollar = getGlobalDollar(); setGlobalDollar($); try { - await Promise.all([ - registerActionCatalogIfInstalled( - loadModule, - projectRoot, - executeAction, - func.allowedConnectionIds, - ), - registerBackendRuntimeIfInstalled(loadModule, projectRoot, $), - ]); + const actionCatalogRegistration = registerActionCatalogIfInstalled( + loadModule, + projectRoot, + executeAction, + func.allowedConnectionIds, + ); + const backendRuntimeRegistration = registerBackendRuntimeIfInstalled( + loadModule, + projectRoot, + $, + ); + await Promise.all([actionCatalogRegistration, backendRuntimeRegistration]); const result = await fn(...args); return { data: result }; From c748efd3e4311404087b9cc63e11a872b2b9d607 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Mon, 31 Aug 2026 12:45:35 -0400 Subject: [PATCH 14/15] fix(apps): remove as-casts reintroduced in index.test.ts The transform-object narrowing extracted into getTransformObject handles the filter/handler access without a cast; extractTransformedCode narrows the awaited transform result to its object form the same way, avoiding four `as` casts a later commit had reintroduced in a file an earlier commit already cleaned up. --- packages/plugins/apps/src/vite/index.test.ts | 74 +++++++++++++++----- 1 file changed, 55 insertions(+), 19 deletions(-) diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 5010b8418..3db466d64 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -14,10 +14,10 @@ import { encodeQueryName } from '../backend/encodeQueryName'; import type { BackendFunction } from '../backend/types'; import { LOCAL_EXECUTION_LOAD_SUFFIX } from '../constants'; -type TransformHandler = (code: string, id: string) => unknown; +type TransformHandler = (code: string, id: string, transformOptions?: { ssr?: boolean }) => unknown; -// Narrows `plugin.transform` to the object-hook form via a runtime check, then wraps `handler` in `Reflect.apply` to match `TransformHandler` without casting its wider real signature. -function getTransformHandler(plugin: ReturnType): TransformHandler { +// Narrows `plugin.transform` to the object-hook form via a runtime check, since tests need to access both `handler` and `filter` without an `as` cast. +function getTransformObject(plugin: ReturnType) { const { transform } = plugin ?? {}; if ( typeof transform !== 'object' || @@ -28,13 +28,32 @@ function getTransformHandler(plugin: ReturnType): Transfor 'Expected plugin.transform to be the object-hook form with a handler function', ); } + return transform; +} - const handler = transform.handler; - return function callTransformHandler(this: unknown, code: string, id: string): unknown { - return Reflect.apply(handler, this, [code, id]); +// Wraps `handler` in `Reflect.apply` to match `TransformHandler` without casting its wider real signature. +function getTransformHandler(plugin: ReturnType): TransformHandler { + const { handler } = getTransformObject(plugin); + return function callTransformHandler( + this: unknown, + code: string, + id: string, + transformOptions?: { ssr?: boolean }, + ): unknown { + return Reflect.apply(handler, this, [code, id, transformOptions]); }; } +/** Extracts `.code` from a transform hook's result if it's the object form — avoids an `as` cast on the otherwise-broad Rollup `TransformResult` union, since these tests only ever care about the code string. */ +function extractTransformedCode(result: unknown): string | undefined { + return typeof result === 'object' && + result !== null && + 'code' in result && + typeof result.code === 'string' + ? result.code + : undefined; +} + const functions: BackendFunction[] = [ { relativePath: 'src/backend/myHandler', @@ -262,7 +281,7 @@ describe('Backend Functions - getVitePlugin', () => { const plugin = getVitePlugin(defaultOptions); const transformHandler = getTransformHandler(plugin); - const result = (await transformHandler.call( + const result = await transformHandler.call( { parse: parseAst, resolve: jest.fn(async () => null), @@ -271,9 +290,11 @@ describe('Backend Functions - getVitePlugin', () => { }, 'export function myHandler() { return 42; }', `/build/src/backend/myHandler.backend.ts${LOCAL_EXECUTION_LOAD_SUFFIX}`, - )) as { code: string } | null; + ); - expect(result?.code).toEqual(expect.stringContaining('executeBackendFunction')); + expect(extractTransformedCode(result)).toEqual( + expect.stringContaining('executeBackendFunction'), + ); }); test('Should still generate the frontend RPC-proxy for a normal (unsuffixed) import of the same file', async () => { @@ -291,18 +312,31 @@ describe('Backend Functions - getVitePlugin', () => { '/build/src/backend/myHandler.backend.ts', ); - expect( - typeof result === 'object' && result !== null && 'code' in result - ? result.code - : undefined, - ).toEqual(expect.stringContaining('executeBackendFunction')); + expect(extractTransformedCode(result)).toEqual( + expect.stringContaining('executeBackendFunction'), + ); }); // Regression test: an unrecognized query string must still be caught by the transform filter, or Vite falls back to its default loader and leaks the real backend source. test('Transform filter should match a backend file carrying an unrecognized query string', () => { const plugin = getVitePlugin(defaultOptions); - const filter = (plugin!.transform as { filter?: { id?: { include?: RegExp[] } } }).filter; - const includePatterns = filter?.id?.include ?? []; + const { filter } = getTransformObject(plugin); + const filterId = filter?.id; + // This plugin always configures `filter.id` as `{ include: RegExp[] }` (see vite/index.ts) — + // narrowed here rather than asserted, since Rollup's own StringFilter type also allows a bare + // string/RegExp/array for other plugins' use. + const includePatterns = + typeof filterId === 'object' && + filterId !== null && + !Array.isArray(filterId) && + !(filterId instanceof RegExp) + ? (Array.isArray(filterId.include) + ? filterId.include + : filterId.include + ? [filterId.include] + : [] + ).filter((pattern): pattern is RegExp => pattern instanceof RegExp) + : []; const idsThatMustMatch = [ '/build/src/backend/myHandler.backend.ts', @@ -321,7 +355,7 @@ describe('Backend Functions - getVitePlugin', () => { const plugin = getVitePlugin(defaultOptions); const transformHandler = getTransformHandler(plugin); - const result = (await transformHandler.call( + const result = await transformHandler.call( { parse: parseAst, resolve: jest.fn(async () => null), @@ -330,9 +364,11 @@ describe('Backend Functions - getVitePlugin', () => { }, 'export function myHandler() { return 42; }', '/build/src/backend/myHandler.backend.ts?x', - )) as { code: string } | null; + ); - expect(result?.code).toEqual(expect.stringContaining('executeBackendFunction')); + expect(extractTransformedCode(result)).toEqual( + expect.stringContaining('executeBackendFunction'), + ); }); test('Should inject the apps runtime', () => { From 2f451384248d3913794a177cbf433e7d002091a6 Mon Sep 17 00:00:00 2001 From: Tiffany Trinh Date: Mon, 31 Aug 2026 15:57:15 -0400 Subject: [PATCH 15/15] fix(apps): don't clear a real registration when a query-bearing zero-export id reaches the transform handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BACKEND_FILE_WITH_QUERY_RE deliberately matches a backend file with any query, so an id that isn't the customer's own real re-transform (e.g. some other Vite load hook resolving to zero-export content) can still reach the "no exported functions" branch, which unconditionally cleared this file's registered functions. That's correct for a genuine no-query re-transform (the case HMR relies on), but wrong for a query-bearing id reaching this branch for an unrelated reason — it would silently and permanently break the file's real (unsuffixed) registration until a file edit or server restart, over an import that never touched its real source. Restricts the destructive clear (and its warning) to the exact no-query case. Vite's own ?raw/?url/?worker load hooks all produce a default export, which enumerateBackendExports already rejects with a loud throw before this branch is reached, so this covers whatever else might legitimately produce zero exports without throwing. --- packages/plugins/apps/src/vite/index.test.ts | 46 ++++++++++++++++++++ packages/plugins/apps/src/vite/index.ts | 24 +++++++--- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 3db466d64..1cc04ea73 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -371,6 +371,52 @@ describe('Backend Functions - getVitePlugin', () => { ); }); + // Regression test: a query-bearing id with zero exports must not clear a DIFFERENT, + // already-registered import of the same file's real (unsuffixed) id — otherwise one + // unrelated query-bearing import anywhere in the app permanently breaks the file's real + // registration until an edit or server restart. Vite's own `?raw`/`?url`/`?worker` load hooks + // all produce a default export, which is already rejected with a loud throw before this + // branch is reached — this covers whatever else might legitimately produce zero exports + // without throwing. + test('Should not clear an already-registered function when a query-bearing import of the same file has zero exports', async () => { + const plugin = getVitePlugin(defaultOptions); + const handler = getTransformHandler(plugin); + + // Real, unsuffixed import — registers myHandler normally. + await handler.call( + { + parse: parseAst, + resolve: jest.fn(async () => null), + load: jest.fn(async () => null), + addWatchFile: jest.fn(), + }, + 'export function myHandler() { return 42; }', + '/build/src/backend/myHandler.backend.ts', + ); + + // An unrelated query-bearing import of the SAME file with zero exports (not `export + // default` — Vite's own `?raw`/`?url`/`?worker` load hooks all produce a default export, + // which this file's static checks already reject with a loud throw before this branch is + // ever reached; this covers whatever else might legitimately produce no named exports + // without throwing). + await handler.call( + { + parse: parseAst, + resolve: jest.fn(async () => null), + load: jest.fn(async () => null), + addWatchFile: jest.fn(), + }, + '', + '/build/src/backend/myHandler.backend.ts?some-other-query', + ); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + await (plugin as any).closeBundle(); + + // Still built once for myHandler — the ?raw import didn't clear its real registration. + expect(mockViteBuild).toHaveBeenCalledTimes(1); + }); + test('Should inject the apps runtime', () => { getVitePlugin(defaultOptions); diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index a2b898fd7..e34d6f150 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -153,13 +153,23 @@ export const getVitePlugin = ({ runBackendStaticChecks(ast, normalizedId, log, scopeAnalysis); const exportNames = extractExportedFunctions(ast, normalizedId); if (exportNames.length === 0) { - log.warn( - `Backend file ${normalizedId} has no exported functions. ` + - `Did you forget to add a named export?`, - ); - // Clear any previously registered functions for this file - // so stale entries don't persist across HMR re-transforms. - setBackendFunctions(normalizedId, []); + // Only a genuinely no-query id can be trusted as a real re-transform of this + // exact file's own source. Vite's own `?raw`/`?url`/`?worker` load hooks all + // produce a default export, which enumerateBackendExports already rejects + // with a loud throw before this branch is reached — but some other + // query-bearing load producing zero-export content isn't ruled out, and + // clearing the registry for that case would silently and permanently break + // the file's real (unsuffixed) registration until a file edit or server + // restart, over an import that never touched its real source. + if (queryIndex === -1) { + log.warn( + `Backend file ${normalizedId} has no exported functions. ` + + `Did you forget to add a named export?`, + ); + // Clear any previously registered functions for this file + // so stale entries don't persist across HMR re-transforms. + setBackendFunctions(normalizedId, []); + } return { code: '', map: null }; }