fix(templates): make Tier 3 deliverable — CDK entrypoint, Helm chart survives render - #300
Conversation
…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.
Independent review — verdict: fix-needed (one real defect)Reviewed the full diff and re-ran the gate myself in a clean worktree off Verified: the two defects are real, and the gate is a real gateNegative 1 — render throws on the pre-fix tree. Copied the three new test files ( Negative 2 — the entrypoint. Same base worktree with only the Helm/engine half of the fix applied ( And the Positive — on this branch, No existing gate was weakened: The defect:
|
`**/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.
|
Reviewer critical addressed in 20740ce — the third ignore layer. Fix. Reproduced first on the unmodified branch head ( Verified the negation is not over-broad: seeded Also took the two adjacent notes.
The suggested gate is in, as Runs. Local No gate weakened — the diff is three files, additions only. Not addressed: |
Re-review (round 2) — verdict: approveThe round-1 critical is genuinely closed. I reproduced the negative and the positive myself in a fresh worktree at The
|
| 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..20740ceboverV1TemplateCompileTests.cs,V1TemplateHarness.csandFixtures/is empty. Every change undersrc/StackAlchemist.Engine.Testsis additions only (641 insertions, 0 deletions). - Guards still hard-fail on CI.
IntegrationToolchain.Available(..., requiredOnCi: true)isAssert.False(IsContinuousIntegration, …), andFindRepositoryRoot's null branch is too. GitHub Actions always setsCI=true, so the Backend job being green is proof docker/npm/Helm were found and the gates ran rather than early-returned. app.fullnamerename is sound.Chart.yamlkeepsname: {{ProjectNameKebab}}and stays on the Handlebars path, so.Chart.Namecarries the project name into every resource. No shipped file underinfra/helm/templates/retains one of our tokens (grepped).Tier3-Infrastructure/infra/helmis still the onlyhelmdirectory in the whole templates tree, soIsForeignTemplatecannot reach anything else today.- Buyer-facing ignores land in the right tree.
AppendTier3InfrastructureFilesdoesfinalFiles[path] = contentinto the same dictionary as the V1 set, so the overlay really does merge at the root whereV1-DotNet-NextJs/.gitignoreand.dockerignoresit. 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.
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
MockFileSystemseeded with synthetic inline files.GenerationPipelineTestswrites its owninfra/cdk/bin/app.tsbefore 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/swallowedsrc/StackAlchemist.Templates/Tier3-Infrastructure/infra/cdk/bin/. The file existed untracked on one machine, which is why it looked fine there.cdk.jsonwas missing outright, so the CLI had no--appeither.2. Rendering the Tier-3 set threw
Helm charts are Go
text/templatefiles 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:That exception leaves
AppendTier3InfrastructureFiles(GenerationOrchestratorL321-322) and lands in the orchestrator's catch, so every Tier-3 generation endedFailedwith no zip.The fix
Engine —
ForeignTemplatePathsexempts 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.yamlandChart.yamldeliberately 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}}.fullnameto a staticapp.fullname, and a test asserts no shipped file carries one of our tokens. Resource names still carry the project name, via.Chart.Namefrom the renderedChart.yaml.Template — added
bin/app.ts,cdk.json, and apackage-lock.jsonso the documentednpm ciworks. Scopedtsconfig.jsontobin/+lib/: itsexclude: ["cdk.out"]had replaced TypeScript's defaults, sonode_moduleswas being pulled into the compile.Runbook —
npm run synthas documented also failed: the stack raisesMissing CDK context value: imageUriby design rather than synthesising a template pointing at a placeholder image.DEPLOYMENT.mdnow passes the context and documents that synth needs no AWS credentials.The
.gitignorewe ship had the samebin/trap, so a Tier-3 buyer's own firstgit add .would have dropped the entrypoint again. Negated there too, pluscdk.out/and CDK's emitted JS.The gate
Tier3InfrastructureCompileTestsrenders the real V1 tree and appends the real Tier-3 set through the sameLoadTemplate/Rendercalls 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 parsescdk.out/InvoiceHubStack.template.jsonand asserts it containsAWS::ECS::Service,AWS::RDS::DBInstanceandAWS::ElasticLoadBalancingV2::LoadBalancer. Exit 0 alone would also describe a CLI that printed help and stopped.helm linton 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
validateneeds aninitthat 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:
The Helm defect's red is the
HandlebarsCompilerExceptionabove — 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:The one skip is the pre-existing
CloudflareR2UploadServiceTestsreal-credentials test.Reviewer notes
IntegrationToolchainduplicates the toolchain-probe and process-runner helpers currently private toV1TemplateCompileTests. 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.aws-cdk-lib2.265.0 / CLI 2.1136.0. It will need the same periodic refresh asV1-DotNet-NextJs/nextjs/package-lock.json.cdk.jsonuses the canonicalnpx ts-node --prefer-ts-exts bin/app.ts, verified against thistsconfig(NodeNext) rather than assumed.