P1 [CRF-4] The base embeds catalog-named module blocks, so selecting the same module in the wizard produces a duplicate module call and a template that fails to build (Hisoka P1, Meruem P1, Pariston P1, Luffy P1, Mafuuu P1, Bisky P2).

The base declares module "code-server", "cursor", "zed", "windsurf", and "git-clone" (lines 273, 282, 304, 313, 324), each name identical to a catalog module ID under coderd/templatebuilder/modules/. Compose renders the base into main.tf and selected modules into modules.tf in the same directory; validateModules (compose.go:203-231) only dedupes within the requested list and never inspects what the base declares. Multiple reviewers reproduced it by running Compose(BaseTemplateID: "quickstart", Modules: [{ID: "code-server"}]): both files emit module "code-server", and Terraform rejects the duplicate.

Verified in a worktree that quickstart is the only base embedding catalog-named modules (siblings declare at most azure_region/gcp_region), so this PR introduces the hazard. The recommended, first-position base breaks on the wizard's core action (add an IDE). This is open question #1, but with no ticket it is an open finding, not a deferral.

Two durable directions: strip the modules so the base is pure infra (option a), or add a base/module collision guard, e.g. declare included_modules in base.json and seed validateModules' seen-map from it so the disjoint-namespace invariant is enforced for every base. A human decides; the reviewer cannot accept this gap as permanent.

🤖

Confirmed, and not deferring silently. This P1 — together with CRF-5, CRF-6, and CRF-10 — stems from one root: this is a batteries-included template sitting in the base slot, where the builder's model is base (infra) + modules composed on top.

The resolution is the scope decision tracked as open question #1: either (a) strip the modules/presets so the base is pure infra (which dissolves CRF-4/5/6/10 together), or (b) add a base/module collision guard — e.g. declare included_modules in base.json and seed validateModules' seen-map from it. Option (b) changes shared compose logic and is an architectural call.

I won't preempt that decision or ship a first-position base that breaks on the wizard's core action (adding an IDE), so I've escalated it to the scope owner. Once it's decided I'll implement the chosen option and re-request review. The PR stays draft / do-not-merge until then. (The independent CRF-9 total-order fix landed in 76aabd3.)

P2 [CRF-5] The JetBrains IDE selector, preset, and README all promise installation, but the module that would do it is commented out, so the selection is a silent no-op (Leorio P2, Hisoka Note).

The ides parameter offers "JetBrains IDEs"; picking it renders a jetbrains_ides parameter described as "Select the JetBrains IDEs to install", the Backend (Go) preset ships jetbrains, and README.md:68 says JetBrains IDEs are "Filtered by your language selection ... and installed." But the module "jetbrains" that consumes the selection is commented out (its source at line 297 is disabled), and local.jetbrains_selected has no other consumer. The user picks GoLand, the build goes green, and nothing installs, with no error or warning.

Ported verbatim from the example, but it ships as the recommended first base now. Either restore the module, remove the JetBrains option from ides and the README bullet, or (floor, not goal) change the description to state the selection has no effect.

🤖

Subsumed by CRF-4 / open question #1. If the scope decision is (a) strip to infra, the JetBrains selector, the Backend (Go) preset, and the README bullet all go with it. If IDEs stay, I'll fix the dead path explicitly — either restore the jetbrains module or remove the option and the README claim. Holding rather than editing content that may be removed wholesale.

P2 [CRF-6] The base's bundled module sources hardcode registry.coder.com, ignoring the deployment's configured TemplateBuilder.RegistryURL that every builder-composed module honors (Hisoka).

Builder-composed modules are templated with {{ .RegistryBase }}, fed from api.DeploymentValues.TemplateBuilder.RegistryURL, so air-gapped or private-mirror deployments can redirect module pulls. The quickstart base carries no Go-template directives (why the golden is byte-identical), so its source = "registry.coder.com/coder/code-server/coder" lines are frozen. On a deployment with a custom RegistryURL, the base modules still reach for the public registry and terraform init fails when it is unreachable.

