Skip to content

fix(templates): make Tier 3 deliverable — CDK entrypoint, Helm chart survives render - #300

Merged
stevenfackley merged 2 commits into
mainfrom
fix/tier3-cdk-entrypoint
Aug 15, 2026
Merged

fix(templates): make Tier 3 deliverable — CDK entrypoint, Helm chart survives render#300
stevenfackley merged 2 commits into
mainfrom
fix/tier3-cdk-entrypoint

Conversation

@stevenfackley

Copy link
Copy Markdown
Owner

What was actually wrong

The lane brief was "Tier3 ships a CDK app with no entrypoint." That is true, and it is the smaller half. Tier 3 ($999) could not deliver an archive at all.

Two defects, both total, both invisible to the suite for the same reason: every existing test of the Tier-3 overlay ran against a MockFileSystem seeded with synthetic inline files. GenerationPipelineTests writes its own infra/cdk/bin/app.ts before asserting one comes out. So the suite proved the orchestrator copies files it was handed, and proved nothing about the files we ship.

1. The CDK entrypoint was never in the repository

The root .gitignore's .NET rule [Bb]in/ swallowed src/StackAlchemist.Templates/Tier3-Infrastructure/infra/cdk/bin/. The file existed untracked on one machine, which is why it looked fine there. cdk.json was missing outright, so the CLI had no --app either.

$ git check-ignore -v src/.../infra/cdk/bin/app.ts
.gitignore:4:[Bb]in/    src/.../infra/cdk/bin/app.ts

2. Rendering the Tier-3 set threw

Helm charts are Go text/template files that share Handlebars' {{ }} delimiters, and {{- toYaml .Values.resources | nindent 12 }} parses as a Handlebars block-params declaration. Against the real tree, on the current default branch:

HandlebarsDotNet.HandlebarsCompilerException : blockParams definition has incorrect syntax
  at StackAlchemist.Engine.Services.TemplateProvider.Render
  at Tier3TemplateHarness.RenderTo

That exception leaves AppendTier3InfrastructureFiles (GenerationOrchestrator L321-322) and lands in the orchestrator's catch, so every Tier-3 generation ended Failed with no zip.

The fix

EngineForeignTemplatePaths exempts Helm chart templates from the Handlebars pass so they reach the buyer byte-for-byte. Only the file body is exempt; paths are still rendered. values.yaml and Chart.yaml deliberately stay on the Handlebars path (plain YAML, no Go syntax, and they carry the project name).

The exemption creates its own hazard — a token left inside an exempt file ships literally — so the chart helpers are renamed from {{ProjectNameKebab}}.fullname to a static app.fullname, and a test asserts no shipped file carries one of our tokens. Resource names still carry the project name, via .Chart.Name from the rendered Chart.yaml.

Template — added bin/app.ts, cdk.json, and a package-lock.json so the documented npm ci works. Scoped tsconfig.json to bin/+lib/: its exclude: ["cdk.out"] had replaced TypeScript's defaults, so node_modules was being pulled into the compile.

Runbooknpm run synth as documented also failed: the stack raises Missing CDK context value: imageUri by design rather than synthesising a template pointing at a placeholder image. DEPLOYMENT.md now passes the context and documents that synth needs no AWS credentials.

The .gitignore we ship had the same bin/ trap, so a Tier-3 buyer's own first git add . would have dropped the entrypoint again. Negated there too, plus cdk.out/ and CDK's emitted JS.

The gate

Tier3InfrastructureCompileTests renders the real V1 tree and appends the real Tier-3 set through the same LoadTemplate/Render calls the orchestrator makes, then runs the customer's own commands:

  • npm ci + npm run synth -- --context imageUri=…, with AWS credentials stripped from the child environment, then parses cdk.out/InvoiceHubStack.template.json and asserts it contains AWS::ECS::Service, AWS::RDS::DBInstance and AWS::ElasticLoadBalancingV2::LoadBalancer. Exit 0 alone would also describe a CLI that printed help and stopped.
  • helm lint on the chart.
  • terraform init -backend=false + terraform validate.

