Skip to content

Commit 82831aa

Browse files
docs(test): correct bundling opt-out to postCliContext (#366)
The documented per-test opt-out used constructor `context`, which CDK silently overwrites with CDK_CONTEXT_JSON in App.loadContext — so `new App({ context: { 'aws:cdk:bundling-stacks': ['**'] } })` does NOT re-enable bundling. Only `postCliContext` (applied last) overrides the global disable. Corrects the opt-out instruction and precedence comment in all three documented locations (disable-bundling.ts, AGENTS.md, review_pr.md). Verified empirically against aws-cdk-lib@2.257.0. Comment/doc-only; runtime logic unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6cf7a98 commit 82831aa

3 files changed

Lines changed: 15 additions & 6 deletions

File tree

.abca/commands/review_pr.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,9 @@ Then apply principal-architect judgment over the diff:
100100
- **Test performance (CDK synth)** — New/changed CDK tests must not re-enable Lambda bundling at
101101
synth or synthesize the same stack repeatedly. `cdk/` disables bundling globally via
102102
`test/setup/disable-bundling.ts` (~15× faster synth); flag any test that turns
103-
`aws:cdk:bundling-stacks` back on without asserting on a bundled asset, or that calls
104-
`new App()` + `Template.fromStack()` per-test instead of once in `beforeAll`. See
103+
`aws:cdk:bundling-stacks` back on (only valid via `postCliContext`, not constructor
104+
`context` — the env var overwrites the latter) without asserting on a bundled asset, or
105+
that calls `new App()` + `Template.fromStack()` per-test instead of once in `beforeAll`. See
105106
[CI build performance](../../docs/design/CI_BUILD_PERFORMANCE.md).
106107
- **Routing** — Changes should land in the right package per the AGENTS.md routing table
107108
(agent runtime in `agent/`, API/Lambdas in `cdk/`, CLI in `cli/`).

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ Handler entry tests: `cdk/test/handlers/orchestrate-task.test.ts`, `create-task.
4545
- Adding or editing files in **`docs/design/`** or **`docs/guides/`** without running **`cd docs && node scripts/sync-starlight.mjs`** — CI will reject ("Fail build on mutation") because the Starlight mirror files in `docs/src/content/docs/` are stale. Always commit the regenerated mirrors alongside source changes.
4646
- Changing **`cdk/.../types.ts`** without updating **`cli/src/types.ts`** — CLI and API drift.
4747
- Running raw **`jest`/`tsc`/`cdk`** from muscle memory — prefer **`mise //cdk:test`**, **`mise //cdk:compile`**, **`mise //cdk:synth`** (see [Commands you can use](#commands-you-can-use)).
48-
- **Bundling Lambda assets in CDK unit tests**`Template.fromStack()` triggers a full synth that bundles every `NodejsFunction` via esbuild (~28s for `AgentStack`). Unit tests assert on CloudFormation structure, not bundled code, so this is pure overhead. The `cdk/` Jest config disables it globally via `test/setup/disable-bundling.ts` (sets `aws:cdk:bundling-stacks: []` in `CDK_CONTEXT_JSON`), which a bare `new App()` picks up automatically. **Do not re-enable bundling** in a test unless you are specifically asserting on a bundled asset (hash / S3 key) — and if you must, opt out narrowly in that test's `App` context, not globally. Minimize full-stack synths regardless: synthesize each distinct stack config once in `beforeAll` and assert against the cached `Template`. See [CI build performance](./docs/design/CI_BUILD_PERFORMANCE.md) and #366.
48+
- **Bundling Lambda assets in CDK unit tests** — `Template.fromStack()` triggers a full synth that bundles every `NodejsFunction` via esbuild (~28s for `AgentStack`). Unit tests assert on CloudFormation structure, not bundled code, so this is pure overhead. The `cdk/` Jest config disables it globally via `test/setup/disable-bundling.ts` (sets `aws:cdk:bundling-stacks: []` in `CDK_CONTEXT_JSON`), which a bare `new App()` picks up automatically. **Do not re-enable bundling** in a test unless you are specifically asserting on a bundled asset (hash / S3 key) — and if you must, opt out narrowly via that test's `App` `postCliContext` (e.g. `new App({ postCliContext: { 'aws:cdk:bundling-stacks': ['**'] } })`), not globally. Note that constructor `context` does **not** work for this key: CDK overwrites it with `CDK_CONTEXT_JSON`, so only `postCliContext` (applied last) overrides the global disable. Minimize full-stack synths regardless: synthesize each distinct stack config once in `beforeAll` and assert against the cached `Template`. See [CI build performance](./docs/design/CI_BUILD_PERFORMANCE.md) and #366.
4949
- **`MISE_EXPERIMENTAL=1`** — required for namespaced tasks like **`mise //cdk:build`** (see [CONTRIBUTING.md](./CONTRIBUTING.md)).
5050
- **`mise run build`** builds **`//agent:quality`** alongside **`//cdk:build`** (the deployed image bundles **`agent/`**, so agent quality is part of the build) — these run as parallel `depends`, not in a fixed order; agent changes belong in the **`agent/`** tree.
5151
- **`prek install`** fails if Git **`core.hooksPath`** is set — another hook manager owns hooks; see [CONTRIBUTING.md](./CONTRIBUTING.md).

cdk/test/setup/disable-bundling.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,19 @@
2828
// `aws:cdk:bundling-stacks: []` tells the CLI/synth to bundle no stacks. CDK
2929
// reads CDK_CONTEXT_JSON when an `App` is constructed, so a bare `new App()`
3030
// (the overwhelming-majority pattern in our tests) picks this up with no
31-
// call-site changes. Tests that pass their own `{ context }` still merge on top.
31+
// call-site changes.
32+
//
33+
// Precedence matters for the opt-out. CDK's `App.loadContext(props.context,
34+
// props.postCliContext)` applies `props.context` FIRST, then overwrites it with
35+
// CDK_CONTEXT_JSON, then applies `postCliContext` LAST. So for this key the env
36+
// var beats constructor `context` — `new App({ context: { 'aws:cdk:bundling-
37+
// stacks': ['**'] } })` does NOT re-enable bundling (the env var clobbers it).
3238
//
3339
// If a test ever needs real bundled assets (e.g. asserting on an asset hash /
34-
// S3 key), it must opt out by constructing its `App` with
35-
// `aws:cdk:bundling-stacks: ['**']` (or the specific stack id) in its context.
40+
// S3 key), it must opt out via `postCliContext` (which wins over the env var),
41+
// constructing its `App` with
42+
// `new App({ postCliContext: { 'aws:cdk:bundling-stacks': ['**'] } })`
43+
// (or the specific stack id).
3644
const BUNDLING_DISABLED_CONTEXT = { 'aws:cdk:bundling-stacks': [] as string[] };
3745

3846
const existing = process.env.CDK_CONTEXT_JSON;

0 commit comments

Comments
 (0)