The runtime failure is unverified (no air-gapped sandbox), but the hardcoding and the divergence from the builder's own registry contract are confirmed in the diff. Stripping the modules (open question #1 option a) also removes this divergence.

🤖

Also subsumed by CRF-4 / open question #1. Stripping the bundled modules (option a) removes the hardcoded registry.coder.com sources entirely. If the modules stay, I'll route their sources through the builder's RegistryBase so they honor TemplateBuilder.RegistryURL like composed modules do. Holding for the scope decision.

P2 [CRF-6] Re-raising. The IDE module hardcodes were removed, but git-clone still hardcodes source = "registry.coder.com/coder/git-clone/coder" (main.tf.tmpl:157), and this is structural (Mafuuu P2, Melody P2, Knov P2, Meruem P2, Chopper P2, Luffy P2; Pariston P3; Hisoka/Zoro Note).

Verified in a worktree: BaseRenderContext (render.go:19-23) has only ContainerImage, ImageOptions, Variables, with no RegistryBase field, and renderBase (compose.go:112) is never passed req.RegistryURL. Only ModuleRenderContext carries RegistryBase (fed at compose.go:270). So the base cannot honor RegistryURL, and the same git-clone module resolves through the mirror when the wizard renders it but reaches the public registry when the base embeds it, on the same deployment. When git_repo is set, terraform init on a mirror/air-gapped deployment pulls the base git-clone from an unreachable registry and the build fails; the wizard also hides git-clone for this base (it is in included_modules), so there is no non-broken path to build-time clone there.

Context for the human decision: this base is unusable fully air-gapped regardless (image, apt, nodesource, go.dev, rustup), so the value is mirror consistency, and three shipped bases already hardcode region-module sources, so this is a base-layer class. No ticket exists, so this needs a human call: thread RegistryBase into base rendering, drop git-clone from the base and let the wizard provide it, or track it.

🤖

Scope owner's call: keep git-clone in the base and take option (c) here (accept + track), with the real fix stacked so this PR stays tight. 658b74b adds an in-code note that the base renders its module source verbatim against the public registry and does not yet honor a deployment's registry mirror. Threading RegistryBase into base rendering is tracked in DOCS-610 and lands as a PR stacked on this branch. As you noted, this base isn't fully air-gap-usable regardless, and three shipped bases already hardcode region-module sources, so we're not blocking the base PR on it.

P3 [CRF-10] The base template is a byte-for-byte copy of the already-embedded examples/templates/quickstart/main.tf, so the binary carries two identical copies with nothing to keep them aligned (Robin).

The example is embedded via //go:embed templates/quickstart and the base via //go:embed bases; diff reports all 437 lines identical. Every other base diverges from its example on purpose (trimmed to infra), so only this one is a verbatim clone. coderd/templatebuilder imports nothing from examples and no test compares the two, so editing the example (a module bump, a prerequisites edit) silently drifts the wizard's Quickstart from the shipped one. The same applies to install-languages.sh.tftpl and README.md. Resolving open question #1 by trimming to infra makes the copies legitimately different; keeping verbatim needs a test asserting the base equals the embedded example.

🤖

Same root as CRF-4 (open question #1). Trimming to infra makes the base legitimately diverge from the embedded example. If we instead keep it verbatim, I'll add a test asserting the base equals examples/templates/quickstart so the two can't silently drift. Holding for the scope decision.

P3 [CRF-10] Re-raising. Trimming main.tf.tmpl resolved the copy for that file, but the drift class survived: bases/quickstart/install-languages.sh.tftpl is byte-for-byte identical to examples/templates/quickstart/install-languages.sh.tftpl (diff reports identical), and both are consumed via templatefile(...) (Robin).

No test asserts the two stay equal, so adding a language to one silently leaves the other installing the old set, and the example and the builder-rendered workspace diverge under the same "Coder Quickstart" name. Either embed a shared source, or add a test that fails when the base file and the example file diverge.

🤖

Pushing back on coupling the files. After 658b74b the base's install-languages.sh.tftpl is no longer byte-identical to the example (the dispatch was rewritten to has_language), so that coupling is already broken. The base is intentionally its own artifact now (main.tf.tmpl was trimmed to diverge), so a test asserting base == examples/templates/quickstart would wrongly block legitimate divergence. The real drift risk (a base's rendered modules vs its manifest) is covered by the new CRF-12 invariant test. If you'd rather share a single embedded source across both surfaces, that's a larger refactor than this base PR and worth its own change.