Toolchain guards match the runner image: npm and Helm hard-fail on CI (ubuntu-24.04 ships Node 22 and Helm 3.21). Terraform skips cleanly even on CI — the image no longer ships Terraform since HashiCorp's licence change, and validate needs an init that pulls the ~600 MB AWS provider. It runs on a dev machine, where it is nearly free. That tradeoff is documented in the test rather than hidden.

Red before green

Entrypoint removed, everything else fixed:

Failed Tier3InfrastructureCompileTests.Render_ProducesACompleteCdkAppAndAnIntactHelmChart
  Expected files {…} to contain "infra/cdk/bin/app.ts"

Failed Tier3InfrastructureCompileTests.CdkApp_InstallsAndSynthesisesTheStack [28 s]
  Expected synthExit to be 0 …
  Error: Cannot find module './app.ts'

The Helm defect's red is the HandlebarsCompilerException above — on the current default branch the render throws before any assertion can run.

Verification

Full Engine suite, CI=true (so guards hard-fail rather than skip), Docker up:

Passed!  - Failed: 0, Passed: 420, Skipped: 1, Total: 421, Duration: 4 m 44 s

The one skip is the pre-existing CloudflareR2UploadServiceTests real-credentials test.

Reviewer notes

  • IntegrationToolchain duplicates the toolchain-probe and process-runner helpers currently private to V1TemplateCompileTests. Deliberate: folding them together means editing the repo's most safety-critical test file, and that belongs in its own change, not a Tier-3 fix. Left a comment saying so.
  • The lockfile pins aws-cdk-lib 2.265.0 / CLI 2.1136.0. It will need the same periodic refresh as V1-DotNet-NextJs/nextjs/package-lock.json.
  • cdk.json uses the canonical npx ts-node --prefer-ts-exts bin/app.ts, verified against this tsconfig (NodeNext) rather than assumed.

…survives render

Tier 3 ($999) could not deliver. Two total defects, both invisible to the suite
because every existing test of the infra overlay ran against a MockFileSystem
that seeded its own copies of the files under test.

1. `infra/cdk/bin/app.ts` was never in the repository. The root .gitignore's
   .NET `[Bb]in/` rule swallowed the directory, so the CDK entrypoint existed
   only as an untracked file on one machine. Every clone shipped a CDK app the
   CLI cannot find an app in. `cdk.json` was missing outright.

2. Rendering the set threw. Helm charts are Go text/template files sharing
   Handlebars' `{{ }}` delimiters, and `{{- toYaml .Values.x | nindent 8 }}`
   parses as a Handlebars block-params declaration:
   HandlebarsCompilerException out of AppendTier3InfrastructureFiles into the
   orchestrator's catch. No archive was produced at all.

Engine: ForeignTemplatePaths exempts helm chart templates from the Handlebars
pass so they reach the buyer byte-for-byte; chart helpers are renamed to a
static `app.*` prefix so nothing inside them needs substituting.

Template: add bin/app.ts + cdk.json + a package-lock so `npm ci` works, scope
tsconfig to bin/lib (it was pulling node_modules into the compile), and correct
the runbook — `npm run synth` as documented also failed, since the stack
requires an `imageUri` context value.

Gate: Tier3InfrastructureCompileTests renders the real tree the way tier-3
generation does, then runs `npm ci` + `npm run synth` credential-free and
asserts the synthesised CloudFormation carries the ECS/RDS/ALB stack, plus
`helm lint` and (locally only) `terraform validate`. npm and Helm hard-fail on
CI; Terraform skips cleanly because ubuntu-latest no longer ships it.

Also fixes the same `bin/` trap in the .gitignore we SHIP, which would have
dropped the entrypoint from the buyer's own first commit.
@stevenfackley

stevenfackley commented Aug 15, 2026

Copy link
Copy Markdown
Owner Author

Independent review — verdict: fix-needed (one real defect)

Reviewed the full diff and re-ran the gate myself in a clean worktree off origin/main (54bd0854), CI=true. The diagnosis in the description is correct on both counts and I reproduced both negatives. But the entrypoint fix does not reach the artifact that actually serves production, so the headline defect — "Tier 3 ships a CDK app with no entrypoint" — is still live after this PR merges.


Verified: the two defects are real, and the gate is a real gate

