refactor(coderd/templatebuilder): address review feedback

- Replace stub module with real code-server module manifest
- Export ParseModulesFromFS(fs.FS) for test isolation (no sync.Once coupling)
- Use sync.OnceValues for the cached loader
- Validate module ID (non-empty, unique), pinned_version, and variable types
  during parsing; reject unknown types at load time
- Normalize nil slices to empty in ToSDK() to prevent null in JSON responses
- Distinguish fs.ErrNotExist from other ReadFile errors
- Improve error messages to describe what failed, not internal plumbing
- Rewrite tests with fstest.MapFS fixtures covering all variable types,
  default pointer, validation errors, and full SDK field assertions
- Trim zero-value godoc comments
- Add future tense to README for unimplemented behavior; link RFC

coderd/templatebuilder/README.md

@@ -1,9 +1,10 @@
 # templatebuilder
 
 Package `templatebuilder` implements the bundled module catalog for the guided
-template creation workflow. It embeds module metadata (`module.json` manifests)
-into the Coder binary via `go:embed` and provides functions to load and convert
-them for the API layer.
+template creation workflow
+([RFC](https://www.notion.so/coderhq/RFC-Guided-Template-Creation-Workflow-342d579be59280dfbf8eea2e5006dbda)).
+It embeds module metadata (`module.json` manifests) into the Coder binary via
+`go:embed` and provides functions to load and convert them for the API layer.
 
 ## Directory layout
 
@@ -57,18 +58,18 @@ silently skipped.
 
 Key fields:
 
-- **`builder_managed`**: Variables the compose engine injects automatically
-  (e.g. `agent_id`). These are never shown to users.
-- **`sensitive`**: Variables containing secrets. The builder does not collect
-  these; they become bare Terraform `variable` blocks so values are supplied
-  at workspace creation time.
+- **`builder_managed`**: Variables the compose engine will inject automatically
+  (e.g. `agent_id`). These should not be shown to users.
+- **`sensitive`**: Variables containing secrets. The builder will not collect
+  these; they will become bare Terraform `variable` blocks so values are
+  supplied at a later stage.
 - **`pinned_version`**: The exact module version shipped with this Coder
-  release. The compose endpoint always emits this version; `latest` is never
-  used.
-- **`compatible_os`**: Matched against the base template's OS to filter
+  release. The compose endpoint will always emit this version; `latest` is
+  never used.
+- **`compatible_os`**: Will be matched against the base template's OS to filter
   incompatible modules.
 - **`conflicts_with`**: Module IDs that should not be selected together. The
-  UI surfaces a warning but does not block selection.
+  UI will surface a warning but not block selection.
 
 ## Two type layers
 
@@ -83,7 +84,13 @@ Key fields:
 
 1. Create `modules/<module-id>/module.json` following the schema above.
 2. Run `go build ./coderd/templatebuilder/` to verify the embed compiles.
-3. Run `go test ./coderd/templatebuilder/` to verify parsing.
+3. Run `go test ./coderd/templatebuilder/` to verify parsing and validation.
 
 The catalog is bundled at build time. New modules or version bumps require a
 Coder release to appear in the builder.
+
+## Testing
+
+`ParseModulesFromFS(fs.FS)` accepts an arbitrary filesystem, so tests can
+supply custom fixtures via `fstest.MapFS` without modifying the embedded
+production catalog.

coderd/templatebuilder/catalog.go

@@ -1,8 +1,10 @@
 package templatebuilder
 
 import (
+	"bytes"
 	"embed"
 	"encoding/json"
+	"errors"
 	"io/fs"
 	"path"
 	"sync"
@@ -14,11 +16,11 @@ import (
 
 var (
 	//go:embed modules
-	files embed.FS
+	modulesFS embed.FS
 
-	catalogOnce    sync.Once
-	catalogModules []ModuleManifest
-	errCatalogLoad error
+	loadModules = sync.OnceValues(func() ([]ModuleManifest, error) {
+		return ParseModulesFromFS(modulesFS)
+	})
 )
 
 const modulesDir = "modules"
@@ -49,26 +51,44 @@ type ModuleVariable struct {
 	BuilderManaged bool    `json:"builder_managed"`
 }
 
-// LoadModules reads all module.json files from the embedded catalog.
-// Results are cached after the first successful call.
-func LoadModules() ([]ModuleManifest, error) {
-	catalogOnce.Do(func() {
-		catalogModules, errCatalogLoad = parseModules()
-	})
-	return catalogModules, errCatalogLoad
+// validVariableTypes maps module.json type strings to their SDK equivalents.
+// Used both for validation in ParseModulesFromFS and for conversion in ToSDK.
+var validVariableTypes = map[string]codersdk.TemplateBuilderVariableType{
+	"string": codersdk.TemplateBuilderVariableTypeString,
+	"number": codersdk.TemplateBuilderVariableTypeNumber,
+	"bool":   codersdk.TemplateBuilderVariableTypeBool,
 }
 
-func parseModules() ([]ModuleManifest, error) {
-	modulesFS, err := fs.Sub(files, modulesDir)
+// LoadModules returns all module manifests from the embedded catalog.
+// Results are cached after the first call, including errors. Each call
+// returns a fresh slice so callers can filter or sort without corrupting
+// the cache.
+func LoadModules() ([]ModuleManifest, error) {
+	modules, err := loadModules()
 	if err != nil {
-		return nil, xerrors.Errorf("get modules fs: %w", err)
+		return nil, err
+	}
+	out := make([]ModuleManifest, len(modules))
+	copy(out, modules)
+	return out, nil
+}
+
+// ParseModulesFromFS reads and validates all module.json files from the
+// given filesystem. Most callers should use LoadModules, which reads from
+// the embedded catalog. ParseModulesFromFS is exposed for tests that need
+// to supply custom fixtures.
+func ParseModulesFromFS(fsys fs.FS) ([]ModuleManifest, error) {
+	sub, err := fs.Sub(fsys, modulesDir)
+	if err != nil {
+		return nil, xerrors.Errorf("open embedded module catalog: %w", err)
 	}
 
-	dirs, err := fs.ReadDir(modulesFS, ".")
+	dirs, err := fs.ReadDir(sub, ".")
 	if err != nil {
-		return nil, xerrors.Errorf("read modules dir: %w", err)
+		return nil, xerrors.Errorf("list module catalog entries: %w", err)
 	}
 
+	seen := make(map[string]bool)
 	var modules []ModuleManifest
 	for _, dir := range dirs {
 		if !dir.IsDir() {
@@ -76,17 +96,46 @@ func parseModules() ([]ModuleManifest, error) {
 		}
 
 		manifestPath := path.Join(dir.Name(), "module.json")
-		data, err := fs.ReadFile(modulesFS, manifestPath)
+		data, err := fs.ReadFile(sub, manifestPath)
 		if err != nil {
-			// Skip directories without a module.json.
-			continue
+			if errors.Is(err, fs.ErrNotExist) {
+				continue
+			}
+			return nil, xerrors.Errorf("read %s: %w", manifestPath, err)
 		}
 
 		var manifest ModuleManifest
-		if err := json.Unmarshal(data, &manifest); err != nil {
+		dec := json.NewDecoder(bytes.NewReader(data))
+		dec.DisallowUnknownFields()
+		if err := dec.Decode(&manifest); err != nil {
 			return nil, xerrors.Errorf("decode %s: %w", manifestPath, err)
 		}
 
+		if manifest.ID == "" {
+			return nil, xerrors.Errorf("module in %s has empty id", dir.Name())
+		}
+		if manifest.PinnedVersion == "" {
+			return nil, xerrors.Errorf("module %q has empty pinned_version", manifest.ID)
+		}
+		if seen[manifest.ID] {
+			return nil, xerrors.Errorf("duplicate module id %q", manifest.ID)
+		}
+		seen[manifest.ID] = true
+
+		seenVars := make(map[string]bool)
+		for i, v := range manifest.Variables {
+			if v.Name == "" {
+				return nil, xerrors.Errorf("module %q variable %d has empty name", manifest.ID, i)
+			}
+			if seenVars[v.Name] {
+				return nil, xerrors.Errorf("module %q has duplicate variable name %q", manifest.ID, v.Name)
+			}
+			seenVars[v.Name] = true
+			if _, ok := validVariableTypes[v.Type]; !ok {
+				return nil, xerrors.Errorf("module %q variable %d (%q): unknown type %q", manifest.ID, i, v.Name, v.Type)
+			}
+		}
+
 		modules = append(modules, manifest)
 	}
 
@@ -94,14 +143,13 @@ func parseModules() ([]ModuleManifest, error) {
 }
 
 // ToSDK converts a ModuleManifest to the API response type.
-// PinnedVersion is mapped to Version; tags are excluded from the
-// API surface.
+// PinnedVersion is mapped to Version; tags are not part of the API surface.
 func (m ModuleManifest) ToSDK() codersdk.TemplateBuilderModule {
 	variables := make([]codersdk.TemplateBuilderModuleVariable, 0, len(m.Variables))
 	for _, v := range m.Variables {
 		variables = append(variables, codersdk.TemplateBuilderModuleVariable{
 			Name:           v.Name,
-			Type:           codersdk.TemplateBuilderVariableType(v.Type),
+			Type:           validVariableTypes[v.Type],
 			Description:    v.Description,
 			Default:        v.Default,
 			Required:       v.Required,
@@ -110,6 +158,15 @@ func (m ModuleManifest) ToSDK() codersdk.TemplateBuilderModule {
 		})
 	}
 
+	compatibleOS := m.CompatibleOS
+	if compatibleOS == nil {
+		compatibleOS = []string{}
+	}
+	conflictsWith := m.ConflictsWith
+	if conflictsWith == nil {
+		conflictsWith = []string{}
+	}
+
 	return codersdk.TemplateBuilderModule{
 		ID:            m.ID,
 		DisplayName:   m.DisplayName,
@@ -117,8 +174,8 @@ func (m ModuleManifest) ToSDK() codersdk.TemplateBuilderModule {
 		Icon:          m.Icon,
 		Category:      m.Category,
 		Version:       m.PinnedVersion,
-		CompatibleOS:  m.CompatibleOS,
-		ConflictsWith: m.ConflictsWith,
+		CompatibleOS:  compatibleOS,
+		ConflictsWith: conflictsWith,
 		Variables:     variables,
 	}
 }

coderd/templatebuilder/catalog_test.go

@@ -2,6 +2,7 @@ package templatebuilder_test
 
 import (
 	"testing"
+	"testing/fstest"
 
 	"github.com/stretchr/testify/require"
 
@@ -14,116 +15,385 @@ func TestLoadModules(t *testing.T) {
 
 	modules, err := templatebuilder.LoadModules()
 	require.NoError(t, err)
-	require.NotEmpty(t, modules, "catalog should contain at least the stub module")
+	require.NotEmpty(t, modules, "embedded catalog should contain at least one module")
 
-	// Find the stub module used for testing.
-	var stub *templatebuilder.ModuleManifest
-	for i := range modules {
-		if modules[i].ID == "stub" {
-			stub = &modules[i]
+	// Verify the code-server module is present and valid.
+	var found bool
+	for _, m := range modules {
+		if m.ID == "code-server" {
+			found = true
+			require.Equal(t, "code-server", m.DisplayName)
+			require.Equal(t, "IDE", m.Category)
+			require.Equal(t, []string{"linux"}, m.CompatibleOS)
+			require.NotEmpty(t, m.PinnedVersion)
 			break
 		}
 	}
-	require.NotNil(t, stub, "stub module must be present in the catalog")
+	require.True(t, found, "code-server module must be in the embedded catalog")
+}
 
-	t.Run("ManifestFields", func(t *testing.T) {
+func TestParseModulesFromFS(t *testing.T) {
+	t.Parallel()
+
+	t.Run("ValidManifest", func(t *testing.T) {
 		t.Parallel()
 
-		require.Equal(t, "stub", stub.ID)
-		require.Equal(t, "Stub Module", stub.DisplayName)
-		require.Equal(t, "Utility", stub.Category)
-		require.Equal(t, "0.0.0", stub.PinnedVersion)
-		require.Equal(t, []string{"linux"}, stub.CompatibleOS)
-		require.Empty(t, stub.ConflictsWith)
-		require.Len(t, stub.Variables, 2)
+		fsys := fstest.MapFS{
+			"modules/mymod/module.json": &fstest.MapFile{
+				Data: []byte(`{
+					"id": "mymod",
+					"display_name": "My Module",
+					"description": "A test module.",
+					"icon": "/icons/mymod.svg",
+					"category": "IDE",
+					"tags": ["ide"],
+					"compatible_os": ["linux"],
+					"conflicts_with": ["other"],
+					"pinned_version": "1.2.3",
+					"variables": [
+						{
+							"name": "agent_id",
+							"type": "string",
+							"description": "The Coder agent ID.",
+							"required": true,
+							"sensitive": false,
+							"builder_managed": true
+						},
+						{
+							"name": "port",
+							"type": "number",
+							"description": "Port number.",
+							"default": "8080",
+							"required": false,
+							"sensitive": false,
+							"builder_managed": false
+						},
+						{
+							"name": "enable_debug",
+							"type": "bool",
+							"description": "Enable debug mode.",
+							"required": false,
+							"sensitive": false,
+							"builder_managed": false
+						},
+						{
+							"name": "api_key",
+							"type": "string",
+							"description": "Secret API key.",
+							"required": true,
+							"sensitive": true,
+							"builder_managed": false
+						}
+					]
+				}`),
+			},
+		}
+
+		modules, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.NoError(t, err)
+		require.Len(t, modules, 1)
+
+		m := modules[0]
+		require.Equal(t, "mymod", m.ID)
+		require.Equal(t, "My Module", m.DisplayName)
+		require.Equal(t, "A test module.", m.Description)
+		require.Equal(t, "/icons/mymod.svg", m.Icon)
+		require.Equal(t, "IDE", m.Category)
+		require.Equal(t, []string{"ide"}, m.Tags)
+		require.Equal(t, []string{"linux"}, m.CompatibleOS)
+		require.Equal(t, []string{"other"}, m.ConflictsWith)
+		require.Equal(t, "1.2.3", m.PinnedVersion)
+		require.Len(t, m.Variables, 4)
+
+		// Verify variable types parsed correctly.
+		require.Equal(t, "string", m.Variables[0].Type)
+		require.Equal(t, "number", m.Variables[1].Type)
+		require.Equal(t, "bool", m.Variables[2].Type)
+
+		// Verify builder_managed and sensitive fields.
+		require.True(t, m.Variables[0].BuilderManaged)
+		require.True(t, m.Variables[3].Sensitive)
+
+		// Verify default pointer.
+		require.Nil(t, m.Variables[0].Default)
+		require.NotNil(t, m.Variables[1].Default)
+		require.Equal(t, "8080", *m.Variables[1].Default)
 	})
 
-	t.Run("BuilderManagedVariable", func(t *testing.T) {
+	t.Run("MultipleModules", func(t *testing.T) {
 		t.Parallel()
 
-		agentVar := findVariable(t, stub.Variables, "agent_id")
-		require.True(t, agentVar.BuilderManaged, "agent_id should be builder_managed")
-		require.True(t, agentVar.Required)
-		require.False(t, agentVar.Sensitive)
-		require.Equal(t, "string", agentVar.Type)
+		fsys := fstest.MapFS{
+			"modules/alpha/module.json": &fstest.MapFile{
+				Data: []byte(`{"id": "alpha", "pinned_version": "1.0.0"}`),
+			},
+			"modules/beta/module.json": &fstest.MapFile{
+				Data: []byte(`{"id": "beta", "pinned_version": "2.0.0"}`),
+			},
+		}
+
+		modules, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.NoError(t, err)
+		require.Len(t, modules, 2)
+
+		ids := []string{modules[0].ID, modules[1].ID}
+		require.Contains(t, ids, "alpha")
+		require.Contains(t, ids, "beta")
 	})
 
-	t.Run("SensitiveVariable", func(t *testing.T) {
+	t.Run("EmptyCatalog", func(t *testing.T) {
 		t.Parallel()
 
-		secretVar := findVariable(t, stub.Variables, "example_secret")
-		require.True(t, secretVar.Sensitive, "example_secret should be sensitive")
-		require.False(t, secretVar.BuilderManaged)
-		require.False(t, secretVar.Required)
-		require.Equal(t, "string", secretVar.Type)
+		fsys := fstest.MapFS{
+			"modules/.keep": &fstest.MapFile{Data: []byte{}},
+		}
+
+		modules, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.NoError(t, err)
+		require.Empty(t, modules)
+	})
+
+	t.Run("SkipsDirWithoutManifest", func(t *testing.T) {
+		t.Parallel()
+
+		fsys := fstest.MapFS{
+			"modules/nomod/readme.txt": &fstest.MapFile{Data: []byte("hi")},
+		}
+
+		modules, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.NoError(t, err)
+		require.Empty(t, modules)
+	})
+
+	t.Run("RejectsEmptyID", func(t *testing.T) {
+		t.Parallel()
+
+		fsys := fstest.MapFS{
+			"modules/bad/module.json": &fstest.MapFile{
+				Data: []byte(`{"id": "", "pinned_version": "1.0.0"}`),
+			},
+		}
+
+		_, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.ErrorContains(t, err, "empty id")
+	})
+
+	t.Run("RejectsEmptyPinnedVersion", func(t *testing.T) {
+		t.Parallel()
+
+		fsys := fstest.MapFS{
+			"modules/bad/module.json": &fstest.MapFile{
+				Data: []byte(`{"id": "bad", "pinned_version": ""}`),
+			},
+		}
+
+		_, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.ErrorContains(t, err, "empty pinned_version")
+	})
+
+	t.Run("RejectsDuplicateID", func(t *testing.T) {
+		t.Parallel()
+
+		fsys := fstest.MapFS{
+			"modules/a/module.json": &fstest.MapFile{
+				Data: []byte(`{"id": "dupe", "pinned_version": "1.0.0"}`),
+			},
+			"modules/b/module.json": &fstest.MapFile{
+				Data: []byte(`{"id": "dupe", "pinned_version": "2.0.0"}`),
+			},
+		}
+
+		_, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.ErrorContains(t, err, "duplicate module id")
+	})
+
+	t.Run("RejectsUnknownVariableType", func(t *testing.T) {
+		t.Parallel()
+
+		fsys := fstest.MapFS{
+			"modules/bad/module.json": &fstest.MapFile{
+				Data: []byte(`{
+					"id": "bad",
+					"pinned_version": "1.0.0",
+					"variables": [{"name": "x", "type": "list"}]
+				}`),
+			},
+		}
+
+		_, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.ErrorContains(t, err, `unknown type "list"`)
+	})
+
+	t.Run("RejectsUnknownField", func(t *testing.T) {
+		t.Parallel()
+
+		fsys := fstest.MapFS{
+			"modules/bad/module.json": &fstest.MapFile{
+				Data: []byte(`{"id": "bad", "pinned_version": "1.0.0", "dispaly_name": "typo"}`),
+			},
+		}
+
+		_, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.ErrorContains(t, err, "decode")
+	})
+
+	t.Run("RejectsEmptyVariableName", func(t *testing.T) {
+		t.Parallel()
+
+		fsys := fstest.MapFS{
+			"modules/bad/module.json": &fstest.MapFile{
+				Data: []byte(`{
+					"id": "bad",
+					"pinned_version": "1.0.0",
+					"variables": [{"name": "", "type": "string"}]
+				}`),
+			},
+		}
+
+		_, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.ErrorContains(t, err, "empty name")
+	})
+
+	t.Run("RejectsDuplicateVariableName", func(t *testing.T) {
+		t.Parallel()
+
+		fsys := fstest.MapFS{
+			"modules/bad/module.json": &fstest.MapFile{
+				Data: []byte(`{
+					"id": "bad",
+					"pinned_version": "1.0.0",
+					"variables": [
+						{"name": "x", "type": "string"},
+						{"name": "x", "type": "number"}
+					]
+				}`),
+			},
+		}
+
+		_, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.ErrorContains(t, err, "duplicate variable name")
+	})
+
+	t.Run("RejectsInvalidJSON", func(t *testing.T) {
+		t.Parallel()
+
+		fsys := fstest.MapFS{
+			"modules/bad/module.json": &fstest.MapFile{
+				Data: []byte(`{not json`),
+			},
+		}
+
+		_, err := templatebuilder.ParseModulesFromFS(fsys)
+		require.ErrorContains(t, err, "decode")
 	})
 }
 
 func TestToSDK(t *testing.T) {
 	t.Parallel()
 
-	modules, err := templatebuilder.LoadModules()
-	require.NoError(t, err)
-
-	var stub templatebuilder.ModuleManifest
-	for _, m := range modules {
-		if m.ID == "stub" {
-			stub = m
-			break
-		}
+	defaultVal := "8080"
+	manifest := templatebuilder.ModuleManifest{
+		ID:            "test-mod",
+		DisplayName:   "Test Module",
+		Description:   "A module for testing.",
+		Icon:          "/icons/test.svg",
+		Category:      "Utility",
+		Tags:          []string{"test"},
+		CompatibleOS:  []string{"linux", "darwin"},
+		ConflictsWith: []string{"conflicting-mod"},
+		PinnedVersion: "2.5.0",
+		Variables: []templatebuilder.ModuleVariable{
+			{
+				Name:           "agent_id",
+				Type:           "string",
+				Description:    "The Coder agent ID.",
+				Required:       true,
+				Sensitive:      false,
+				BuilderManaged: true,
+			},
+			{
+				Name:           "port",
+				Type:           "number",
+				Description:    "Port to listen on.",
+				Default:        &defaultVal,
+				Required:       false,
+				Sensitive:      false,
+				BuilderManaged: false,
+			},
+			{
+				Name:           "secret_key",
+				Type:           "string",
+				Description:    "A sensitive value.",
+				Required:       true,
+				Sensitive:      true,
+				BuilderManaged: false,
+			},
+		},
 	}
-	require.NotEmpty(t, stub.ID, "stub module must be present")
 
-	sdk := stub.ToSDK()
+	sdk := manifest.ToSDK()
 
-	t.Run("PinnedVersionMapsToVersion", func(t *testing.T) {
+	t.Run("TopLevelFields", func(t *testing.T) {
 		t.Parallel()
-		require.Equal(t, stub.PinnedVersion, sdk.Version)
-		require.Equal(t, "0.0.0", sdk.Version)
+
+		require.Equal(t, "test-mod", sdk.ID)
+		require.Equal(t, "Test Module", sdk.DisplayName)
+		require.Equal(t, "A module for testing.", sdk.Description)
+		require.Equal(t, "/icons/test.svg", sdk.Icon)
+		require.Equal(t, "Utility", sdk.Category)
+		require.Equal(t, "2.5.0", sdk.Version, "PinnedVersion should map to Version")
+		require.Equal(t, []string{"linux", "darwin"}, sdk.CompatibleOS)
+		require.Equal(t, []string{"conflicting-mod"}, sdk.ConflictsWith)
 	})
 
-	t.Run("FieldsPreserved", func(t *testing.T) {
+	t.Run("AllVariableFields", func(t *testing.T) {
 		t.Parallel()
-		require.Equal(t, stub.ID, sdk.ID)
-		require.Equal(t, stub.DisplayName, sdk.DisplayName)
-		require.Equal(t, stub.Description, sdk.Description)
-		require.Equal(t, stub.Category, sdk.Category)
-		require.Equal(t, stub.CompatibleOS, sdk.CompatibleOS)
-		require.Equal(t, stub.ConflictsWith, sdk.ConflictsWith)
+
+		require.Len(t, sdk.Variables, 3)
+
+		agent := sdk.Variables[0]
+		require.Equal(t, "agent_id", agent.Name)
+		require.Equal(t, codersdk.TemplateBuilderVariableTypeString, agent.Type)
+		require.Equal(t, "The Coder agent ID.", agent.Description)
+		require.Nil(t, agent.Default)
+		require.True(t, agent.Required)
+		require.False(t, agent.Sensitive)
+		require.True(t, agent.BuilderManaged)
+
+		port := sdk.Variables[1]
+		require.Equal(t, "port", port.Name)
+		require.Equal(t, codersdk.TemplateBuilderVariableTypeNumber, port.Type)
+		require.Equal(t, "Port to listen on.", port.Description)
+		require.NotNil(t, port.Default)
+		require.Equal(t, "8080", *port.Default)
+		require.False(t, port.Required)
+		require.False(t, port.Sensitive)
+		require.False(t, port.BuilderManaged)
+
+		secret := sdk.Variables[2]
+		require.Equal(t, "secret_key", secret.Name)
+		require.Equal(t, codersdk.TemplateBuilderVariableTypeString, secret.Type)
+		require.Equal(t, "A sensitive value.", secret.Description)
+		require.Nil(t, secret.Default)
+		require.True(t, secret.Required)
+		require.True(t, secret.Sensitive)
+		require.False(t, secret.BuilderManaged)
 	})
 
-	t.Run("VariablesConverted", func(t *testing.T) {
+	t.Run("NilSlicesNormalizedToEmpty", func(t *testing.T) {
 		t.Parallel()
-		require.Len(t, sdk.Variables, 2)
 
-		agentVar := findSDKVariable(t, sdk.Variables, "agent_id")
-		require.Equal(t, codersdk.TemplateBuilderVariableTypeString, agentVar.Type)
-		require.True(t, agentVar.BuilderManaged)
-
-		secretVar := findSDKVariable(t, sdk.Variables, "example_secret")
-		require.True(t, secretVar.Sensitive)
-		require.False(t, secretVar.BuilderManaged)
+		m := templatebuilder.ModuleManifest{
+			ID:            "nil-slices",
+			PinnedVersion: "1.0.0",
+			// CompatibleOS and ConflictsWith are nil.
+		}
+		s := m.ToSDK()
+		require.NotNil(t, s.CompatibleOS, "nil CompatibleOS should become empty slice")
+		require.NotNil(t, s.ConflictsWith, "nil ConflictsWith should become empty slice")
+		require.NotNil(t, s.Variables, "nil Variables should become empty slice")
+		require.Empty(t, s.CompatibleOS)
+		require.Empty(t, s.ConflictsWith)
+		require.Empty(t, s.Variables)
 	})
 }
-
-func findVariable(t *testing.T, vars []templatebuilder.ModuleVariable, name string) templatebuilder.ModuleVariable {
-	t.Helper()
-	for _, v := range vars {
-		if v.Name == name {
-			return v
-		}
-	}
-	t.Fatalf("variable %q not found", name)
-	return templatebuilder.ModuleVariable{}
-}
-
-func findSDKVariable(t *testing.T, vars []codersdk.TemplateBuilderModuleVariable, name string) codersdk.TemplateBuilderModuleVariable {
-	t.Helper()
-	for _, v := range vars {
-		if v.Name == name {
-			return v
-		}
-	}
-	t.Fatalf("variable %q not found", name)
-	return codersdk.TemplateBuilderModuleVariable{}
-}

coderd/templatebuilder/modules/code-server/module.json

@@ -0,0 +1,30 @@
+{
+  "id": "code-server",
+  "display_name": "code-server",
+  "description": "VS Code in the browser",
+  "icon": "/icon/code.svg",
+  "category": "IDE",
+  "tags": ["ide", "web"],
+  "compatible_os": ["linux"],
+  "conflicts_with": [],
+  "pinned_version": "1.2.3",
+  "variables": [
+    {
+      "name": "agent_id",
+      "type": "string",
+      "description": "The ID of the Coder agent. Injected automatically by the builder.",
+      "required": true,
+      "sensitive": false,
+      "builder_managed": true
+    },
+    {
+      "name": "port",
+      "type": "number",
+      "description": "Port to run code-server on",
+      "default": "13337",
+      "required": false,
+      "sensitive": false,
+      "builder_managed": false
+    }
+  ]
+}

coderd/templatebuilder/modules/stub/module.json

@@ -1,29 +0,0 @@
-{
-  "id": "stub",
-  "display_name": "Stub Module",
-  "description": "Stub module for testing the catalog system.",
-  "icon": "",
-  "category": "Utility",
-  "tags": ["test"],
-  "compatible_os": ["linux"],
-  "conflicts_with": [],
-  "pinned_version": "0.0.0",
-  "variables": [
-    {
-      "name": "agent_id",
-      "type": "string",
-      "description": "The ID of the Coder agent.",
-      "required": true,
-      "sensitive": false,
-      "builder_managed": true
-    },
-    {
-      "name": "example_secret",
-      "type": "string",
-      "description": "An example sensitive variable.",
-      "required": false,
-      "sensitive": true,
-      "builder_managed": false
-    }
-  ]
-}

codersdk/templatebuilder.go

@@ -1,6 +1,7 @@
 package codersdk
 
-// TemplateBuilderVariableType represents the type of a template builder variable.
+// TemplateBuilderVariableType enumerates the variable types
+// supported by template builder module manifests.
 type TemplateBuilderVariableType string
 
 const (
@@ -9,7 +10,6 @@ const (
 	TemplateBuilderVariableTypeBool   TemplateBuilderVariableType = "bool"
 )
 
-// TemplateBuilderModuleVariable represents a variable within a template builder module.
 type TemplateBuilderModuleVariable struct {
 	Name           string                      `json:"name"`
 	Type           TemplateBuilderVariableType `json:"type"`
@@ -22,7 +22,7 @@ type TemplateBuilderModuleVariable struct {
 
 // TemplateBuilderModule is the API response type returned by
 // GET /api/v2/templatebuilder/modules. The Version field is
-// populated from the catalog's PinnedVersion at serving time.
+// populated from the catalog manifest's PinnedVersion at serving time.
 type TemplateBuilderModule struct {
 	ID            string                          `json:"id"`
 	DisplayName   string                          `json:"display_name"`
@@ -35,8 +35,6 @@ type TemplateBuilderModule struct {
 	Variables     []TemplateBuilderModuleVariable `json:"variables"`
 }
 
-// TemplateBuilderModulesResponse is the response body for listing
-// template builder modules.
 type TemplateBuilderModulesResponse struct {
 	Modules []TemplateBuilderModule `json:"modules"`
 }

site/src/api/typesGenerated.ts

@@ -8131,7 +8131,7 @@ export interface TemplateBuilderConfig {
 /**
  * TemplateBuilderModule is the API response type returned by
  * GET /api/v2/templatebuilder/modules. The Version field is
- * populated from the catalog's PinnedVersion at serving time.
+ * populated from the catalog manifest's PinnedVersion at serving time.
  */
 export interface TemplateBuilderModule {
 	readonly id: string;
@@ -8146,9 +8146,6 @@ export interface TemplateBuilderModule {
 }
 
 // From codersdk/templatebuilder.go
-/**
- * TemplateBuilderModuleVariable represents a variable within a template builder module.
- */
 export interface TemplateBuilderModuleVariable {
 	readonly name: string;
 	readonly type: TemplateBuilderVariableType;
@@ -8160,10 +8157,6 @@ export interface TemplateBuilderModuleVariable {
 }
 
 // From codersdk/templatebuilder.go
-/**
- * TemplateBuilderModulesResponse is the response body for listing
- * template builder modules.
- */
 export interface TemplateBuilderModulesResponse {
 	readonly modules: readonly TemplateBuilderModule[];
 }