diff --git a/workspace-server/internal/handlers/handlers_additional_test.go b/workspace-server/internal/handlers/handlers_additional_test.go index 46ebe317f..805a491e9 100644 --- a/workspace-server/internal/handlers/handlers_additional_test.go +++ b/workspace-server/internal/handlers/handlers_additional_test.go @@ -33,7 +33,7 @@ func TestWorkspaceCreate_WithParentID(t *testing.T) { // Default tier is 3 (Privileged) — see workspace.go create-handler comment. // delivery_mode defaults to "push" when payload omits it (#2339). mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Child Agent", nil, 3, "claude-code", &parentID, nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Child Agent", nil, 3, "claude-code", "", &parentID, nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO workspace_secrets"). @@ -81,7 +81,7 @@ func TestWorkspaceCreate_DefaultsParentToPlatformRoot(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Team Member", nil, 3, "claude-code", rootID, nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Team Member", nil, 3, "claude-code", "", rootID, nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO workspace_secrets").WillReturnResult(sqlmock.NewResult(0, 1)) @@ -118,7 +118,7 @@ func TestWorkspaceCreate_NoPlatformRoot_KeepsNullParent(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Bootstrap Root", nil, 3, "claude-code", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Bootstrap Root", nil, 3, "claude-code", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO workspace_secrets").WillReturnResult(sqlmock.NewResult(0, 1)) @@ -150,7 +150,7 @@ func TestWorkspaceCreate_ExplicitClaudeCodeRuntime(t *testing.T) { mock.ExpectBegin() // delivery_mode defaults to "push" when payload omits it (#2339). mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "CC Agent", nil, 2, "claude-code", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "CC Agent", nil, 2, "claude-code", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO canvas_layouts"). @@ -374,7 +374,7 @@ func TestWorkspaceCreate_MaxConcurrentTasksOverride(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Leader Agent", nil, 3, "claude-code", (*string)(nil), nil, "none", (*int64)(nil), 3, "push"). + WithArgs(sqlmock.AnyArg(), "Leader Agent", nil, 3, "claude-code", "", (*string)(nil), nil, "none", (*int64)(nil), 3, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO canvas_layouts"). @@ -1111,8 +1111,8 @@ func TestRestart_ParentPaused(t *testing.T) { // Workspace lookup succeeds mock.ExpectQuery("SELECT status, name, tier"). WithArgs("dddddddd-0001-0000-0000-000000000000"). - WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}). - AddRow("offline", "Child Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime", "template"}). + AddRow("offline", "Child Agent", 1, "claude-code", "")) // isParentPaused: get parent_id mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id"). @@ -1154,8 +1154,8 @@ func TestRestart_ProvisionerNil(t *testing.T) { mock.ExpectQuery("SELECT status, name, tier"). WithArgs("ws-noprov"). - WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}). - AddRow("offline", "Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime", "template"}). + AddRow("offline", "Agent", 1, "claude-code", "")) // isParentPaused: no parent mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id"). @@ -1389,8 +1389,8 @@ func TestResume_ProvisionerNil(t *testing.T) { mock.ExpectQuery("SELECT name, tier"). WithArgs("ws-resume-noprov"). - WillReturnRows(sqlmock.NewRows([]string{"name", "tier", "runtime"}). - AddRow("Paused Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"name", "tier", "runtime", "template"}). + AddRow("Paused Agent", 1, "claude-code", "")) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) diff --git a/workspace-server/internal/handlers/handlers_extended_test.go b/workspace-server/internal/handlers/handlers_extended_test.go index dfd742f58..ebcdb7c29 100644 --- a/workspace-server/internal/handlers/handlers_extended_test.go +++ b/workspace-server/internal/handlers/handlers_extended_test.go @@ -190,7 +190,7 @@ func TestExtended_WorkspaceRestart_NoProvisioner(t *testing.T) { // Expect SELECT for workspace existence check (includes runtime column) mock.ExpectQuery("SELECT status, name, tier"). WithArgs("ws-restart"). - WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}).AddRow("offline", "Restarting Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime", "template"}).AddRow("offline", "Restarting Agent", 1, "claude-code", "")) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) diff --git a/workspace-server/internal/handlers/handlers_test.go b/workspace-server/internal/handlers/handlers_test.go index 3d0fa850f..7d3c2eb1c 100644 --- a/workspace-server/internal/handlers/handlers_test.go +++ b/workspace-server/internal/handlers/handlers_test.go @@ -385,7 +385,7 @@ func TestWorkspaceCreate(t *testing.T) { // Default tier is 3 (Privileged) — see workspace.go create-handler comment. // delivery_mode defaults to "push" when payload omits it (#2339). mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Test Agent", nil, 3, "claude-code", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Test Agent", nil, 3, "claude-code", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) // Expect transaction commit (no secrets in this payload) @@ -464,7 +464,7 @@ func TestWorkspaceCreate_ReturnsAuthToken_201(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Token Holder", nil, 3, "claude-code", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Token Holder", nil, 3, "claude-code", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO canvas_layouts"). diff --git a/workspace-server/internal/handlers/org_import.go b/workspace-server/internal/handlers/org_import.go index fbc7b3a7f..519548e49 100644 --- a/workspace-server/internal/handlers/org_import.go +++ b/workspace-server/internal/handlers/org_import.go @@ -298,6 +298,7 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX // whether to do prep at all. payload := models.CreateWorkspacePayload{ Name: ws.Name, Tier: tier, Runtime: runtime, Model: model, + Template: ws.Template, WorkspaceDir: ws.WorkspaceDir, WorkspaceAccess: workspaceAccess, } diff --git a/workspace-server/internal/handlers/restart_template.go b/workspace-server/internal/handlers/restart_template.go index 08e9c5706..5de08e2b9 100644 --- a/workspace-server/internal/handlers/restart_template.go +++ b/workspace-server/internal/handlers/restart_template.go @@ -39,7 +39,11 @@ type restartTemplateInput struct { // 3. `RebuildConfig=true` → org-templates recovery fallback (#239). // 4. `ApplyTemplate=true` + non-empty dbRuntime → runtime-default template // (e.g. `hermes-default/`) for runtime-change workflows. -// 5. Fall through → empty path + "existing-volume" label. Provisioner +// 5. Persisted `dbTemplate` (set by PATCH /workspaces/:id/template) when no +// explicit body template was supplied. This makes `PATCH template=...` +// followed by a plain `POST /restart` actually apply the newly stored +// template instead of silently reusing the existing config volume. +// 6. Fall through → empty path + "existing-volume" label. Provisioner // reuses the workspace's existing config volume from the previous run. // // Returns (templatePath, configLabel). An empty templatePath is the signal @@ -48,7 +52,7 @@ type restartTemplateInput struct { // // Pure function: no writes, no DB access, no network. Safe to unit-test // with just a temp directory. -func resolveRestartTemplate(configsDir, wsName, dbRuntime string, body restartTemplateInput) (templatePath, configLabel string) { +func resolveRestartTemplate(configsDir, wsName, dbRuntime, dbTemplate string, body restartTemplateInput) (templatePath, configLabel string) { template := body.Template // Tier 2: name-based auto-match, gated on ApplyTemplate. @@ -102,7 +106,22 @@ func resolveRestartTemplate(configsDir, wsName, dbRuntime string, body restartTe } } - // Tier 5: reuse existing volume. This is the default, and the path + // Tier 5: persisted template from PATCH /workspaces/:id/template. + // When the caller supplied no explicit body template, fall back to the + // template stored in the DB. This makes `PATCH template=seo-agent` + // followed by `POST /restart` with no body apply the newly installed + // template instead of reusing the stale config volume. + if body.Template == "" && dbTemplate != "" { + if candidatePath, resolveErr := resolveInsideRoot(configsDir, dbTemplate); resolveErr != nil { + log.Printf("Restart: invalid persisted template %q: %v — proceeding without it", dbTemplate, resolveErr) + } else if _, err := os.Stat(candidatePath); err == nil { + return candidatePath, dbTemplate + } else { + log.Printf("Restart: persisted template %q dir not found — proceeding without it", dbTemplate) + } + } + + // Tier 6: reuse existing volume. This is the default, and the path // the Canvas Save+Restart flow MUST hit to preserve user edits. return "", "existing-volume" } diff --git a/workspace-server/internal/handlers/restart_template_test.go b/workspace-server/internal/handlers/restart_template_test.go index f84fb84f8..80f43ad68 100644 --- a/workspace-server/internal/handlers/restart_template_test.go +++ b/workspace-server/internal/handlers/restart_template_test.go @@ -54,7 +54,7 @@ func TestResolveRestartTemplate_DefaultRestart_PreservesVolume(t *testing.T) { t.Fatal(err) } - path, label := resolveRestartTemplate(root, "Hermes Agent", "hermes", restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Hermes Agent", "hermes", "", restartTemplateInput{ // ApplyTemplate intentionally omitted — this is the default restart. }) if path != "" { @@ -71,7 +71,7 @@ func TestResolveRestartTemplate_DefaultRestart_PreservesVolume(t *testing.T) { func TestResolveRestartTemplate_ExplicitTemplate_AlwaysHonoured(t *testing.T) { root := newTemplateDir(t, "claude-code") - path, label := resolveRestartTemplate(root, "Some Agent", "", restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Some Agent", "", "", restartTemplateInput{ Template: "claude-code", }) if path == "" || label != "claude-code" { @@ -85,7 +85,7 @@ func TestResolveRestartTemplate_ExplicitTemplate_AlwaysHonoured(t *testing.T) { func TestResolveRestartTemplate_ApplyTemplate_NameMatch(t *testing.T) { root := newTemplateDir(t, "hermes") - path, label := resolveRestartTemplate(root, "Hermes", "", restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Hermes", "", "", restartTemplateInput{ ApplyTemplate: true, }) if path == "" || label != "hermes" { @@ -100,7 +100,7 @@ func TestResolveRestartTemplate_ApplyTemplate_NameMatch(t *testing.T) { func TestResolveRestartTemplate_ApplyTemplate_RuntimeDefault(t *testing.T) { root := newTemplateDir(t, "hermes-default") - path, label := resolveRestartTemplate(root, "Some Workspace", "hermes", restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Some Workspace", "hermes", "", restartTemplateInput{ ApplyTemplate: true, }) if path == "" || label != "hermes-default" { @@ -179,7 +179,7 @@ func TestRestartRuntimeFromConfig_DefaultRestartPreservesContainerRuntime(t *tes func TestResolveRestartTemplate_ApplyTemplate_NoMatch_NoRuntime(t *testing.T) { root := newTemplateDir(t) // empty templates dir - path, label := resolveRestartTemplate(root, "Orphan", "", restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Orphan", "", "", restartTemplateInput{ ApplyTemplate: true, }) if path != "" { @@ -197,7 +197,7 @@ func TestResolveRestartTemplate_ApplyTemplate_NoMatch_NoRuntime(t *testing.T) { func TestResolveRestartTemplate_InvalidExplicitTemplate_ProceedsWithout(t *testing.T) { root := newTemplateDir(t, "claude-code") - path, label := resolveRestartTemplate(root, "Some Agent", "", restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Some Agent", "", "", restartTemplateInput{ Template: "../../etc/passwd", }) if path != "" { @@ -214,7 +214,7 @@ func TestResolveRestartTemplate_InvalidExplicitTemplate_ProceedsWithout(t *testi func TestResolveRestartTemplate_NonExistentExplicitTemplate(t *testing.T) { root := newTemplateDir(t, "claude-code") - path, label := resolveRestartTemplate(root, "Some Agent", "", restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Some Agent", "", "", restartTemplateInput{ Template: "deleted-template", }) if path != "" { @@ -232,7 +232,7 @@ func TestResolveRestartTemplate_NonExistentExplicitTemplate(t *testing.T) { func TestResolveRestartTemplate_Priority_ExplicitBeatsApplyTemplate(t *testing.T) { root := newTemplateDir(t, "hermes", "claude-code") - path, label := resolveRestartTemplate(root, "Hermes", "", restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Hermes", "", "", restartTemplateInput{ Template: "claude-code", ApplyTemplate: true, }) @@ -279,7 +279,7 @@ func TestResolveRestartTemplate_CWE22_TraversalRuntime_FallsThrough(t *testing.T {"deep traversal", "a/b/c/../../../d"}, } { t.Run(tc.name, func(t *testing.T) { - path, label := resolveRestartTemplate(root, "Some Workspace", tc.dbRuntime, restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Some Workspace", tc.dbRuntime, "", restartTemplateInput{ ApplyTemplate: true, }) // Must NOT return a path that escapes root @@ -300,7 +300,7 @@ func TestResolveRestartTemplate_CWE22_TraversalRuntime_FallsThrough(t *testing.T func TestResolveRestartTemplate_CWE22_TraversalRuntime_CannotOverrideKnownRuntime(t *testing.T) { root := newTemplateDir(t, "claude-code-default") - path, label := resolveRestartTemplate(root, "Some Workspace", "../../../etc", restartTemplateInput{ + path, label := resolveRestartTemplate(root, "Some Workspace", "../../../etc", "", restartTemplateInput{ ApplyTemplate: true, }) // Must resolve to claude-code-default (the safe default after sanitizeRuntime), @@ -313,3 +313,56 @@ func TestResolveRestartTemplate_CWE22_TraversalRuntime_CannotOverrideKnownRuntim t.Errorf("label must be claude-code-default; got %q", label) } } + +// TestResolveRestartTemplate_PersistedTemplate_FallsBack verifies that a +// workspace with a non-empty DB template uses that template on a plain +// restart when no body template or apply/rebuild flags are supplied. +// Regression test for core#2980 review feedback. +func TestResolveRestartTemplate_PersistedTemplate_FallsBack(t *testing.T) { + root := newTemplateDir(t, "seo-agent") + + path, label := resolveRestartTemplate(root, "Some Workspace", "claude-code", "seo-agent", restartTemplateInput{ + // no body.Template, no ApplyTemplate, no RebuildConfig + }) + expected := filepath.Join(root, "seo-agent") + if path != expected { + t.Errorf("persisted template fallback: expected path %q, got %q", expected, path) + } + if label != "seo-agent" { + t.Errorf("persisted template fallback: expected label %q, got %q", "seo-agent", label) + } +} + +// TestResolveRestartTemplate_PersistedTemplate_EmptyPreservesVolume verifies +// that workspaces with an empty DB template still reuse the existing config +// volume on a plain restart. +func TestResolveRestartTemplate_PersistedTemplate_EmptyPreservesVolume(t *testing.T) { + root := newTemplateDir(t, "seo-agent") + + path, label := resolveRestartTemplate(root, "Some Workspace", "claude-code", "", restartTemplateInput{}) + if path != "" { + t.Errorf("empty persisted template must preserve volume; got path=%q", path) + } + if label != "existing-volume" { + t.Errorf("empty persisted template must fall back to existing-volume; got label=%q", label) + } +} + +// TestResolveRestartTemplate_ExplicitBodyTemplateOverridesPersistedTemplate +// verifies that a template named in the request body still wins over the +// stored template. +func TestResolveRestartTemplate_ExplicitBodyTemplateOverridesPersistedTemplate(t *testing.T) { + root := newTemplateDir(t, "seo-agent", "hermes") + + path, label := resolveRestartTemplate(root, "Some Workspace", "claude-code", "seo-agent", restartTemplateInput{ + Template: "hermes", + }) + expected := filepath.Join(root, "hermes") + if path != expected { + t.Errorf("explicit body template must win; expected path %q, got %q", expected, path) + } + if label != "hermes" { + t.Errorf("explicit body template must win; expected label %q, got %q", "hermes", label) + } +} + diff --git a/workspace-server/internal/handlers/runtime_registry.go b/workspace-server/internal/handlers/runtime_registry.go index a355a6bf2..2adb2b05b 100644 --- a/workspace-server/internal/handlers/runtime_registry.go +++ b/workspace-server/internal/handlers/runtime_registry.go @@ -348,3 +348,32 @@ func templateIdentityForRuntime(runtime string) (string, bool) { } return rr.Repo + "@" + rr.Ref, true } + +// isKnownTemplate reports whether name is a registered workspace template in +// manifest.json. The empty string is intentionally NOT known — it is the +// "no installed template" sentinel and falls back to runtime resolution. +func isKnownTemplate(name string) bool { + if name == "" { + return false + } + _, ok := templateRepoByName[name] + return ok +} + +// resolveTemplateIdentity returns the Gitea template identity (repo@ref) for a +// workspace. If a template is explicitly installed, it wins; otherwise the +// runtime's default template is used. This is the SSOT for the RFC #2843 asset +// fetcher and for the control-plane provision wire. +// +// Fail-closed: an explicitly set but unknown template returns ("", false) so +// callers can surface a 422 instead of silently degrading to the runtime +// fallback (matches the create-boundary posture for unknown runtimes). +func resolveTemplateIdentity(template, runtime string) (string, bool) { + if template != "" { + if rr, ok := templateRepoByName[template]; ok { + return rr.Repo + "@" + rr.Ref, true + } + return "", false + } + return templateIdentityForRuntime(runtime) +} diff --git a/workspace-server/internal/handlers/runtime_registry_test.go b/workspace-server/internal/handlers/runtime_registry_test.go index 057fd12fc..a830eab23 100644 --- a/workspace-server/internal/handlers/runtime_registry_test.go +++ b/workspace-server/internal/handlers/runtime_registry_test.go @@ -223,23 +223,94 @@ func TestTemplateIdentityForRuntime(t *testing.T) { } } -// TestTemplateIdentityForRuntimeOrEmpty pins the -// single-expression wrapper used at the call site in -// buildProvisionerConfig. -func TestTemplateIdentityForRuntimeOrEmpty(t *testing.T) { +// TestTemplateIdentityOrEmpty pins the single-expression wrapper used at the +// call site in buildProvisionerConfig. +func TestTemplateIdentityOrEmpty(t *testing.T) { if manifestPath() == "" { t.Skip("manifest.json not discoverable from this test cwd") } initTemplateRepoByName() - if got := templateIdentityForRuntimeOrEmpty("claude-code"); got == "" { + if got := templateIdentityOrEmpty(resolveTemplateIdentity("", "claude-code")); got == "" { t.Error("claude-code should return a non-empty identity") } - if got := templateIdentityForRuntimeOrEmpty("external"); got != "" { + if got := templateIdentityOrEmpty(resolveTemplateIdentity("", "external")); got != "" { t.Errorf("external should return empty, got %q", got) } - if got := templateIdentityForRuntimeOrEmpty("unknown-xyz"); got != "" { + if got := templateIdentityOrEmpty(resolveTemplateIdentity("", "unknown-xyz")); got != "" { t.Errorf("unknown-xyz should return empty, got %q", got) } + // Phase 1: explicit template wins over runtime fallback. + if got := templateIdentityOrEmpty(resolveTemplateIdentity("claude-code", "external")); got == "" { + t.Error("explicit claude-code template should return a non-empty identity even when runtime is external") + } + if got := templateIdentityOrEmpty(resolveTemplateIdentity("unknown-template-xyz", "claude-code")); got != "" { + t.Errorf("unknown explicit template should fail-closed to empty, got %q", got) + } +} + +// TestResolveTemplateIdentity pins the Phase 1 template-first resolver. +func TestResolveTemplateIdentity(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "manifest.json") + manifest := `{ + "workspace_templates": [ + {"name": "claude-code-default", "repo": "molecule-ai/t-cc", "ref": "main"}, + {"name": "seo-agent", "repo": "molecule-ai/t-seo", "ref": "v1"}, + {"name": "hermes", "repo": "molecule-ai/t-hermes", "ref": "v2"} + ] + }` + if err := os.WriteFile(p, []byte(manifest), 0600); err != nil { + t.Fatalf("write temp manifest: %v", err) + } + t.Setenv("WORKSPACE_MANIFEST_PATH", p) + initTemplateRepoByName() + + cases := []struct { + name string + template string + runtime string + wantOk bool + wantRepo string + }{ + {"template wins", "seo-agent", "claude-code", true, "molecule-ai/t-seo@v1"}, + {"template empty falls back to runtime", "", "claude-code", true, "molecule-ai/t-cc@main"}, + {"template empty external returns empty", "", "external", false, ""}, + {"unknown explicit template fail-closed", "no-such-template", "claude-code", false, ""}, + {"unknown runtime empty", "", "no-such-runtime", false, ""}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + id, ok := resolveTemplateIdentity(c.template, c.runtime) + if ok != c.wantOk { + t.Fatalf("template=%q runtime=%q: want ok=%v, got ok=%v", c.template, c.runtime, c.wantOk, ok) + } + if id != c.wantRepo { + t.Errorf("template=%q runtime=%q: want id=%q, got %q", c.template, c.runtime, c.wantRepo, id) + } + }) + } +} + +// TestIsKnownTemplate pins the manifest-entry gate used by PATCH /template. +func TestIsKnownTemplate(t *testing.T) { + dir := t.TempDir() + p := filepath.Join(dir, "manifest.json") + manifest := `{"workspace_templates": [{"name": "seo-agent", "repo": "r", "ref": "main"}]}` + if err := os.WriteFile(p, []byte(manifest), 0600); err != nil { + t.Fatalf("write temp manifest: %v", err) + } + t.Setenv("WORKSPACE_MANIFEST_PATH", p) + initTemplateRepoByName() + + if isKnownTemplate("") { + t.Error("empty template should not be known") + } + if !isKnownTemplate("seo-agent") { + t.Error("seo-agent should be known") + } + if isKnownTemplate("ghost") { + t.Error("ghost template should not be known") + } } // TestTemplateIdentityForTemplateOrRuntime is the #32 regression gate: a @@ -309,7 +380,7 @@ func TestInitTemplateRepoByName_PopulatesMap_FromTempManifest(t *testing.T) { // Assert the map is populated for the shipped runtimes. cases := []struct { - runtime string + runtime string wantRepo string wantRef string }{ diff --git a/workspace-server/internal/handlers/workspace.go b/workspace-server/internal/handlers/workspace.go index b74c9fab7..55bf252fb 100644 --- a/workspace-server/internal/handlers/workspace.go +++ b/workspace-server/internal/handlers/workspace.go @@ -681,10 +681,10 @@ func (h *WorkspaceHandler) Create(c *gin.Context) { // returns the actually-persisted name (which we MUST thread back into // payload + broadcast so the canvas displays what the DB has). const insertWorkspaceSQL = ` - INSERT INTO workspaces (id, name, role, tier, runtime, status, parent_id, workspace_dir, workspace_access, budget_limit, max_concurrent_tasks, delivery_mode) - VALUES ($1, $2, $3, $4, $5, 'provisioning', $6, $7, $8, $9, $10, $11) + INSERT INTO workspaces (id, name, role, tier, runtime, template, status, parent_id, workspace_dir, workspace_access, budget_limit, max_concurrent_tasks, delivery_mode) + VALUES ($1, $2, $3, $4, $5, $6, 'provisioning', $7, $8, $9, $10, $11, $12) ` - insertArgs := []any{id, payload.Name, role, payload.Tier, payload.Runtime, payload.ParentID, workspaceDir, workspaceAccess, payload.BudgetLimit, maxConcurrent, deliveryMode} + insertArgs := []any{id, payload.Name, role, payload.Tier, payload.Runtime, payload.Template, payload.ParentID, workspaceDir, workspaceAccess, payload.BudgetLimit, maxConcurrent, deliveryMode} persistedName, currentTx, err := insertWorkspaceWithNameRetry( ctx, tx, diff --git a/workspace-server/internal/handlers/workspace_budget_test.go b/workspace-server/internal/handlers/workspace_budget_test.go index 828a1794f..2f65c2169 100644 --- a/workspace-server/internal/handlers/workspace_budget_test.go +++ b/workspace-server/internal/handlers/workspace_budget_test.go @@ -152,6 +152,7 @@ func TestWorkspaceBudget_Create_WithLimit(t *testing.T) { nil, // role 3, // tier (default, workspace.go create-handler) "claude-code", // runtime + "", // template (*string)(nil), // parent_id nil, // workspace_dir "none", // workspace_access diff --git a/workspace-server/internal/handlers/workspace_crud.go b/workspace-server/internal/handlers/workspace_crud.go index b5f1d8f09..86591ca5d 100644 --- a/workspace-server/internal/handlers/workspace_crud.go +++ b/workspace-server/internal/handlers/workspace_crud.go @@ -752,6 +752,62 @@ func (h *WorkspaceHandler) CascadeDelete(ctx context.Context, id string, erase b return descendantIDs, stopErrs, nil } +// PatchTemplate handles PATCH /workspaces/:id/template. +// It sets the installed template for an existing workspace without changing +// its engine runtime. A restart/re-provision is required for the template +// assets to be fetched and applied. +// +// Auth: admin/CP-gated at the router (mirrors PATCH /workspaces/:id/budget). +func (h *WorkspaceHandler) PatchTemplate(c *gin.Context) { + id := c.Param("id") + if err := validateWorkspaceID(id); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + + var body struct { + Template string `json:"template" binding:"required"` + } + if err := c.ShouldBindJSON(&body); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": "template is required"}) + return + } + + if !isKnownTemplate(body.Template) { + log.Printf("PatchTemplate: %q is not a known workspace template", body.Template) + c.JSON(http.StatusUnprocessableEntity, gin.H{ + "error": "unsupported workspace template", + "template": body.Template, + "code": "TEMPLATE_UNSUPPORTED", + }) + return + } + + var exists bool + if err := db.DB.QueryRowContext(c.Request.Context(), + `SELECT EXISTS(SELECT 1 FROM workspaces WHERE id = $1)`, id, + ).Scan(&exists); err != nil { + log.Printf("PatchTemplate: existence check failed for %s: %v", id, err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "lookup failed"}) + return + } + if !exists { + c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found"}) + return + } + + if _, err := db.DB.ExecContext(c.Request.Context(), + `UPDATE workspaces SET template = $2, updated_at = now() WHERE id = $1`, id, body.Template, + ); err != nil { + log.Printf("PatchTemplate: update failed for %s: %v", id, err) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to update template"}) + return + } + + log.Printf("PatchTemplate: workspace %s template updated to %q", id, body.Template) + c.JSON(http.StatusOK, gin.H{"status": "updated", "needs_restart": true}) +} + // validateWorkspaceID returns an error when id is not a valid UUID. // #687: prevents 500s from Postgres when a garbage string (e.g. ../../etc/passwd) // is passed as the :id path parameter. diff --git a/workspace-server/internal/handlers/workspace_crud_test.go b/workspace-server/internal/handlers/workspace_crud_test.go index 65bdfdbdb..78c47a4f6 100644 --- a/workspace-server/internal/handlers/workspace_crud_test.go +++ b/workspace-server/internal/handlers/workspace_crud_test.go @@ -8,6 +8,7 @@ import ( "errors" "net/http" "net/http/httptest" + "os" "strings" "testing" @@ -751,6 +752,113 @@ func TestUpdate_Runtime_ModelUnresolved_SkipsCheckAndProceeds(t *testing.T) { } } +// TestPatchTemplate pins the admin/CP-gated PATCH /workspaces/:id/template endpoint. +func TestPatchTemplate(t *testing.T) { + wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + + // Wire a temp manifest so seo-agent is a known template. + dir := t.TempDir() + manifestPath := dir + "/manifest.json" + manifest := `{"workspace_templates": [{"name": "seo-agent", "repo": "molecule-ai/t-seo", "ref": "main"}]}` + if err := os.WriteFile(manifestPath, []byte(manifest), 0600); err != nil { + t.Fatalf("write manifest: %v", err) + } + t.Setenv("WORKSPACE_MANIFEST_PATH", manifestPath) + initTemplateRepoByName() + + mock, r := setupWorkspaceCrudTest(t) + h := newWorkspaceCrudHandler(t) + r.PATCH("/workspaces/:id/template", h.PatchTemplate) + + mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1\)`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(true)) + mock.ExpectExec(`UPDATE workspaces SET template = \$2, updated_at = now\(\) WHERE id = \$1`). + WithArgs(wsID, "seo-agent"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + body := map[string]interface{}{"template": "seo-agent"} + b, _ := json.Marshal(body) + req, _ := http.NewRequest("PATCH", "/workspaces/"+wsID+"/template", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var resp map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("parse response: %v", err) + } + if resp["status"] != "updated" { + t.Errorf("expected status=updated, got %v", resp["status"]) + } + if resp["needs_restart"] != true { + t.Errorf("expected needs_restart=true, got %v", resp["needs_restart"]) + } +} + +func TestPatchTemplate_UnknownTemplate_Fails422(t *testing.T) { + wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + + dir := t.TempDir() + manifestPath := dir + "/manifest.json" + manifest := `{"workspace_templates": [{"name": "seo-agent", "repo": "r", "ref": "main"}]}` + if err := os.WriteFile(manifestPath, []byte(manifest), 0600); err != nil { + t.Fatalf("write manifest: %v", err) + } + t.Setenv("WORKSPACE_MANIFEST_PATH", manifestPath) + initTemplateRepoByName() + + _, r := setupWorkspaceCrudTest(t) + h := newWorkspaceCrudHandler(t) + r.PATCH("/workspaces/:id/template", h.PatchTemplate) + + body := map[string]interface{}{"template": "ghost-template"} + b, _ := json.Marshal(body) + req, _ := http.NewRequest("PATCH", "/workspaces/"+wsID+"/template", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusUnprocessableEntity { + t.Errorf("expected 422, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestPatchTemplate_NotFound_Fails404(t *testing.T) { + wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + + dir := t.TempDir() + manifestPath := dir + "/manifest.json" + manifest := `{"workspace_templates": [{"name": "seo-agent", "repo": "r", "ref": "main"}]}` + if err := os.WriteFile(manifestPath, []byte(manifest), 0600); err != nil { + t.Fatalf("write manifest: %v", err) + } + t.Setenv("WORKSPACE_MANIFEST_PATH", manifestPath) + initTemplateRepoByName() + + mock, r := setupWorkspaceCrudTest(t) + h := newWorkspaceCrudHandler(t) + r.PATCH("/workspaces/:id/template", h.PatchTemplate) + + mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1\)`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"exists"}).AddRow(false)) + + body := map[string]interface{}{"template": "seo-agent"} + b, _ := json.Marshal(body) + req, _ := http.NewRequest("PATCH", "/workspaces/"+wsID+"/template", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + + if w.Code != http.StatusNotFound { + t.Errorf("expected 404, got %d: %s", w.Code, w.Body.String()) + } +} + // TestUpdate_Runtime_ModelSecretDBError_Fails500 pins that a genuine DB error // reading the MODEL workspace_secret is fail-closed (500). Only sql.ErrNoRows // (unresolved model) skips the strict compat-check; real DB/decrypt errors diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index b7dd50ea9..7b29785de 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -384,6 +384,7 @@ func (h *WorkspaceHandler) buildProvisionerConfig( return provisioner.WorkspaceConfig{ WorkspaceID: workspaceID, TemplatePath: templatePath, + Template: payload.Template, ConfigFiles: configFiles, PluginsPath: pluginsPath, WorkspacePath: workspacePath, @@ -418,34 +419,25 @@ func (h *WorkspaceHandler) buildProvisionerConfig( // reader's sql.ErrNoRows path was producing already. Image: "", - // RFC #2843 #24 PR-B — wire the generic template-asset - // channel. cfg.TemplateIdentity is derived from the - // runtime_registry (manifest.json's workspace_templates - // entry for this runtime) — the format is "@" - // (the giteaTemplateAssetFetcher parses this further as - // "/@"). External-like runtimes - // (external / kimi / kimi-cli / mock) have NO template - // repo, so the identity is left empty — the SCAFFOLD - // gate in collectCPConfigFiles treats empty identity as - // "skip the fetcher" (pre-scaffold behavior preserved). - // - // The fetcher itself is assigned by the caller (main.go - // for SaaS, or a test helper) via h.giteaTemplateFetcher - // — wired here so the fetcher resolution is one place, - // not duplicated across first-provision + restart paths. - // nil fetcher = "no fetcher wired" (self-host default; - // falls through to the local TemplatePath path). - TemplateIdentity: templateIdentityForTemplateOrRuntime(conciergeTemplateOrDefault(kind, payload.Template), payload.Runtime), + // RFC #2843 #24 PR-B + Phase 1 template decoupling: derive the + // template identity from the installed template first, falling back + // to the runtime's default template. For kind='platform' concierges + // with no explicit template, force "platform-agent" so the concierge + // persona/config is delivered (RFC §5.7; #30/#2970). The empty + // identity tells the SCAFFOLD gate in collectCPConfigFiles to skip + // the fetcher (external runtimes). + TemplateIdentity: templateIdentityOrEmpty(resolveTemplateIdentity(conciergeTemplateOrDefault(kind, payload.Template), payload.Runtime)), TemplateAssetFetcher: h.giteaTemplateFetcher, } } -// templateIdentityForRuntimeOrEmpty is a tiny wrapper around -// templateIdentityForRuntime that returns "" on miss (rather -// than the (string, bool) tuple). Used at the call site so -// the assignment can be a single expression. -func templateIdentityForRuntimeOrEmpty(runtime string) string { - id, _ := templateIdentityForRuntime(runtime) +// templateIdentityOrEmpty is a tiny wrapper around resolveTemplateIdentity +// that returns "" on miss (rather than the (string, bool) tuple). Used at the +// call site so the assignment can be a single expression. +func templateIdentityOrEmpty(id string, ok bool) string { + if !ok { + return "" + } return id } @@ -469,7 +461,7 @@ func templateIdentityForTemplateOrRuntime(template, runtime string) string { return id } } - return templateIdentityForRuntimeOrEmpty(runtime) + return templateIdentityOrEmpty(templateIdentityForRuntime(runtime)) } // issueAndInjectToken rotates the workspace auth token and injects the diff --git a/workspace-server/internal/handlers/workspace_restart.go b/workspace-server/internal/handlers/workspace_restart.go index fdb267b10..b70726014 100644 --- a/workspace-server/internal/handlers/workspace_restart.go +++ b/workspace-server/internal/handlers/workspace_restart.go @@ -315,11 +315,11 @@ func (h *WorkspaceHandler) Restart(c *gin.Context) { id := c.Param("id") ctx := c.Request.Context() - var status, wsName, dbRuntime string + var status, wsName, dbRuntime, dbTemplate string var tier int err := db.DB.QueryRowContext(ctx, - `SELECT status, name, tier, COALESCE(runtime, 'claude-code') FROM workspaces WHERE id = $1`, id, - ).Scan(&status, &wsName, &tier, &dbRuntime) + `SELECT status, name, tier, COALESCE(runtime, 'claude-code'), COALESCE(template, '') FROM workspaces WHERE id = $1`, id, + ).Scan(&status, &wsName, &tier, &dbRuntime, &dbTemplate) if err == sql.ErrNoRows { c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found"}) return @@ -418,7 +418,7 @@ func (h *WorkspaceHandler) Restart(c *gin.Context) { "runtime": containerRuntime, }) - templatePath, configLabel := resolveRestartTemplate(h.configsDir, wsName, dbRuntime, restartTemplateInput{ + templatePath, configLabel := resolveRestartTemplate(h.configsDir, wsName, dbRuntime, dbTemplate, restartTemplateInput{ Template: body.Template, ApplyTemplate: body.ApplyTemplate, RebuildConfig: body.RebuildConfig, @@ -442,8 +442,8 @@ func (h *WorkspaceHandler) Restart(c *gin.Context) { } var configFiles map[string][]byte - payload := withStoredCompute(ctx, id, models.CreateWorkspacePayload{Name: wsName, Tier: tier, Runtime: containerRuntime}) - log.Printf("Restart: workspace %s (%s) runtime=%q", wsName, id, containerRuntime) + payload := withStoredCompute(ctx, id, models.CreateWorkspacePayload{Name: wsName, Tier: tier, Runtime: containerRuntime, Template: dbTemplate}) + log.Printf("Restart: workspace %s (%s) runtime=%q template=%q", wsName, id, containerRuntime, dbTemplate) // #12: ?reset=true (or body.Reset) discards the claude-sessions volume // before restart, giving the agent a clean /root/.claude/sessions dir. @@ -931,11 +931,11 @@ func (h *WorkspaceHandler) runRestartCycle(workspaceID string) { gate.Lock() defer gate.Unlock() - var wsName, status, dbRuntime string + var wsName, status, dbRuntime, dbTemplate string var tier int err := db.DB.QueryRowContext(ctx, - `SELECT name, status, tier, COALESCE(runtime, 'claude-code') FROM workspaces WHERE id = $1 AND status NOT IN ('removed', 'paused', 'hibernated')`, workspaceID, - ).Scan(&wsName, &status, &tier, &dbRuntime) + `SELECT name, status, tier, COALESCE(runtime, 'claude-code'), COALESCE(template, '') FROM workspaces WHERE id = $1 AND status NOT IN ('removed', 'paused', 'hibernated')`, workspaceID, + ).Scan(&wsName, &status, &tier, &dbRuntime, &dbTemplate) if err != nil { return // includes paused/hibernated — don't auto-restart those } @@ -958,7 +958,7 @@ func (h *WorkspaceHandler) runRestartCycle(workspaceID string) { time.Sleep(10 * time.Second) } - log.Printf("Auto-restart: restarting %s (%s) runtime=%q (was: %s)", wsName, workspaceID, dbRuntime, status) + log.Printf("Auto-restart: restarting %s (%s) runtime=%q template=%q (was: %s)", wsName, workspaceID, dbRuntime, dbTemplate, status) // #125 Phase 1: send pre-restart drain signal to the workspace agent. // For native_session targets, A2A messages go directly to the SDK session @@ -979,11 +979,11 @@ func (h *WorkspaceHandler) runRestartCycle(workspaceID string) { log.Printf("Auto-restart: failed to set provisioning status for %s: %v", workspaceID, err) } h.broadcaster.RecordAndBroadcast(ctx, string(events.EventWorkspaceProvisioning), workspaceID, map[string]interface{}{ - "name": wsName, "tier": tier, "runtime": dbRuntime, + "name": wsName, "tier": tier, "runtime": dbRuntime, "template": dbTemplate, }) // Runtime from DB — no more config file parsing - payload := withStoredCompute(ctx, workspaceID, models.CreateWorkspacePayload{Name: wsName, Tier: tier, Runtime: dbRuntime}) + payload := withStoredCompute(ctx, workspaceID, models.CreateWorkspacePayload{Name: wsName, Tier: tier, Runtime: dbRuntime, Template: dbTemplate}) // RFC#2843 #33 + SaaS restart re-stub fix: on SaaS (cpProv), restore the // persisted template AND resolve its LOCAL template dir so the re-provision @@ -1018,6 +1018,18 @@ func (h *WorkspaceHandler) runRestartCycle(workspaceID string) { } } + // RFC#2843 #33: on SaaS (cpProv), restore the persisted template so the + // re-provision re-delivers config.yaml + prompts — TemplateIdentity is + // derived from payload.Template (workspace_provision.go). Without this the + // SaaS re-provision ran with template="" → 218-byte stub config + dropped + // skills on every restart. Docker keeps its persistent config volume, so it + // retains the "do not re-apply templates" behavior (template left empty). + if h.cpProv != nil { + if storedTmpl := storedWorkspaceTemplate(ctx, workspaceID); storedTmpl != "" { + payload.Template = storedTmpl + } + } + // Snapshot restart-context data before the new session overwrites // last_heartbeat_at. Issue #19 Layer 1. restartData := loadRestartContextData(ctx, workspaceID) @@ -1142,11 +1154,11 @@ func (h *WorkspaceHandler) Resume(c *gin.Context) { id := c.Param("id") ctx := c.Request.Context() - var wsName, dbRuntime string + var wsName, dbRuntime, dbTemplate string var tier int err := db.DB.QueryRowContext(ctx, - `SELECT name, tier, COALESCE(runtime, 'claude-code') FROM workspaces WHERE id = $1 AND status = 'paused'`, id, - ).Scan(&wsName, &tier, &dbRuntime) + `SELECT name, tier, COALESCE(runtime, 'claude-code'), COALESCE(template, '') FROM workspaces WHERE id = $1 AND status = 'paused'`, id, + ).Scan(&wsName, &tier, &dbRuntime, &dbTemplate) if err == sql.ErrNoRows { c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found or not paused"}) return @@ -1172,17 +1184,17 @@ func (h *WorkspaceHandler) Resume(c *gin.Context) { // Collect this workspace + all paused descendants to resume type wsInfo struct { - id, name, runtime string - tier int + id, name, runtime, template string + tier int } - toResume := []wsInfo{{id, wsName, dbRuntime, tier}} + toResume := []wsInfo{{id, wsName, dbRuntime, dbTemplate, tier}} var descendantList []gin.H rows, err := db.DB.QueryContext(ctx, `WITH RECURSIVE descendants AS ( - SELECT id, name, tier, COALESCE(runtime, 'claude-code') AS runtime FROM workspaces WHERE parent_id = $1 AND status = 'paused' + SELECT id, name, tier, COALESCE(runtime, 'claude-code') AS runtime, COALESCE(template, '') AS template FROM workspaces WHERE parent_id = $1 AND status = 'paused' UNION ALL - SELECT w.id, w.name, w.tier, COALESCE(w.runtime, 'claude-code') FROM workspaces w JOIN descendants d ON w.parent_id = d.id WHERE w.status = 'paused' - ) SELECT id, name, tier, runtime FROM descendants`, id) + SELECT w.id, w.name, w.tier, COALESCE(w.runtime, 'claude-code'), COALESCE(w.template, '') FROM workspaces w JOIN descendants d ON w.parent_id = d.id WHERE w.status = 'paused' + ) SELECT id, name, tier, runtime, template FROM descendants`, id) if err != nil { log.Printf("Resume: descendant query failed for %s: %v", id, err) } @@ -1190,7 +1202,7 @@ func (h *WorkspaceHandler) Resume(c *gin.Context) { defer rows.Close() for rows.Next() { var ws wsInfo - if rows.Scan(&ws.id, &ws.name, &ws.tier, &ws.runtime) == nil { + if rows.Scan(&ws.id, &ws.name, &ws.tier, &ws.runtime, &ws.template) == nil { toResume = append(toResume, ws) descendantList = append(descendantList, gin.H{"id": ws.id, "name": ws.name}) } @@ -1216,12 +1228,14 @@ func (h *WorkspaceHandler) Resume(c *gin.Context) { log.Printf("Resume: failed to set provisioning status for %s: %v", ws.id, err) } h.broadcaster.RecordAndBroadcast(ctx, string(events.EventWorkspaceProvisioning), ws.id, map[string]interface{}{ - "name": ws.name, "tier": ws.tier, "runtime": ws.runtime, + "name": ws.name, "tier": ws.tier, "runtime": ws.runtime, "template": ws.template, }) - payload := withStoredCompute(ctx, ws.id, models.CreateWorkspacePayload{Name: ws.name, Tier: ws.tier, Runtime: ws.runtime}) - // RFC#2843 #33: restore the persisted template on SaaS resume so config + - // prompts re-deliver (see runRestartCycle for the full rationale). - if h.cpProv != nil { + // Phase 1 template decoupling: the workspace row stores the template + // explicitly, so resume carries it through CreateWorkspacePayload. + payload := withStoredCompute(ctx, ws.id, models.CreateWorkspacePayload{Name: ws.name, Tier: ws.tier, Runtime: ws.runtime, Template: ws.template}) + // RFC#2843 #33: if the row template is empty (legacy row), restore the + // persisted template on SaaS resume so config + prompts re-deliver. + if payload.Template == "" && h.cpProv != nil { if storedTmpl := storedWorkspaceTemplate(ctx, ws.id); storedTmpl != "" { payload.Template = storedTmpl } diff --git a/workspace-server/internal/handlers/workspace_restart_test.go b/workspace-server/internal/handlers/workspace_restart_test.go index 92c1e39c1..22f85689c 100644 --- a/workspace-server/internal/handlers/workspace_restart_test.go +++ b/workspace-server/internal/handlers/workspace_restart_test.go @@ -78,8 +78,8 @@ func TestRestartHandler_RemovedWorkspaceReturns404(t *testing.T) { mock.ExpectQuery("SELECT status, name, tier, COALESCE"). WithArgs("ws-removed"). - WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}). - AddRow("removed", "Removed Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime", "template"}). + AddRow("removed", "Removed Agent", 1, "claude-code", "")) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) @@ -106,8 +106,8 @@ func TestRestartHandler_AncestorPausedBlocksRestart(t *testing.T) { // Lookup workspace mock.ExpectQuery("SELECT status, name, tier, COALESCE"). WithArgs("ws-grandchild"). - WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}). - AddRow("offline", "Grandchild Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime", "template"}). + AddRow("offline", "Grandchild Agent", 1, "claude-code", "")) // isParentPaused: get parent_id of grandchild -> child mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id ="). @@ -163,8 +163,8 @@ func TestRestartHandler_ExternalRuntimeNoOps(t *testing.T) { mock.ExpectQuery("SELECT status, name, tier, COALESCE"). WithArgs("ws-external"). - WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}). - AddRow("offline", "External Agent", 1, "external")) + WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime", "template"}). + AddRow("offline", "External Agent", 1, "external", "")) // isParentPaused: no parent mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id ="). @@ -214,8 +214,8 @@ func TestRestartHandler_KimiRuntimeNoOps(t *testing.T) { mock.ExpectQuery("SELECT status, name, tier, COALESCE"). WithArgs("ws-kimi"). - WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}). - AddRow("offline", "Kimi Agent", 1, "kimi-cli")) + WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime", "template"}). + AddRow("offline", "Kimi Agent", 1, "kimi-cli", "")) mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id ="). WithArgs("ws-kimi"). @@ -259,8 +259,8 @@ func TestRestartHandler_NilProvisionerReturns503(t *testing.T) { mock.ExpectQuery("SELECT status, name, tier, COALESCE"). WithArgs("ws-no-prov"). - WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime"}). - AddRow("offline", "Test Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"status", "name", "tier", "runtime", "template"}). + AddRow("offline", "Test Agent", 1, "claude-code", "")) // isParentPaused: no parent mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id ="). @@ -528,8 +528,8 @@ func TestResumeHandler_NilProvisionerReturns503(t *testing.T) { mock.ExpectQuery("SELECT name, tier, COALESCE"). WithArgs("ws-resume-noprov"). - WillReturnRows(sqlmock.NewRows([]string{"name", "tier", "runtime"}). - AddRow("Test Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"name", "tier", "runtime", "template"}). + AddRow("Test Agent", 1, "claude-code", "")) // provisioner nil check happens BEFORE isParentPaused, so no parent query expected @@ -562,8 +562,8 @@ func TestResumeHandler_DescendantsNoCascadeReturns409(t *testing.T) { mock.ExpectQuery("SELECT name, tier, COALESCE"). WithArgs("ws-resume-parent"). - WillReturnRows(sqlmock.NewRows([]string{"name", "tier", "runtime"}). - AddRow("Parent Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"name", "tier", "runtime", "template"}). + AddRow("Parent Agent", 1, "claude-code", "")) // isParentPaused: no parent mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id ="). @@ -572,9 +572,9 @@ func TestResumeHandler_DescendantsNoCascadeReturns409(t *testing.T) { mock.ExpectQuery("WITH RECURSIVE descendants"). WithArgs("ws-resume-parent"). - WillReturnRows(sqlmock.NewRows([]string{"id", "name", "tier", "runtime"}). - AddRow("ws-child-1", "Child 1", 1, "claude-code"). - AddRow("ws-child-2", "Child 2", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"id", "name", "tier", "runtime", "template"}). + AddRow("ws-child-1", "Child 1", 1, "claude-code", ""). + AddRow("ws-child-2", "Child 2", 1, "claude-code", "")) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) @@ -610,8 +610,8 @@ func TestResumeHandler_DescendantsWithCascadeReturns200(t *testing.T) { mock.ExpectQuery("SELECT name, tier, COALESCE"). WithArgs("ws-resume-parent-cascade"). - WillReturnRows(sqlmock.NewRows([]string{"name", "tier", "runtime"}). - AddRow("Parent Agent", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"name", "tier", "runtime", "template"}). + AddRow("Parent Agent", 1, "claude-code", "")) // isParentPaused: no parent mock.ExpectQuery("SELECT parent_id FROM workspaces WHERE id ="). @@ -620,9 +620,9 @@ func TestResumeHandler_DescendantsWithCascadeReturns200(t *testing.T) { mock.ExpectQuery("WITH RECURSIVE descendants"). WithArgs("ws-resume-parent-cascade"). - WillReturnRows(sqlmock.NewRows([]string{"id", "name", "tier", "runtime"}). - AddRow("ws-child-1", "Child 1", 1, "claude-code"). - AddRow("ws-child-2", "Child 2", 1, "claude-code")) + WillReturnRows(sqlmock.NewRows([]string{"id", "name", "tier", "runtime", "template"}). + AddRow("ws-child-1", "Child 1", 1, "claude-code", ""). + AddRow("ws-child-2", "Child 2", 1, "claude-code", "")) for _, wsID := range []string{"ws-resume-parent-cascade", "ws-child-1", "ws-child-2"} { mock.ExpectExec("UPDATE workspaces SET status ="). diff --git a/workspace-server/internal/handlers/workspace_switch_provider.go b/workspace-server/internal/handlers/workspace_switch_provider.go index 96d0a0377..8e8aacb31 100644 --- a/workspace-server/internal/handlers/workspace_switch_provider.go +++ b/workspace-server/internal/handlers/workspace_switch_provider.go @@ -59,15 +59,15 @@ func (h *WorkspaceHandler) SwitchProvider(c *gin.Context) { return } - var status, wsName, dbRuntime, oldProvider, dataPersistence string + var status, wsName, dbRuntime, dbTemplate, oldProvider, dataPersistence string var oldInstanceID sql.NullString var tier int err := db.DB.QueryRowContext(ctx, ` - SELECT status, name, tier, COALESCE(runtime, 'claude-code'), + SELECT status, name, tier, COALESCE(runtime, 'claude-code'), COALESCE(template, ''), COALESCE(compute->>'provider', ''), COALESCE(compute->>'data_persistence', ''), instance_id FROM workspaces WHERE id = $1`, id, - ).Scan(&status, &wsName, &tier, &dbRuntime, &oldProvider, &dataPersistence, &oldInstanceID) + ).Scan(&status, &wsName, &tier, &dbRuntime, &dbTemplate, &oldProvider, &dataPersistence, &oldInstanceID) if err == sql.ErrNoRows || status == string(models.StatusRemoved) { c.JSON(http.StatusNotFound, gin.H{"error": "workspace not found"}) return @@ -251,7 +251,7 @@ func (h *WorkspaceHandler) SwitchProvider(c *gin.Context) { // context: the reprovision outlives the request. Routes through // provisionWorkspaceAuto (not provisionWorkspaceCP directly) per // TestNoCallSiteCallsDirectProvisionerExceptAuto (core#2422 RCA tick). - payload := withStoredCompute(context.Background(), id, models.CreateWorkspacePayload{Name: wsName, Tier: tier, Runtime: dbRuntime}) + payload := withStoredCompute(context.Background(), id, models.CreateWorkspacePayload{Name: wsName, Tier: tier, Runtime: dbRuntime, Template: dbTemplate}) h.provisionWorkspaceAuto(id, "", nil, payload) // All 5 steps completed; mark the switch COMMITTED so the rollback diff --git a/workspace-server/internal/handlers/workspace_test.go b/workspace-server/internal/handlers/workspace_test.go index f6bce8645..2f16cc19a 100644 --- a/workspace-server/internal/handlers/workspace_test.go +++ b/workspace-server/internal/handlers/workspace_test.go @@ -343,7 +343,7 @@ func TestWorkspaceCreate_DBInsertError(t *testing.T) { // Transaction begins, workspace INSERT fails, transaction is rolled back. mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Failing Agent", nil, 3, "claude-code", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Failing Agent", nil, 3, "claude-code", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnError(sql.ErrConnDone) mock.ExpectRollback() @@ -376,7 +376,7 @@ func TestWorkspaceCreate_DefaultsApplied(t *testing.T) { // Expect workspace INSERT with defaulted tier=3 (Privileged — the // handler default in workspace.go), runtime="claude-code" mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Default Agent", nil, 3, "claude-code", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Default Agent", nil, 3, "claude-code", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO workspace_secrets"). @@ -428,7 +428,7 @@ func TestWorkspaceCreate_SaaSHardForcesTier4(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "SaaS External Agent", nil, 4, "external", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "SaaS External Agent", nil, 4, "external", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO canvas_layouts"). @@ -472,7 +472,7 @@ func TestWorkspaceCreate_WithSecrets_Persists(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "External Agent", nil, 3, "external", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "External Agent", nil, 3, "external", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) // Secret inserted inside the same transaction. mock.ExpectExec("INSERT INTO workspace_secrets"). @@ -597,7 +597,7 @@ func TestWorkspaceCreate_ExternalURL_SSRFSafe(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Ext Agent", nil, 3, "external", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Ext Agent", nil, 3, "external", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() // External URL update (localhost is explicitly allowed by validateAgentURL). @@ -637,7 +637,7 @@ func TestWorkspaceCreate_KimiRuntime_PreservesLabel(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Kimi Agent", nil, 3, "kimi", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Kimi Agent", nil, 3, "kimi", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() // Pre-register flow: awaiting_agent + runtime preserved as "kimi" @@ -702,7 +702,7 @@ func TestWorkspaceCreate_ExternalFlagDefaultsRuntimeExternal(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "External Agent", nil, 3, "external", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "External Agent", nil, 3, "external", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("UPDATE workspaces SET status"). @@ -1818,7 +1818,7 @@ runtime_config: // and hand the completed values to the INSERT. mock.ExpectExec("INSERT INTO workspaces"). WithArgs( - sqlmock.AnyArg(), "Hermes Agent", nil, 3, "hermes", + sqlmock.AnyArg(), "Hermes Agent", nil, 3, "hermes", "hermes-template", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() @@ -1877,7 +1877,7 @@ model: moonshot/kimi-k2.5 // this assertion should flip back to 1. mock.ExpectExec("INSERT INTO workspaces"). WithArgs( - sqlmock.AnyArg(), "Legacy Agent", nil, 3, "hermes", + sqlmock.AnyArg(), "Legacy Agent", nil, 3, "hermes", "legacy-template", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() @@ -1934,7 +1934,7 @@ runtime_config: // absence of a handler error to mean the model passthrough was honored. mock.ExpectExec("INSERT INTO workspaces"). WithArgs( - sqlmock.AnyArg(), "Custom Hermes", nil, 3, "hermes", + sqlmock.AnyArg(), "Custom Hermes", nil, 3, "hermes", "hermes-template", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() @@ -2232,7 +2232,7 @@ func TestWorkspaceCreate_188_ExplicitRuntimeNoTemplate_OK(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). - WithArgs(sqlmock.AnyArg(), "Explicit Codex", nil, 3, "codex", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). + WithArgs(sqlmock.AnyArg(), "Explicit Codex", nil, 3, "codex", "", (*string)(nil), nil, "none", (*int64)(nil), models.DefaultMaxConcurrentTasks, "push"). WillReturnResult(sqlmock.NewResult(0, 1)) mock.ExpectCommit() mock.ExpectExec("INSERT INTO canvas_layouts"). diff --git a/workspace-server/internal/provisioner/cp_provisioner.go b/workspace-server/internal/provisioner/cp_provisioner.go index b9326107d..191fedbd6 100644 --- a/workspace-server/internal/provisioner/cp_provisioner.go +++ b/workspace-server/internal/provisioner/cp_provisioner.go @@ -159,6 +159,7 @@ type cpProvisionRequest struct { OrgID string `json:"org_id"` WorkspaceID string `json:"workspace_id"` Runtime string `json:"runtime"` + Template string `json:"template,omitempty"` Tier int `json:"tier"` InstanceType string `json:"instance_type,omitempty"` DiskGB int32 `json:"disk_gb,omitempty"` @@ -295,6 +296,7 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, OrgID: p.orgID, WorkspaceID: cfg.WorkspaceID, Runtime: cfg.Runtime, + Template: cfg.Template, Tier: cfg.Tier, InstanceType: cfg.InstanceType, DiskGB: cfg.DiskGB, diff --git a/workspace-server/internal/provisioner/provision_request.contract.json b/workspace-server/internal/provisioner/provision_request.contract.json index c26c0a51c..5855c8403 100644 --- a/workspace-server/internal/provisioner/provision_request.contract.json +++ b/workspace-server/internal/provisioner/provision_request.contract.json @@ -24,6 +24,7 @@ "org_id": {"type": "string", "cp_consumes": true}, "workspace_id": {"type": "string", "cp_consumes": true}, "runtime": {"type": "string", "cp_consumes": true}, + "template": {"type": "string", "cp_consumes": true}, "tier": {"type": "int", "cp_consumes": true}, "instance_type": {"type": "string", "cp_consumes": true}, "disk_gb": {"type": "int", "cp_consumes": true}, diff --git a/workspace-server/internal/provisioner/provisioner.go b/workspace-server/internal/provisioner/provisioner.go index 8cf1021c1..d598bdd08 100644 --- a/workspace-server/internal/provisioner/provisioner.go +++ b/workspace-server/internal/provisioner/provisioner.go @@ -112,6 +112,7 @@ const ( type WorkspaceConfig struct { WorkspaceID string TemplatePath string // Host path to template dir to copy from (e.g. claude-code-default/) + Template string // RFC #2948 Phase 1: installed template name, distinct from engine runtime. TemplateIdentity string // RFC #2843 #24: opaque token the TemplateAssetFetcher resolves to the template repo+ref (e.g. "claudius-v1.2.3" or a sha). Used by SaaS; ignored by the local-dir TemplatePath path. ConfigFiles map[string][]byte // Generated config files to write into /configs volume PluginsPath string // Host path to plugins directory (mounted at /plugins) diff --git a/workspace-server/internal/router/router.go b/workspace-server/internal/router/router.go index 6deab403a..b245e9f25 100644 --- a/workspace-server/internal/router/router.go +++ b/workspace-server/internal/router/router.go @@ -527,6 +527,7 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi budgeth := handlers.NewBudgetHandler() wsAuth.GET("/budget", budgeth.GetBudget) r.PATCH("/workspaces/:id/budget", middleware.AdminAuth(db.DB), budgeth.PatchBudget) + r.PATCH("/workspaces/:id/template", middleware.AdminAuth(db.DB), wh.PatchTemplate) // Token management (user-facing create/list/revoke) tokh := handlers.NewTokenHandler() diff --git a/workspace-server/migrations/20260616000000_workspaces_template.down.sql b/workspace-server/migrations/20260616000000_workspaces_template.down.sql new file mode 100644 index 000000000..eb6fa2224 --- /dev/null +++ b/workspace-server/migrations/20260616000000_workspaces_template.down.sql @@ -0,0 +1,3 @@ +-- Revert workspaces.template addition. +ALTER TABLE workspaces + DROP COLUMN IF EXISTS template; diff --git a/workspace-server/migrations/20260616000000_workspaces_template.up.sql b/workspace-server/migrations/20260616000000_workspaces_template.up.sql new file mode 100644 index 000000000..46adf8ac3 --- /dev/null +++ b/workspace-server/migrations/20260616000000_workspaces_template.up.sql @@ -0,0 +1,18 @@ +-- Add workspaces.template to decouple installed template from engine runtime. +-- Phase 1 of RFC #2948; closes the rejected PATCH runtime=seo-agent path. +-- +-- Empty string means "no installed template — resolve assets from runtime". +-- NOT NULL DEFAULT '' keeps the Go model a plain string and preserves every +-- existing workspace without a backfill step for the column itself. +ALTER TABLE workspaces + ADD COLUMN IF NOT EXISTS template TEXT NOT NULL DEFAULT ''; + +-- Backfill template from workspace_config.data when it was already recorded +-- at create time (e.g. TemplatePalette / org import flows that stored it +-- there but did not persist it on the workspaces row). +UPDATE workspaces w +SET template = COALESCE(NULLIF(c.data ->> 'template', ''), w.template) +FROM workspace_config c +WHERE c.workspace_id = w.id + AND w.template = '' + AND c.data ->> 'template' IS NOT NULL;