Negative 1 — render throws on the pre-fix tree. Copied the three new test files (IntegrationToolchain.cs, Tier3TemplateHarness.cs, Tier3InfrastructureCompileTests.cs) onto an unmodified 54bd0854 worktree and ran them:

Failed  Tier3InfrastructureCompileTests.CdkApp_InstallsAndSynthesisesTheStack
Failed  Tier3InfrastructureCompileTests.RenderedTree_CarriesNoneOfOurUnsubstitutedTokens
  HandlebarsDotNet.HandlebarsCompilerException : blockParams definition has incorrect syntax
   at StackAlchemist.Engine.Services.TemplateProvider.RenderString(...) TemplateProvider.cs:line 117
   at StackAlchemist.Engine.Services.TemplateProvider.Render(...) TemplateProvider.cs:line 86
Total tests: 5   Failed: 5

Negative 2 — the entrypoint. Same base worktree with only the Helm/engine half of the fix applied (ForeignTemplatePaths.cs, TemplateProvider.cs, infra/helm/templates/*, package.json/package-lock.json/tsconfig.json) and bin/app.ts + cdk.json deliberately left out:

Failed  Render_ProducesACompleteCdkAppAndAnIntactHelmChart
  Expected files {...} to contain "infra/cdk/bin/app.ts"

Failed  CdkApp_InstallsAndSynthesisesTheStack [35 s]
  Expected synthExit to be 0 ...
  --app is required either in command-line, in cdk.json or in ~/.cdk.json

And the .gitignore claim, on both trees:

$ git check-ignore -v src/StackAlchemist.Templates/Tier3-Infrastructure/infra/cdk/bin/app.ts
54bd0854:  .gitignore:4:[Bb]in/    src/.../infra/cdk/bin/app.ts   (exit 0)
this PR:   (exit 1 — no longer ignored, and `git ls-files` shows bin/app.ts and cdk.json tracked)

Positive — on this branch, CI=true, real toolchains present locally (npm 12.0.2 / Helm 4.2.3 / Terraform 1.15.8):

Passed  Render_ProducesACompleteCdkAppAndAnIntactHelmChart [412 ms]
Passed  HelmChart_Lints [567 ms]
Passed  RenderedTree_CarriesNoneOfOurUnsubstitutedTokens [462 ms]
Passed  TerraformBaseline_Validates [31 s]
Passed  CdkApp_InstallsAndSynthesisesTheStack [1 m]        <- real npm ci + real cdk synth
Passed  ForeignTemplatePathsTests (12 cases)
Total tests: 17   Passed: 17

No existing gate was weakened: V1TemplateCompileTests.cs, V1TemplateHarness.cs and Fixtures/ are byte-identical to main (empty git diff --stat). IsForeignTemplate is scoped tightly — Tier3-Infrastructure/infra/helm is the only helm directory in the whole templates tree — and values.yaml / Chart.yaml correctly stay on the Handlebars path, so .Chart.Name really does carry invoice-hub into every resource name. The app.* helper rename is complete; no shipped chart file carries one of our tokens.


The defect: .dockerignore drops bin/ from the engine image

.gitignore (repo) and V1-DotNet-NextJs/.gitignore (buyer) were both fixed. .dockerignore — the third ignore layer, and the one that decides what production actually runs — was not.

.dockerignore:15 is **/bin/. Dockerfile:103 in the engine stage is COPY src/StackAlchemist.Templates/ ./StackAlchemist.Templates/, and TemplateProvider.LoadTemplate enumerates that directory at runtime. docker-compose.prod.yml's sa-engine builds context: ., target: engine, with no volume mount for templates — so the image copy is the only copy.

Probed the actual build context of this branch (COPY src/StackAlchemist.Templates/ /t/ into an alpine, context = this worktree):

$ docker run --rm sa-ctxprobe sh -c "ls -la /t/Tier3-Infrastructure/infra/cdk/; find /t -name app.ts"
drwxr-xr-x  lib
-rwxr-xr-x  cdk.json
-rwxr-xr-x  package-lock.json
-rwxr-xr-x  package.json
-rwxr-xr-x  tsconfig.json
        <- no bin/, and `find` returns nothing

lib/, cdk.json and package-lock.json all make it. bin/ alone is stripped. So on the deployed engine a Tier-3 buyer still downloads a CDK app whose cdk.json names a file that is not in the zip — --app is required / Cannot find module './app.ts', exactly the failure this PR set out to kill. The new gate can't see it because it renders from the working tree via V1TemplateHarness.ResolveTemplatesRoot(), and Docker Build Validation (engine) only proves the image builds.

Note BuildResiduePaths already anticipated this trap in the engine (// "bin" alone is NOT excludable: a CDK app's entrypoint lives at infra/cdk/bin/app.ts) — .dockerignore is the one place the same reasoning wasn't applied.

Fix, verified working (edited .dockerignore, rebuilt the probe, bin/app.ts present in the image; reverted after):

 **/bin/
+# ...except the Tier-3 CDK entrypoint — bin/ is a SOURCE directory in the CDK
+# convention, and the engine image is where the shipped template tree comes from.
+!src/StackAlchemist.Templates/*/infra/cdk/bin/
+!src/StackAlchemist.Templates/*/infra/cdk/bin/**
$ docker run --rm sa-ctxprobe2 sh -c "ls /t/Tier3-Infrastructure/infra/cdk/bin"
app.ts

Worth also making the gate able to catch this class, since the whole point of the PR is that an ignore rule was invisible — e.g. assert that every relative path LoadTemplate returns for Tier3-Infrastructure survives the .dockerignore patterns, or run one render inside the built engine image. Otherwise the next **/… rule silently re-opens it.


Minor, non-blocking

  • src/StackAlchemist.Templates/V1-DotNet-NextJs/.dockerignore has the same **/bin rule. No functional impact today (the buyer's Dockerfile builds dotnet/ and nextjs/, never infra/cdk/), but it's the identical trap one directory over and is one line to close while you're in here.
  • npm run build (tsc) is not exercised by the gate — only ts-node via cdk synth. The rescoped include makes that path plausible but unproven.
  • Agreed on leaving the IntegrationToolchain / V1TemplateCompileTests helper de-duplication to its own change.

Everything except the .dockerignore line is good work — the harness that renders through the real LoadTemplate/Render instead of a MockFileSystem is the right correction to the thing that hid both bugs.


Full Engine suite independently confirmed on this branch, CI=true, Docker up: Passed! - Failed: 0, Passed: 420, Skipped: 1, Total: 421, Duration: 3 m 57 s — matches the description. CI on main @ 54bd0854 is red on E2E Integration (Playwright, Main/Nightly), which is pre-existing and skipped on PRs.

`**/bin/` in .dockerignore stripped src/StackAlchemist.Templates/**/bin/ out of
the build context, so the engine image — the only source of templates in
production — shipped the Tier 3 CDK app without bin/app.ts, the entrypoint
cdk.json names. The repository .gitignore and the buyer-facing .gitignore were
both corrected earlier on this branch; this third ignore layer was not, so the
headline defect survived unchanged into every image built from it.

Negate the rule for the template sets only, and keep bin/<Configuration>/
ignored — the same distinction BuildResiduePaths already draws for the runtime
loader, so the two layers agree on what counts as build output. The buyer-facing
.dockerignore carries the identical rule and the Tier 3 overlay merges into that
same tree root, so it gets the same narrow negation.

Add DockerBuildContext_KeepsEveryFileTheEngineLoads, which exports the real
build context and asserts every TemplateProvider.LoadTemplate path survives it.
Asserting on the one path would only re-fight this defect; this way the next
`**/…` rule that swallows a template file fails in CI instead of in a paid
customer archive.
@stevenfackley

Copy link
Copy Markdown
Owner Author

Reviewer critical addressed in 20740ce — the third ignore layer.

Fix. .dockerignore:15 **/bin/ stripped src/StackAlchemist.Templates/**/bin/ from the engine build context. Negated for the template sets only, keeping bin/<Configuration>/ ignored — the same distinction BuildResiduePaths already draws for the runtime loader, so the two layers agree on what counts as build output.

Reproduced first on the unmodified branch head (docker build of the real context, FROM scratch + --output type=local): exactly one tracked template file was missing, Tier3-Infrastructure/infra/cdk/bin/app.ts. After the fix, the real engine image (docker build --target engine) contains it:

--- app.ts in image ---
/app/StackAlchemist.Templates/Tier3-Infrastructure/infra/cdk/bin/app.ts

Verified the negation is not over-broad: seeded dotnet/bin/Debug/net10.0/residue.dll, dotnet/bin/Release/residue2.dll, dotnet/obj/project.assets.json and a bin/keepme-source.ts — only keepme-source.ts reached the context.

Also took the two adjacent notes.

V1-DotNet-NextJs/.dockerignore got the same narrow negation. Not just symmetry: Tier3TemplateHarness shows the Tier-3 overlay merges into the V1 tree at one root, so that shipped .dockerignore sits beside the buyer's own infra/cdk/bin/app.ts. Verified on a synthetic Tier-3 tree — infra/cdk/bin/app.ts survives, dotnet/bin/Debug/** and dotnet/obj/** do not.

The suggested gate is in, as DockerBuildContext_KeepsEveryFileTheEngineLoads. It exports the real build context and asserts every TemplateProvider.LoadTemplate path survives it, across every template set — asserting the one path would only re-fight this defect. Negative control, fix reverted:

Expected missing to be empty ... but found at least one item
{"Tier3-Infrastructure/infra/cdk/bin/app.ts"}
Failed! - Failed: 1, Passed: 0, Total: 1, Duration: 1 s

Runs. Local CI=true, real toolchains: Tier3 + ForeignTemplatePaths Passed! - Failed: 0, Passed: 18 (was 17). Full Engine suite Failed: 0, Passed: 421, Skipped: 1, Total: 422 — the skip is the pre-existing CloudflareR2 real-credentials test. CI Backend job matches exactly (422/421/1, 3m37s) and the only SKIP in the log is that same test, so the new docker gate really ran on the runner. All checks green.

No gate weakened — the diff is three files, additions only.

Not addressed: npm run build (tsc) is still not exercised, only ts-node via cdk synth. Left as-is; it is a separate change to the Tier-3 gate's scope.

@stevenfackley

Copy link
Copy Markdown
Owner Author

Re-review (round 2) — verdict: approve

The round-1 critical is genuinely closed. I reproduced the negative and the positive myself in a fresh worktree at 20740ceb, against the real build context — not by reading the diff.


The .dockerignore defect is fixed, proved both directions

Negative control — worktree at 70dca1f0 (everything else on this branch already fixed, .dockerignore untouched), real context exported with FROM scratch + --output type=local:

$ ls ctx-pre/Tier3-Infrastructure/infra/cdk/
cdk.json  lib/  package.json  package-lock.json  tsconfig.json
$ find ctx-pre -name app.ts
        <- nothing

Positive — same probe at 20740ceb:

$ ls ctx-fixed/Tier3-Infrastructure/infra/cdk/
bin/  cdk.json  lib/  package.json  package-lock.json  tsconfig.json
$ find ctx-fixed -name app.ts
ctx-fixed/Tier3-Infrastructure/infra/cdk/bin/app.ts

The new gate really gates. Reverted only .dockerignore to its 70dca1f0 content in an otherwise-fixed tree and ran the new test alone, CI=true:

Failed StackAlchemist.Engine.Tests.Integration.Tier3InfrastructureCompileTests.DockerBuildContext_KeepsEveryFileTheEngineLoads [2 s]
  Expected missing to be empty ... but found at least one item
  {"Tier3-Infrastructure/infra/cdk/bin/app.ts"}
Failed! - Failed: 1, Passed: 0, Total: 1

Restored, re-ran, green. Asserting the invariant (every LoadTemplate path survives the context) rather than the one path was the right call, and checkedPaths.Should().BeGreaterThan(0) closes the vacuous-pass hole.

The negation is not over-broad where it matters. Seeded residue into the template tree and re-exported:

seeded in context
V1-DotNet-NextJs/dotnet/bin/Debug/net10.0/residue.dll no
V1-DotNet-NextJs/dotnet/bin/Release/residue2.dll no
V1-DotNet-NextJs/dotnet/obj/project.assets.json no
Tier3-Infrastructure/infra/cdk/bin/keepme-source.ts yes (intended)

Severity was right. Re-confirmed the chain independently: Dockerfile:103 COPY src/StackAlchemist.Templates/ ./StackAlchemist.Templates/, and docker-compose.prod.yml's sa-engine is context: . / target: engine with no volumes: and no Templates__Root anywhere in any compose file or workflow. The image is the only copy of the templates in production, so this was the layer that decided what buyers received.


The rest of the PR re-checked

  • No gate weakened. git diff 54bd0854..20740ceb over V1TemplateCompileTests.cs, V1TemplateHarness.cs and Fixtures/ is empty. Every change under src/StackAlchemist.Engine.Tests is additions only (641 insertions, 0 deletions).
  • Guards still hard-fail on CI. IntegrationToolchain.Available(..., requiredOnCi: true) is Assert.False(IsContinuousIntegration, …), and FindRepositoryRoot's null branch is too. GitHub Actions always sets CI=true, so the Backend job being green is proof docker/npm/Helm were found and the gates ran rather than early-returned.
  • app.fullname rename is sound. Chart.yaml keeps name: {{ProjectNameKebab}} and stays on the Handlebars path, so .Chart.Name carries the project name into every resource. No shipped file under infra/helm/templates/ retains one of our tokens (grepped). Tier3-Infrastructure/infra/helm is still the only helm directory in the whole templates tree, so IsForeignTemplate cannot reach anything else today.
  • Buyer-facing ignores land in the right tree. AppendTier3InfrastructureFiles does finalFiles[path] = content into the same dictionary as the V1 set, so the overlay really does merge at the root where V1-DotNet-NextJs/.gitignore and .dockerignore sit. The negations are correctly placed.

Runs (mine, CI=true, Docker up, real toolchains):

Tier3InfrastructureCompileTests + ForeignTemplatePathsTests
  Passed! - Failed: 0, Passed: 18, Skipped: 0, Total: 18, Duration: 1 m 38 s

Full Engine suite
  Passed! - Failed: 0, Passed: 421, Skipped: 1, Total: 422, Duration: 5 m 27 s

The one skip is the pre-existing CloudflareR2UploadServiceTests real-credentials test. Matches the CI Backend job exactly (421/1/422, 3 m 37 s), whose log shows that same test as its only SKIP.


The open note from round 1, resolved

npm run build is still not exercised by the gate — so I ran it. Rendered the CDK set by hand (InvoiceHub / invoice-hub), then:

$ npm ci      -> added 26 packages, exit 0
$ npm run build  (tsc) -> exit 0
   emits bin/app.js, bin/app.d.ts, lib/invoice-hub-stack.js, lib/invoice-hub-stack.d.ts

So the rescoped include holds for tsc, not just ts-node, and the emit-in-place output is exactly what the new buyer-facing infra/cdk/**/*.js + **/*.d.ts rules cover. Not a defect; leaving it unexercised is a fair scope call.


Minor, non-blocking

The .dockerignore negation is wider than the .gitignore one it mirrors. .gitignore re-includes only src/StackAlchemist.Templates/*/infra/cdk/bin/; .dockerignore re-includes **/bin/** and then subtracts only bin/Debug/ and bin/Release/. Two shapes slip through:

V1-DotNet-NextJs/dotnet/bin/x64/Debug/residue3.dll   -> reaches the context
V1-DotNet-NextJs/dotnet/bin/loose-at-bin-root.dll    -> reaches the context

Zero impact on CI or prod — those build from a fresh checkout where no such residue exists, and nothing in the repo builds a template project in place. It only bites someone running docker build locally after building a template by hand, and the cost is a stray file in an image, not a broken archive. Tightening to the .gitignore's narrower */infra/cdk/bin/ shape would make the two layers say the same thing, which is the stated goal of the comment above the rule. Worth a follow-up line, not a blocker.


Approving. The defect I raised is fixed at the layer that mattered, the fix is proved by a negative I reproduced independently, and the new gate fails on the pre-fix tree and passes on the fix. Leaving the PR open for the merge decision.

@stevenfackley
stevenfackley merged commit 9f931e4 into main Aug 15, 2026
17 checks passed
@stevenfackley
stevenfackley deleted the fix/tier3-cdk-entrypoint branch August 15, 2026 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant