apps init: address code review findings for pre-rendered templates

- Check os.WriteFile error for package.json instead of discarding it
- Check enc.Close() error for YAML encoder
- Warn when --set values are used with pre-rendered templates
- Fix stale comment about --features warning behavior
- Add unit tests for isPreRenderedTemplate, replaceProjectName,
  setYAMLValue, setFirstAppName, and yamlMapLookup

Co-authored-by: Isaac

Author: pkosiec

cmd/apps/init.go

@@ -684,7 +684,9 @@ func replaceProjectName(destDir, newName string) error {
 			if err == nil {
 				// Preserve trailing newline convention.
 				out = append(out, '\n')
-				_ = os.WriteFile(pkgPath, out, 0o644)
+				if err := os.WriteFile(pkgPath, out, 0o644); err != nil {
+					return fmt.Errorf("write package.json: %w", err)
+				}
 			}
 		}
 	}
@@ -710,7 +712,9 @@ func replaceProjectName(destDir, newName string) error {
 	if err := enc.Encode(&doc); err != nil {
 		return fmt.Errorf("encode %s: %w", bundleConfigFile, err)
 	}
-	enc.Close()
+	if err := enc.Close(); err != nil {
+		return fmt.Errorf("encode %s: %w", bundleConfigFile, err)
+	}
 
 	return os.WriteFile(ymlPath, buf.Bytes(), 0o644)
 }
@@ -1217,12 +1221,18 @@ func runCreate(ctx context.Context, opts createOptions) error {
 		maps.Copy(resourceValues, setVals)
 	}
 
+	// Pre-rendered templates have no Go template placeholders, so --set
+	// values cannot be injected into the output files.
+	if preRendered && len(setVals) > 0 {
+		log.Warnf(ctx, "--set values are ignored for pre-rendered templates (resources are already configured in %s)", bundleConfigFile)
+	}
+
 	// Always include mandatory plugins regardless of user selection or flags.
 	selectedPlugins = appendUnique(selectedPlugins, m.GetMandatoryPluginNames()...)
 
 	// Warn when --features adds plugins that the pre-rendered template