Note [CRF-7] The container image is hardcoded (codercom/enterprise-base:ubuntu in main.tf.tmpl) while sibling bases like docker expose a container_image builder variable (Mafuuu, Luffy).

base.json declares no variables, so the flagship, first-position base is the one base a user cannot re-image without editing generated Terraform. This is the author's open question #3; noting it so the inconsistency is a conscious decision, and it is worth deciding alongside #1.

🤖

Acknowledged — this is open question #3. The image is hardcoded to stay faithful to the ported example; I'll decide alongside open question #1 whether the flagship base should expose a container_image variable like the docker base does.

Decision: accept, keep the image hardcoded. The quickstart base is the opinionated starter; the Docker base is the bring-your-own-image path, and exposing container_image here blurs that line. The baseSpec row locks quickstart to no builder variables, so adding one later is a deliberate change. Tracked in DOCS-611 to revisit on user demand.

Note [CRF-8] Quickstart-first ordering is a hardcoded ID pin inside the comparator rather than data (Pariston, Meruem).

The comparator sorts every base by Name but special-cases one base by ID. It works and is proportional for a single pinned base (open question #2). If a second pinned base ever appears, the comparator stops expressing intent and an order field in base.json would move the policy back to data. Recording so the decision is explicit; no change needed now.

🤖

Acknowledged — open question #2 (placement pending sign-off). The single-ID pin is intentional for now; if a second pinned base ever appears I'll move the policy into an order field in base.json rather than special-casing IDs in the comparator. No change now.

Note [CRF-3] The golden is byte-for-byte identical to the embedded main.tf.tmpl because the ported template has no Go-template directives, so the snapshot asserts source == source (Netero, Law).

Low incremental value, but not vacuous: it guards against accidental corruption of the embedded file and confirms RenderBaseTemplate handles directive-free input. This is a third copy of the same 437 lines in the tree (see CRF-10). No change required; flagged for awareness.

🤖

Acknowledged — the golden is identical to the embedded template because the port carries no Go-template directives, and it still guards against accidental corruption of the embedded file. Tied to CRF-10; I'll revisit it with the scope decision.

P3 [CRF-12] The collision guard's invariant is not enforced: included_modules is a hand-maintained duplicate of the base's actual module blocks, and nothing binds them (Hisoka, Mafuuu, Pariston, Chopper, Melody, Meruem, Zoro P3; Knov P2).

Today included_modules: ["git-clone"] matches the single module "git-clone" block. But the two live in separate files with no link, and no test cross-checks them (the only reference is BaseExcludesIncludedModules, which asserts the endpoint honors the JSON, not that the JSON describes reality). If a future edit adds a catalog-named module block and forgets the JSON line (or renames the block), validateModules stops reserving the name and the endpoint starts offering it, and CRF-4's P1 duplicate-module break returns with no failing test. There is also no check that included_modules entries are real catalog IDs, so a typo silently protects the wrong name.

The R4 fix closed the git-clone instance; this closes the class. Mechanical fix (AGENTS.md prefers this): a test that renders each base, scans for module "<id>" labels that are catalog IDs, and asserts the set equals that base's included_modules, or derive the set from the rendered base (the package already parses base HCL in ExtractAgentResourceName) and delete the field.

🤖

Enforced in 657abbb. Added ExtractModuleNames and TestBaseIncludedModulesMatchRendered: it renders each base, keeps the module "<id>" labels that are catalog IDs, and asserts that set equals the base's included_modules. A base that adds a catalog-named module block without the manifest line (or lists a phantom/typo'd one) now fails. Verified empirically: it fails when I temporarily emptied quickstart's included_modules, and passes once restored.

