From 6cf7a984fd1e7d544424a66107a3d00aa9f968de Mon Sep 17 00:00:00 2001 From: bgagent <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 17 Jun 2026 02:18:29 +0000 Subject: [PATCH 1/8] perf(test): disable Lambda bundling in CDK unit-test synths (#366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Template.fromStack() triggers a full CDK synth that bundles every NodejsFunction via esbuild — ~28s for the 413-resource AgentStack. Unit tests assert on CloudFormation structure, not bundled Lambda code, so the bundling is pure overhead. A jest setupFiles hook sets aws:cdk:bundling-stacks: [] via CDK_CONTEXT_JSON, which a bare new App() picks up with no call-site changes. Measured: single AgentStack synth 28.7s -> 1.9s (~15x); github-tags.test.ts ~135s -> 9s; full //cdk:test 155s -> 25s (8-core local). All 2177 tests pass; coverage thresholds unchanged (lines 92.36 >= 91). Durable regression guards added: AGENTS.md "Common mistakes" entry and a .abca/commands/review_pr.md checklist item — do not re-enable bundling in tests unless asserting on a bundled asset, and minimize full-stack synths (synth once in beforeAll). Both link docs/design/CI_BUILD_PERFORMANCE.md (PR #364). Refs #366. Part of #363. Likely supersedes the sharding approach (#365) — to be re-evaluated once this lands. Co-Authored-By: Claude Fable 5 --- .abca/commands/review_pr.md | 6 ++++ AGENTS.md | 1 + cdk/package.json | 3 ++ cdk/test/setup/disable-bundling.ts | 45 ++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+) create mode 100644 cdk/test/setup/disable-bundling.ts diff --git a/.abca/commands/review_pr.md b/.abca/commands/review_pr.md index 10cb2bb0..8f486e32 100644 --- a/.abca/commands/review_pr.md +++ b/.abca/commands/review_pr.md @@ -97,6 +97,12 @@ Then apply principal-architect judgment over the diff: IDs, cdk-nag clean. Watch for cost and operational footguns. - **Tests** — Are unit tests added/updated under the matching `*/test/` tree? Do they cover the new behavior and failure paths, not just the happy path? +- **Test performance (CDK synth)** — New/changed CDK tests must not re-enable Lambda bundling at + synth or synthesize the same stack repeatedly. `cdk/` disables bundling globally via + `test/setup/disable-bundling.ts` (~15× faster synth); flag any test that turns + `aws:cdk:bundling-stacks` back on without asserting on a bundled asset, or that calls + `new App()` + `Template.fromStack()` per-test instead of once in `beforeAll`. See + [CI build performance](../../docs/design/CI_BUILD_PERFORMANCE.md). - **Routing** — Changes should land in the right package per the AGENTS.md routing table (agent runtime in `agent/`, API/Lambdas in `cdk/`, CLI in `cli/`). diff --git a/AGENTS.md b/AGENTS.md index 0f6f2c0f..421ebf22 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,6 +45,7 @@ Handler entry tests: `cdk/test/handlers/orchestrate-task.test.ts`, `create-task. - 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. - Changing **`cdk/.../types.ts`** without updating **`cli/src/types.ts`** — CLI and API drift. - 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)). +- **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. - **`MISE_EXPERIMENTAL=1`** — required for namespaced tasks like **`mise //cdk:build`** (see [CONTRIBUTING.md](./CONTRIBUTING.md)). - **`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. - **`prek install`** fails if Git **`core.hooksPath`** is set — another hook manager owns hooks; see [CONTRIBUTING.md](./CONTRIBUTING.md). diff --git a/cdk/package.json b/cdk/package.json index 5968b096..a1097dda 100644 --- a/cdk/package.json +++ b/cdk/package.json @@ -106,6 +106,9 @@ "watchPathIgnorePatterns": [ "/node_modules/" ], + "setupFiles": [ + "/test/setup/disable-bundling.ts" + ], "setupFilesAfterEnv": [ "aws-cdk-lib/testhelpers/jest-autoclean" ], diff --git a/cdk/test/setup/disable-bundling.ts b/cdk/test/setup/disable-bundling.ts new file mode 100644 index 00000000..f0462582 --- /dev/null +++ b/cdk/test/setup/disable-bundling.ts @@ -0,0 +1,45 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +// Disable Lambda asset bundling during unit-test synthesis. +// +// `Template.fromStack()` triggers a full CDK synth, which bundles every +// NodejsFunction via esbuild — ~28s for the AgentStack (413 resources). Unit +// tests assert on CloudFormation structure/properties, not bundled Lambda code, +// so bundling is pure overhead here: skipping it cuts a single synth ~15× +// (~28.7s -> ~1.9s). See #366 and docs/design/CI_BUILD_PERFORMANCE.md. +// +// `aws:cdk:bundling-stacks: []` tells the CLI/synth to bundle no stacks. CDK +// reads CDK_CONTEXT_JSON when an `App` is constructed, so a bare `new App()` +// (the overwhelming-majority pattern in our tests) picks this up with no +// call-site changes. Tests that pass their own `{ context }` still merge on top. +// +// If a test ever needs real bundled assets (e.g. asserting on an asset hash / +// S3 key), it must opt out by constructing its `App` with +// `aws:cdk:bundling-stacks: ['**']` (or the specific stack id) in its context. +const BUNDLING_DISABLED_CONTEXT = { 'aws:cdk:bundling-stacks': [] as string[] }; + +const existing = process.env.CDK_CONTEXT_JSON; +if (existing) { + // Preserve any context already provided; our key wins only if unset. + const merged = { ...BUNDLING_DISABLED_CONTEXT, ...JSON.parse(existing) }; + process.env.CDK_CONTEXT_JSON = JSON.stringify(merged); +} else { + process.env.CDK_CONTEXT_JSON = JSON.stringify(BUNDLING_DISABLED_CONTEXT); +} From 82831aab75992263dadd203eaf66c75b83a88b0d Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 17 Jun 2026 13:55:11 +0000 Subject: [PATCH 2/8] docs(test): correct bundling opt-out to postCliContext (#366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .abca/commands/review_pr.md | 5 +++-- AGENTS.md | 2 +- cdk/test/setup/disable-bundling.ts | 14 +++++++++++--- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/.abca/commands/review_pr.md b/.abca/commands/review_pr.md index 8f486e32..7126db0c 100644 --- a/.abca/commands/review_pr.md +++ b/.abca/commands/review_pr.md @@ -100,8 +100,9 @@ Then apply principal-architect judgment over the diff: - **Test performance (CDK synth)** — New/changed CDK tests must not re-enable Lambda bundling at synth or synthesize the same stack repeatedly. `cdk/` disables bundling globally via `test/setup/disable-bundling.ts` (~15× faster synth); flag any test that turns - `aws:cdk:bundling-stacks` back on without asserting on a bundled asset, or that calls - `new App()` + `Template.fromStack()` per-test instead of once in `beforeAll`. See + `aws:cdk:bundling-stacks` back on (only valid via `postCliContext`, not constructor + `context` — the env var overwrites the latter) without asserting on a bundled asset, or + that calls `new App()` + `Template.fromStack()` per-test instead of once in `beforeAll`. See [CI build performance](../../docs/design/CI_BUILD_PERFORMANCE.md). - **Routing** — Changes should land in the right package per the AGENTS.md routing table (agent runtime in `agent/`, API/Lambdas in `cdk/`, CLI in `cli/`). diff --git a/AGENTS.md b/AGENTS.md index 421ebf22..9013bf39 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,7 @@ Handler entry tests: `cdk/test/handlers/orchestrate-task.test.ts`, `create-task. - 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. - Changing **`cdk/.../types.ts`** without updating **`cli/src/types.ts`** — CLI and API drift. - 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)). -- **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. +- **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. - **`MISE_EXPERIMENTAL=1`** — required for namespaced tasks like **`mise //cdk:build`** (see [CONTRIBUTING.md](./CONTRIBUTING.md)). - **`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. - **`prek install`** fails if Git **`core.hooksPath`** is set — another hook manager owns hooks; see [CONTRIBUTING.md](./CONTRIBUTING.md). diff --git a/cdk/test/setup/disable-bundling.ts b/cdk/test/setup/disable-bundling.ts index f0462582..094b8368 100644 --- a/cdk/test/setup/disable-bundling.ts +++ b/cdk/test/setup/disable-bundling.ts @@ -28,11 +28,19 @@ // `aws:cdk:bundling-stacks: []` tells the CLI/synth to bundle no stacks. CDK // reads CDK_CONTEXT_JSON when an `App` is constructed, so a bare `new App()` // (the overwhelming-majority pattern in our tests) picks this up with no -// call-site changes. Tests that pass their own `{ context }` still merge on top. +// call-site changes. +// +// Precedence matters for the opt-out. CDK's `App.loadContext(props.context, +// props.postCliContext)` applies `props.context` FIRST, then overwrites it with +// CDK_CONTEXT_JSON, then applies `postCliContext` LAST. So for this key the env +// var beats constructor `context` — `new App({ context: { 'aws:cdk:bundling- +// stacks': ['**'] } })` does NOT re-enable bundling (the env var clobbers it). // // If a test ever needs real bundled assets (e.g. asserting on an asset hash / -// S3 key), it must opt out by constructing its `App` with -// `aws:cdk:bundling-stacks: ['**']` (or the specific stack id) in its context. +// S3 key), it must opt out via `postCliContext` (which wins over the env var), +// constructing its `App` with +// `new App({ postCliContext: { 'aws:cdk:bundling-stacks': ['**'] } })` +// (or the specific stack id). const BUNDLING_DISABLED_CONTEXT = { 'aws:cdk:bundling-stacks': [] as string[] }; const existing = process.env.CDK_CONTEXT_JSON; From c1872c070690ceee44bd9c66c1b3edf0df98cfc4 Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 17 Jun 2026 14:12:59 +0000 Subject: [PATCH 3/8] docs(test): clarify bundling opt-out is for asset output, not synth (#366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Disabling bundling does not stop tests from synthesizing — `Template.fromStack()` still runs a full synth; it only skips the esbuild asset-bundling step. The `postCliContext` opt-out exists solely for the rare test that needs real bundled-asset output (asset hash / S3 key), where an unbundled synth would silently yield a placeholder. Makes this explicit in disable-bundling.ts and AGENTS.md. Doc-only. Co-Authored-By: Claude Opus 4.8 --- AGENTS.md | 2 +- cdk/test/setup/disable-bundling.ts | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9013bf39..34706641 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,7 @@ Handler entry tests: `cdk/test/handlers/orchestrate-task.test.ts`, `create-task. - 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. - Changing **`cdk/.../types.ts`** without updating **`cli/src/types.ts`** — CLI and API drift. - 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)). -- **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. +- **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. This does not stop tests from synthesizing — `Template.fromStack()` still runs a full synth; it only skips the esbuild step. **Do not re-enable bundling** in a test unless you are specifically asserting on the bundled-asset output (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. - **`MISE_EXPERIMENTAL=1`** — required for namespaced tasks like **`mise //cdk:build`** (see [CONTRIBUTING.md](./CONTRIBUTING.md)). - **`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. - **`prek install`** fails if Git **`core.hooksPath`** is set — another hook manager owns hooks; see [CONTRIBUTING.md](./CONTRIBUTING.md). diff --git a/cdk/test/setup/disable-bundling.ts b/cdk/test/setup/disable-bundling.ts index 094b8368..38ac646d 100644 --- a/cdk/test/setup/disable-bundling.ts +++ b/cdk/test/setup/disable-bundling.ts @@ -36,8 +36,12 @@ // var beats constructor `context` — `new App({ context: { 'aws:cdk:bundling- // stacks': ['**'] } })` does NOT re-enable bundling (the env var clobbers it). // -// If a test ever needs real bundled assets (e.g. asserting on an asset hash / -// S3 key), it must opt out via `postCliContext` (which wins over the env var), +// This does NOT stop tests from synthesizing — `Template.fromStack()` still +// runs a full synth; it only skips the esbuild asset-bundling step within that +// synth. The opt-out below exists solely for the rare test that needs the +// *bundled-asset output itself* (e.g. asserting on a real asset hash / S3 key), +// where an unbundled synth would silently yield a placeholder value. Such a +// test must opt out via `postCliContext` (which wins over the env var), // constructing its `App` with // `new App({ postCliContext: { 'aws:cdk:bundling-stacks': ['**'] } })` // (or the specific stack id). From 95ebf79cb63d98341c84cd6cb57bee4355b596a0 Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 17 Jun 2026 14:22:08 +0000 Subject: [PATCH 4/8] docs(test): drop dead CI_BUILD_PERFORMANCE.md links (#366) The doc lives in draft PR #364 and exists on neither this branch nor main, so the three links (disable-bundling.ts, AGENTS.md, review_pr.md) are dead. Drop them and keep the stable #366 reference, removing the merge-order dependency on #364 and the link-checker (#300) risk. Co-Authored-By: Claude Opus 4.8 --- .abca/commands/review_pr.md | 4 ++-- AGENTS.md | 2 +- cdk/test/setup/disable-bundling.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.abca/commands/review_pr.md b/.abca/commands/review_pr.md index 7126db0c..7a597dd6 100644 --- a/.abca/commands/review_pr.md +++ b/.abca/commands/review_pr.md @@ -102,8 +102,8 @@ Then apply principal-architect judgment over the diff: `test/setup/disable-bundling.ts` (~15× faster synth); flag any test that turns `aws:cdk:bundling-stacks` back on (only valid via `postCliContext`, not constructor `context` — the env var overwrites the latter) without asserting on a bundled asset, or - that calls `new App()` + `Template.fromStack()` per-test instead of once in `beforeAll`. See - [CI build performance](../../docs/design/CI_BUILD_PERFORMANCE.md). + that calls `new App()` + `Template.fromStack()` per-test instead of once in `beforeAll`. + See #366. - **Routing** — Changes should land in the right package per the AGENTS.md routing table (agent runtime in `agent/`, API/Lambdas in `cdk/`, CLI in `cli/`). diff --git a/AGENTS.md b/AGENTS.md index 34706641..b2ba5f9f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,7 @@ Handler entry tests: `cdk/test/handlers/orchestrate-task.test.ts`, `create-task. - 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. - Changing **`cdk/.../types.ts`** without updating **`cli/src/types.ts`** — CLI and API drift. - 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)). -- **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. This does not stop tests from synthesizing — `Template.fromStack()` still runs a full synth; it only skips the esbuild step. **Do not re-enable bundling** in a test unless you are specifically asserting on the bundled-asset output (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. +- **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. This does not stop tests from synthesizing — `Template.fromStack()` still runs a full synth; it only skips the esbuild step. **Do not re-enable bundling** in a test unless you are specifically asserting on the bundled-asset output (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 #366. - **`MISE_EXPERIMENTAL=1`** — required for namespaced tasks like **`mise //cdk:build`** (see [CONTRIBUTING.md](./CONTRIBUTING.md)). - **`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. - **`prek install`** fails if Git **`core.hooksPath`** is set — another hook manager owns hooks; see [CONTRIBUTING.md](./CONTRIBUTING.md). diff --git a/cdk/test/setup/disable-bundling.ts b/cdk/test/setup/disable-bundling.ts index 38ac646d..306b7639 100644 --- a/cdk/test/setup/disable-bundling.ts +++ b/cdk/test/setup/disable-bundling.ts @@ -23,7 +23,7 @@ // NodejsFunction via esbuild — ~28s for the AgentStack (413 resources). Unit // tests assert on CloudFormation structure/properties, not bundled Lambda code, // so bundling is pure overhead here: skipping it cuts a single synth ~15× -// (~28.7s -> ~1.9s). See #366 and docs/design/CI_BUILD_PERFORMANCE.md. +// (~28.7s -> ~1.9s). See #366. // // `aws:cdk:bundling-stacks: []` tells the CLI/synth to bundle no stacks. CDK // reads CDK_CONTEXT_JSON when an `App` is constructed, so a bare `new App()` From 94c604ed3c2d65432788e07e220af2d50d3ce071 Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 17 Jun 2026 22:52:47 +0000 Subject: [PATCH 5/8] fix(test): harden bundling-disable against ambient CDK_CONTEXT_JSON (#366) Two robustness fixes to the global bundling-disable setup file, both from PR #371 review (finding 1 hardening nits): - Spread BUNDLING_DISABLED_CONTEXT LAST when merging a pre-set CDK_CONTEXT_JSON, so an ambient `aws:cdk:bundling-stacks` value can no longer silently win and re-enable bundling. This is safe precisely because the documented per-test opt-out is `postCliContext` (applied after the env var regardless), so the escape hatch still works. - Guard `JSON.parse` of the pre-set var with try/catch. A malformed value previously threw inside setupFiles and failed every test in every file with an opaque parse error; now the blame is localized to the offending value. Co-Authored-By: Claude Opus 4.8 --- cdk/test/setup/disable-bundling.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/cdk/test/setup/disable-bundling.ts b/cdk/test/setup/disable-bundling.ts index 306b7639..9abf54bd 100644 --- a/cdk/test/setup/disable-bundling.ts +++ b/cdk/test/setup/disable-bundling.ts @@ -49,8 +49,23 @@ const BUNDLING_DISABLED_CONTEXT = { 'aws:cdk:bundling-stacks': [] as string[] }; const existing = process.env.CDK_CONTEXT_JSON; if (existing) { - // Preserve any context already provided; our key wins only if unset. - const merged = { ...BUNDLING_DISABLED_CONTEXT, ...JSON.parse(existing) }; + // Preserve any ambient context, but let the bundling disable win + // unconditionally: a pre-set CDK_CONTEXT_JSON that carries + // `aws:cdk:bundling-stacks` must not silently re-enable bundling and defeat + // the speedup. The documented per-test opt-out is `postCliContext` (applied + // AFTER the env var regardless — see above), so spreading our key LAST keeps + // that escape hatch working while making the global disable robust to + // whatever the environment already set. + let ambient: Record; + try { + ambient = JSON.parse(existing); + } catch { + // Without this guard a malformed pre-set var throws here, inside + // `setupFiles`, and fails EVERY test in EVERY file with an opaque parse + // error. Re-throw with the offending value so the blame is localized. + throw new Error(`malformed CDK_CONTEXT_JSON in environment (must be valid JSON): ${existing}`); + } + const merged = { ...ambient, ...BUNDLING_DISABLED_CONTEXT }; process.env.CDK_CONTEXT_JSON = JSON.stringify(merged); } else { process.env.CDK_CONTEXT_JSON = JSON.stringify(BUNDLING_DISABLED_CONTEXT); From 09b1f24dd3ad52b9ec8bec35e90849b01220fe96 Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 17 Jun 2026 22:54:14 +0000 Subject: [PATCH 6/8] test(cdk): add executable guard that Lambda bundling is disabled (#366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #371 review finding 1: the ~15× synth speedup rested entirely on the `disable-bundling.ts` setupFiles entry, but the only regression guards were honor-system prose (AGENTS.md) and a review_pr.md checklist item. Dropping the setupFiles wiring, reordering Jest setup, or a CDK rename of the context key would silently revert the suite to full-bundling synths with no failing check. Adds two assertions: the synth context carries `aws:cdk:bundling-stacks: []`, and a bare-App stack reports `bundlingRequired === false`. Verified empirically against aws-cdk-lib@2.259.0 that without the disable a bare App yields `bundlingRequired === true`, so these discriminate a real regression. Co-Authored-By: Claude Opus 4.8 --- cdk/test/setup/disable-bundling.test.ts | 43 +++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 cdk/test/setup/disable-bundling.test.ts diff --git a/cdk/test/setup/disable-bundling.test.ts b/cdk/test/setup/disable-bundling.test.ts new file mode 100644 index 00000000..696186fd --- /dev/null +++ b/cdk/test/setup/disable-bundling.test.ts @@ -0,0 +1,43 @@ +/** + * MIT No Attribution + * + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +// Executable guard for the global Lambda-bundling disable (#366). +// +// The speedup in this package depends entirely on `test/setup/disable-bundling.ts` +// running as a Jest `setupFiles` entry. Until now the only regression guards +// were prose in AGENTS.md and a checklist item in review_pr.md — both +// honor-system. If someone drops the `setupFiles` wiring, reorders Jest setup, +// or a CDK rename breaks the context key, the suite silently reverts to a +// full-bundling (~15× slower) synth with no failing check. These assertions +// make that floor machine-enforced. +import { App, Stack } from 'aws-cdk-lib'; + +describe('global Lambda bundling disable', () => { + it('sets aws:cdk:bundling-stacks to [] in the synth context', () => { + // A bare `new App()` reads CDK_CONTEXT_JSON, which setupFiles populated. + expect(new App().node.tryGetContext('aws:cdk:bundling-stacks')).toEqual([]); + }); + + it('makes a bare-App stack report bundlingRequired === false', () => { + const stack = new Stack(new App(), 'BundlingProbe', { + env: { account: '123456789012', region: 'us-east-1' }, + }); + expect(stack.bundlingRequired).toBe(false); + }); +}); From 77b0c7a9fdb837178894c20bf4e973f756fd5425 Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 17 Jun 2026 22:55:25 +0000 Subject: [PATCH 7/8] test(cdk): cover the postCliContext bundling opt-out precedence (#366) PR #371 review finding 2: the documented per-test opt-out (`postCliContext`) and the `loadContext` precedence this PR corrected had zero test coverage, so a future CDK version reordering context application could silently break the one supported escape hatch. Locks the contract with two assertions verified against aws-cdk-lib@2.259.0: `postCliContext: { 'aws:cdk:bundling-stacks': ['**'] }` re-enables bundling (`bundlingRequired === true`), while constructor `context` does NOT (the env var set by the global disable clobbers it). Uses a bare `Stack` rather than a heavy AgentStack since the precedence is a property of CDK context loading, not of any particular stack. Co-Authored-By: Claude Opus 4.8 --- cdk/test/setup/disable-bundling.test.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cdk/test/setup/disable-bundling.test.ts b/cdk/test/setup/disable-bundling.test.ts index 696186fd..d7c5d9c8 100644 --- a/cdk/test/setup/disable-bundling.test.ts +++ b/cdk/test/setup/disable-bundling.test.ts @@ -41,3 +41,28 @@ describe('global Lambda bundling disable', () => { expect(stack.bundlingRequired).toBe(false); }); }); + +describe('per-test bundling opt-out (#366)', () => { + // The documented escape hatch for a test that needs real bundled-asset output + // is `postCliContext`, NOT constructor `context`. CDK's + // `App.loadContext(props.context, props.postCliContext)` applies + // `props.context` first, overwrites it with CDK_CONTEXT_JSON (which the global + // disable sets), then applies `postCliContext` last — so only the latter wins. + // These lock that precedence contract against a future CDK reordering; without + // them the one supported opt-out path is unguarded. + it('re-enables bundling via postCliContext (the documented opt-out)', () => { + const app = new App({ postCliContext: { 'aws:cdk:bundling-stacks': ['**'] } }); + const stack = new Stack(app, 'OptOutProbe', { + env: { account: '123456789012', region: 'us-east-1' }, + }); + expect(stack.bundlingRequired).toBe(true); + }); + + it('does NOT re-enable bundling via constructor context (env var clobbers it)', () => { + const app = new App({ context: { 'aws:cdk:bundling-stacks': ['**'] } }); + const stack = new Stack(app, 'ContextOptOutProbe', { + env: { account: '123456789012', region: 'us-east-1' }, + }); + expect(stack.bundlingRequired).toBe(false); + }); +}); From 3f6ff1ff4f29e624497a683c3e57e7221d2ad45d Mon Sep 17 00:00:00 2001 From: scottschreckengaust <345885+scottschreckengaust@users.noreply.github.com> Date: Wed, 17 Jun 2026 22:56:43 +0000 Subject: [PATCH 8/8] docs(test): reframe bundling as overhead-plus-smoke-test, not "pure overhead" (#366) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #371 review finding 3: "pure overhead" undersold the tradeoff. esbuild bundling also validates that every Lambda `entry` resolves and the handler compiles, so disabling it in unit tests means a broken entry path or handler TS error stops surfacing at `Template.fromStack()` and only fails at real synth/deploy or in the `agent/` build. The tradeoff is still acceptable — real synth/deploy bundles, and handler code has its own compile/test under `agent/` — but the wording now names what unit tests cede. Updated in both places the phrase lived: the disable-bundling.ts header and AGENTS.md. Co-Authored-By: Claude Opus 4.8 --- AGENTS.md | 2 +- cdk/test/setup/disable-bundling.ts | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b2ba5f9f..e11a9acf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,7 @@ Handler entry tests: `cdk/test/handlers/orchestrate-task.test.ts`, `create-task. - 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. - Changing **`cdk/.../types.ts`** without updating **`cli/src/types.ts`** — CLI and API drift. - 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)). -- **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. This does not stop tests from synthesizing — `Template.fromStack()` still runs a full synth; it only skips the esbuild step. **Do not re-enable bundling** in a test unless you are specifically asserting on the bundled-asset output (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 #366. +- **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 for those assertions it is overhead — plus a smoke-test of Lambda `entry` resolution and handler compilation that unit tests intentionally cede to the `agent/` build (a broken entry path or handler TS error stops surfacing at `Template.fromStack()` and instead fails at real synth/deploy or in the `agent/` build). 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. This does not stop tests from synthesizing — `Template.fromStack()` still runs a full synth; it only skips the esbuild step. **Do not re-enable bundling** in a test unless you are specifically asserting on the bundled-asset output (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 #366. - **`MISE_EXPERIMENTAL=1`** — required for namespaced tasks like **`mise //cdk:build`** (see [CONTRIBUTING.md](./CONTRIBUTING.md)). - **`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. - **`prek install`** fails if Git **`core.hooksPath`** is set — another hook manager owns hooks; see [CONTRIBUTING.md](./CONTRIBUTING.md). diff --git a/cdk/test/setup/disable-bundling.ts b/cdk/test/setup/disable-bundling.ts index 9abf54bd..9ec21a9f 100644 --- a/cdk/test/setup/disable-bundling.ts +++ b/cdk/test/setup/disable-bundling.ts @@ -22,8 +22,10 @@ // `Template.fromStack()` triggers a full CDK synth, which bundles every // NodejsFunction via esbuild — ~28s for the AgentStack (413 resources). Unit // tests assert on CloudFormation structure/properties, not bundled Lambda code, -// so bundling is pure overhead here: skipping it cuts a single synth ~15× -// (~28.7s -> ~1.9s). See #366. +// so for those assertions bundling is overhead — plus a smoke-test of Lambda +// `entry` resolution and handler compilation that unit tests intentionally cede +// to the `agent/` build (real synth/deploy still bundles). Skipping it cuts a +// single synth ~15× (~28.7s -> ~1.9s). See #366. // // `aws:cdk:bundling-stacks: []` tells the CLI/synth to bundle no stacks. CDK // reads CDK_CONTEXT_JSON when an `App` is constructed, so a bare `new App()`