[APPS][Connections Part 14] Use static string values for imported connection IDs by sdkennedy2 · Pull Request #377 · DataDog/build-plugins · GitHub
Skip to content

[APPS][Connections Part 14] Use static string values for imported connection IDs#377

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
sdkennedy2/connection-id-static-string-resolution
May 18, 2026
Merged

[APPS][Connections Part 14] Use static string values for imported connection IDs#377
gh-worker-dd-mergequeue-cf854d[bot] merged 2 commits into
masterfrom
sdkennedy2/connection-id-static-string-resolution

Conversation

@sdkennedy2

@sdkennedy2 sdkennedy2 commented May 16, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Backend connection ID extraction already walks reachable app-local modules, but it only supported inline and same-module values. Real apps often keep connection IDs in shared modules and import them into backend helpers. This PR wires the generic static string resolver into module-graph connection ID extraction so those imported values can be analyzed conservatively.

Changes

Module-graph extraction now uses each reachable ParsedModuleRecord directly instead of reparsing through the same-file-only extraction path. For each action-catalog call, the connection ID adapter passes the full connectionId expression to:

resolveStaticStringValue(modules, record.id, connectionIdExpression)

Resolved strings are added to the conservative backend-file allowlist. Unsupported results fail closed using the existing connection ID error path while preserving the generic unsupported reason and message for diagnostics.

This broadens supported connection ID shapes to imported constants, imported static templates, imported const chains, imported object roots, nested static object paths, local export aliases, named re-exports, import/export relays, and unambiguous star exports. Default imports, namespace imports, ambiguous star exports, missing exports, mutable or reassigned bindings, cycles, and dynamic expressions remain unsupported and fail closed.

QA Instructions

Review extract-connection-ids-from-module-graph.test.ts for the imported constant, const-chain, object-path, re-export, star-export, and unsupported-case coverage. The stack remains conservative: unsupported imported or dynamic forms throw instead of silently omitting a possible connection ID.

Blast Radius

This affects backend manifest connection ID extraction for reachable app-local backend modules. It does not change module collection, graph walking, upload plumbing, dev middleware, or the manifest schema.

Documentation

@sdkennedy2 sdkennedy2 changed the title Use static string resolver for connection IDs [APPS][Connections Part 14] Use static string values for imported connection IDs May 16, 2026
@sdkennedy2 sdkennedy2 changed the base branch from sdkennedy2/static-string-resolution to graphite-base/377 May 16, 2026 17:01
@sdkennedy2 sdkennedy2 force-pushed the graphite-base/377 branch from e1d6fb2 to 6287251 Compare May 16, 2026 17:40
@sdkennedy2 sdkennedy2 force-pushed the sdkennedy2/connection-id-static-string-resolution branch from eced601 to 5cf3f1f Compare May 16, 2026 17:40
@sdkennedy2 sdkennedy2 changed the base branch from graphite-base/377 to sdkennedy2/static-string-resolution May 16, 2026 17:40
@sdkennedy2 sdkennedy2 changed the base branch from sdkennedy2/static-string-resolution to graphite-base/377 May 16, 2026 18:27
@sdkennedy2 sdkennedy2 force-pushed the graphite-base/377 branch from 6287251 to 91fc379 Compare May 16, 2026 18:36
@sdkennedy2 sdkennedy2 force-pushed the sdkennedy2/connection-id-static-string-resolution branch from 5cf3f1f to 94d1c0c Compare May 16, 2026 18:36
@sdkennedy2 sdkennedy2 changed the base branch from graphite-base/377 to sdkennedy2/static-string-resolution May 16, 2026 18:36
@sdkennedy2 sdkennedy2 marked this pull request as ready for review May 16, 2026 19:13
@sdkennedy2 sdkennedy2 requested review from a team and yoannmoinet as code owners May 16, 2026 19:13

@sarenji sarenji left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One observation: Seems like the tests are quite similar across PRs. I wonder if we only need to test the public interface (as you do in this PR), rather than the implementation details. For example if a change goes wrong, would we have multiple tests fail across the stack? But you know best whether the more "implementation-y"/unit-y tests are a useful signal to have as well.

@sdkennedy2 sdkennedy2 changed the base branch from sdkennedy2/static-string-resolution to graphite-base/377 May 18, 2026 16:10
@sdkennedy2

Copy link
Copy Markdown
Collaborator Author

@sdkennedy2 sdkennedy2 force-pushed the sdkennedy2/connection-id-static-string-resolution branch from 94d1c0c to 6ed08aa Compare May 18, 2026 17:06
@sdkennedy2 sdkennedy2 force-pushed the graphite-base/377 branch from 91fc379 to b9ed474 Compare May 18, 2026 17:06
@sdkennedy2 sdkennedy2 changed the base branch from graphite-base/377 to master May 18, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants