From 513da3c719446c98b89217125c7ab3b3b276a585 Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Mon, 27 Apr 2026 17:37:03 -0400 Subject: [PATCH 01/11] [APPS] Add connections.ts support: emit backend/manifest.json and forward allowedConnectionIds to dev preview --- .../src/backend/extract-connections.test.ts | 384 ++++++++++++++++++ .../apps/src/backend/extract-connections.ts | 167 ++++++++ packages/plugins/apps/src/index.test.ts | 77 ++++ packages/plugins/apps/src/index.ts | 81 +++- .../plugins/apps/src/vite/dev-server.test.ts | 108 +++-- packages/plugins/apps/src/vite/dev-server.ts | 55 ++- packages/plugins/apps/src/vite/index.test.ts | 1 + packages/plugins/apps/src/vite/index.ts | 11 +- 8 files changed, 844 insertions(+), 40 deletions(-) create mode 100644 packages/plugins/apps/src/backend/extract-connections.test.ts create mode 100644 packages/plugins/apps/src/backend/extract-connections.ts diff --git a/packages/plugins/apps/src/backend/extract-connections.test.ts b/packages/plugins/apps/src/backend/extract-connections.test.ts new file mode 100644 index 000000000..4968625fb --- /dev/null +++ b/packages/plugins/apps/src/backend/extract-connections.test.ts @@ -0,0 +1,384 @@ +// 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. + +import { + extractConnectionIds, + findConnectionsFile, +} from '@dd/apps-plugin/backend/extract-connections'; +import type { + ExportNamedDeclaration, + ObjectExpression, + Program, + Property, + SpreadElement, +} from 'estree'; +import { promises as fsp } from 'fs'; +import os from 'os'; +import path from 'path'; + +/** + * Build a minimal ESTree Program node containing the given top-level statements. + */ +function program(body: Program['body']): Program { + return { type: 'Program', sourceType: 'module', body }; +} + +/** + * Build an `export const connections = ` declaration. + */ +function exportConnections(properties: ObjectExpression['properties']): ExportNamedDeclaration { + return { + type: 'ExportNamedDeclaration', + declaration: { + type: 'VariableDeclaration', + kind: 'const', + declarations: [ + { + type: 'VariableDeclarator', + id: { type: 'Identifier', name: 'connections' }, + init: { type: 'ObjectExpression', properties }, + }, + ], + }, + specifiers: [], + source: null, + attributes: [], + }; +} + +/** + * Build a `KEY: 'value'` ObjectExpression property whose value is a string literal. + */ +function stringProperty(key: string, value: string): Property { + return { + type: 'Property', + key: { type: 'Identifier', name: key }, + value: { type: 'Literal', value }, + kind: 'init', + method: false, + shorthand: false, + computed: false, + }; +} + +const filePath = '/project/connections.ts'; + +describe('extract-connections - extractConnectionIds', () => { + const acceptedCases = [ + { + description: 'single literal value', + ast: program([exportConnections([stringProperty('OPEN_AI', 'uuid-1')])]), + expected: ['uuid-1'], + }, + { + description: 'multiple values, sorted and deduplicated', + ast: program([ + exportConnections([ + stringProperty('A', 'uuid-z'), + stringProperty('B', 'uuid-a'), + stringProperty('C', 'uuid-z'), + ]), + ]), + expected: ['uuid-a', 'uuid-z'], + }, + { + description: 'string-literal keys', + ast: program([ + exportConnections([ + { + type: 'Property', + key: { type: 'Literal', value: 'open-ai' }, + value: { type: 'Literal', value: 'uuid-1' }, + kind: 'init', + method: false, + shorthand: false, + computed: false, + }, + ]), + ]), + expected: ['uuid-1'], + }, + { + description: 'template literal value with no interpolation', + ast: program([ + exportConnections([ + { + type: 'Property', + key: { type: 'Identifier', name: 'OPEN_AI' }, + value: { + type: 'TemplateLiteral', + expressions: [], + quasis: [ + { + type: 'TemplateElement', + value: { cooked: 'uuid-tmpl', raw: 'uuid-tmpl' }, + tail: true, + }, + ], + }, + kind: 'init', + method: false, + shorthand: false, + computed: false, + }, + ]), + ]), + expected: ['uuid-tmpl'], + }, + { + description: 'empty object', + ast: program([exportConnections([])]), + expected: [], + }, + ]; + + test.each(acceptedCases)('Should accept $description', ({ ast, expected }) => { + expect(extractConnectionIds(ast, filePath)).toEqual(expected); + }); + + test('Should throw when no "export const connections" is present', () => { + const ast = program([ + { + type: 'VariableDeclaration', + kind: 'const', + declarations: [ + { + type: 'VariableDeclarator', + id: { type: 'Identifier', name: 'connections' }, + init: { type: 'ObjectExpression', properties: [] }, + }, + ], + }, + ]); + expect(() => extractConnectionIds(ast, filePath)).toThrow( + 'connections file must define "export const connections = { ... }"', + ); + }); + + test('Should throw when default-exported instead of named export', () => { + const ast = program([ + { + type: 'ExportDefaultDeclaration', + declaration: { type: 'ObjectExpression', properties: [] }, + }, + ]); + expect(() => extractConnectionIds(ast, filePath)).toThrow( + 'connections file must define "export const connections = { ... }"', + ); + }); + + test('Should throw when initialized with a non-object expression', () => { + const ast = program([ + { + type: 'ExportNamedDeclaration', + declaration: { + type: 'VariableDeclaration', + kind: 'const', + declarations: [ + { + type: 'VariableDeclarator', + id: { type: 'Identifier', name: 'connections' }, + init: { type: 'Literal', value: 'oops' }, + }, + ], + }, + specifiers: [], + source: null, + attributes: [], + }, + ]); + expect(() => extractConnectionIds(ast, filePath)).toThrow( + '"export const connections" must be initialized with an object literal', + ); + }); + + test('Should throw on multiple "export const connections" declarations', () => { + const ast = program([ + exportConnections([stringProperty('A', 'uuid-1')]), + exportConnections([stringProperty('B', 'uuid-2')]), + ]); + expect(() => extractConnectionIds(ast, filePath)).toThrow( + 'multiple top-level "export const connections" declarations are not allowed', + ); + }); + + test('Should throw on computed keys', () => { + const ast = program([ + exportConnections([ + { + type: 'Property', + key: { type: 'Identifier', name: 'KEY' }, + value: { type: 'Literal', value: 'uuid' }, + kind: 'init', + method: false, + shorthand: false, + computed: true, + }, + ]), + ]); + expect(() => extractConnectionIds(ast, filePath)).toThrow('computed keys'); + }); + + test('Should throw on spread elements', () => { + const ast = program([ + exportConnections([ + { + type: 'SpreadElement', + argument: { type: 'Identifier', name: 'other' }, + } as SpreadElement, + ]), + ]); + expect(() => extractConnectionIds(ast, filePath)).toThrow('spread elements'); + }); + + const rejectedValueCases: Array<{ + description: string; + value: Property['value']; + reasonContains: string; + }> = [ + { + description: 'identifier reference', + value: { type: 'Identifier', name: 'someConst' }, + reasonContains: 'must be a string literal', + }, + { + description: 'env var (member expression)', + value: { + type: 'MemberExpression', + object: { + type: 'MemberExpression', + object: { type: 'Identifier', name: 'process' }, + property: { type: 'Identifier', name: 'env' }, + computed: false, + optional: false, + }, + property: { type: 'Identifier', name: 'OPEN_AI_ID' }, + computed: false, + optional: false, + }, + reasonContains: 'must be a string literal', + }, + { + description: 'binary expression (concatenation)', + value: { + type: 'BinaryExpression', + operator: '+', + left: { type: 'Literal', value: 'a-' }, + right: { type: 'Literal', value: 'b' }, + }, + reasonContains: 'must be a string literal', + }, + { + description: 'function call', + value: { + type: 'CallExpression', + callee: { type: 'Identifier', name: 'getId' }, + arguments: [], + optional: false, + }, + reasonContains: 'must be a string literal', + }, + { + description: 'template literal with interpolation', + value: { + type: 'TemplateLiteral', + expressions: [{ type: 'Identifier', name: 'suffix' }], + quasis: [ + { + type: 'TemplateElement', + value: { cooked: 'pre-', raw: 'pre-' }, + tail: false, + }, + { + type: 'TemplateElement', + value: { cooked: '', raw: '' }, + tail: true, + }, + ], + }, + reasonContains: 'template literals with interpolations', + }, + { + description: 'numeric literal', + value: { type: 'Literal', value: 42 }, + reasonContains: 'must be a string literal', + }, + ]; + + test.each(rejectedValueCases)( + 'Should throw on non-literal value: $description', + ({ value, reasonContains }) => { + const property: Property = { + type: 'Property', + key: { type: 'Identifier', name: 'BAD' }, + value, + kind: 'init', + method: false, + shorthand: false, + computed: false, + }; + const ast = program([exportConnections([property])]); + expect(() => extractConnectionIds(ast, filePath)).toThrow(reasonContains); + }, + ); +}); + +describe('extract-connections - findConnectionsFile', () => { + let buildRoot: string; + + beforeEach(async () => { + buildRoot = await fsp.mkdtemp(path.join(os.tmpdir(), 'connections-test-')); + }); + + afterEach(async () => { + await fsp.rm(buildRoot, { recursive: true, force: true }); + }); + + test('Should return undefined when no connections file exists', async () => { + await expect(findConnectionsFile(buildRoot)).resolves.toBeUndefined(); + }); + + test.each([ + { ext: '.ts' as const }, + { ext: '.tsx' as const }, + { ext: '.js' as const }, + { ext: '.jsx' as const }, + ])('Should find connections$ext when it exists', async ({ ext }) => { + const expected = path.join(buildRoot, `connections${ext}`); + await fsp.writeFile(expected, 'export const connections = {} as const;'); + await expect(findConnectionsFile(buildRoot)).resolves.toBe(expected); + }); + + test('Should prefer .ts over other extensions', async () => { + await fsp.writeFile( + path.join(buildRoot, 'connections.ts'), + 'export const connections = {} as const;', + ); + await fsp.writeFile( + path.join(buildRoot, 'connections.tsx'), + 'export const connections = {} as const;', + ); + await fsp.writeFile( + path.join(buildRoot, 'connections.js'), + 'export const connections = {};', + ); + await expect(findConnectionsFile(buildRoot)).resolves.toBe( + path.join(buildRoot, 'connections.ts'), + ); + }); + + test('Should prefer .tsx over .js when .ts is absent', async () => { + await fsp.writeFile( + path.join(buildRoot, 'connections.tsx'), + 'export const connections = {} as const;', + ); + await fsp.writeFile( + path.join(buildRoot, 'connections.js'), + 'export const connections = {};', + ); + await expect(findConnectionsFile(buildRoot)).resolves.toBe( + path.join(buildRoot, 'connections.tsx'), + ); + }); +}); diff --git a/packages/plugins/apps/src/backend/extract-connections.ts b/packages/plugins/apps/src/backend/extract-connections.ts new file mode 100644 index 000000000..16e31f1d7 --- /dev/null +++ b/packages/plugins/apps/src/backend/extract-connections.ts @@ -0,0 +1,167 @@ +// 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. + +import type { Expression, Node, ObjectExpression, Program, Property } from 'estree'; +import { promises as fsp } from 'fs'; +import path from 'path'; + +const CONNECTIONS_BASENAME = 'connections'; +const CONNECTIONS_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx'] as const; + +/** + * Locate the project's connections file. Looks for `connections.{ts,tsx,js,jsx}` + * at `buildRoot` and returns the absolute path of the first match in priority + * order, or `undefined` when none exists. + */ +export async function findConnectionsFile(buildRoot: string): Promise { + for (const ext of CONNECTIONS_EXTENSIONS) { + const candidate = path.join(buildRoot, `${CONNECTIONS_BASENAME}${ext}`); + try { + await fsp.access(candidate); + return candidate; + } catch { + // not found at this extension — try the next. + } + } + return undefined; +} + +/** + * Extract connection IDs from a parsed connections-file AST. + * + * The file must contain exactly one top-level export of the form: + * + * export const connections = { + * NAME_A: 'uuid-a', + * NAME_B: 'uuid-b', + * } as const; + * + * Values must be plain string literals or interpolation-free template literals. + * Anything else (identifiers, env vars, concatenation, function calls, computed + * keys, spread elements, …) throws with a framed source location so the caller + * can surface a build-time error. + * + * Returns the union of values, deduplicated and sorted lexicographically for + * deterministic manifests. + */ +export function extractConnectionIds(ast: Program, filePath: string): string[] { + if (ast.type !== 'Program') { + throw new Error( + `Expected a Program node from this.parse() for ${filePath}, got ${(ast as Node).type}`, + ); + } + + let connectionsObject: ObjectExpression | undefined; + + for (const node of ast.body) { + if (node.type !== 'ExportNamedDeclaration' || !node.declaration) { + continue; + } + const decl = node.declaration; + if (decl.type !== 'VariableDeclaration') { + continue; + } + for (const d of decl.declarations) { + if (d.id.type !== 'Identifier' || d.id.name !== CONNECTIONS_BASENAME) { + continue; + } + if (connectionsObject) { + throw fail( + filePath, + d.loc, + `multiple top-level "export const ${CONNECTIONS_BASENAME}" declarations are not allowed`, + ); + } + if (!d.init || d.init.type !== 'ObjectExpression') { + throw fail( + filePath, + (d.init ?? d).loc, + `"export const ${CONNECTIONS_BASENAME}" must be initialized with an object literal`, + ); + } + connectionsObject = d.init; + } + } + + if (!connectionsObject) { + throw fail( + filePath, + null, + `connections file must define "export const ${CONNECTIONS_BASENAME} = { ... }"`, + ); + } + + const ids = new Set(); + for (const property of connectionsObject.properties) { + if (property.type === 'SpreadElement') { + throw fail( + filePath, + property.loc, + `spread elements are not supported inside "${CONNECTIONS_BASENAME}"`, + ); + } + if (property.computed) { + throw fail( + filePath, + property.loc, + `computed keys are not supported inside "${CONNECTIONS_BASENAME}"`, + ); + } + const keyName = readKeyName(property); + const value = extractStaticString(property.value, keyName, filePath); + ids.add(value); + } + + return [...ids].sort(); +} + +/** + * Resolve a property value node to its static string. Accepts string literals + * and interpolation-free template literals; throws on anything else. + */ +function extractStaticString(value: Property['value'], keyName: string, filePath: string): string { + if (value.type === 'Literal' && typeof value.value === 'string') { + return value.value; + } + if (value.type === 'TemplateLiteral') { + if (value.expressions.length > 0) { + throw fail( + filePath, + value.loc, + `value for "${keyName}" must be a static string — template literals with interpolations are not allowed`, + ); + } + const quasi = value.quasis[0]; + return quasi.value.cooked ?? quasi.value.raw; + } + throw fail( + filePath, + value.loc, + `value for "${keyName}" must be a string literal; got ${describeNode(value)}`, + ); +} + +/** + * Read a property's key name as a string. Computed keys are rejected upstream, + * so this only handles `Identifier` (e.g. `OPEN_AI: '...'`) and string + * `Literal` (`'open-ai': '...'`) forms. + */ +function readKeyName(property: Property): string { + if (property.key.type === 'Identifier') { + return property.key.name; + } + if (property.key.type === 'Literal') { + return String(property.key.value); + } + return ''; +} + +function describeNode(node: Expression | Property['value']): string { + return node.type; +} + +function fail(filePath: string, loc: Node['loc'], reason: string): Error { + const where = loc?.start ? `${filePath}:${loc.start.line}:${loc.start.column + 1}` : filePath; + return new Error(`[connections] ${reason} (at ${where})`); +} diff --git a/packages/plugins/apps/src/index.test.ts b/packages/plugins/apps/src/index.test.ts index 553f29c2a..4d37d7d60 100644 --- a/packages/plugins/apps/src/index.test.ts +++ b/packages/plugins/apps/src/index.test.ts @@ -4,6 +4,7 @@ import * as archive from '@dd/apps-plugin/archive'; import * as assets from '@dd/apps-plugin/assets'; +import * as extractConnections from '@dd/apps-plugin/backend/extract-connections'; import * as identifier from '@dd/apps-plugin/identifier'; import * as uploader from '@dd/apps-plugin/upload'; import { getPlugins } from '@dd/apps-plugin'; @@ -29,6 +30,29 @@ function extractCloseBundle(plugins: PluginOptions[]) { return plugin.vite!.closeBundle as () => Promise; } +/** Extract and assert buildStart from the first plugin. */ +function extractBuildStart(plugins: PluginOptions[]) { + const plugin = plugins[0]; + expect(typeof plugin?.buildStart).toBe('function'); + return plugin.buildStart as (this: unknown) => Promise; +} + +/** Build a minimal mock of the unplugin/Rollup PluginContext used by buildStart. */ +function createMockPluginContext(loadedCode: string | null) { + return { + addWatchFile: jest.fn(), + load: jest.fn().mockResolvedValue({ code: loadedCode }), + parse: jest.fn().mockImplementation(() => ({ + // The actual extraction is asserted via the `extractConnectionIds` + // spy in the calling test — `parse` just needs to return something + // program-shaped that can flow through the buildStart hook. + type: 'Program', + sourceType: 'module', + body: [], + })), + }; +} + describe('Apps Plugin - getPlugins', () => { const buildRoot = '/project'; const outDir = '/project/dist'; @@ -217,6 +241,59 @@ describe('Apps Plugin - getPlugins', () => { expect(fsHelpers.rm).toHaveBeenCalledWith(path.resolve('/tmp/dd-apps-456')); }); + describe('buildStart - connection IDs', () => { + test('Should be a no-op when no connections file exists', async () => { + const findSpy = jest + .spyOn(extractConnections, 'findConnectionsFile') + .mockResolvedValue(undefined); + const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); + + const buildStart = extractBuildStart(getPlugins(getArgs())); + const ctx = createMockPluginContext(null); + + await buildStart.call(ctx); + + expect(findSpy).toHaveBeenCalledWith(buildRoot); + expect(ctx.load).not.toHaveBeenCalled(); + expect(ctx.addWatchFile).not.toHaveBeenCalled(); + expect(extractSpy).not.toHaveBeenCalled(); + }); + + test('Should load, parse, and extract IDs when connections file is present', async () => { + const connectionsPath = path.join(buildRoot, 'connections.ts'); + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( + connectionsPath, + ); + const extractSpy = jest + .spyOn(extractConnections, 'extractConnectionIds') + .mockReturnValue(['uuid-a', 'uuid-b']); + + const buildStart = extractBuildStart(getPlugins(getArgs())); + const ctx = createMockPluginContext('export const connections = {} as const;'); + + await buildStart.call(ctx); + + expect(ctx.addWatchFile).toHaveBeenCalledWith(connectionsPath); + expect(ctx.load).toHaveBeenCalledWith({ id: connectionsPath }); + expect(ctx.parse).toHaveBeenCalledWith('export const connections = {} as const;'); + expect(extractSpy).toHaveBeenCalled(); + }); + + test('Should throw when ctx.load returns no code for the connections file', async () => { + const connectionsPath = path.join(buildRoot, 'connections.ts'); + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( + connectionsPath, + ); + + const buildStart = extractBuildStart(getPlugins(getArgs())); + const ctx = createMockPluginContext(null); + + await expect(buildStart.call(ctx)).rejects.toThrow( + `connections file '${connectionsPath}' produced no code when loaded`, + ); + }); + }); + test('Should upload assets with vite bundler', async () => { const intakeHost = 'https://api.example.com'; const scope = nock(intakeHost).post(`/${APPS_API_PATH}/app-id/upload`).reply(200, { diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index 13af8ff23..915fd94b7 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -6,6 +6,9 @@ import { rm } from '@dd/core/helpers/fs'; import type { GetPlugins } from '@dd/core/types'; import { InjectPosition } from '@dd/core/types'; import chalk from 'chalk'; +import type { Program } from 'estree'; +import fsp from 'fs/promises'; +import os from 'os'; import path from 'path'; import { createArchive } from './archive'; @@ -14,6 +17,7 @@ import { collectAssets } from './assets'; import type { BackendFunction } from './backend/discovery'; import { extractExportedFunctions } from './backend/discovery'; import { encodeQueryName } from './backend/encodeQueryName'; +import { extractConnectionIds, findConnectionsFile } from './backend/extract-connections'; import { generateProxyModule } from './backend/proxy-codegen'; import { BACKEND_FILE_RE, CONFIG_KEY, PLUGIN_NAME } from './constants'; import { resolveIdentifier } from './identifier'; @@ -71,6 +75,24 @@ function createBackendFunctionRegistry() { }; } +/** + * Create a registry for the connection IDs extracted from `connections.ts`. + * Populated by the `buildStart` hook (once per build) and read by + * `handleUpload` (production manifest emission) and the dev middleware + * (preview-async request body). + */ +function createConnectionIdsRegistry() { + let connectionIds: string[] = []; + return { + setConnectionIds(ids: string[]) { + connectionIds = ids; + }, + getConnectionIds() { + return connectionIds; + }, + }; +} + export type types = { // Add the types you'd like to expose here. AppsOptions: AppsOptions; @@ -105,10 +127,12 @@ export const getPlugins: GetPlugins = ({ options, context, bundler }) => { }); const { setBackendFunctions, getBackendFunctions } = createBackendFunctionRegistry(); + const { setConnectionIds, getConnectionIds } = createConnectionIdsRegistry(); const handleUpload = async (backendOutputs: Map) => { const handleTimer = log.time('handle assets'); let archiveDir: string | undefined; + let manifestDir: string | undefined; try { const identifierTimer = log.time('resolve identifier'); @@ -158,6 +182,29 @@ Either: }); } + // Emit backend/manifest.json with the per-function allowed connection + // IDs so the server-side actions runtime can allowlist the connections + // each function uses. The same union list (from connections.ts) is + // applied to every function — the server supports distinct lists, but + // the RFC explicitly accepts a flat union as the chosen design. + if (backendOutputs.size > 0) { + const allowedConnectionIds = getConnectionIds(); + const manifest: Record = {}; + for (const bundleName of backendOutputs.keys()) { + manifest[bundleName] = { allowedConnectionIds }; + } + manifestDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'dd-apps-manifest-')); + const manifestPath = path.join(manifestDir, 'manifest.json'); + await fsp.writeFile(manifestPath, JSON.stringify(manifest, null, 2)); + allAssets.push({ + absolutePath: manifestPath, + relativePath: 'backend/manifest.json', + }); + log.debug( + `Emitted backend/manifest.json with ${allowedConnectionIds.length} connection ID(s)`, + ); + } + const archiveTimer = log.time('archive assets'); const archive = await createArchive(allAssets); archiveTimer.end(); @@ -198,10 +245,13 @@ Either: log.error(`${red('Failed to upload assets:')}\n${error?.message || error}`); } - // Clean temporary directory + // Clean temporary directories if (archiveDir) { await rm(archiveDir); } + if (manifestDir) { + await rm(manifestDir); + } handleTimer.end(); if (toThrow) { @@ -216,6 +266,34 @@ Either: { name: PLUGIN_NAME, enforce: 'post', + // Fires once per build (and re-fires on watch invalidation when + // connections.ts changes, because addWatchFile registers it as a + // build dependency). Each /__dd/executeAction dev request triggers + // its own nested viteBuild() and therefore its own buildStart, so + // dev always reads fresh state before bundling. + // + // The `load` method is provided by Vite/Rollup but isn't on + // unplugin's common UnpluginBuildContext. We only run under Vite + // (the plugin returns early for other bundlers above), so a + // structural cast keeps types clean without importing from + // rollup/vite. + async buildStart() { + const filePath = await findConnectionsFile(context.buildRoot); + if (!filePath) { + setConnectionIds([]); + return; + } + this.addWatchFile(filePath); + const ctx = this as unknown as { + load: (options: { id: string }) => Promise<{ code?: string | null }>; + }; + const info = await ctx.load({ id: filePath }); + if (info.code == null) { + throw new Error(`connections file '${filePath}' produced no code when loaded`); + } + const ast = this.parse(info.code); + setConnectionIds(extractConnectionIds(ast as unknown as Program, filePath)); + }, transform: { filter: { id: { @@ -254,6 +332,7 @@ Either: viteBuild: bundler.build, buildRoot: context.buildRoot, getBackendFunctions, + getConnectionIds, handleUpload, log, auth: context.auth, diff --git a/packages/plugins/apps/src/vite/dev-server.test.ts b/packages/plugins/apps/src/vite/dev-server.test.ts index 2c7b203a0..20c96ba99 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -94,13 +94,14 @@ describe('Dev Server Middleware', () => { }); describe('createDevServerMiddleware routing', () => { - const middleware = createDevServerMiddleware( - mockViteBuild, - () => mockFunctions, - mockAuth, - '/project', - mockLog, - ); + const middleware = createDevServerMiddleware({ + viteBuild: mockViteBuild, + getBackendFunctions: () => mockFunctions, + getConnectionIds: () => [], + auth: mockAuth, + projectRoot: '/project', + log: mockLog, + }); test('Should call next() for non-POST requests', () => { const req = { method: 'GET', url: '/__dd/debugBundle' } as unknown as IncomingMessage; @@ -181,13 +182,14 @@ describe('Dev Server Middleware', () => { }); describe('debugBundle handler', () => { - const middleware = createDevServerMiddleware( - mockViteBuild, - () => mockFunctions, - mockAuth, - '/project', - mockLog, - ); + const middleware = createDevServerMiddleware({ + viteBuild: mockViteBuild, + getBackendFunctions: () => mockFunctions, + getConnectionIds: () => [], + auth: mockAuth, + projectRoot: '/project', + log: mockLog, + }); test('Should return 400 for missing functionRef', async () => { const req = createMockRequest('/__dd/debugBundle', {}); @@ -256,13 +258,14 @@ describe('Dev Server Middleware', () => { }); describe('executeAction handler', () => { - const middleware = createDevServerMiddleware( - mockViteBuild, - () => mockFunctions, - mockAuth, - '/project', - mockLog, - ); + const middleware = createDevServerMiddleware({ + viteBuild: mockViteBuild, + getBackendFunctions: () => mockFunctions, + getConnectionIds: () => [], + auth: mockAuth, + projectRoot: '/project', + log: mockLog, + }); test('Should return 400 for missing functionRef', async () => { const req = createMockRequest('/__dd/executeAction', {}); @@ -375,6 +378,54 @@ describe('Dev Server Middleware', () => { expect(body.error).toContain('Script threw an error'); }); + test('Should include allowedConnectionIds inside inputs of the preview-async body', async () => { + mockViteBuild.mockResolvedValue(mockBuildResult('// code')); + + const middlewareWithIds = createDevServerMiddleware({ + viteBuild: mockViteBuild, + getBackendFunctions: () => mockFunctions, + getConnectionIds: () => ['uuid-1', 'uuid-2'], + auth: mockAuth, + projectRoot: '/project', + log: mockLog, + }); + + let capturedBody: unknown; + const apiScope = nock(`https://${DD_SITE}`) + .post('/api/v2/app-builder/queries/preview-async', (body) => { + capturedBody = body; + return true; + }) + .reply(200, { data: { id: 'receipt-conn' } }) + .get('/api/v2/app-builder/queries/execution-long-polling/receipt-conn') + .reply(200, { + data: { attributes: { done: true, outputs: { data: { ok: true } } } }, + }); + + const req = createMockRequest('/__dd/executeAction', { + functionName: encodeQueryName(mockFunctions[0]), + args: [], + }); + const res = createMockResponse(); + + middlewareWithIds(req, res, jest.fn()); + await res.done; + + expect(res.statusCode).toBe(200); + expect(apiScope.isDone()).toBe(true); + + const inputs = ( + capturedBody as { + data: { + attributes: { query: { properties: { spec: { inputs: unknown } } } }; + }; + } + ).data.attributes.query.properties.spec.inputs as { + allowedConnectionIds: string[]; + }; + expect(inputs.allowedConnectionIds).toEqual(['uuid-1', 'uuid-2']); + }); + test('Should retry when long-poll returns done: false', async () => { mockViteBuild.mockResolvedValue(mockBuildResult('// code')); @@ -408,13 +459,14 @@ describe('Dev Server Middleware', () => { describe('dynamic discovery', () => { test('Should not find stale function after re-transform (HMR)', async () => { let currentFunctions: BackendFunction[] = [...mockFunctions]; - const middleware = createDevServerMiddleware( - mockViteBuild, - () => currentFunctions, - mockAuth, - '/project', - mockLog, - ); + const middleware = createDevServerMiddleware({ + viteBuild: mockViteBuild, + getBackendFunctions: () => currentFunctions, + getConnectionIds: () => [], + auth: mockAuth, + projectRoot: '/project', + log: mockLog, + }); // Simulate HMR: greet is renamed to greetV2 in the same file. currentFunctions = [ diff --git a/packages/plugins/apps/src/vite/dev-server.ts b/packages/plugins/apps/src/vite/dev-server.ts index f2ecbb43b..9f54d87f6 100644 --- a/packages/plugins/apps/src/vite/dev-server.ts +++ b/packages/plugins/apps/src/vite/dev-server.ts @@ -111,10 +111,16 @@ async function bundleBackendFunction( /** * Execute a script via Datadog's app-builder queries API. + * + * `allowedConnectionIds` is forwarded inside the action's `inputs` so the + * server-side actions runtime allowlists the connections this preview can use. + * Empty list means no connection-using actions are permitted (same effect as + * not declaring any connections). */ async function executeScriptViaDatadog( scriptBody: string, displayName: string, + allowedConnectionIds: string[], auth: AuthConfig, log: Logger, ): Promise { @@ -133,7 +139,7 @@ async function executeScriptViaDatadog( properties: { spec: { fqn: 'com.datadoghq.datatransformation.jsFunctionWithActions', - inputs: { script: scriptBody }, + inputs: { script: scriptBody, allowedConnectionIds }, }, onlyTriggerManually: true, }, @@ -294,6 +300,7 @@ async function handleExecuteAction( res: ServerResponse, functionsByName: Map, bundle: BundleFn, + allowedConnectionIds: string[], auth: AuthConfig, log: Logger, ): Promise { @@ -302,7 +309,13 @@ async function handleExecuteAction( log.debug(`Executing action: ${displayName} with args`); - const result = await executeScriptViaDatadog(code, displayName, auth, log); + const result = await executeScriptViaDatadog( + code, + displayName, + allowedConnectionIds, + auth, + log, + ); res.statusCode = 200; res.setHeader('Content-Type', 'application/json'); @@ -322,6 +335,15 @@ function buildFunctionMap(backendFunctions: BackendFunction[]): Map [encodeQueryName(f), f])); } +export interface DevServerMiddlewareOptions { + viteBuild: typeof build; + getBackendFunctions: () => BackendFunction[]; + getConnectionIds: () => string[]; + auth: AuthOptionsWithDefaults; + projectRoot: string; + log: Logger; +} + /** * Create a Connect-compatible middleware for the Vite dev server. * Intercepts backend function requests and handles them via Datadog API. @@ -330,13 +352,18 @@ function buildFunctionMap(backendFunctions: BackendFunction[]): Map BackendFunction[], - auth: AuthOptionsWithDefaults, - projectRoot: string, - log: Logger, -): (req: IncomingMessage, res: ServerResponse, next: () => void) => void { +export function createDevServerMiddleware({ + viteBuild, + getBackendFunctions, + getConnectionIds, + auth, + projectRoot, + log, +}: DevServerMiddlewareOptions): ( + req: IncomingMessage, + res: ServerResponse, + next: () => void, +) => void { const bundle = (func: BackendFunction, args: unknown[]) => bundleBackendFunction(viteBuild, func, args, projectRoot, log); @@ -381,7 +408,15 @@ export function createDevServerMiddleware( ); return; } - handleExecuteAction(req, res, functionsByName, bundle, fullAuth, log).catch(() => { + handleExecuteAction( + req, + res, + functionsByName, + bundle, + getConnectionIds(), + fullAuth, + log, + ).catch(() => { sendError(res, 500, 'Unexpected error'); }); } else { diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 5d923f8a7..9b7550d97 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -38,6 +38,7 @@ const defaultOptions = { viteBuild: mockViteBuild, buildRoot: '/build', getBackendFunctions: () => functions, + getConnectionIds: () => [], handleUpload: mockHandleUpload, log, auth: { site: 'datadoghq.com' }, diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index b35658349..cacc3f38b 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -15,6 +15,7 @@ export interface VitePluginOptions { viteBuild: typeof build; buildRoot: string; getBackendFunctions: () => BackendFunction[]; + getConnectionIds: () => string[]; handleUpload: (backendOutputs: Map) => Promise; log: Logger; auth: AuthOptionsWithDefaults; @@ -33,6 +34,7 @@ export const getVitePlugin = ({ viteBuild, buildRoot, getBackendFunctions, + getConnectionIds, handleUpload, log, auth, @@ -56,7 +58,14 @@ export const getVitePlugin = ({ }, configureServer(server) { server.middlewares.use( - createDevServerMiddleware(viteBuild, getBackendFunctions, auth, buildRoot, log), + createDevServerMiddleware({ + viteBuild, + getBackendFunctions, + getConnectionIds, + auth, + projectRoot: buildRoot, + log, + }), ); }, }); From 808fd00a635c475175c5b6ac8a5e03f9fec5a7a6 Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Tue, 28 Apr 2026 10:25:18 -0400 Subject: [PATCH 02/11] Move buildStart into vite sub-plugin to drop unsafe cast on this.load --- packages/plugins/apps/src/index.test.ts | 151 ++++++++++++++++++- packages/plugins/apps/src/index.ts | 95 +++++++----- packages/plugins/apps/src/vite/index.test.ts | 6 +- packages/plugins/apps/src/vite/index.ts | 64 +++++++- 4 files changed, 272 insertions(+), 44 deletions(-) diff --git a/packages/plugins/apps/src/index.test.ts b/packages/plugins/apps/src/index.test.ts index 4d37d7d60..13fd2dbea 100644 --- a/packages/plugins/apps/src/index.test.ts +++ b/packages/plugins/apps/src/index.test.ts @@ -30,11 +30,32 @@ function extractCloseBundle(plugins: PluginOptions[]) { return plugin.vite!.closeBundle as () => Promise; } -/** Extract and assert buildStart from the first plugin. */ +/** Extract and assert buildStart from the first plugin's vite hooks. */ function extractBuildStart(plugins: PluginOptions[]) { const plugin = plugins[0]; - expect(typeof plugin?.buildStart).toBe('function'); - return plugin.buildStart as (this: unknown) => Promise; + const buildStart = (plugin?.vite as { buildStart?: unknown } | undefined)?.buildStart; + expect(typeof buildStart).toBe('function'); + return buildStart as (this: unknown) => Promise; +} + +/** Extract and assert handleHotUpdate from the first plugin's vite hooks. */ +function extractHandleHotUpdate(plugins: PluginOptions[]) { + const plugin = plugins[0]; + const handler = (plugin?.vite as { handleHotUpdate?: unknown } | undefined)?.handleHotUpdate; + expect(typeof handler).toBe('function'); + return handler as (ctx: { file: string; server: unknown }) => Promise; +} + +/** Build a minimal mock HmrContext for handleHotUpdate. */ +function createMockHmrContext(file: string, transformedCode: string | null) { + return { + file, + server: { + transformRequest: jest + .fn() + .mockResolvedValue(transformedCode == null ? null : { code: transformedCode }), + }, + }; } /** Build a minimal mock of the unplugin/Rollup PluginContext used by buildStart. */ @@ -294,6 +315,130 @@ describe('Apps Plugin - getPlugins', () => { }); }); + describe('handleHotUpdate - connection IDs', () => { + // Run buildStart first so the apps plugin captures `this.parse` from + // the mock plugin context. handleHotUpdate reuses that captured fn — + // production order is guaranteed (buildStart fires once at server + // start, before any HMR), so the tests mirror that order. + const primeParser = async (plugins: PluginOptions[]) => { + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValueOnce(undefined); + const buildStart = extractBuildStart(plugins); + await buildStart.call(createMockPluginContext(null)); + }; + + test('Should ignore unrelated file changes', async () => { + const findSpy = jest.spyOn(extractConnections, 'findConnectionsFile'); + const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); + + const handleHotUpdate = extractHandleHotUpdate(getPlugins(getArgs())); + const ctx = createMockHmrContext( + path.join(buildRoot, 'src/some-other-file.ts'), + 'irrelevant', + ); + + await handleHotUpdate(ctx); + + // Cheap basename filter must skip the disk lookup entirely. + expect(findSpy).not.toHaveBeenCalled(); + expect(extractSpy).not.toHaveBeenCalled(); + expect(ctx.server.transformRequest).not.toHaveBeenCalled(); + }); + + test('Should refresh connection IDs when connections.ts changes', async () => { + const plugins = getPlugins(getArgs()); + await primeParser(plugins); + + const connectionsPath = path.join(buildRoot, 'connections.ts'); + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( + connectionsPath, + ); + const extractSpy = jest + .spyOn(extractConnections, 'extractConnectionIds') + .mockReturnValue(['fresh-uuid-1', 'fresh-uuid-2']); + + const handleHotUpdate = extractHandleHotUpdate(plugins); + const ctx = createMockHmrContext( + connectionsPath, + "export const connections = { A: 'fresh-uuid-1' };", + ); + + await handleHotUpdate(ctx); + + expect(ctx.server.transformRequest).toHaveBeenCalledWith(connectionsPath); + expect(extractSpy).toHaveBeenCalledWith(expect.anything(), connectionsPath); + }); + + test('Should not throw when transformRequest returns null', async () => { + const plugins = getPlugins(getArgs()); + await primeParser(plugins); + + const connectionsPath = path.join(buildRoot, 'connections.ts'); + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( + connectionsPath, + ); + const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); + + const handleHotUpdate = extractHandleHotUpdate(plugins); + const ctx = createMockHmrContext(connectionsPath, null); + + await handleHotUpdate(ctx); + + expect(extractSpy).not.toHaveBeenCalled(); + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining('Failed to refresh connection IDs'), + 'error', + ); + }); + + test('Should swallow extraction errors as logged warnings', async () => { + const plugins = getPlugins(getArgs()); + await primeParser(plugins); + + const connectionsPath = path.join(buildRoot, 'connections.ts'); + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( + connectionsPath, + ); + jest.spyOn(extractConnections, 'extractConnectionIds').mockImplementation(() => { + throw new Error('boom'); + }); + + const handleHotUpdate = extractHandleHotUpdate(plugins); + const ctx = createMockHmrContext( + connectionsPath, + "export const connections = { A: 'x' };", + ); + + await expect(handleHotUpdate(ctx)).resolves.toBeUndefined(); + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining('Failed to refresh connection IDs: boom'), + 'error', + ); + }); + + test('Should log error when handleHotUpdate fires before buildStart', async () => { + const connectionsPath = path.join(buildRoot, 'connections.ts'); + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( + connectionsPath, + ); + const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); + + // Skip primeParser — simulate parser not yet captured. + const handleHotUpdate = extractHandleHotUpdate(getPlugins(getArgs())); + const ctx = createMockHmrContext( + connectionsPath, + "export const connections = { A: 'x' };", + ); + + await handleHotUpdate(ctx); + + expect(extractSpy).not.toHaveBeenCalled(); + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining('connection registry parse not set'), + 'error', + ); + }); + }); + test('Should upload assets with vite bundler', async () => { const intakeHost = 'https://api.example.com'; const scope = nock(intakeHost).post(`/${APPS_API_PATH}/app-id/upload`).reply(200, { diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index 915fd94b7..365e0c2fd 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -77,19 +77,61 @@ function createBackendFunctionRegistry() { /** * Create a registry for the connection IDs extracted from `connections.ts`. - * Populated by the `buildStart` hook (once per build) and read by - * `handleUpload` (production manifest emission) and the dev middleware - * (preview-async request body). + * Owns the find → load → parse → extract → store pipeline so both buildStart + * (production / dev startup) and the vite sub-plugin's handleHotUpdate + * (dev edit) drive the same code path. Callers differ only in *how* they + * load post-TS-stripped code (PluginContext.load vs. ViteDevServer.transformRequest), + * which is the single argument to `loadAndSetConnectionIds`. + * + * `setParse` is called from inside the vite sub-plugin's buildStart hook, + * where Rollup's `this.parse` is available. `loadAndSetConnectionIds` will + * throw if invoked before `setParse` ran. */ -function createConnectionIdsRegistry() { +export interface ConnectionIdsRegistry { + /** + * Wires Rollup's `this.parse` into the registry. Called once from the + * vite sub-plugin's buildStart, where the plugin context is available. + * Rollup's `ProgramNode` extends estree's `Program` (adds `start`/`end`), + * so the narrower estree type is what consumers actually need. + */ + setParse(parse: (code: string) => Program): void; + getConnectionIds(): string[]; + /** + * Find connections.ts → load it via the supplied loader → parse → + * extract IDs → store. Throws if `setParse` hasn't been called yet, + * or if the loader returns null for an existing connections file. + */ + loadAndSetConnectionIds( + load: (filePath: string) => Promise, + ): Promise<{ filePath: string | null; connectionIds: string[] }>; +} + +function createConnectionIdsRegistry(opts: { buildRoot: string }): ConnectionIdsRegistry { + let parse: ((code: string) => Program) | null = null; let connectionIds: string[] = []; return { - setConnectionIds(ids: string[]) { - connectionIds = ids; + setParse(p) { + parse = p; }, getConnectionIds() { return connectionIds; }, + async loadAndSetConnectionIds(load) { + if (!parse) { + throw new Error('connection registry parse not set — buildStart did not run'); + } + const filePath = await findConnectionsFile(opts.buildRoot); + if (!filePath) { + connectionIds = []; + return { filePath: null, connectionIds }; + } + const code = await load(filePath); + if (code == null) { + throw new Error(`connections file '${filePath}' produced no code when loaded`); + } + connectionIds = extractConnectionIds(parse(code), filePath); + return { filePath, connectionIds }; + }, }; } @@ -127,7 +169,14 @@ export const getPlugins: GetPlugins = ({ options, context, bundler }) => { }); const { setBackendFunctions, getBackendFunctions } = createBackendFunctionRegistry(); - const { setConnectionIds, getConnectionIds } = createConnectionIdsRegistry(); + + // The registry is shared by handleUpload (closeBundle) and the vite + // sub-plugin (configureServer / handleHotUpdate). It's safe to construct + // upfront because all consumers run after the vite buildStart hook calls + // setParse — `loadAndSetConnectionIds` throws if invoked before that. + const connectionRegistry = createConnectionIdsRegistry({ + buildRoot: context.buildRoot, + }); const handleUpload = async (backendOutputs: Map) => { const handleTimer = log.time('handle assets'); @@ -188,7 +237,7 @@ Either: // applied to every function — the server supports distinct lists, but // the RFC explicitly accepts a flat union as the chosen design. if (backendOutputs.size > 0) { - const allowedConnectionIds = getConnectionIds(); + const allowedConnectionIds = connectionRegistry.getConnectionIds(); const manifest: Record = {}; for (const bundleName of backendOutputs.keys()) { manifest[bundleName] = { allowedConnectionIds }; @@ -266,34 +315,6 @@ Either: { name: PLUGIN_NAME, enforce: 'post', - // Fires once per build (and re-fires on watch invalidation when - // connections.ts changes, because addWatchFile registers it as a - // build dependency). Each /__dd/executeAction dev request triggers - // its own nested viteBuild() and therefore its own buildStart, so - // dev always reads fresh state before bundling. - // - // The `load` method is provided by Vite/Rollup but isn't on - // unplugin's common UnpluginBuildContext. We only run under Vite - // (the plugin returns early for other bundlers above), so a - // structural cast keeps types clean without importing from - // rollup/vite. - async buildStart() { - const filePath = await findConnectionsFile(context.buildRoot); - if (!filePath) { - setConnectionIds([]); - return; - } - this.addWatchFile(filePath); - const ctx = this as unknown as { - load: (options: { id: string }) => Promise<{ code?: string | null }>; - }; - const info = await ctx.load({ id: filePath }); - if (info.code == null) { - throw new Error(`connections file '${filePath}' produced no code when loaded`); - } - const ast = this.parse(info.code); - setConnectionIds(extractConnectionIds(ast as unknown as Program, filePath)); - }, transform: { filter: { id: { @@ -332,7 +353,7 @@ Either: viteBuild: bundler.build, buildRoot: context.buildRoot, getBackendFunctions, - getConnectionIds, + connectionRegistry, handleUpload, log, auth: context.auth, diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 9b7550d97..701e3e0f1 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -38,7 +38,11 @@ const defaultOptions = { viteBuild: mockViteBuild, buildRoot: '/build', getBackendFunctions: () => functions, - getConnectionIds: () => [], + connectionRegistry: { + setParse: jest.fn(), + getConnectionIds: () => [], + loadAndSetConnectionIds: jest.fn().mockResolvedValue({ filePath: null, connectionIds: [] }), + }, handleUpload: mockHandleUpload, log, auth: { site: 'datadoghq.com' }, diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index cacc3f38b..92a9161b3 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -4,9 +4,12 @@ import { rm } from '@dd/core/helpers/fs'; import type { AuthOptionsWithDefaults, Logger, PluginOptions } from '@dd/core/types'; +import path from 'path'; import type { build } from 'vite'; import type { BackendFunction } from '../backend/discovery'; +import { findConnectionsFile } from '../backend/extract-connections'; +import type { ConnectionIdsRegistry } from '../index'; import { buildBackendFunctions } from './build-backend-functions'; import { createDevServerMiddleware } from './dev-server'; @@ -15,7 +18,7 @@ export interface VitePluginOptions { viteBuild: typeof build; buildRoot: string; getBackendFunctions: () => BackendFunction[]; - getConnectionIds: () => string[]; + connectionRegistry: ConnectionIdsRegistry; handleUpload: (backendOutputs: Map) => Promise; log: Logger; auth: AuthOptionsWithDefaults; @@ -24,21 +27,46 @@ export interface VitePluginOptions { /** * Returns the Vite-specific plugin hooks for the apps plugin. * + * buildStart: captures Rollup's `this.parse` for the connection-IDs + * registry and primes it by loading connections.ts through `this.load`. + * Fires once per production build and once at dev-server start. In + * `vite build --watch` it also re-fires when connections.ts changes + * because addWatchFile registers it as a build dependency. In the dev + * server, addWatchFile only registers chokidar tracking — buildStart + * does NOT re-run on edits there. The handleHotUpdate hook below is + * what refreshes the registry mid-session (the per-request nested + * viteBuild uses a different config without the apps plugin, so its + * buildStart can't help). + * * Production (closeBundle): builds backend functions (if any) then uploads * all assets sequentially. * * Dev (configureServer): registers middleware for local backend function * testing when auth credentials are available. + * + * Dev (handleHotUpdate): re-extracts connection IDs when connections.ts is + * edited so the next /__dd/executeAction request sends fresh + * allowedConnectionIds. */ export const getVitePlugin = ({ viteBuild, buildRoot, getBackendFunctions, - getConnectionIds, + connectionRegistry, handleUpload, log, auth, }: VitePluginOptions): PluginOptions['vite'] => ({ + async buildStart() { + connectionRegistry.setParse((code) => this.parse(code)); + const { filePath } = await connectionRegistry.loadAndSetConnectionIds(async (id) => { + const info = await this.load({ id }); + return info.code ?? null; + }); + if (filePath) { + this.addWatchFile(filePath); + } + }, async closeBundle() { let backendOutDir: string | undefined; let backendOutputs = new Map(); @@ -61,11 +89,41 @@ export const getVitePlugin = ({ createDevServerMiddleware({ viteBuild, getBackendFunctions, - getConnectionIds, + getConnectionIds: connectionRegistry.getConnectionIds, auth, projectRoot: buildRoot, log, }), ); }, + async handleHotUpdate({ file, server }) { + if (!CONNECTIONS_BASENAME_RE.test(path.basename(file))) { + return; + } + const connectionsPath = await findConnectionsFile(buildRoot); + if (!connectionsPath || path.resolve(file) !== path.resolve(connectionsPath)) { + return; + } + + try { + // The apps closure handles find + load + parse + extract + store. + // Pull post-TS-stripped JS through Vite's full transform pipeline + // (esbuild handles `as const`, type annotations, etc.) so this + // matches the buildStart code path; the module-graph cache is + // invalidated before handleHotUpdate fires, so we're guaranteed + // fresh code. + const { connectionIds } = await connectionRegistry.loadAndSetConnectionIds( + async (id) => { + const result = await server.transformRequest(id); + return result?.code ?? null; + }, + ); + log.debug(`Refreshed connection IDs from ${connectionsPath} (${connectionIds.length})`); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + log.error(`Failed to refresh connection IDs: ${message}`); + } + }, }); + +const CONNECTIONS_BASENAME_RE = /^connections\.(?:ts|tsx|js|jsx)$/; From ef8ff6886a0629f531d9c177e33598c4b8d02887 Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Tue, 28 Apr 2026 10:41:45 -0400 Subject: [PATCH 03/11] Trim verbose comments in apps plugin entry and vite hooks --- packages/plugins/apps/src/index.ts | 27 ------------------ packages/plugins/apps/src/vite/index.ts | 38 ++++++------------------- 2 files changed, 8 insertions(+), 57 deletions(-) diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index 365e0c2fd..4ce8835cf 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -75,32 +75,9 @@ function createBackendFunctionRegistry() { }; } -/** - * Create a registry for the connection IDs extracted from `connections.ts`. - * Owns the find → load → parse → extract → store pipeline so both buildStart - * (production / dev startup) and the vite sub-plugin's handleHotUpdate - * (dev edit) drive the same code path. Callers differ only in *how* they - * load post-TS-stripped code (PluginContext.load vs. ViteDevServer.transformRequest), - * which is the single argument to `loadAndSetConnectionIds`. - * - * `setParse` is called from inside the vite sub-plugin's buildStart hook, - * where Rollup's `this.parse` is available. `loadAndSetConnectionIds` will - * throw if invoked before `setParse` ran. - */ export interface ConnectionIdsRegistry { - /** - * Wires Rollup's `this.parse` into the registry. Called once from the - * vite sub-plugin's buildStart, where the plugin context is available. - * Rollup's `ProgramNode` extends estree's `Program` (adds `start`/`end`), - * so the narrower estree type is what consumers actually need. - */ setParse(parse: (code: string) => Program): void; getConnectionIds(): string[]; - /** - * Find connections.ts → load it via the supplied loader → parse → - * extract IDs → store. Throws if `setParse` hasn't been called yet, - * or if the loader returns null for an existing connections file. - */ loadAndSetConnectionIds( load: (filePath: string) => Promise, ): Promise<{ filePath: string | null; connectionIds: string[] }>; @@ -170,10 +147,6 @@ export const getPlugins: GetPlugins = ({ options, context, bundler }) => { const { setBackendFunctions, getBackendFunctions } = createBackendFunctionRegistry(); - // The registry is shared by handleUpload (closeBundle) and the vite - // sub-plugin (configureServer / handleHotUpdate). It's safe to construct - // upfront because all consumers run after the vite buildStart hook calls - // setParse — `loadAndSetConnectionIds` throws if invoked before that. const connectionRegistry = createConnectionIdsRegistry({ buildRoot: context.buildRoot, }); diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 92a9161b3..861459673 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -24,30 +24,6 @@ export interface VitePluginOptions { auth: AuthOptionsWithDefaults; } -/** - * Returns the Vite-specific plugin hooks for the apps plugin. - * - * buildStart: captures Rollup's `this.parse` for the connection-IDs - * registry and primes it by loading connections.ts through `this.load`. - * Fires once per production build and once at dev-server start. In - * `vite build --watch` it also re-fires when connections.ts changes - * because addWatchFile registers it as a build dependency. In the dev - * server, addWatchFile only registers chokidar tracking — buildStart - * does NOT re-run on edits there. The handleHotUpdate hook below is - * what refreshes the registry mid-session (the per-request nested - * viteBuild uses a different config without the apps plugin, so its - * buildStart can't help). - * - * Production (closeBundle): builds backend functions (if any) then uploads - * all assets sequentially. - * - * Dev (configureServer): registers middleware for local backend function - * testing when auth credentials are available. - * - * Dev (handleHotUpdate): re-extracts connection IDs when connections.ts is - * edited so the next /__dd/executeAction request sends fresh - * allowedConnectionIds. - */ export const getVitePlugin = ({ viteBuild, buildRoot, @@ -57,6 +33,14 @@ export const getVitePlugin = ({ log, auth, }: VitePluginOptions): PluginOptions['vite'] => ({ + // Fires once per production build and once at dev-server start. In + // `vite build --watch` it also re-fires when connections.ts changes + // because addWatchFile registers it as a build dependency. In the dev + // server, addWatchFile only registers chokidar tracking — buildStart + // does NOT re-run on edits there; handleHotUpdate refreshes the + // registry mid-session instead (the per-request nested viteBuild uses + // a different config without the apps plugin, so its buildStart can't + // help). async buildStart() { connectionRegistry.setParse((code) => this.parse(code)); const { filePath } = await connectionRegistry.loadAndSetConnectionIds(async (id) => { @@ -106,12 +90,6 @@ export const getVitePlugin = ({ } try { - // The apps closure handles find + load + parse + extract + store. - // Pull post-TS-stripped JS through Vite's full transform pipeline - // (esbuild handles `as const`, type annotations, etc.) so this - // matches the buildStart code path; the module-graph cache is - // invalidated before handleHotUpdate fires, so we're guaranteed - // fresh code. const { connectionIds } = await connectionRegistry.loadAndSetConnectionIds( async (id) => { const result = await server.transformRequest(id); From 5bd5aef409f18add4d2327e5520fbf7ef465627f Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Tue, 28 Apr 2026 10:51:22 -0400 Subject: [PATCH 04/11] Accept either 'connections' or 'CONNECTIONS' as the export name in connections.ts --- .../src/backend/extract-connections.test.ts | 34 +++++++++++++++---- .../apps/src/backend/extract-connections.ts | 26 +++++++++----- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/packages/plugins/apps/src/backend/extract-connections.test.ts b/packages/plugins/apps/src/backend/extract-connections.test.ts index 4968625fb..ae1ad3806 100644 --- a/packages/plugins/apps/src/backend/extract-connections.test.ts +++ b/packages/plugins/apps/src/backend/extract-connections.test.ts @@ -25,9 +25,12 @@ function program(body: Program['body']): Program { } /** - * Build an `export const connections = ` declaration. + * Build an `export const = ` declaration. */ -function exportConnections(properties: ObjectExpression['properties']): ExportNamedDeclaration { +function exportConnections( + properties: ObjectExpression['properties'], + name: 'connections' | 'CONNECTIONS' = 'connections', +): ExportNamedDeclaration { return { type: 'ExportNamedDeclaration', declaration: { @@ -36,7 +39,7 @@ function exportConnections(properties: ObjectExpression['properties']): ExportNa declarations: [ { type: 'VariableDeclarator', - id: { type: 'Identifier', name: 'connections' }, + id: { type: 'Identifier', name }, init: { type: 'ObjectExpression', properties }, }, ], @@ -131,6 +134,13 @@ describe('extract-connections - extractConnectionIds', () => { ast: program([exportConnections([])]), expected: [], }, + { + description: 'uppercase CONNECTIONS variable name', + ast: program([ + exportConnections([stringProperty('OPEN_AI', 'uuid-upper')], 'CONNECTIONS'), + ]), + expected: ['uuid-upper'], + }, ]; test.each(acceptedCases)('Should accept $description', ({ ast, expected }) => { @@ -152,7 +162,7 @@ describe('extract-connections - extractConnectionIds', () => { }, ]); expect(() => extractConnectionIds(ast, filePath)).toThrow( - 'connections file must define "export const connections = { ... }"', + 'connections file must define "export const CONNECTIONS" (or "connections") = { ... }', ); }); @@ -164,7 +174,7 @@ describe('extract-connections - extractConnectionIds', () => { }, ]); expect(() => extractConnectionIds(ast, filePath)).toThrow( - 'connections file must define "export const connections = { ... }"', + 'connections file must define "export const CONNECTIONS" (or "connections") = { ... }', ); }); @@ -189,7 +199,7 @@ describe('extract-connections - extractConnectionIds', () => { }, ]); expect(() => extractConnectionIds(ast, filePath)).toThrow( - '"export const connections" must be initialized with an object literal', + '"export const CONNECTIONS" (or "connections") must be initialized with an object literal', ); }); @@ -199,7 +209,17 @@ describe('extract-connections - extractConnectionIds', () => { exportConnections([stringProperty('B', 'uuid-2')]), ]); expect(() => extractConnectionIds(ast, filePath)).toThrow( - 'multiple top-level "export const connections" declarations are not allowed', + 'multiple top-level "export const CONNECTIONS" (or "connections") declarations are not allowed', + ); + }); + + test('Should throw when both "connections" and "CONNECTIONS" are exported', () => { + const ast = program([ + exportConnections([stringProperty('A', 'uuid-1')], 'connections'), + exportConnections([stringProperty('B', 'uuid-2')], 'CONNECTIONS'), + ]); + expect(() => extractConnectionIds(ast, filePath)).toThrow( + 'multiple top-level "export const CONNECTIONS" (or "connections") declarations are not allowed', ); }); diff --git a/packages/plugins/apps/src/backend/extract-connections.ts b/packages/plugins/apps/src/backend/extract-connections.ts index 16e31f1d7..d0481a7c7 100644 --- a/packages/plugins/apps/src/backend/extract-connections.ts +++ b/packages/plugins/apps/src/backend/extract-connections.ts @@ -6,8 +6,10 @@ import type { Expression, Node, ObjectExpression, Program, Property } from 'estr import { promises as fsp } from 'fs'; import path from 'path'; -const CONNECTIONS_BASENAME = 'connections'; +const CONNECTIONS_FILE_BASENAME = 'connections'; const CONNECTIONS_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx'] as const; +const CONNECTIONS_EXPORT_NAMES = ['connections', 'CONNECTIONS'] as const; +const EXPECTED_EXPORT_DESCRIPTION = '"export const CONNECTIONS" (or "connections")'; /** * Locate the project's connections file. Looks for `connections.{ts,tsx,js,jsx}` @@ -16,7 +18,7 @@ const CONNECTIONS_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx'] as const; */ export async function findConnectionsFile(buildRoot: string): Promise { for (const ext of CONNECTIONS_EXTENSIONS) { - const candidate = path.join(buildRoot, `${CONNECTIONS_BASENAME}${ext}`); + const candidate = path.join(buildRoot, `${CONNECTIONS_FILE_BASENAME}${ext}`); try { await fsp.access(candidate); return candidate; @@ -32,11 +34,13 @@ export async function findConnectionsFile(buildRoot: string): Promise Date: Tue, 28 Apr 2026 12:20:02 -0400 Subject: [PATCH 05/11] Fix dev startup, surface framed errors, add line:col to connections.ts validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - buildStart now uses server.transformRequest in dev (vite serve), where this.load returns a ModuleInfo proxy whose `code` getter throws. server is captured via configureServer, which fires before buildStart in dev. - buildStart logs the framed [connections] error before re-throwing so it survives any masking by downstream closeBundle errors (e.g. error-tracking sourcemaps' "No output files found"). - extractConnectionIds now derives line:col from node.start byte offsets (which SWC produces) against the source text. Previously the loc?.start branch was dead — rollup's parser doesn't emit ESTree loc info. --- .../src/backend/extract-connections.test.ts | 51 +++++- .../apps/src/backend/extract-connections.ts | 59 ++++--- packages/plugins/apps/src/index.test.ts | 87 ++++++++- packages/plugins/apps/src/index.ts | 2 +- packages/plugins/apps/src/vite/index.ts | 166 ++++++++++-------- 5 files changed, 260 insertions(+), 105 deletions(-) diff --git a/packages/plugins/apps/src/backend/extract-connections.test.ts b/packages/plugins/apps/src/backend/extract-connections.test.ts index ae1ad3806..f5bc53c40 100644 --- a/packages/plugins/apps/src/backend/extract-connections.test.ts +++ b/packages/plugins/apps/src/backend/extract-connections.test.ts @@ -144,7 +144,7 @@ describe('extract-connections - extractConnectionIds', () => { ]; test.each(acceptedCases)('Should accept $description', ({ ast, expected }) => { - expect(extractConnectionIds(ast, filePath)).toEqual(expected); + expect(extractConnectionIds(ast, filePath, '')).toEqual(expected); }); test('Should throw when no "export const connections" is present', () => { @@ -161,7 +161,7 @@ describe('extract-connections - extractConnectionIds', () => { ], }, ]); - expect(() => extractConnectionIds(ast, filePath)).toThrow( + expect(() => extractConnectionIds(ast, filePath, '')).toThrow( 'connections file must define "export const CONNECTIONS" (or "connections") = { ... }', ); }); @@ -173,7 +173,7 @@ describe('extract-connections - extractConnectionIds', () => { declaration: { type: 'ObjectExpression', properties: [] }, }, ]); - expect(() => extractConnectionIds(ast, filePath)).toThrow( + expect(() => extractConnectionIds(ast, filePath, '')).toThrow( 'connections file must define "export const CONNECTIONS" (or "connections") = { ... }', ); }); @@ -198,7 +198,7 @@ describe('extract-connections - extractConnectionIds', () => { attributes: [], }, ]); - expect(() => extractConnectionIds(ast, filePath)).toThrow( + expect(() => extractConnectionIds(ast, filePath, '')).toThrow( '"export const CONNECTIONS" (or "connections") must be initialized with an object literal', ); }); @@ -208,7 +208,7 @@ describe('extract-connections - extractConnectionIds', () => { exportConnections([stringProperty('A', 'uuid-1')]), exportConnections([stringProperty('B', 'uuid-2')]), ]); - expect(() => extractConnectionIds(ast, filePath)).toThrow( + expect(() => extractConnectionIds(ast, filePath, '')).toThrow( 'multiple top-level "export const CONNECTIONS" (or "connections") declarations are not allowed', ); }); @@ -218,7 +218,7 @@ describe('extract-connections - extractConnectionIds', () => { exportConnections([stringProperty('A', 'uuid-1')], 'connections'), exportConnections([stringProperty('B', 'uuid-2')], 'CONNECTIONS'), ]); - expect(() => extractConnectionIds(ast, filePath)).toThrow( + expect(() => extractConnectionIds(ast, filePath, '')).toThrow( 'multiple top-level "export const CONNECTIONS" (or "connections") declarations are not allowed', ); }); @@ -237,7 +237,7 @@ describe('extract-connections - extractConnectionIds', () => { }, ]), ]); - expect(() => extractConnectionIds(ast, filePath)).toThrow('computed keys'); + expect(() => extractConnectionIds(ast, filePath, '')).toThrow('computed keys'); }); test('Should throw on spread elements', () => { @@ -249,7 +249,7 @@ describe('extract-connections - extractConnectionIds', () => { } as SpreadElement, ]), ]); - expect(() => extractConnectionIds(ast, filePath)).toThrow('spread elements'); + expect(() => extractConnectionIds(ast, filePath, '')).toThrow('spread elements'); }); const rejectedValueCases: Array<{ @@ -339,9 +339,42 @@ describe('extract-connections - extractConnectionIds', () => { computed: false, }; const ast = program([exportConnections([property])]); - expect(() => extractConnectionIds(ast, filePath)).toThrow(reasonContains); + expect(() => extractConnectionIds(ast, filePath, '')).toThrow(reasonContains); }, ); + + // Rollup's this.parse() (SWC) emits character offsets but no line:col, + // so we derive line:col from `node.start` against the source text. + // These tests use the real parser so a regression in offset handling + // surfaces immediately. + describe('framed source location from parseAst', () => { + test('Should include line:col when value is not a string literal', async () => { + const { parseAst } = await import('rollup/parseAst'); + // parseAst is a JS-only parser, so the source has no `as const`. + const code = [ + 'export const CONNECTIONS = {', + " A: 'good-uuid',", + ' B: process.env.OPEN_AI_ID,', + '};', + '', + ].join('\n'); + const ast = parseAst(code) as unknown as Program; + + expect(() => extractConnectionIds(ast, filePath, code)).toThrow( + `must be a string literal; got MemberExpression (at ${filePath}:3:8)`, + ); + }); + + test('Should include line:col for the missing-export case', async () => { + const { parseAst } = await import('rollup/parseAst'); + const code = 'const CONNECTIONS = {};\nexport default CONNECTIONS;\n'; + const ast = parseAst(code) as unknown as Program; + + expect(() => extractConnectionIds(ast, filePath, code)).toThrow( + `connections file must define "export const CONNECTIONS" (or "connections") = { ... } (at ${filePath})`, + ); + }); + }); }); describe('extract-connections - findConnectionsFile', () => { diff --git a/packages/plugins/apps/src/backend/extract-connections.ts b/packages/plugins/apps/src/backend/extract-connections.ts index d0481a7c7..8300ab9c3 100644 --- a/packages/plugins/apps/src/backend/extract-connections.ts +++ b/packages/plugins/apps/src/backend/extract-connections.ts @@ -29,6 +29,8 @@ export async function findConnectionsFile(buildRoot: string): Promise { + const where = + node?.start != null ? `${filePath}:${formatLineCol(code, node.start)}` : filePath; + return new Error(`[connections] ${reason} (at ${where})`); + }; + let connectionsObject: ObjectExpression | undefined; for (const node of ast.body) { @@ -72,15 +81,13 @@ export function extractConnectionIds(ast: Program, filePath: string): string[] { } if (connectionsObject) { throw fail( - filePath, - d.loc, + d, `multiple top-level ${EXPECTED_EXPORT_DESCRIPTION} declarations are not allowed`, ); } if (!d.init || d.init.type !== 'ObjectExpression') { throw fail( - filePath, - (d.init ?? d).loc, + d.init ?? d, `${EXPECTED_EXPORT_DESCRIPTION} must be initialized with an object literal`, ); } @@ -89,31 +96,25 @@ export function extractConnectionIds(ast: Program, filePath: string): string[] { } if (!connectionsObject) { - throw fail( - filePath, - null, - `connections file must define ${EXPECTED_EXPORT_DESCRIPTION} = { ... }`, - ); + throw fail(null, `connections file must define ${EXPECTED_EXPORT_DESCRIPTION} = { ... }`); } const ids = new Set(); for (const property of connectionsObject.properties) { if (property.type === 'SpreadElement') { throw fail( - filePath, - property.loc, + property, `spread elements are not supported inside ${EXPECTED_EXPORT_DESCRIPTION}`, ); } if (property.computed) { throw fail( - filePath, - property.loc, + property, `computed keys are not supported inside ${EXPECTED_EXPORT_DESCRIPTION}`, ); } const keyName = readKeyName(property); - const value = extractStaticString(property.value, keyName, filePath); + const value = extractStaticString(property.value, keyName, fail); ids.add(value); } @@ -124,15 +125,18 @@ export function extractConnectionIds(ast: Program, filePath: string): string[] { * Resolve a property value node to its static string. Accepts string literals * and interpolation-free template literals; throws on anything else. */ -function extractStaticString(value: Property['value'], keyName: string, filePath: string): string { +function extractStaticString( + value: Property['value'], + keyName: string, + fail: (node: WithOffset | null | undefined, reason: string) => Error, +): string { if (value.type === 'Literal' && typeof value.value === 'string') { return value.value; } if (value.type === 'TemplateLiteral') { if (value.expressions.length > 0) { throw fail( - filePath, - value.loc, + value, `value for "${keyName}" must be a static string — template literals with interpolations are not allowed`, ); } @@ -140,8 +144,7 @@ function extractStaticString(value: Property['value'], keyName: string, filePath return quasi.value.cooked ?? quasi.value.raw; } throw fail( - filePath, - value.loc, + value, `value for "${keyName}" must be a string literal; got ${describeNode(value)}`, ); } @@ -169,7 +172,15 @@ function isConnectionsExportName(name: string): name is (typeof CONNECTIONS_EXPO return (CONNECTIONS_EXPORT_NAMES as readonly string[]).includes(name); } -function fail(filePath: string, loc: Node['loc'], reason: string): Error { - const where = loc?.start ? `${filePath}:${loc.start.line}:${loc.start.column + 1}` : filePath; - return new Error(`[connections] ${reason} (at ${where})`); +/** + * Convert a 0-based byte offset into a `line:column` string (1-based, like + * editor jump-to-line targets). + */ +function formatLineCol(code: string, offset: number): string { + const before = code.slice(0, offset); + const newlineCount = (before.match(/\n/g) ?? []).length; + const lastNewline = before.lastIndexOf('\n'); + const line = newlineCount + 1; + const column = offset - (lastNewline + 1) + 1; + return `${line}:${column}`; } diff --git a/packages/plugins/apps/src/index.test.ts b/packages/plugins/apps/src/index.test.ts index 13fd2dbea..136594c82 100644 --- a/packages/plugins/apps/src/index.test.ts +++ b/packages/plugins/apps/src/index.test.ts @@ -46,6 +46,14 @@ function extractHandleHotUpdate(plugins: PluginOptions[]) { return handler as (ctx: { file: string; server: unknown }) => Promise; } +/** Extract and assert configureServer from the first plugin's vite hooks. */ +function extractConfigureServer(plugins: PluginOptions[]) { + const plugin = plugins[0]; + const handler = (plugin?.vite as { configureServer?: unknown } | undefined)?.configureServer; + expect(typeof handler).toBe('function'); + return handler as (server: { middlewares: { use: (fn: unknown) => void } }) => void; +} + /** Build a minimal mock HmrContext for handleHotUpdate. */ function createMockHmrContext(file: string, transformedCode: string | null) { return { @@ -313,6 +321,79 @@ describe('Apps Plugin - getPlugins', () => { `connections file '${connectionsPath}' produced no code when loaded`, ); }); + + // Vite's dev plugin context returns a ModuleInfo proxy whose `code` + // getter throws — code is only resolvable through the dev server's + // transformRequest. configureServer fires before buildStart in dev, + // so the loader uses the captured server instead of this.load. + test('Should use server.transformRequest in dev (after configureServer fires)', async () => { + const connectionsPath = path.join(buildRoot, 'connections.ts'); + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( + connectionsPath, + ); + const extractSpy = jest + .spyOn(extractConnections, 'extractConnectionIds') + .mockReturnValue(['uuid-from-dev']); + + const plugins = getPlugins(getArgs()); + const configureServer = extractConfigureServer(plugins); + const transformRequest = jest + .fn() + .mockResolvedValue({ code: 'export const connections = {};' }); + configureServer({ + middlewares: { use: jest.fn() }, + transformRequest, + } as unknown as Parameters[0]); + + const buildStart = extractBuildStart(plugins); + // ctx.load returns a ModuleInfo whose `code` getter throws — + // mirrors vite's EnvironmentPluginContainer proxy. + const moduleInfoStub = Object.defineProperty({}, 'code', { + get() { + throw new Error('[vite] The "code" property of ModuleInfo is not supported.'); + }, + }); + const ctx = { + addWatchFile: jest.fn(), + load: jest.fn().mockResolvedValue(moduleInfoStub), + parse: jest.fn().mockReturnValue({ + type: 'Program', + sourceType: 'module', + body: [], + }), + }; + + await buildStart.call(ctx); + + expect(transformRequest).toHaveBeenCalledWith(connectionsPath); + expect(ctx.load).not.toHaveBeenCalled(); + expect(ctx.addWatchFile).toHaveBeenCalledWith(connectionsPath); + expect(extractSpy).toHaveBeenCalled(); + }); + + // Strict-validation throws need to surface in the build log because + // downstream plugins (e.g. error-tracking sourcemaps) can throw their + // own errors during teardown and mask ours from vite's final report. + test('Should log the framed error before re-throwing', async () => { + const connectionsPath = path.join(buildRoot, 'connections.ts'); + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( + connectionsPath, + ); + jest.spyOn(extractConnections, 'extractConnectionIds').mockImplementation(() => { + throw new Error('[connections] bad value (at /project/connections.ts:3:8)'); + }); + + const buildStart = extractBuildStart(getPlugins(getArgs())); + const ctx = createMockPluginContext('export const connections = {};'); + + await expect(buildStart.call(ctx)).rejects.toThrow( + '[connections] bad value (at /project/connections.ts:3:8)', + ); + expect(mockLogFn).toHaveBeenCalledWith( + expect.stringContaining('[connections] bad value (at /project/connections.ts:3:8)'), + 'error', + ); + }); }); describe('handleHotUpdate - connection IDs', () => { @@ -365,7 +446,11 @@ describe('Apps Plugin - getPlugins', () => { await handleHotUpdate(ctx); expect(ctx.server.transformRequest).toHaveBeenCalledWith(connectionsPath); - expect(extractSpy).toHaveBeenCalledWith(expect.anything(), connectionsPath); + expect(extractSpy).toHaveBeenCalledWith( + expect.anything(), + connectionsPath, + "export const connections = { A: 'fresh-uuid-1' };", + ); }); test('Should not throw when transformRequest returns null', async () => { diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index 4ce8835cf..ea0ec067f 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -106,7 +106,7 @@ function createConnectionIdsRegistry(opts: { buildRoot: string }): ConnectionIds if (code == null) { throw new Error(`connections file '${filePath}' produced no code when loaded`); } - connectionIds = extractConnectionIds(parse(code), filePath); + connectionIds = extractConnectionIds(parse(code), filePath, code); return { filePath, connectionIds }; }, }; diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 861459673..020e7e2f8 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -5,7 +5,7 @@ import { rm } from '@dd/core/helpers/fs'; import type { AuthOptionsWithDefaults, Logger, PluginOptions } from '@dd/core/types'; import path from 'path'; -import type { build } from 'vite'; +import type { build, ViteDevServer } from 'vite'; import type { BackendFunction } from '../backend/discovery'; import { findConnectionsFile } from '../backend/extract-connections'; @@ -32,76 +32,102 @@ export const getVitePlugin = ({ handleUpload, log, auth, -}: VitePluginOptions): PluginOptions['vite'] => ({ - // Fires once per production build and once at dev-server start. In - // `vite build --watch` it also re-fires when connections.ts changes - // because addWatchFile registers it as a build dependency. In the dev - // server, addWatchFile only registers chokidar tracking — buildStart - // does NOT re-run on edits there; handleHotUpdate refreshes the - // registry mid-session instead (the per-request nested viteBuild uses - // a different config without the apps plugin, so its buildStart can't - // help). - async buildStart() { - connectionRegistry.setParse((code) => this.parse(code)); - const { filePath } = await connectionRegistry.loadAndSetConnectionIds(async (id) => { - const info = await this.load({ id }); - return info.code ?? null; - }); - if (filePath) { - this.addWatchFile(filePath); - } - }, - async closeBundle() { - let backendOutDir: string | undefined; - let backendOutputs = new Map(); - const functions = getBackendFunctions(); - if (functions.length > 0) { - const result = await buildBackendFunctions(viteBuild, functions, buildRoot, log); - backendOutDir = result.outDir; - backendOutputs = result.outputs; - } - try { - await handleUpload(backendOutputs); - } finally { - if (backendOutDir) { - await rm(backendOutDir); - } - } - }, - configureServer(server) { - server.middlewares.use( - createDevServerMiddleware({ - viteBuild, - getBackendFunctions, - getConnectionIds: connectionRegistry.getConnectionIds, - auth, - projectRoot: buildRoot, - log, - }), - ); - }, - async handleHotUpdate({ file, server }) { - if (!CONNECTIONS_BASENAME_RE.test(path.basename(file))) { - return; - } - const connectionsPath = await findConnectionsFile(buildRoot); - if (!connectionsPath || path.resolve(file) !== path.resolve(connectionsPath)) { - return; - } +}: VitePluginOptions): PluginOptions['vite'] => { + // Captured from configureServer so buildStart can branch on dev vs build. + // In `vite serve`, this.load returns a ModuleInfo whose `code` getter + // throws — code is only resolvable through the dev server's transformRequest. + let devServer: ViteDevServer | undefined; - try { - const { connectionIds } = await connectionRegistry.loadAndSetConnectionIds( - async (id) => { - const result = await server.transformRequest(id); - return result?.code ?? null; - }, + return { + // Fires once per production build and once at dev-server start. In + // `vite build --watch` it also re-fires when connections.ts changes + // because addWatchFile registers it as a build dependency. In the dev + // server, addWatchFile only registers chokidar tracking — buildStart + // does NOT re-run on edits there; handleHotUpdate refreshes the + // registry mid-session instead (the per-request nested viteBuild uses + // a different config without the apps plugin, so its buildStart can't + // help). + async buildStart() { + connectionRegistry.setParse((code) => this.parse(code)); + try { + const { filePath } = await connectionRegistry.loadAndSetConnectionIds( + async (id) => { + if (devServer) { + const result = await devServer.transformRequest(id); + return result?.code ?? null; + } + const info = await this.load({ id }); + return info.code ?? null; + }, + ); + if (filePath) { + this.addWatchFile(filePath); + } + } catch (error) { + // Surface the framed error before re-throwing — downstream + // plugins (e.g. error-tracking's sourcemaps upload) may throw + // their own errors during build teardown and mask ours from + // vite's final error report. + const message = error instanceof Error ? error.message : String(error); + log.error(message); + throw error; + } + }, + async closeBundle() { + let backendOutDir: string | undefined; + let backendOutputs = new Map(); + const functions = getBackendFunctions(); + if (functions.length > 0) { + const result = await buildBackendFunctions(viteBuild, functions, buildRoot, log); + backendOutDir = result.outDir; + backendOutputs = result.outputs; + } + try { + await handleUpload(backendOutputs); + } finally { + if (backendOutDir) { + await rm(backendOutDir); + } + } + }, + configureServer(server) { + devServer = server; + server.middlewares.use( + createDevServerMiddleware({ + viteBuild, + getBackendFunctions, + getConnectionIds: connectionRegistry.getConnectionIds, + auth, + projectRoot: buildRoot, + log, + }), ); - log.debug(`Refreshed connection IDs from ${connectionsPath} (${connectionIds.length})`); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - log.error(`Failed to refresh connection IDs: ${message}`); - } - }, -}); + }, + async handleHotUpdate({ file, server }) { + if (!CONNECTIONS_BASENAME_RE.test(path.basename(file))) { + return; + } + const connectionsPath = await findConnectionsFile(buildRoot); + if (!connectionsPath || path.resolve(file) !== path.resolve(connectionsPath)) { + return; + } + + try { + const { connectionIds } = await connectionRegistry.loadAndSetConnectionIds( + async (id) => { + const result = await server.transformRequest(id); + return result?.code ?? null; + }, + ); + log.debug( + `Refreshed connection IDs from ${connectionsPath} (${connectionIds.length})`, + ); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + log.error(`Failed to refresh connection IDs: ${message}`); + } + }, + }; +}; const CONNECTIONS_BASENAME_RE = /^connections\.(?:ts|tsx|js|jsx)$/; From cbd16e2cbb40bf8926f13b09699c0436633589fe Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Tue, 28 Apr 2026 12:29:29 -0400 Subject: [PATCH 06/11] Move manifest.json to zip root and nest under backend.functions per updated RFC --- packages/plugins/apps/src/index.ts | 20 ++++++++++-------- .../src/e2e/appsPlugin/appsPlugin.spec.ts | 21 +++++++++++++++++-- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index ea0ec067f..d87518b68 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -204,26 +204,28 @@ Either: }); } - // Emit backend/manifest.json with the per-function allowed connection - // IDs so the server-side actions runtime can allowlist the connections - // each function uses. The same union list (from connections.ts) is - // applied to every function — the server supports distinct lists, but - // the RFC explicitly accepts a flat union as the chosen design. + // Emit manifest.json at the zip root with the per-function allowed + // connection IDs so the server-side actions runtime can allowlist + // the connections each function uses. The same union list (from + // connections.ts) is applied to every function — the server + // supports distinct lists, but the RFC explicitly accepts a flat + // union as the chosen design. if (backendOutputs.size > 0) { const allowedConnectionIds = connectionRegistry.getConnectionIds(); - const manifest: Record = {}; + const functions: Record = {}; for (const bundleName of backendOutputs.keys()) { - manifest[bundleName] = { allowedConnectionIds }; + functions[bundleName] = { allowedConnectionIds }; } + const manifest = { backend: { functions } }; manifestDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'dd-apps-manifest-')); const manifestPath = path.join(manifestDir, 'manifest.json'); await fsp.writeFile(manifestPath, JSON.stringify(manifest, null, 2)); allAssets.push({ absolutePath: manifestPath, - relativePath: 'backend/manifest.json', + relativePath: 'manifest.json', }); log.debug( - `Emitted backend/manifest.json with ${allowedConnectionIds.length} connection ID(s)`, + `Emitted manifest.json with ${allowedConnectionIds.length} connection ID(s)`, ); } diff --git a/packages/tests/src/e2e/appsPlugin/appsPlugin.spec.ts b/packages/tests/src/e2e/appsPlugin/appsPlugin.spec.ts index 3e8687e62..d7e784f44 100644 --- a/packages/tests/src/e2e/appsPlugin/appsPlugin.spec.ts +++ b/packages/tests/src/e2e/appsPlugin/appsPlugin.spec.ts @@ -160,14 +160,31 @@ describe('Apps Plugin', () => { const filePaths = Object.keys(zip.files); expect(filePaths.length).toBeGreaterThan(0); - // Every file should be under frontend/ or backend/. + // Every file should be under frontend/ or backend/, or the root + // manifest.json emitted alongside backend functions. for (const filePath of filePaths) { - expect(filePath).toMatch(/^(frontend|backend)\//); + expect(filePath).toMatch(/^(frontend|backend)\/|^manifest\.json$/); } // There should be at least one frontend asset. const frontendFiles = filePaths.filter((f) => f.startsWith('frontend/')); expect(frontendFiles.length).toBeGreaterThan(0); + + // The root manifest.json should describe each backend function with + // an allowedConnectionIds list. + const manifestFile = zip.file('manifest.json'); + expect(manifestFile).not.toBeNull(); + const manifest = JSON.parse(await manifestFile!.async('string')) as { + backend: { functions: Record }; + }; + const backendFiles = filePaths.filter((f) => f.startsWith('backend/') && !zip.files[f].dir); + const expectedKeys = backendFiles.map((f) => + f.replace(/^backend\//, '').replace(/\.js$/, ''), + ); + expect(Object.keys(manifest.backend.functions).sort()).toEqual(expectedKeys.sort()); + for (const entry of Object.values(manifest.backend.functions)) { + expect(Array.isArray(entry.allowedConnectionIds)).toBe(true); + } }); // Backend function injection is only supported for vite. From e7ee68bc6af671540e541b3f6cc577fef8c5c7a3 Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Tue, 28 Apr 2026 13:10:27 -0400 Subject: [PATCH 07/11] Refresh connections registry via server.watcher; fail-closed on extraction errors --- packages/plugins/apps/src/index.test.ts | 195 +++++++++++-------- packages/plugins/apps/src/index.ts | 4 + packages/plugins/apps/src/vite/index.test.ts | 1 + packages/plugins/apps/src/vite/index.ts | 72 ++++--- 4 files changed, 167 insertions(+), 105 deletions(-) diff --git a/packages/plugins/apps/src/index.test.ts b/packages/plugins/apps/src/index.test.ts index 136594c82..a8c87f41c 100644 --- a/packages/plugins/apps/src/index.test.ts +++ b/packages/plugins/apps/src/index.test.ts @@ -38,31 +38,45 @@ function extractBuildStart(plugins: PluginOptions[]) { return buildStart as (this: unknown) => Promise; } -/** Extract and assert handleHotUpdate from the first plugin's vite hooks. */ -function extractHandleHotUpdate(plugins: PluginOptions[]) { - const plugin = plugins[0]; - const handler = (plugin?.vite as { handleHotUpdate?: unknown } | undefined)?.handleHotUpdate; - expect(typeof handler).toBe('function'); - return handler as (ctx: { file: string; server: unknown }) => Promise; -} - /** Extract and assert configureServer from the first plugin's vite hooks. */ function extractConfigureServer(plugins: PluginOptions[]) { const plugin = plugins[0]; const handler = (plugin?.vite as { configureServer?: unknown } | undefined)?.configureServer; expect(typeof handler).toBe('function'); - return handler as (server: { middlewares: { use: (fn: unknown) => void } }) => void; + return handler as (server: { + middlewares: { use: (fn: unknown) => void }; + transformRequest?: jest.Mock; + watcher?: { on: jest.Mock }; + }) => void; } -/** Build a minimal mock HmrContext for handleHotUpdate. */ -function createMockHmrContext(file: string, transformedCode: string | null) { +type WatcherEvent = 'add' | 'change' | 'unlink'; + +/** + * Build a minimal mock dev server and capture chokidar listeners registered + * via `server.watcher.on(...)`. The returned `listeners` map exposes them so + * tests can fire watcher events without a real chokidar instance. + */ +function createMockServer(transformedCode: string | null) { + const listeners = new Map Promise>(); + const transformRequest = jest + .fn() + .mockResolvedValue(transformedCode == null ? null : { code: transformedCode }); + const watcher: { on: jest.Mock } = { + on: jest.fn((event: WatcherEvent, fn: (file: string) => Promise) => { + listeners.set(event, fn); + return watcher; + }), + }; return { - file, server: { - transformRequest: jest - .fn() - .mockResolvedValue(transformedCode == null ? null : { code: transformedCode }), + middlewares: { use: jest.fn() }, + transformRequest, + watcher, }, + listeners, + transformRequest, + watcher, }; } @@ -343,7 +357,8 @@ describe('Apps Plugin - getPlugins', () => { configureServer({ middlewares: { use: jest.fn() }, transformRequest, - } as unknown as Parameters[0]); + watcher: { on: jest.fn() }, + }); const buildStart = extractBuildStart(plugins); // ctx.load returns a ModuleInfo whose `code` getter throws — @@ -396,40 +411,58 @@ describe('Apps Plugin - getPlugins', () => { }); }); - describe('handleHotUpdate - connection IDs', () => { - // Run buildStart first so the apps plugin captures `this.parse` from - // the mock plugin context. handleHotUpdate reuses that captured fn — - // production order is guaranteed (buildStart fires once at server - // start, before any HMR), so the tests mirror that order. - const primeParser = async (plugins: PluginOptions[]) => { + describe('configureServer - connection-file watcher', () => { + const connectionsPath = path.join(buildRoot, 'connections.ts'); + + // Wire up configureServer + buildStart so the parser is captured (the + // refresh path needs it). Returns the watcher listener map and the + // connectionRegistry-driven middleware getConnectionIds closure for + // assertions on the registry's post-event state. + const setup = async (transformedCode: string | null = '') => { + const plugins = getPlugins(getArgs()); + const mock = createMockServer(transformedCode); + extractConfigureServer(plugins)(mock.server); + + // primeParser via buildStart — start with no connections file so + // buildStart succeeds without touching extractConnectionIds. jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValueOnce(undefined); - const buildStart = extractBuildStart(plugins); - await buildStart.call(createMockPluginContext(null)); + await extractBuildStart(plugins).call(createMockPluginContext(null)); + + // Capture getConnectionIds from the middleware passed to use(). + const middlewareCalls = (mock.server.middlewares.use as jest.Mock).mock.calls; + expect(middlewareCalls.length).toBeGreaterThan(0); + + return mock; }; test('Should ignore unrelated file changes', async () => { const findSpy = jest.spyOn(extractConnections, 'findConnectionsFile'); const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); - const handleHotUpdate = extractHandleHotUpdate(getPlugins(getArgs())); - const ctx = createMockHmrContext( - path.join(buildRoot, 'src/some-other-file.ts'), - 'irrelevant', - ); + const mock = await setup(); + findSpy.mockClear(); - await handleHotUpdate(ctx); + await mock.listeners.get('change')!(path.join(buildRoot, 'src/some-other-file.ts')); - // Cheap basename filter must skip the disk lookup entirely. + // Basename filter must short-circuit before any disk lookup. expect(findSpy).not.toHaveBeenCalled(); expect(extractSpy).not.toHaveBeenCalled(); - expect(ctx.server.transformRequest).not.toHaveBeenCalled(); + expect(mock.transformRequest).not.toHaveBeenCalled(); }); - test('Should refresh connection IDs when connections.ts changes', async () => { - const plugins = getPlugins(getArgs()); - await primeParser(plugins); + test('Should ignore connections.ts in subdirectories (only project-root file)', async () => { + const findSpy = jest.spyOn(extractConnections, 'findConnectionsFile'); - const connectionsPath = path.join(buildRoot, 'connections.ts'); + const mock = await setup(); + findSpy.mockClear(); + + await mock.listeners.get('change')!(path.join(buildRoot, 'src/connections.ts')); + + expect(findSpy).not.toHaveBeenCalled(); + expect(mock.transformRequest).not.toHaveBeenCalled(); + }); + + test('Should refresh connection IDs when connections.ts changes', async () => { jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( connectionsPath, ); @@ -437,15 +470,11 @@ describe('Apps Plugin - getPlugins', () => { .spyOn(extractConnections, 'extractConnectionIds') .mockReturnValue(['fresh-uuid-1', 'fresh-uuid-2']); - const handleHotUpdate = extractHandleHotUpdate(plugins); - const ctx = createMockHmrContext( - connectionsPath, - "export const connections = { A: 'fresh-uuid-1' };", - ); + const mock = await setup("export const connections = { A: 'fresh-uuid-1' };"); - await handleHotUpdate(ctx); + await mock.listeners.get('change')!(connectionsPath); - expect(ctx.server.transformRequest).toHaveBeenCalledWith(connectionsPath); + expect(mock.transformRequest).toHaveBeenCalledWith(connectionsPath); expect(extractSpy).toHaveBeenCalledWith( expect.anything(), connectionsPath, @@ -453,72 +482,84 @@ describe('Apps Plugin - getPlugins', () => { ); }); - test('Should not throw when transformRequest returns null', async () => { - const plugins = getPlugins(getArgs()); - await primeParser(plugins); + test('Should refresh connection IDs when connections.ts is created (add event)', async () => { + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( + connectionsPath, + ); + const extractSpy = jest + .spyOn(extractConnections, 'extractConnectionIds') + .mockReturnValue(['new-uuid']); - const connectionsPath = path.join(buildRoot, 'connections.ts'); + const mock = await setup("export const connections = { A: 'new-uuid' };"); + + await mock.listeners.get('add')!(connectionsPath); + + expect(mock.transformRequest).toHaveBeenCalledWith(connectionsPath); + expect(extractSpy).toHaveBeenCalled(); + }); + + test('Should clear registry when connections.ts is deleted (unlink event)', async () => { + // First, populate the registry via a successful change refresh. jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( connectionsPath, ); - const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); + jest.spyOn(extractConnections, 'extractConnectionIds').mockReturnValue(['uuid-x']); - const handleHotUpdate = extractHandleHotUpdate(plugins); - const ctx = createMockHmrContext(connectionsPath, null); + const mock = await setup("export const connections = { A: 'uuid-x' };"); + await mock.listeners.get('change')!(connectionsPath); - await handleHotUpdate(ctx); + // Now simulate the file going away — findConnectionsFile returns + // undefined, so loadAndSetConnectionIds clears the registry. + jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue(undefined); + await mock.listeners.get('unlink')!(connectionsPath); - expect(extractSpy).not.toHaveBeenCalled(); expect(mockLogFn).toHaveBeenCalledWith( - expect.stringContaining('Failed to refresh connection IDs'), - 'error', + expect.stringContaining('Cleared connection IDs (no connections file present)'), + 'debug', ); }); - test('Should swallow extraction errors as logged warnings', async () => { - const plugins = getPlugins(getArgs()); - await primeParser(plugins); - - const connectionsPath = path.join(buildRoot, 'connections.ts'); + test('Should clear registry on extraction failure (allowlist fail-closed)', async () => { jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( connectionsPath, ); - jest.spyOn(extractConnections, 'extractConnectionIds').mockImplementation(() => { + // First successful refresh seeds the registry. + const extractSpy = jest + .spyOn(extractConnections, 'extractConnectionIds') + .mockReturnValueOnce(['uuid-good']); + + const mock = await setup("export const connections = { A: 'uuid-good' };"); + await mock.listeners.get('change')!(connectionsPath); + + // Subsequent edit fails — registry must be cleared, not retained. + extractSpy.mockImplementationOnce(() => { throw new Error('boom'); }); + mock.transformRequest.mockResolvedValueOnce({ code: 'broken' }); - const handleHotUpdate = extractHandleHotUpdate(plugins); - const ctx = createMockHmrContext( - connectionsPath, - "export const connections = { A: 'x' };", - ); + await mock.listeners.get('change')!(connectionsPath); - await expect(handleHotUpdate(ctx)).resolves.toBeUndefined(); expect(mockLogFn).toHaveBeenCalledWith( - expect.stringContaining('Failed to refresh connection IDs: boom'), + expect.stringContaining( + 'Failed to refresh connection IDs (cleared registry): boom', + ), 'error', ); }); - test('Should log error when handleHotUpdate fires before buildStart', async () => { - const connectionsPath = path.join(buildRoot, 'connections.ts'); + test('Should log error when transformRequest returns null', async () => { jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( connectionsPath, ); const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); - // Skip primeParser — simulate parser not yet captured. - const handleHotUpdate = extractHandleHotUpdate(getPlugins(getArgs())); - const ctx = createMockHmrContext( - connectionsPath, - "export const connections = { A: 'x' };", - ); + const mock = await setup(null); - await handleHotUpdate(ctx); + await mock.listeners.get('change')!(connectionsPath); expect(extractSpy).not.toHaveBeenCalled(); expect(mockLogFn).toHaveBeenCalledWith( - expect.stringContaining('connection registry parse not set'), + expect.stringContaining('Failed to refresh connection IDs (cleared registry):'), 'error', ); }); diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index d87518b68..58ab1dacd 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -78,6 +78,7 @@ function createBackendFunctionRegistry() { export interface ConnectionIdsRegistry { setParse(parse: (code: string) => Program): void; getConnectionIds(): string[]; + clearConnectionIds(): void; loadAndSetConnectionIds( load: (filePath: string) => Promise, ): Promise<{ filePath: string | null; connectionIds: string[] }>; @@ -93,6 +94,9 @@ function createConnectionIdsRegistry(opts: { buildRoot: string }): ConnectionIds getConnectionIds() { return connectionIds; }, + clearConnectionIds() { + connectionIds = []; + }, async loadAndSetConnectionIds(load) { if (!parse) { throw new Error('connection registry parse not set — buildStart did not run'); diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 701e3e0f1..1f54a4e21 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -41,6 +41,7 @@ const defaultOptions = { connectionRegistry: { setParse: jest.fn(), getConnectionIds: () => [], + clearConnectionIds: jest.fn(), loadAndSetConnectionIds: jest.fn().mockResolvedValue({ filePath: null, connectionIds: [] }), }, handleUpload: mockHandleUpload, diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 020e7e2f8..314278ebc 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -8,7 +8,6 @@ import path from 'path'; import type { build, ViteDevServer } from 'vite'; import type { BackendFunction } from '../backend/discovery'; -import { findConnectionsFile } from '../backend/extract-connections'; import type { ConnectionIdsRegistry } from '../index'; import { buildBackendFunctions } from './build-backend-functions'; @@ -43,10 +42,10 @@ export const getVitePlugin = ({ // `vite build --watch` it also re-fires when connections.ts changes // because addWatchFile registers it as a build dependency. In the dev // server, addWatchFile only registers chokidar tracking — buildStart - // does NOT re-run on edits there; handleHotUpdate refreshes the - // registry mid-session instead (the per-request nested viteBuild uses - // a different config without the apps plugin, so its buildStart can't - // help). + // does NOT re-run on edits there; the dev-server watcher subscriptions + // in configureServer refresh the registry on add/change/unlink (the + // per-request nested viteBuild uses a different config without the + // apps plugin, so its buildStart can't help). async buildStart() { connectionRegistry.setParse((code) => this.parse(code)); try { @@ -102,30 +101,47 @@ export const getVitePlugin = ({ log, }), ); - }, - async handleHotUpdate({ file, server }) { - if (!CONNECTIONS_BASENAME_RE.test(path.basename(file))) { - return; - } - const connectionsPath = await findConnectionsFile(buildRoot); - if (!connectionsPath || path.resolve(file) !== path.resolve(connectionsPath)) { - return; - } - try { - const { connectionIds } = await connectionRegistry.loadAndSetConnectionIds( - async (id) => { - const result = await server.transformRequest(id); - return result?.code ?? null; - }, - ); - log.debug( - `Refreshed connection IDs from ${connectionsPath} (${connectionIds.length})`, - ); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - log.error(`Failed to refresh connection IDs: ${message}`); - } + // Watch for connections-file lifecycle events. `handleHotUpdate` + // only fires for updates to already-tracked files; it misses + // creates and deletes. We subscribe to the underlying chokidar + // watcher directly so create/change/unlink at the project root + // all refresh (or clear) the registry — important because the + // IDs are an allowlist and stale state could keep removed + // connections allowed mid-session. + const buildRootResolved = path.resolve(buildRoot); + const isConnectionsFile = (filePath: string) => + CONNECTIONS_BASENAME_RE.test(path.basename(filePath)) && + path.resolve(path.dirname(filePath)) === buildRootResolved; + + const refresh = async (filePath: string) => { + if (!isConnectionsFile(filePath)) { + return; + } + try { + const { filePath: resolved, connectionIds } = + await connectionRegistry.loadAndSetConnectionIds(async (id) => { + const result = await server.transformRequest(id); + return result?.code ?? null; + }); + log.debug( + resolved + ? `Refreshed connection IDs from ${resolved} (${connectionIds.length})` + : 'Cleared connection IDs (no connections file present)', + ); + } catch (error) { + // Fail closed: an allowlist that silently retains + // removed UUIDs is more dangerous than one that + // temporarily denies everything until the file is fixed. + connectionRegistry.clearConnectionIds(); + const message = error instanceof Error ? error.message : String(error); + log.error(`Failed to refresh connection IDs (cleared registry): ${message}`); + } + }; + + server.watcher.on('add', refresh); + server.watcher.on('change', refresh); + server.watcher.on('unlink', refresh); }, }; }; From 2b56c08dfe4d407f287fb66ed7729b3660db555e Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Tue, 28 Apr 2026 13:25:36 -0400 Subject: [PATCH 08/11] Inline describeNode helper and drop unused Expression import --- .../plugins/apps/src/backend/extract-connections.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/plugins/apps/src/backend/extract-connections.ts b/packages/plugins/apps/src/backend/extract-connections.ts index 8300ab9c3..813984cf1 100644 --- a/packages/plugins/apps/src/backend/extract-connections.ts +++ b/packages/plugins/apps/src/backend/extract-connections.ts @@ -2,7 +2,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import type { Expression, Node, ObjectExpression, Program, Property } from 'estree'; +import type { Node, ObjectExpression, Program, Property } from 'estree'; import { promises as fsp } from 'fs'; import path from 'path'; @@ -143,10 +143,7 @@ function extractStaticString( const quasi = value.quasis[0]; return quasi.value.cooked ?? quasi.value.raw; } - throw fail( - value, - `value for "${keyName}" must be a string literal; got ${describeNode(value)}`, - ); + throw fail(value, `value for "${keyName}" must be a string literal; got ${value.type}`); } /** @@ -164,10 +161,6 @@ function readKeyName(property: Property): string { return ''; } -function describeNode(node: Expression | Property['value']): string { - return node.type; -} - function isConnectionsExportName(name: string): name is (typeof CONNECTIONS_EXPORT_NAMES)[number] { return (CONNECTIONS_EXPORT_NAMES as readonly string[]).includes(name); } From 3173652e311ce7796c5901bb6c4c5073ffd4e730 Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Tue, 28 Apr 2026 13:30:50 -0400 Subject: [PATCH 09/11] Defer reading buildRoot in connections registry until configResolved --- packages/plugins/apps/src/index.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index 58ab1dacd..e55b41532 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -84,7 +84,7 @@ export interface ConnectionIdsRegistry { ): Promise<{ filePath: string | null; connectionIds: string[] }>; } -function createConnectionIdsRegistry(opts: { buildRoot: string }): ConnectionIdsRegistry { +function createConnectionIdsRegistry(opts: { getBuildRoot: () => string }): ConnectionIdsRegistry { let parse: ((code: string) => Program) | null = null; let connectionIds: string[] = []; return { @@ -101,7 +101,7 @@ function createConnectionIdsRegistry(opts: { buildRoot: string }): ConnectionIds if (!parse) { throw new Error('connection registry parse not set — buildStart did not run'); } - const filePath = await findConnectionsFile(opts.buildRoot); + const filePath = await findConnectionsFile(opts.getBuildRoot()); if (!filePath) { connectionIds = []; return { filePath: null, connectionIds }; @@ -152,7 +152,7 @@ export const getPlugins: GetPlugins = ({ options, context, bundler }) => { const { setBackendFunctions, getBackendFunctions } = createBackendFunctionRegistry(); const connectionRegistry = createConnectionIdsRegistry({ - buildRoot: context.buildRoot, + getBuildRoot: () => context.buildRoot, }); const handleUpload = async (backendOutputs: Map) => { From b64302d91ebc18c9a18c93436a593a6d8a7e92c5 Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Wed, 29 Apr 2026 08:16:03 -0400 Subject: [PATCH 10/11] Tighten connections export name to CONNECTIONS only; add E2E manifest assertion --- .../src/backend/extract-connections.test.ts | 77 +--- .../apps/src/backend/extract-connections.ts | 18 +- packages/plugins/apps/src/index.test.ts | 348 ------------------ .../plugins/apps/src/vite/dev-server.test.ts | 28 +- packages/plugins/apps/src/vite/index.ts | 15 + .../src/e2e/appsPlugin/appsPlugin.spec.ts | 8 +- .../src/e2e/appsPlugin/project/connections.js | 8 + 7 files changed, 67 insertions(+), 435 deletions(-) create mode 100644 packages/tests/src/e2e/appsPlugin/project/connections.js diff --git a/packages/plugins/apps/src/backend/extract-connections.test.ts b/packages/plugins/apps/src/backend/extract-connections.test.ts index f5bc53c40..76e2ef4b9 100644 --- a/packages/plugins/apps/src/backend/extract-connections.test.ts +++ b/packages/plugins/apps/src/backend/extract-connections.test.ts @@ -25,12 +25,9 @@ function program(body: Program['body']): Program { } /** - * Build an `export const = ` declaration. + * Build an `export const CONNECTIONS = ` declaration. */ -function exportConnections( - properties: ObjectExpression['properties'], - name: 'connections' | 'CONNECTIONS' = 'connections', -): ExportNamedDeclaration { +function exportConnections(properties: ObjectExpression['properties']): ExportNamedDeclaration { return { type: 'ExportNamedDeclaration', declaration: { @@ -39,7 +36,7 @@ function exportConnections( declarations: [ { type: 'VariableDeclarator', - id: { type: 'Identifier', name }, + id: { type: 'Identifier', name: 'CONNECTIONS' }, init: { type: 'ObjectExpression', properties }, }, ], @@ -134,20 +131,13 @@ describe('extract-connections - extractConnectionIds', () => { ast: program([exportConnections([])]), expected: [], }, - { - description: 'uppercase CONNECTIONS variable name', - ast: program([ - exportConnections([stringProperty('OPEN_AI', 'uuid-upper')], 'CONNECTIONS'), - ]), - expected: ['uuid-upper'], - }, ]; test.each(acceptedCases)('Should accept $description', ({ ast, expected }) => { expect(extractConnectionIds(ast, filePath, '')).toEqual(expected); }); - test('Should throw when no "export const connections" is present', () => { + test('Should throw when no "export const CONNECTIONS" is present', () => { const ast = program([ { type: 'VariableDeclaration', @@ -155,14 +145,14 @@ describe('extract-connections - extractConnectionIds', () => { declarations: [ { type: 'VariableDeclarator', - id: { type: 'Identifier', name: 'connections' }, + id: { type: 'Identifier', name: 'CONNECTIONS' }, init: { type: 'ObjectExpression', properties: [] }, }, ], }, ]); expect(() => extractConnectionIds(ast, filePath, '')).toThrow( - 'connections file must define "export const CONNECTIONS" (or "connections") = { ... }', + 'connections file must define "export const CONNECTIONS" = { ... }', ); }); @@ -174,7 +164,7 @@ describe('extract-connections - extractConnectionIds', () => { }, ]); expect(() => extractConnectionIds(ast, filePath, '')).toThrow( - 'connections file must define "export const CONNECTIONS" (or "connections") = { ... }', + 'connections file must define "export const CONNECTIONS" = { ... }', ); }); @@ -188,7 +178,7 @@ describe('extract-connections - extractConnectionIds', () => { declarations: [ { type: 'VariableDeclarator', - id: { type: 'Identifier', name: 'connections' }, + id: { type: 'Identifier', name: 'CONNECTIONS' }, init: { type: 'Literal', value: 'oops' }, }, ], @@ -199,27 +189,17 @@ describe('extract-connections - extractConnectionIds', () => { }, ]); expect(() => extractConnectionIds(ast, filePath, '')).toThrow( - '"export const CONNECTIONS" (or "connections") must be initialized with an object literal', + '"export const CONNECTIONS" must be initialized with an object literal', ); }); - test('Should throw on multiple "export const connections" declarations', () => { + test('Should throw on multiple "export const CONNECTIONS" declarations', () => { const ast = program([ exportConnections([stringProperty('A', 'uuid-1')]), exportConnections([stringProperty('B', 'uuid-2')]), ]); expect(() => extractConnectionIds(ast, filePath, '')).toThrow( - 'multiple top-level "export const CONNECTIONS" (or "connections") declarations are not allowed', - ); - }); - - test('Should throw when both "connections" and "CONNECTIONS" are exported', () => { - const ast = program([ - exportConnections([stringProperty('A', 'uuid-1')], 'connections'), - exportConnections([stringProperty('B', 'uuid-2')], 'CONNECTIONS'), - ]); - expect(() => extractConnectionIds(ast, filePath, '')).toThrow( - 'multiple top-level "export const CONNECTIONS" (or "connections") declarations are not allowed', + 'multiple top-level "export const CONNECTIONS" declarations are not allowed', ); }); @@ -246,7 +226,7 @@ describe('extract-connections - extractConnectionIds', () => { { type: 'SpreadElement', argument: { type: 'Identifier', name: 'other' }, - } as SpreadElement, + } satisfies SpreadElement, ]), ]); expect(() => extractConnectionIds(ast, filePath, '')).toThrow('spread elements'); @@ -342,39 +322,6 @@ describe('extract-connections - extractConnectionIds', () => { expect(() => extractConnectionIds(ast, filePath, '')).toThrow(reasonContains); }, ); - - // Rollup's this.parse() (SWC) emits character offsets but no line:col, - // so we derive line:col from `node.start` against the source text. - // These tests use the real parser so a regression in offset handling - // surfaces immediately. - describe('framed source location from parseAst', () => { - test('Should include line:col when value is not a string literal', async () => { - const { parseAst } = await import('rollup/parseAst'); - // parseAst is a JS-only parser, so the source has no `as const`. - const code = [ - 'export const CONNECTIONS = {', - " A: 'good-uuid',", - ' B: process.env.OPEN_AI_ID,', - '};', - '', - ].join('\n'); - const ast = parseAst(code) as unknown as Program; - - expect(() => extractConnectionIds(ast, filePath, code)).toThrow( - `must be a string literal; got MemberExpression (at ${filePath}:3:8)`, - ); - }); - - test('Should include line:col for the missing-export case', async () => { - const { parseAst } = await import('rollup/parseAst'); - const code = 'const CONNECTIONS = {};\nexport default CONNECTIONS;\n'; - const ast = parseAst(code) as unknown as Program; - - expect(() => extractConnectionIds(ast, filePath, code)).toThrow( - `connections file must define "export const CONNECTIONS" (or "connections") = { ... } (at ${filePath})`, - ); - }); - }); }); describe('extract-connections - findConnectionsFile', () => { diff --git a/packages/plugins/apps/src/backend/extract-connections.ts b/packages/plugins/apps/src/backend/extract-connections.ts index 813984cf1..f73f35403 100644 --- a/packages/plugins/apps/src/backend/extract-connections.ts +++ b/packages/plugins/apps/src/backend/extract-connections.ts @@ -8,8 +8,8 @@ import path from 'path'; const CONNECTIONS_FILE_BASENAME = 'connections'; const CONNECTIONS_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx'] as const; -const CONNECTIONS_EXPORT_NAMES = ['connections', 'CONNECTIONS'] as const; -const EXPECTED_EXPORT_DESCRIPTION = '"export const CONNECTIONS" (or "connections")'; +const CONNECTIONS_EXPORT_NAME = 'CONNECTIONS'; +const EXPECTED_EXPORT_DESCRIPTION = `"export const ${CONNECTIONS_EXPORT_NAME}"`; /** * Locate the project's connections file. Looks for `connections.{ts,tsx,js,jsx}` @@ -41,8 +41,6 @@ type WithOffset = Node & { start?: number }; * NAME_B: 'uuid-b', * } as const; * - * `connections` (lowercase) is also accepted as the variable name. - * * Values must be plain string literals or interpolation-free template literals. * Anything else (identifiers, env vars, concatenation, function calls, computed * keys, spread elements, …) throws with a framed source location so the caller @@ -67,6 +65,7 @@ export function extractConnectionIds(ast: Program, filePath: string, code: strin let connectionsObject: ObjectExpression | undefined; + // Find: export const CONNECTIONS = {}; for (const node of ast.body) { if (node.type !== 'ExportNamedDeclaration' || !node.declaration) { continue; @@ -76,7 +75,7 @@ export function extractConnectionIds(ast: Program, filePath: string, code: strin continue; } for (const d of decl.declarations) { - if (d.id.type !== 'Identifier' || !isConnectionsExportName(d.id.name)) { + if (d.id.type !== 'Identifier' || d.id.name !== CONNECTIONS_EXPORT_NAME) { continue; } if (connectionsObject) { @@ -100,6 +99,7 @@ export function extractConnectionIds(ast: Program, filePath: string, code: strin } const ids = new Set(); + // Validate and extract the CONNECTIONS object data for (const property of connectionsObject.properties) { if (property.type === 'SpreadElement') { throw fail( @@ -124,6 +124,10 @@ export function extractConnectionIds(ast: Program, filePath: string, code: strin /** * Resolve a property value node to its static string. Accepts string literals * and interpolation-free template literals; throws on anything else. + * + * This return the value of literal string ('HELLO') and template literals with no expressions: (`World`). + * It will throw on everything else. + * */ function extractStaticString( value: Property['value'], @@ -161,10 +165,6 @@ function readKeyName(property: Property): string { return ''; } -function isConnectionsExportName(name: string): name is (typeof CONNECTIONS_EXPORT_NAMES)[number] { - return (CONNECTIONS_EXPORT_NAMES as readonly string[]).includes(name); -} - /** * Convert a 0-based byte offset into a `line:column` string (1-based, like * editor jump-to-line targets). diff --git a/packages/plugins/apps/src/index.test.ts b/packages/plugins/apps/src/index.test.ts index a8c87f41c..553f29c2a 100644 --- a/packages/plugins/apps/src/index.test.ts +++ b/packages/plugins/apps/src/index.test.ts @@ -4,7 +4,6 @@ import * as archive from '@dd/apps-plugin/archive'; import * as assets from '@dd/apps-plugin/assets'; -import * as extractConnections from '@dd/apps-plugin/backend/extract-connections'; import * as identifier from '@dd/apps-plugin/identifier'; import * as uploader from '@dd/apps-plugin/upload'; import { getPlugins } from '@dd/apps-plugin'; @@ -30,72 +29,6 @@ function extractCloseBundle(plugins: PluginOptions[]) { return plugin.vite!.closeBundle as () => Promise; } -/** Extract and assert buildStart from the first plugin's vite hooks. */ -function extractBuildStart(plugins: PluginOptions[]) { - const plugin = plugins[0]; - const buildStart = (plugin?.vite as { buildStart?: unknown } | undefined)?.buildStart; - expect(typeof buildStart).toBe('function'); - return buildStart as (this: unknown) => Promise; -} - -/** Extract and assert configureServer from the first plugin's vite hooks. */ -function extractConfigureServer(plugins: PluginOptions[]) { - const plugin = plugins[0]; - const handler = (plugin?.vite as { configureServer?: unknown } | undefined)?.configureServer; - expect(typeof handler).toBe('function'); - return handler as (server: { - middlewares: { use: (fn: unknown) => void }; - transformRequest?: jest.Mock; - watcher?: { on: jest.Mock }; - }) => void; -} - -type WatcherEvent = 'add' | 'change' | 'unlink'; - -/** - * Build a minimal mock dev server and capture chokidar listeners registered - * via `server.watcher.on(...)`. The returned `listeners` map exposes them so - * tests can fire watcher events without a real chokidar instance. - */ -function createMockServer(transformedCode: string | null) { - const listeners = new Map Promise>(); - const transformRequest = jest - .fn() - .mockResolvedValue(transformedCode == null ? null : { code: transformedCode }); - const watcher: { on: jest.Mock } = { - on: jest.fn((event: WatcherEvent, fn: (file: string) => Promise) => { - listeners.set(event, fn); - return watcher; - }), - }; - return { - server: { - middlewares: { use: jest.fn() }, - transformRequest, - watcher, - }, - listeners, - transformRequest, - watcher, - }; -} - -/** Build a minimal mock of the unplugin/Rollup PluginContext used by buildStart. */ -function createMockPluginContext(loadedCode: string | null) { - return { - addWatchFile: jest.fn(), - load: jest.fn().mockResolvedValue({ code: loadedCode }), - parse: jest.fn().mockImplementation(() => ({ - // The actual extraction is asserted via the `extractConnectionIds` - // spy in the calling test — `parse` just needs to return something - // program-shaped that can flow through the buildStart hook. - type: 'Program', - sourceType: 'module', - body: [], - })), - }; -} - describe('Apps Plugin - getPlugins', () => { const buildRoot = '/project'; const outDir = '/project/dist'; @@ -284,287 +217,6 @@ describe('Apps Plugin - getPlugins', () => { expect(fsHelpers.rm).toHaveBeenCalledWith(path.resolve('/tmp/dd-apps-456')); }); - describe('buildStart - connection IDs', () => { - test('Should be a no-op when no connections file exists', async () => { - const findSpy = jest - .spyOn(extractConnections, 'findConnectionsFile') - .mockResolvedValue(undefined); - const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); - - const buildStart = extractBuildStart(getPlugins(getArgs())); - const ctx = createMockPluginContext(null); - - await buildStart.call(ctx); - - expect(findSpy).toHaveBeenCalledWith(buildRoot); - expect(ctx.load).not.toHaveBeenCalled(); - expect(ctx.addWatchFile).not.toHaveBeenCalled(); - expect(extractSpy).not.toHaveBeenCalled(); - }); - - test('Should load, parse, and extract IDs when connections file is present', async () => { - const connectionsPath = path.join(buildRoot, 'connections.ts'); - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( - connectionsPath, - ); - const extractSpy = jest - .spyOn(extractConnections, 'extractConnectionIds') - .mockReturnValue(['uuid-a', 'uuid-b']); - - const buildStart = extractBuildStart(getPlugins(getArgs())); - const ctx = createMockPluginContext('export const connections = {} as const;'); - - await buildStart.call(ctx); - - expect(ctx.addWatchFile).toHaveBeenCalledWith(connectionsPath); - expect(ctx.load).toHaveBeenCalledWith({ id: connectionsPath }); - expect(ctx.parse).toHaveBeenCalledWith('export const connections = {} as const;'); - expect(extractSpy).toHaveBeenCalled(); - }); - - test('Should throw when ctx.load returns no code for the connections file', async () => { - const connectionsPath = path.join(buildRoot, 'connections.ts'); - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( - connectionsPath, - ); - - const buildStart = extractBuildStart(getPlugins(getArgs())); - const ctx = createMockPluginContext(null); - - await expect(buildStart.call(ctx)).rejects.toThrow( - `connections file '${connectionsPath}' produced no code when loaded`, - ); - }); - - // Vite's dev plugin context returns a ModuleInfo proxy whose `code` - // getter throws — code is only resolvable through the dev server's - // transformRequest. configureServer fires before buildStart in dev, - // so the loader uses the captured server instead of this.load. - test('Should use server.transformRequest in dev (after configureServer fires)', async () => { - const connectionsPath = path.join(buildRoot, 'connections.ts'); - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( - connectionsPath, - ); - const extractSpy = jest - .spyOn(extractConnections, 'extractConnectionIds') - .mockReturnValue(['uuid-from-dev']); - - const plugins = getPlugins(getArgs()); - const configureServer = extractConfigureServer(plugins); - const transformRequest = jest - .fn() - .mockResolvedValue({ code: 'export const connections = {};' }); - configureServer({ - middlewares: { use: jest.fn() }, - transformRequest, - watcher: { on: jest.fn() }, - }); - - const buildStart = extractBuildStart(plugins); - // ctx.load returns a ModuleInfo whose `code` getter throws — - // mirrors vite's EnvironmentPluginContainer proxy. - const moduleInfoStub = Object.defineProperty({}, 'code', { - get() { - throw new Error('[vite] The "code" property of ModuleInfo is not supported.'); - }, - }); - const ctx = { - addWatchFile: jest.fn(), - load: jest.fn().mockResolvedValue(moduleInfoStub), - parse: jest.fn().mockReturnValue({ - type: 'Program', - sourceType: 'module', - body: [], - }), - }; - - await buildStart.call(ctx); - - expect(transformRequest).toHaveBeenCalledWith(connectionsPath); - expect(ctx.load).not.toHaveBeenCalled(); - expect(ctx.addWatchFile).toHaveBeenCalledWith(connectionsPath); - expect(extractSpy).toHaveBeenCalled(); - }); - - // Strict-validation throws need to surface in the build log because - // downstream plugins (e.g. error-tracking sourcemaps) can throw their - // own errors during teardown and mask ours from vite's final report. - test('Should log the framed error before re-throwing', async () => { - const connectionsPath = path.join(buildRoot, 'connections.ts'); - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( - connectionsPath, - ); - jest.spyOn(extractConnections, 'extractConnectionIds').mockImplementation(() => { - throw new Error('[connections] bad value (at /project/connections.ts:3:8)'); - }); - - const buildStart = extractBuildStart(getPlugins(getArgs())); - const ctx = createMockPluginContext('export const connections = {};'); - - await expect(buildStart.call(ctx)).rejects.toThrow( - '[connections] bad value (at /project/connections.ts:3:8)', - ); - expect(mockLogFn).toHaveBeenCalledWith( - expect.stringContaining('[connections] bad value (at /project/connections.ts:3:8)'), - 'error', - ); - }); - }); - - describe('configureServer - connection-file watcher', () => { - const connectionsPath = path.join(buildRoot, 'connections.ts'); - - // Wire up configureServer + buildStart so the parser is captured (the - // refresh path needs it). Returns the watcher listener map and the - // connectionRegistry-driven middleware getConnectionIds closure for - // assertions on the registry's post-event state. - const setup = async (transformedCode: string | null = '') => { - const plugins = getPlugins(getArgs()); - const mock = createMockServer(transformedCode); - extractConfigureServer(plugins)(mock.server); - - // primeParser via buildStart — start with no connections file so - // buildStart succeeds without touching extractConnectionIds. - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValueOnce(undefined); - await extractBuildStart(plugins).call(createMockPluginContext(null)); - - // Capture getConnectionIds from the middleware passed to use(). - const middlewareCalls = (mock.server.middlewares.use as jest.Mock).mock.calls; - expect(middlewareCalls.length).toBeGreaterThan(0); - - return mock; - }; - - test('Should ignore unrelated file changes', async () => { - const findSpy = jest.spyOn(extractConnections, 'findConnectionsFile'); - const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); - - const mock = await setup(); - findSpy.mockClear(); - - await mock.listeners.get('change')!(path.join(buildRoot, 'src/some-other-file.ts')); - - // Basename filter must short-circuit before any disk lookup. - expect(findSpy).not.toHaveBeenCalled(); - expect(extractSpy).not.toHaveBeenCalled(); - expect(mock.transformRequest).not.toHaveBeenCalled(); - }); - - test('Should ignore connections.ts in subdirectories (only project-root file)', async () => { - const findSpy = jest.spyOn(extractConnections, 'findConnectionsFile'); - - const mock = await setup(); - findSpy.mockClear(); - - await mock.listeners.get('change')!(path.join(buildRoot, 'src/connections.ts')); - - expect(findSpy).not.toHaveBeenCalled(); - expect(mock.transformRequest).not.toHaveBeenCalled(); - }); - - test('Should refresh connection IDs when connections.ts changes', async () => { - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( - connectionsPath, - ); - const extractSpy = jest - .spyOn(extractConnections, 'extractConnectionIds') - .mockReturnValue(['fresh-uuid-1', 'fresh-uuid-2']); - - const mock = await setup("export const connections = { A: 'fresh-uuid-1' };"); - - await mock.listeners.get('change')!(connectionsPath); - - expect(mock.transformRequest).toHaveBeenCalledWith(connectionsPath); - expect(extractSpy).toHaveBeenCalledWith( - expect.anything(), - connectionsPath, - "export const connections = { A: 'fresh-uuid-1' };", - ); - }); - - test('Should refresh connection IDs when connections.ts is created (add event)', async () => { - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( - connectionsPath, - ); - const extractSpy = jest - .spyOn(extractConnections, 'extractConnectionIds') - .mockReturnValue(['new-uuid']); - - const mock = await setup("export const connections = { A: 'new-uuid' };"); - - await mock.listeners.get('add')!(connectionsPath); - - expect(mock.transformRequest).toHaveBeenCalledWith(connectionsPath); - expect(extractSpy).toHaveBeenCalled(); - }); - - test('Should clear registry when connections.ts is deleted (unlink event)', async () => { - // First, populate the registry via a successful change refresh. - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( - connectionsPath, - ); - jest.spyOn(extractConnections, 'extractConnectionIds').mockReturnValue(['uuid-x']); - - const mock = await setup("export const connections = { A: 'uuid-x' };"); - await mock.listeners.get('change')!(connectionsPath); - - // Now simulate the file going away — findConnectionsFile returns - // undefined, so loadAndSetConnectionIds clears the registry. - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue(undefined); - await mock.listeners.get('unlink')!(connectionsPath); - - expect(mockLogFn).toHaveBeenCalledWith( - expect.stringContaining('Cleared connection IDs (no connections file present)'), - 'debug', - ); - }); - - test('Should clear registry on extraction failure (allowlist fail-closed)', async () => { - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( - connectionsPath, - ); - // First successful refresh seeds the registry. - const extractSpy = jest - .spyOn(extractConnections, 'extractConnectionIds') - .mockReturnValueOnce(['uuid-good']); - - const mock = await setup("export const connections = { A: 'uuid-good' };"); - await mock.listeners.get('change')!(connectionsPath); - - // Subsequent edit fails — registry must be cleared, not retained. - extractSpy.mockImplementationOnce(() => { - throw new Error('boom'); - }); - mock.transformRequest.mockResolvedValueOnce({ code: 'broken' }); - - await mock.listeners.get('change')!(connectionsPath); - - expect(mockLogFn).toHaveBeenCalledWith( - expect.stringContaining( - 'Failed to refresh connection IDs (cleared registry): boom', - ), - 'error', - ); - }); - - test('Should log error when transformRequest returns null', async () => { - jest.spyOn(extractConnections, 'findConnectionsFile').mockResolvedValue( - connectionsPath, - ); - const extractSpy = jest.spyOn(extractConnections, 'extractConnectionIds'); - - const mock = await setup(null); - - await mock.listeners.get('change')!(connectionsPath); - - expect(extractSpy).not.toHaveBeenCalled(); - expect(mockLogFn).toHaveBeenCalledWith( - expect.stringContaining('Failed to refresh connection IDs (cleared registry):'), - 'error', - ); - }); - }); - test('Should upload assets with vite bundler', async () => { const intakeHost = 'https://api.example.com'; const scope = nock(intakeHost).post(`/${APPS_API_PATH}/app-id/upload`).reply(200, { diff --git a/packages/plugins/apps/src/vite/dev-server.test.ts b/packages/plugins/apps/src/vite/dev-server.test.ts index 20c96ba99..488229337 100644 --- a/packages/plugins/apps/src/vite/dev-server.test.ts +++ b/packages/plugins/apps/src/vite/dev-server.test.ts @@ -390,10 +390,21 @@ describe('Dev Server Middleware', () => { log: mockLog, }); - let capturedBody: unknown; + type PreviewAsyncBody = { + data: { + attributes: { + query: { + properties: { + spec: { inputs: { allowedConnectionIds: string[] } }; + }; + }; + }; + }; + }; + let capturedBody: PreviewAsyncBody | undefined; const apiScope = nock(`https://${DD_SITE}`) .post('/api/v2/app-builder/queries/preview-async', (body) => { - capturedBody = body; + capturedBody = body as PreviewAsyncBody; return true; }) .reply(200, { data: { id: 'receipt-conn' } }) @@ -414,16 +425,9 @@ describe('Dev Server Middleware', () => { expect(res.statusCode).toBe(200); expect(apiScope.isDone()).toBe(true); - const inputs = ( - capturedBody as { - data: { - attributes: { query: { properties: { spec: { inputs: unknown } } } }; - }; - } - ).data.attributes.query.properties.spec.inputs as { - allowedConnectionIds: string[]; - }; - expect(inputs.allowedConnectionIds).toEqual(['uuid-1', 'uuid-2']); + expect( + capturedBody?.data.attributes.query.properties.spec.inputs.allowedConnectionIds, + ).toEqual(['uuid-1', 'uuid-2']); }); test('Should retry when long-poll returns done: false', async () => { diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index 314278ebc..e04e5069f 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -23,6 +23,14 @@ export interface VitePluginOptions { auth: AuthOptionsWithDefaults; } +/** + * Vite-specific hooks for the apps plugin. Lifecycle: + * + * - `buildStart` (both): primes the connection-IDs registry from `connections.ts`. + * - `closeBundle` (production): builds backend functions and uploads the archive. + * - `configureServer` (dev): registers the local-preview middleware and a + * chokidar watcher that refreshes the registry on connections-file changes. + */ export const getVitePlugin = ({ viteBuild, buildRoot, @@ -51,6 +59,13 @@ export const getVitePlugin = ({ try { const { filePath } = await connectionRegistry.loadAndSetConnectionIds( async (id) => { + // In `vite serve`, this.load returns a ModuleInfo + // proxy whose `code` getter throws — code is only + // reachable through server.transformRequest. The + // captured devServer doubles as our dev/build + // discriminator: configureServer fires only in + // `vite serve` (and before buildStart), so devServer + // being set means we're in dev. if (devServer) { const result = await devServer.transformRequest(id); return result?.code ?? null; diff --git a/packages/tests/src/e2e/appsPlugin/appsPlugin.spec.ts b/packages/tests/src/e2e/appsPlugin/appsPlugin.spec.ts index d7e784f44..5cca236a9 100644 --- a/packages/tests/src/e2e/appsPlugin/appsPlugin.spec.ts +++ b/packages/tests/src/e2e/appsPlugin/appsPlugin.spec.ts @@ -182,8 +182,14 @@ describe('Apps Plugin', () => { f.replace(/^backend\//, '').replace(/\.js$/, ''), ); expect(Object.keys(manifest.backend.functions).sort()).toEqual(expectedKeys.sort()); + // The fixture's connections.js declares two UUIDs; the manifest emits + // the same allowlist (sorted) for every backend function. + const expectedConnectionIds = [ + 'a1111111-1111-1111-1111-111111111111', + 'b2222222-2222-2222-2222-222222222222', + ]; for (const entry of Object.values(manifest.backend.functions)) { - expect(Array.isArray(entry.allowedConnectionIds)).toBe(true); + expect(entry.allowedConnectionIds).toEqual(expectedConnectionIds); } }); diff --git a/packages/tests/src/e2e/appsPlugin/project/connections.js b/packages/tests/src/e2e/appsPlugin/project/connections.js new file mode 100644 index 000000000..b6b6bdf5e --- /dev/null +++ b/packages/tests/src/e2e/appsPlugin/project/connections.js @@ -0,0 +1,8 @@ +// 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. + +export const CONNECTIONS = { + OPEN_AI: 'a1111111-1111-1111-1111-111111111111', + SLACK: 'b2222222-2222-2222-2222-222222222222', +}; From c605f1893d0a05fcd8419bcd427d5fcf6e317e2d Mon Sep 17 00:00:00 2001 From: Scott Kennedy Date: Mon, 4 May 2026 10:19:39 -0400 Subject: [PATCH 11/11] Use injected Vite parser for connection IDs --- packages/plugins/apps/src/index.ts | 16 ++++++---------- packages/plugins/apps/src/vite/index.test.ts | 1 - packages/plugins/apps/src/vite/index.ts | 1 - 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/packages/plugins/apps/src/index.ts b/packages/plugins/apps/src/index.ts index e55b41532..ea3a5b2d5 100644 --- a/packages/plugins/apps/src/index.ts +++ b/packages/plugins/apps/src/index.ts @@ -76,7 +76,6 @@ function createBackendFunctionRegistry() { } export interface ConnectionIdsRegistry { - setParse(parse: (code: string) => Program): void; getConnectionIds(): string[]; clearConnectionIds(): void; loadAndSetConnectionIds( @@ -84,13 +83,12 @@ export interface ConnectionIdsRegistry { ): Promise<{ filePath: string | null; connectionIds: string[] }>; } -function createConnectionIdsRegistry(opts: { getBuildRoot: () => string }): ConnectionIdsRegistry { - let parse: ((code: string) => Program) | null = null; +function createConnectionIdsRegistry(opts: { + getBuildRoot: () => string; + parse: (code: string) => Program; +}): ConnectionIdsRegistry { let connectionIds: string[] = []; return { - setParse(p) { - parse = p; - }, getConnectionIds() { return connectionIds; }, @@ -98,9 +96,6 @@ function createConnectionIdsRegistry(opts: { getBuildRoot: () => string }): Conn connectionIds = []; }, async loadAndSetConnectionIds(load) { - if (!parse) { - throw new Error('connection registry parse not set — buildStart did not run'); - } const filePath = await findConnectionsFile(opts.getBuildRoot()); if (!filePath) { connectionIds = []; @@ -110,7 +105,7 @@ function createConnectionIdsRegistry(opts: { getBuildRoot: () => string }): Conn if (code == null) { throw new Error(`connections file '${filePath}' produced no code when loaded`); } - connectionIds = extractConnectionIds(parse(code), filePath, code); + connectionIds = extractConnectionIds(opts.parse(code), filePath, code); return { filePath, connectionIds }; }, }; @@ -153,6 +148,7 @@ export const getPlugins: GetPlugins = ({ options, context, bundler }) => { const connectionRegistry = createConnectionIdsRegistry({ getBuildRoot: () => context.buildRoot, + parse: (code) => bundler.parseAst(code) as Program, }); const handleUpload = async (backendOutputs: Map) => { diff --git a/packages/plugins/apps/src/vite/index.test.ts b/packages/plugins/apps/src/vite/index.test.ts index 1f54a4e21..3743b1fd1 100644 --- a/packages/plugins/apps/src/vite/index.test.ts +++ b/packages/plugins/apps/src/vite/index.test.ts @@ -39,7 +39,6 @@ const defaultOptions = { buildRoot: '/build', getBackendFunctions: () => functions, connectionRegistry: { - setParse: jest.fn(), getConnectionIds: () => [], clearConnectionIds: jest.fn(), loadAndSetConnectionIds: jest.fn().mockResolvedValue({ filePath: null, connectionIds: [] }), diff --git a/packages/plugins/apps/src/vite/index.ts b/packages/plugins/apps/src/vite/index.ts index e04e5069f..069028c3b 100644 --- a/packages/plugins/apps/src/vite/index.ts +++ b/packages/plugins/apps/src/vite/index.ts @@ -55,7 +55,6 @@ export const getVitePlugin = ({ // per-request nested viteBuild uses a different config without the // apps plugin, so its buildStart can't help). async buildStart() { - connectionRegistry.setParse((code) => this.parse(code)); try { const { filePath } = await connectionRegistry.loadAndSetConnectionIds( async (id) => {