-	// cannot inject (they'll appear in .env but not in databricks.yml,
-	// server.ts, or app.yaml).
+	// cannot inject (its databricks.yml, server.ts, and app.yaml are
+	// already finalised).
 	if preRendered && opts.pluginsChanged {
 		mandatoryNames := m.GetMandatoryPluginNames()
 		mandatory := make(map[string]bool, len(mandatoryNames))

cmd/apps/init_test.go

@@ -2,6 +2,7 @@ package apps
 
 import (
 	"bytes"
+	"encoding/json"
 	"errors"
 	"io"
 	"io/fs"
@@ -18,6 +19,7 @@ import (
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.yaml.in/yaml/v3"
 )
 
 func TestIsTextFile(t *testing.T) {
@@ -1148,3 +1150,231 @@ func TestRunCreate_NameDotAndOutputDirAreMutuallyExclusive(t *testing.T) {
 	require.Error(t, err)
 	assert.ErrorIs(t, err, prompt.ErrNameDotWithOutputDir)
 }
+
+func TestIsPreRenderedTemplate(t *testing.T) {
+	tests := []struct {
+		name     string
+		setup    func(dir string)
+		expected bool
+	}{
+		{
+			name: "normal template with project_name dir",
+			setup: func(dir string) {
+				require.NoError(t, os.MkdirAll(filepath.Join(dir, "{{.project_name}}"), 0o755))
+				require.NoError(t, os.WriteFile(filepath.Join(dir, bundleConfigFile), []byte("bundle:\n  name: myapp\n"), 0o644))
+			},
+			expected: false,
+		},
+		{
+			name: "pre-rendered template",
+			setup: func(dir string) {
+				require.NoError(t, os.WriteFile(filepath.Join(dir, bundleConfigFile), []byte("bundle:\n  name: myapp\n"), 0o644))
+			},
+			expected: true,
+		},
+		{
+			name: "no databricks.yml",
+			setup: func(dir string) {
+				require.NoError(t, os.WriteFile(filepath.Join(dir, "README.md"), []byte("hello"), 0o644))
+			},
+			expected: false,
+		},
+		{
+			name: "databricks.yml with Go template syntax",
+			setup: func(dir string) {
+				require.NoError(t, os.WriteFile(filepath.Join(dir, bundleConfigFile), []byte("bundle:\n  name: {{.project_name}}\n"), 0o644))
+			},
+			expected: false,
+		},
+		{
+			name: "unreadable directory",
+			setup: func(_ string) {
+				// dir is replaced with a nonexistent path below.
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			dir := t.TempDir()
+			if tt.name == "unreadable directory" {
+				dir = filepath.Join(dir, "nonexistent")
+			} else {
+				tt.setup(dir)
+			}
+			assert.Equal(t, tt.expected, isPreRenderedTemplate(dir))
+		})
+	}
+}
+
+func TestReplaceProjectName(t *testing.T) {
+	tests := []struct {
+		name           string
+		yml            string
+		pkgJSON        string // empty means no package.json
+		newName        string
+		wantBundleName string
+		wantAppName    string
+		wantPkgName    string
+		wantErr        bool
+	}{
+		{
+			name:           "bundle and app name replaced",
+			yml:            "bundle:\n  name: old-app\nresources:\n  apps:\n    old-app:\n      name: \"old-app\"\n",
+			newName:        "new-app",
+			wantBundleName: "new-app",
+			wantAppName:    "new-app",
+		},
+		{
+			name:           "bundle name only, no resources",
+			yml:            "bundle:\n  name: old-app\n",
+			newName:        "new-app",
+			wantBundleName: "new-app",
+		},
+		{
+			name:        "no bundle.name, only app name",
+			yml:         "resources:\n  apps:\n    myapp:\n      name: \"myapp\"\n",
+			newName:     "new-app",
+			wantAppName: "new-app",
+		},
+		{
+			name:           "package.json round-trip",
+			yml:            "bundle:\n  name: old-app\n",
+			pkgJSON:        `{"name":"old-app","version":"1.0.0"}`,
+			newName:        "new-app",
+			wantBundleName: "new-app",
+			wantPkgName:    "new-app",
+		},
+		{
+			name:           "no package.json",
+			yml:            "bundle:\n  name: old-app\n",
+			newName:        "new-app",
+			wantBundleName: "new-app",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			dir := t.TempDir()
+
+			require.NoError(t, os.WriteFile(filepath.Join(dir, bundleConfigFile), []byte(tt.yml), 0o644))
+
+			if tt.pkgJSON != "" {
+				require.NoError(t, os.WriteFile(filepath.Join(dir, "package.json"), []byte(tt.pkgJSON), 0o644))
+			}
+
+			err := replaceProjectName(dir, tt.newName)
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+
+			// Verify databricks.yml via yaml.Node to avoid format sensitivity.
+			ymlData, err := os.ReadFile(filepath.Join(dir, bundleConfigFile))
+			require.NoError(t, err)
+
+			var doc yaml.Node
+			require.NoError(t, yaml.Unmarshal(ymlData, &doc))
+
+			if tt.wantBundleName != "" {
+				bundleName := yamlMapLookup(yamlMapLookup(doc.Content[0], "bundle"), "name")
+				require.NotNil(t, bundleName, "bundle.name not found in output")
+				assert.Equal(t, tt.wantBundleName, bundleName.Value)
+			}
+
+			if tt.wantAppName != "" {
+				appsNode := yamlMapLookup(yamlMapLookup(doc.Content[0], "resources"), "apps")
+				require.NotNil(t, appsNode)
+				require.True(t, appsNode.Kind == yaml.MappingNode && len(appsNode.Content) >= 2)
+				appName := yamlMapLookup(appsNode.Content[1], "name")
+				require.NotNil(t, appName, "resources.apps.*.name not found in output")
+				assert.Equal(t, tt.wantAppName, appName.Value)
+			}
+
+			if tt.wantPkgName != "" {
+				pkgData, err := os.ReadFile(filepath.Join(dir, "package.json"))
+				require.NoError(t, err)
+				var pkg map[string]any
+				require.NoError(t, json.Unmarshal(pkgData, &pkg))
+				assert.Equal(t, tt.wantPkgName, pkg["name"])
+			}
+		})
+	}
+}
+
+func TestReplaceProjectNameNoDatabricksYml(t *testing.T) {
+	dir := t.TempDir()
+	err := replaceProjectName(dir, "new-app")
+	require.Error(t, err)
+}
+
+func TestSetYAMLValue(t *testing.T) {
+	input := "top:\n  nested:\n    leaf: old\n"
+	var doc yaml.Node
+	require.NoError(t, yaml.Unmarshal([]byte(input), &doc))
+
+	setYAMLValue(&doc, []string{"top", "nested", "leaf"}, "new")
+
+	val := yamlMapLookup(yamlMapLookup(yamlMapLookup(doc.Content[0], "top"), "nested"), "leaf")
+	require.NotNil(t, val)
+	assert.Equal(t, "new", val.Value)
+}
+
+func TestSetYAMLValueMissingKey(t *testing.T) {
+	input := "top:\n  nested:\n    leaf: old\n"
+	var doc yaml.Node
+	require.NoError(t, yaml.Unmarshal([]byte(input), &doc))
+
+	// Should not panic or modify the tree.
+	setYAMLValue(&doc, []string{"top", "nonexistent", "leaf"}, "new")
+
+	val := yamlMapLookup(yamlMapLookup(yamlMapLookup(doc.Content[0], "top"), "nested"), "leaf")
+	require.NotNil(t, val)
+	assert.Equal(t, "old", val.Value)
+}
+
+func TestYamlMapLookup(t *testing.T) {
+	input := "a: 1\nb: 2\n"
+	var doc yaml.Node
+	require.NoError(t, yaml.Unmarshal([]byte(input), &doc))
+
+	root := doc.Content[0]
+	assert.NotNil(t, yamlMapLookup(root, "a"))
+	assert.Equal(t, "1", yamlMapLookup(root, "a").Value)
+	assert.Nil(t, yamlMapLookup(root, "missing"))
+	assert.Nil(t, yamlMapLookup(nil, "a"))
+}
+
+func TestSetFirstAppName(t *testing.T) {
+	input := "resources:\n  apps:\n    myapp:\n      name: \"old-name\"\n      description: keep\n"
+	var doc yaml.Node
+	require.NoError(t, yaml.Unmarshal([]byte(input), &doc))
+
+	setFirstAppName(&doc, "new-name")
+
+	appsNode := yamlMapLookup(yamlMapLookup(doc.Content[0], "resources"), "apps")
+	require.NotNil(t, appsNode)
+	appName := yamlMapLookup(appsNode.Content[1], "name")
+	require.NotNil(t, appName)
+	assert.Equal(t, "new-name", appName.Value)
+
+	// Verify description is untouched.
+	desc := yamlMapLookup(appsNode.Content[1], "description")
+	require.NotNil(t, desc)
+	assert.Equal(t, "keep", desc.Value)
+}
+
+func TestSetFirstAppNameNoResources(t *testing.T) {
+	input := "bundle:\n  name: myapp\n"
+	var doc yaml.Node
+	require.NoError(t, yaml.Unmarshal([]byte(input), &doc))
+
+	// Should not panic when resources.apps is absent.
+	setFirstAppName(&doc, "new-name")
+
+	bundleName := yamlMapLookup(yamlMapLookup(doc.Content[0], "bundle"), "name")
+	require.NotNil(t, bundleName)
+	assert.Equal(t, "myapp", bundleName.Value)
+}