PterX · pull · May 1, 2025 · May 1, 2025 · May 1, 2025 · May 1, 2025
diff --git a/CODEOWNERS b/CODEOWNERS
@@ -4,3 +4,5 @@ agent/proto/ @spikecurtis @johnstcn
 tailnet/proto/ @spikecurtis @johnstcn
 vpn/vpn.proto @spikecurtis @johnstcn
 vpn/version.go @spikecurtis @johnstcn
+provisionerd/proto/ @spikecurtis @johnstcn
+provisionersdk/proto/ @spikecurtis @johnstcn
diff --git a/agent/agentscripts/agentscripts.go b/agent/agentscripts/agentscripts.go
@@ -10,7 +10,6 @@ import (
 	"os/user"
 	"path/filepath"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/google/uuid"
@@ -104,7 +103,6 @@ type Runner struct {
 	closed          chan struct{}
 	closeMutex      sync.Mutex
 	cron            *cron.Cron
-	initialized     atomic.Bool
 	scripts         []runnerScript
 	dataDir         string
 	scriptCompleted ScriptCompletedFunc
@@ -113,6 +111,9 @@ type Runner struct {
 	// execute startup scripts, and scripts on a cron schedule. Both will increment
 	// this counter.
 	scriptsExecuted *prometheus.CounterVec
+
+	initMutex   sync.Mutex
+	initialized bool
 }
 
 // DataDir returns the directory where scripts data is stored.
@@ -154,10 +155,12 @@ func WithPostStartScripts(scripts ...codersdk.WorkspaceAgentScript) InitOption {
 // It also schedules any scripts that have a schedule.
 // This function must be called before Execute.
 func (r *Runner) Init(scripts []codersdk.WorkspaceAgentScript, scriptCompleted ScriptCompletedFunc, opts ...InitOption) error {
-	if r.initialized.Load() {
+	r.initMutex.Lock()
+	defer r.initMutex.Unlock()
+	if r.initialized {
 		return xerrors.New("init: already initialized")
 	}
-	r.initialized.Store(true)
+	r.initialized = true
 	r.scripts = toRunnerScript(scripts...)
 	r.scriptCompleted = scriptCompleted
 	for _, opt := range opts {
@@ -227,6 +230,18 @@ const (
 
 // Execute runs a set of scripts according to a filter.
 func (r *Runner) Execute(ctx context.Context, option ExecuteOption) error {
+	initErr := func() error {
+		r.initMutex.Lock()
+		defer r.initMutex.Unlock()
+		if !r.initialized {
+			return xerrors.New("execute: not initialized")
+		}
+		return nil
+	}()
+	if initErr != nil {
+		return initErr
+	}
+
 	var eg errgroup.Group
 	for _, script := range r.scripts {
 		runScript := (option == ExecuteStartScripts && script.RunOnStart) ||

diff --git a/codersdk/richparameters.go b/codersdk/richparameters.go
@@ -164,7 +164,7 @@ type ParameterResolver struct {
 // resolves the correct value.  It returns the value of the parameter, if valid, and an error if invalid.
 func (r *ParameterResolver) ValidateResolve(p TemplateVersionParameter, v *WorkspaceBuildParameter) (value string, err error) {
 	prevV := r.findLastValue(p)
-	if !p.Mutable && v != nil && prevV != nil {
+	if !p.Mutable && v != nil && prevV != nil && v.Value != prevV.Value {
 		return "", xerrors.Errorf("Parameter %q is not mutable, so it can't be updated after creating a workspace.", p.Name)
 	}
 	if p.Required && v == nil && prevV == nil {

diff --git a/codersdk/richparameters_test.go b/codersdk/richparameters_test.go
@@ -1,6 +1,7 @@
 package codersdk_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -121,20 +122,60 @@ func TestParameterResolver_ValidateResolve_NewOverridesOld(t *testing.T) {
 func TestParameterResolver_ValidateResolve_Immutable(t *testing.T) {
 	t.Parallel()
 	uut := codersdk.ParameterResolver{
-		Rich: []codersdk.WorkspaceBuildParameter{{Name: "n", Value: "5"}},
+		Rich: []codersdk.WorkspaceBuildParameter{{Name: "n", Value: "old"}},
 	}
 	p := codersdk.TemplateVersionParameter{
 		Name:     "n",
-		Type:     "number",
+		Type:     "string",
 		Required: true,
 		Mutable:  false,
 	}
-	v, err := uut.ValidateResolve(p, &codersdk.WorkspaceBuildParameter{
-		Name:  "n",
-		Value: "6",
-	})
-	require.Error(t, err)
-	require.Equal(t, "", v)
+
+	cases := []struct {
+		name        string
+		newValue    string
+		expectedErr string
+	}{
+		{
+			name:        "mutation",
+			newValue:    "new", // "new" != "old"
+			expectedErr: fmt.Sprintf("Parameter %q is not mutable", p.Name),
+		},
+		{
+			// Values are case-sensitive.
+			name:        "case change",
+			newValue:    "Old", // "Old" != "old"
+			expectedErr: fmt.Sprintf("Parameter %q is not mutable", p.Name),
+		},
+		{
+			name:        "default",
+			newValue:    "", // "" != "old"
+			expectedErr: fmt.Sprintf("Parameter %q is not mutable", p.Name),
+		},
+		{
+			name:     "no change",
+			newValue: "old", // "old" == "old"
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			v, err := uut.ValidateResolve(p, &codersdk.WorkspaceBuildParameter{
+				Name:  "n",
+				Value: tc.newValue,
+			})
+
+			if tc.expectedErr == "" {
+				require.NoError(t, err)
+				require.Equal(t, tc.newValue, v)
+			} else {
+				require.ErrorContains(t, err, tc.expectedErr)
+				require.Equal(t, "", v)
+			}
+		})
+	}
 }
 
 func TestRichParameterValidation(t *testing.T) {

diff --git a/codersdk/toolsdk/toolsdk.go b/codersdk/toolsdk/toolsdk.go
@@ -327,6 +327,7 @@ var ListWorkspaces = Tool[ListWorkspacesArgs, []MinimalWorkspace]{
 					"description": "The owner of the workspaces to list. Use \"me\" to list workspaces for the authenticated user. If you do not specify an owner, \"me\" will be assumed by default.",
 				},
 			},
+			Required: []string{},
 		},
 	},
 	Handler: func(ctx context.Context, deps Deps, args ListWorkspacesArgs) ([]MinimalWorkspace, error) {
@@ -1256,6 +1257,7 @@ var DeleteTemplate = Tool[DeleteTemplateArgs, codersdk.Response]{
 					"type": "string",
 				},
 			},
+			Required: []string{"template_id"},
 		},
 	},
 	Handler: func(ctx context.Context, deps Deps, args DeleteTemplateArgs) (codersdk.Response, error) {

diff --git a/codersdk/toolsdk/toolsdk_test.go b/codersdk/toolsdk/toolsdk_test.go
@@ -539,6 +539,33 @@ func TestWithCleanContext(t *testing.T) {
 	})
 }
 
+func TestToolSchemaFields(t *testing.T) {
+	t.Parallel()
+
+	// Test that all tools have the required Schema fields (Properties and Required)
+	for _, tool := range toolsdk.All {
+		t.Run(tool.Tool.Name, func(t *testing.T) {
+			t.Parallel()
+
+			// Check that Properties is not nil
+			require.NotNil(t, tool.Tool.Schema.Properties,
+				"Tool %q missing Schema.Properties", tool.Tool.Name)
+
+			// Check that Required is not nil
+			require.NotNil(t, tool.Tool.Schema.Required,
+				"Tool %q missing Schema.Required", tool.Tool.Name)
+
+			// Ensure Properties has entries for all required fields
+			for _, requiredField := range tool.Tool.Schema.Required {
+				_, exists := tool.Tool.Schema.Properties[requiredField]
+				require.True(t, exists,
+					"Tool %q requires field %q but it is not defined in Properties",
+					tool.Tool.Name, requiredField)
+			}
+		})
+	}
+}
+
 // TestMain runs after all tests to ensure that all tools in this package have
 // been tested once.
 func TestMain(m *testing.M) {

diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go
@@ -329,9 +329,8 @@ func TestClaimPrebuild(t *testing.T) {
 			require.NoError(t, err)
 
 			stopBuild, err := userClient.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
-				TemplateVersionID:   version.ID,
-				Transition:          codersdk.WorkspaceTransitionStop,
-				RichParameterValues: wp,
+				TemplateVersionID: version.ID,
+				Transition:        codersdk.WorkspaceTransitionStop,
 			})
 			require.NoError(t, err)
 			coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, stopBuild.ID)
@@ -369,6 +368,17 @@ func templateWithAgentAndPresetsWithPrebuilds(desiredInstances int32) *echo.Resp
 								},
 							},
 						},
+						// Make sure immutable params don't break claiming logic
+						Parameters: []*proto.RichParameter{
+							{
+								Name:         "k1",
+								Description:  "immutable param",
+								Type:         "string",
+								DefaultValue: "",
+								Required:     false,
+								Mutable:      false,
+							},
+						},
 						Presets: []*proto.Preset{
 							{
 								Name: "preset-a",

diff --git a/enterprise/coderd/prebuilds/reconcile_test.go b/enterprise/coderd/prebuilds/reconcile_test.go
@@ -901,6 +901,16 @@ func setupTestDBTemplateVersion(
 		ID:              templateID,
 		ActiveVersionID: templateVersion.ID,
 	}))
+	// Make sure immutable params don't break prebuilt workspace deletion logic
+	dbgen.TemplateVersionParameter(t, db, database.TemplateVersionParameter{
+		TemplateVersionID: templateVersion.ID,
+		Name:              "test",
+		Description:       "required & immutable param",
+		Type:              "string",
+		DefaultValue:      "",
+		Required:          true,
+		Mutable:           false,
+	})
 	return templateVersion.ID
 }
 
@@ -999,7 +1009,7 @@ func setupTestDBWorkspace(
 		OrganizationID: orgID,
 		Error:          buildError,
 	})
-	dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{
+	workspaceBuild := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{
 		WorkspaceID:             workspace.ID,
 		InitiatorID:             initiatorID,
 		TemplateVersionID:       templateVersionID,
@@ -1008,6 +1018,13 @@ func setupTestDBWorkspace(
 		Transition:              transition,
 		CreatedAt:               clock.Now(),
 	})
+	dbgen.WorkspaceBuildParameters(t, db, []database.WorkspaceBuildParameter{
+		{
+			WorkspaceBuildID: workspaceBuild.ID,
+			Name:             "test",
+			Value:            "test",
+		},
+	})
 
 	return workspace
 }