P2 [CRF-13] The README promises languages are "installed on first start and cached for subsequent starts," but nothing caches them (Leorio P2, Meruem P2).

The only persistent storage is the home_volume at /home/coder; the container is recreated from codercom/enterprise-base:ubuntu on every start. install-languages.sh.tftpl installs Node/Java/C++ via apt into /usr and Go into /usr/local/go, none of which persist, and its command -v idempotency guard fails after each rebuild, so every start reinstalls from the network (only Rust's ~/.cargo persists). Because the script is run_on_start = true with start_blocks_login = true, each restart blocks login on a full reinstall, and on mirror/air-gapped deployments (see CRF-6) that reinstall fails and login never unblocks.

Fix the doc to state that toolchains reinstall on each start, or change the design to install into the persistent volume / a prebuilt image. The current sentence is a false promise on the recommended starter.

🤖

Fixed in 658b74b. The README now states the toolchains reinstall on every start (most install into the ephemeral container; only Rust's ~/.cargo persists) and that the start script blocks login until it finishes. Persisting toolchains / a prebuilt image is a separate design change, out of scope for this base PR.

P3 [CRF-14] Language dispatch uses an unanchored substring match, so it is correct only because the current values happen not to overlap (Meruem P3; Hisoka, Mafuuu, Knov Note).

Each block tests the comma-joined selection with echo "$LANGUAGES" | grep -q "<token>" (e.g. grep -q "go"). The six values (python, nodejs, go, rust, java, cpp) are substring-disjoint today, so it works, but any future value that contains an existing token (golang, mongo, django) would silently trigger the wrong install branch with no error. Match tokens exactly, e.g. case ",$LANGUAGES," in *,go,*) ... ;;, so the installer is not coupled to option names never overlapping.

🤖

Fixed in 658b74b using exactly the idiom you suggested: a has_language helper wrapping case ",$LANGUAGES," in *",$1,"*). Verified LANGUAGES=python,nodejs,mongodb no longer triggers the go branch, while python,go does.

P3 [CRF-15] The quickstart side of BaseExcludesIncludedModules is a vacuous negative loop that passes on an empty list (Bisky).

for _, m := range quickstartResp.Modules { require.NotEqual(t, "git-clone", m.ID, ...) } asserts nothing if quickstartResp.Modules is empty, so if the filter ever over-filters to an empty list the test still goes green and reports the guard as working. The docker half is honest (require.True(t, dockerHasGitClone)). Add require.NotEmpty(t, quickstartResp.Modules) before the loop so it proves the filter kept the other modules, matching the sibling subtest's pattern.

🤖

Fixed in 657abbb. Added require.NotEmpty(t, quickstartResp.Modules) before the loop, so the test proves the filter kept the other modules instead of passing on an empty list.

P3 [CRF-16] The base/module collision rationale is written about six times across this diff, and the comments narrate the code rather than explain it (Gon, who rated the two clearest instances P2).

The rationale appears at bases.go:47, bases.go:268, compose.go:202, compose.go:209, handler.go:87, and handler.go:131. The inline comment here narrates the sort.Slice and groupQuickstartBeforeDocker statements it precedes and repeats the placement rationale that the function's own doc (:131) already carries; the BaseIncludedModules doc (bases.go:268) restates the field doc and describes Compose's behavior. Duplicated rationale drifts apart on the next edit. Keep the reason at the check site (validateModules) and the data source (the field); drop the narration at the pass-through sites.

🤖

Fixed in 1cf64f1. Kept the rationale at the check site (validateModules) and the data source (BaseManifest.IncludedModules), and reduced the pass-through sites (the BaseIncludedModules doc and the modules-endpoint comment) to short pointers. The inline ordering narration is gone: the sort is now a single slices.SortFunc.

Nit [CRF-18] Stdlib modernization for the ordering code (go 1.26) (ging-go, Robin).

