perf(test): disable Lambda bundling in CDK unit-test synths (~15x per synth) (#366)

[scottschreckengaust]

What

Disables Lambda asset bundling during CDK unit-test synthesis. Template.fromStack() does a full CDK synth, which bundles every NodejsFunction via esbuild — ~28s for the 413-resource AgentStack. Unit tests assert on CloudFormation structure/properties, not bundled Lambda code, so the bundling is pure overhead.

A jest setupFiles hook (cdk/test/setup/disable-bundling.ts) sets aws:cdk:bundling-stacks: [] via CDK_CONTEXT_JSON. A bare new App() — the dominant pattern across our tests (87 call sites) — picks this up with zero call-site changes.

Closes #366. Part of #363.

Results (8-core local)

Metric	Before	After
Single AgentStack synth	28.7s	1.9s (~15×)
github-tags.test.ts	~135s	9s
Full //cdk:test	~155s	25s (~6×)

• All 2177 CDK tests pass. Coverage thresholds unchanged and met (lines 92.36 ≥ 91, branches 82.81 ≥ 82).
• Real 4-core CI before/after will post on this PR's build run.

Why this over sharding (#365)

Profiling under #366 showed the CDK suite's cost is per-synth esbuild bundling, not test count or redundant synths. This single change beats 4-way sharding (~14%) and synth-dedup (~0%) by a wide margin, with no CI complexity. #365 (sharding) is likely now moot — to be re-evaluated once this lands and we see the new CI baseline.

Durable regression guards

• AGENTS.md "Common mistakes": do not re-enable bundling in tests unless asserting on a bundled asset; minimize full-stack synths (synth once in beforeAll).
• .abca/commands/review_pr.md: a Stage-3 checklist item so PR review catches regressions.

Safety / blast radius

• Audited all 31 stack-synth suites — all green. The one asset-hash reference (test/bootstrap/version.test.ts) is a bootstrap policy hash, not a CDK asset hash, and synthesizes no stack — unaffected.
• A test that genuinely needs bundled assets can opt out by setting aws:cdk:bundling-stacks: ['**'] in its own App context (documented in the setup file).

Dependency note

The AGENTS.md / review_pr.md links point to docs/design/CI_BUILD_PERFORMANCE.md, which is added by PR #364 (the design doc, currently draft). Merge #364 first (or together) so the links resolve.

Draft

Opened as draft while under review per WIP discipline. Will mark ready once the 4-core CI build posts green and #364 ordering is settled.

---

🤖 Generated with Claude Code

[isadeks]

Genuinely good, well-scoped optimization. I verified the core mechanism empirically against aws-cdk-lib@2.257.0: bundlingRequired reads context key aws:cdk:bundling-stacks defaulting to ["**"], so setting it to [] makes [].some(...) return false and a bare new App() (the dominant test pattern) picks it up via CDK_CONTEXT_JSON with zero call-site changes. The ~15× synth speedup is real and the safety audit checks out.

✅ What I verified

• Mechanism is correct — new App() reads CDK_CONTEXT_JSON; aws:cdk:bundling-stacks: [] disables bundling (bundlingRequired === false). Confirmed empirically.
• Safety claim holds — version.test.ts computes a bootstrap-policy hash and synthesizes no stack, so it's unaffected by disabling bundling.
• No collateral — no existing CDK test asserts on bundled asset hashes / S3 keys, and none relies on a constructor-context bundling toggle, so the "all 2177 tests pass" claim is consistent.
• Setup-file internal merge is self-consistent — when CDK_CONTEXT_JSON is pre-set, the existing value wins, matching the file's comment.

🚫 Blockers

None.

🔧 Should fix before merge

1. The documented per-test opt-out does not work (latent bug).
The escape hatch documented in all three places (disable-bundling.ts comment, AGENTS.md, review_pr.md) —

new App({ context: { 'aws:cdk:bundling-stacks': ['**'] } })

— does not re-enable bundling. Verified empirically against aws-cdk-lib@2.257.0:

App construction	stack.bundlingRequired
bare new App()	false (disabled ✅ — intended)
new App({ context: { 'aws:cdk:bundling-stacks': ['**'] } })	false ❌ (PR claims true)
new App({ postCliContext: { 'aws:cdk:bundling-stacks': ['**'] } })	true ✅

Root cause: CDK's App.loadContext(props.context, props.postCliContext) sets props.context first, then overwrites it with CDK_CONTEXT_JSON, then applies postCliContext last. So the env var beats constructor context.

Impact: a future author who needs a real bundled asset (asset-hash / S3-key assertion) follows the docs, gets a silently-unbundled synth, and a wrong/empty asset hash with no error. Not a current-test failure — no test uses this path today — so it's latent.

Fix: document postCliContext as the opt-out (it actually overrides), and correct the "Tests that pass their own { context } still merge on top" comment, which is wrong for this key.

💬 Other (self-disclosed)

2. Dangling doc link. AGENTS.md → ./docs/design/CI_BUILD_PERFORMANCE.md doesn't exist on this branch or main (added by draft PR #364). The PR body already flags the merge-order dependency. Note it intersects with the link-checker PR #300: AGENTS.md is a root .md within that checker's scope, so landing #371 + #300 without #364 would turn the dead link into a red CI check. (review_pr.md has the same link but .abca/ is outside the checker's scope, so only AGENTS.md would trip CI.)

---

Reviewed at xhigh effort. Mechanism and precedence claims verified empirically against aws-cdk-lib@2.257.0, not just read from source.

[scottschreckengaust]

@isadeks Thanks for the empirical verification — you're exactly right, and I confirmed it. I've corrected the opt-out to postCliContext when tests need the bundled asset and rewrote the precedence comment in all three documented locations.

[isadeks]

Re-review: ✅ Both findings resolved — recommend approve

Re-reviewed at head 95ebf79. The three new commits map cleanly onto both findings from the prior pass:

Finding	Fix	Status
1. Broken opt-out — docs said use constructor context, which the env var clobbers	82831aa correct opt-out to postCliContext + c1872c0 clarify it's for asset output, not synth	✅ Fixed & verified empirically
2. Dead doc link — CI_BUILD_PERFORMANCE.md (draft #364)	95ebf79 drop dead links	✅ Fixed

Finding 1 — verified against aws-cdk-lib@2.257.0 with the setup file's env var active:

App construction	stack.bundlingRequired
new App({ postCliContext: { 'aws:cdk:bundling-stacks': ['**'] } })	true ✅ (the documented opt-out — now works)
new App({ context: { 'aws:cdk:bundling-stacks': ['**'] } })	false ✅ (docs now correctly warn this does NOT work)

The corrected AGENTS.md / disable-bundling.ts prose is technically accurate: the loadContext(props.context, props.postCliContext) precedence explanation matches CDK's actual behavior (env var overwrites constructor context; postCliContext is applied last and wins). The added "this doesn't stop the synth, only skips the esbuild step" clarification is correct and heads off a likely misconception.

Finding 2 — zero CI_BUILD_PERFORMANCE.md references remain in any of the three touched files, so the hard dependency on #364 is severed. I also confirmed all 8 remaining .md links in the touched docs resolve on the PR head — no new dead links introduced.

Core mechanism, the version.test.ts safety claim, and the "no test relies on bundled-asset assertions" audit were confirmed in the prior pass and are unchanged.

Nothing left blocking. One cosmetic residual: the PR description's dependency note still mentions the #364 merge ordering, which is now stale since the links are gone — worth trimming the description, but it's not a code issue. Good to mark ready once the 4-core CI build posts green.