The comparator is sort.Slice with a reflection-based less func; slices.SortFunc(bases, func(a, b) int { return cmp.Or(cmp.Compare(a.Name, b.Name), cmp.Compare(a.ID, b.ID)) }) is the idiom and drops the sort import (behavior identical, comparator already total). In groupQuickstartBeforeDocker, the manual dockerIdx search is slices.IndexFunc and the three-append splice is slices.Insert(rest, dockerIdx, quickstart). Style/type-safety only; take it or leave it.

🤖

Done in 1cf64f1. The comparator is now slices.SortFunc(bases, func(a, b) int { return cmp.Or(cmp.Compare(a.Name, b.Name), cmp.Compare(a.ID, b.ID)) }), the sort import is dropped, and groupQuickstartBeforeDocker uses slices.IndexFunc + slices.Insert. Behavior identical (TestTemplateBuilderBases green).

P3 [CRF-20] The language selector's option values and the install script's has_language branches are hand-maintained duplicates across two files with no cross-check test (Bisky).

The coder_parameter "languages" block offers six values (python, nodejs, go, rust, java, cpp) and install-languages.sh.tftpl has six matching has_language branches. Verified they agree today, and verified nothing binds them (no test references both). Add an option value and forget the branch, or rename cpp to c++ on one side, and the user selects a language, the workspace starts, and nothing installs, no error. That is the CRF-5 silent-no-op shape, and the CRF-12 hand-synced-lists shape you just made mechanical for included_modules. Close it the same way: render the base, extract the coder_parameter "languages" option values and the has_language arguments, and require.ElementsMatch the two sets. Rationed to P3 because the code is correct today and the fix is one test.

🤖

P3 [CRF-23] CRF-13's doc fix is honest, but the behavioral residual it documents is untracked (Mafu-san).

The README now correctly says languages reinstall on every start and block login. The behavior behind it: coder_script.install_languages runs with run_on_start = true and start_blocks_login = true, and the script fetches every toolchain over the network (nodesource, go.dev, rustup, apt). On a network-restricted or air-gapped deployment, every workspace start blocks login on an install that cannot complete. That is a distinct mechanism from CRF-6/DOCS-610 (the git-clone registry mirror), so DOCS-610 does not cover it. Per the no-follow-up rule an untracked residual is a human decision, not an implicit accept: confirm it is tracked (a ticket) or explicitly accept the limitation. The doc-fix itself is fine.

🤖

Note [CRF-21] The CRF-12 invariant test renders each base with DefaultBaseRenderContext only, but production renderBase overlays merged user variables (Hisoka).

So the guard verifies the manifest-vs-rendered mirror under a context production may not use. A base whose main.tf.tmpl gated a module "<catalog-id>" block on a variable ({{ if .Variables.x }}module "code-server"{{ end }}) would render that block in production while the test, running defaults, never sees it, and included_modules would not need to list it to pass, re-opening CRF-4 with the test green. Dormant today: the only base with included_modules is quickstart, which declares zero variables and is directive-free. Worth a note in the test or a variable-context case if bases ever gain variables.

🤖

Note [CRF-22] The CRF-13 fix's leading clause slightly over-corrects: "reinstalled on every start" is not literally true for Rust (Mafuuu; Leorio and Hisoka consider the current wording acceptable).

Rust installs to $HOME/.cargo on the persistent home volume and is skipped on restart by the [ -f "$HOME/.cargo/bin/rustc" ] guard; every other language installs into the ephemeral container and does reinstall. The trailing "most language tools ... do not persist" hedges the aggregate correctly, so this is a minor optional refinement (e.g. "most selected languages are reinstalled on every start"), not a false promise.

🤖

Nit [CRF-24] Residual of the CRF-16 dedup: the sort-site comment still repeats the placement rationale that groupQuickstartBeforeDocker's own doc carries (Gon).

The CRF-16 fix cut the collision rationale from ~6 copies to a canonical site; this ordering rationale is the one remaining verbatim pair (sort call site + helper doc). The helper owns the reordering and its why, so the sort-site comment can drop the rationale and keep just what the sort does, e.g. "Sort by display name, then regroup Quickstart next to Docker (see groupQuickstartBeforeDocker)."

🤖