From 44469f9835cd75d95f2178fd3317a4715d482c51 Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer A (Kimi)" Date: Mon, 15 Jun 2026 18:33:39 +0000 Subject: [PATCH] fix(handlers): reject PATCH runtime with template variant slugs A live customer conversion (JRS seo-template) set runtime="seo-agent" on PATCH /workspaces/:id. "seo-agent" is a claude-code template variant, not a runtime; persisting it wedges the workspace because no adapter recognizes the pseudo-runtime. Changes: - manifest.json: add optional "runtime":"claude-code" to the seo-agent workspace_template entry so variant templates resolve to their base runtime. - runtime_registry.go: add Runtime to manifestEntry; loadRuntimesFromManifest uses the explicit base runtime when present. Add isKnownRuntime helper. - workspace_crud.go: PATCH /workspaces/:id now validates runtime is a known runtime before the (runtime, model) compatibility check, returning 422 RUNTIME_UNSUPPORTED for unknown/template-variant slugs. - Tests cover manifest variant normalization and PATCH rejection. Test plan: - go test ./workspace-server/internal/handlers/ -count=1 - go vet ./... && go build ./... Co-Authored-By: Claude --- manifest.json | 2 +- .../internal/handlers/runtime_registry.go | 33 +++++++++++++-- .../handlers/runtime_registry_test.go | 40 +++++++++++++++++++ .../internal/handlers/workspace_crud.go | 27 +++++++++++-- .../internal/handlers/workspace_crud_test.go | 37 +++++++++++++++++ 5 files changed, 132 insertions(+), 7 deletions(-) diff --git a/manifest.json b/manifest.json index 6d3fd6f15..b115dc13b 100644 --- a/manifest.json +++ b/manifest.json @@ -31,7 +31,7 @@ {"name": "openclaw", "repo": "molecule-ai/molecule-ai-workspace-template-openclaw", "ref": "143e69b56f2530433141f5a87373e8a76578c52e"}, {"name": "codex", "repo": "molecule-ai/molecule-ai-workspace-template-codex", "ref": "070447a0afdf66ae6f2bb166ac3e2b2884456951"}, {"name": "google-adk", "repo": "molecule-ai/molecule-ai-workspace-template-google-adk", "ref": "3f9fd7ef6ea4dd912bb65446607f3c3c991ea76e"}, - {"name": "seo-agent", "repo": "molecule-ai/molecule-ai-workspace-template-seo-agent", "ref": "51bee3c0de03c7d38ddc153e7b9dc70e19ededd6"} + {"name": "seo-agent", "repo": "molecule-ai/molecule-ai-workspace-template-seo-agent", "ref": "51bee3c0de03c7d38ddc153e7b9dc70e19ededd6", "runtime": "claude-code"} ], "org_templates": [ {"name": "molecule-dev", "repo": "molecule-ai/molecule-ai-org-template-molecule-dev", "ref": "990d7b23f65dadd7afe05958a77eeb74082b4feb"}, diff --git a/workspace-server/internal/handlers/runtime_registry.go b/workspace-server/internal/handlers/runtime_registry.go index 844ba7576..d88a90a1f 100644 --- a/workspace-server/internal/handlers/runtime_registry.go +++ b/workspace-server/internal/handlers/runtime_registry.go @@ -59,9 +59,10 @@ func manifestPath() string { // manifestEntry mirrors the shape of a workspace_templates item. // Only the fields we read are declared; extras are ignored. type manifestEntry struct { - Name string `json:"name"` - Repo string `json:"repo"` - Ref string `json:"ref"` + Name string `json:"name"` + Repo string `json:"repo"` + Ref string `json:"ref"` + Runtime string `json:"runtime"` // optional base runtime identifier for template variants (e.g. "seo-agent" → "claude-code") } type manifestFile struct { @@ -190,7 +191,13 @@ func loadRuntimesFromManifest(path string) (map[string]struct{}, error) { // Convention: "-default" is the vanilla variant of // . Strip the suffix so both `claude-code` and // `claude-code-default` resolve to the same runtime. + // If the manifest entry declares an explicit `runtime`, use it + // as the base runtime identifier (e.g. the "seo-agent" template + // is a claude-code variant, not a runtime of its own). name = strings.TrimSuffix(name, "-default") + if strings.TrimSpace(e.Runtime) != "" { + name = strings.TrimSpace(e.Runtime) + } out[name] = struct{}{} } return out, nil @@ -247,6 +254,26 @@ func initKnownRuntimes() { log.Printf("runtime registry: loaded %d runtimes from %s: %v", len(loaded), path, names) } +// isKnownRuntime reports whether runtime is a recognized workspace runtime +// (first-party template-backed, external-like meta-runtime, or mock). +// Safe to call before initKnownRuntimes — falls back to the compile-time +// fallbackRuntimes map. +func isKnownRuntime(runtime string) bool { + if _, ok := knownRuntimes[runtime]; ok { + return true + } + // externalLikeRuntimes and mock are always valid, even when the manifest + // is missing and knownRuntimes is still the fallback set (which already + // includes them, but re-checking is cheap and explicit). + if isExternalLikeRuntime(runtime) { + return true + } + if runtime == "mock" { + return true + } + return false +} + // templateRepoRef is the parsed manifest entry needed to // derive a TemplateIdentity for the Gitea fetcher. The // identity is "@" (the giteaTemplateAssetFetcher diff --git a/workspace-server/internal/handlers/runtime_registry_test.go b/workspace-server/internal/handlers/runtime_registry_test.go index 7db0badf9..2ab133daf 100644 --- a/workspace-server/internal/handlers/runtime_registry_test.go +++ b/workspace-server/internal/handlers/runtime_registry_test.go @@ -55,6 +55,34 @@ func TestLoadRuntimesFromManifest_StripsDefaultSuffix(t *testing.T) { } } +func TestLoadRuntimesFromManifest_UsesExplicitRuntimeForVariants(t *testing.T) { + // Template variants such as "seo-agent" declare an explicit base + // runtime in manifest.json. The variant name must NOT become a + // standalone runtime identifier, or PATCH /workspaces/:id could + // persist a pseudo-runtime that no adapter recognizes. + dir := t.TempDir() + path := filepath.Join(dir, "manifest.json") + err := os.WriteFile(path, []byte(`{ + "workspace_templates": [ + {"name": "claude-code-default", "repo": "org/t-cc"}, + {"name": "seo-agent", "repo": "org/t-seo", "runtime": "claude-code"} + ] + }`), 0600) + if err != nil { + t.Fatalf("write: %v", err) + } + got, err := loadRuntimesFromManifest(path) + if err != nil { + t.Fatalf("load: %v", err) + } + if _, ok := got["claude-code"]; !ok { + t.Errorf("expected base runtime 'claude-code' in set, got=%v", keys(got)) + } + if _, ok := got["seo-agent"]; ok { + t.Errorf("template variant 'seo-agent' must NOT be exposed as a runtime, got=%v", keys(got)) + } +} + func TestLoadRuntimesFromManifest_ExternalAlwaysInjected(t *testing.T) { // Even a manifest without external (which matches reality — // external has no template repo) must still produce "external" @@ -115,6 +143,11 @@ func TestRealManifestParses(t *testing.T) { t.Errorf("real manifest should not expose unsupported runtime %q — got=%v", removed, keys(got)) } } + for _, variant := range templateVariantNamesForTest() { + if _, ok := got[variant]; ok { + t.Errorf("real manifest should not expose template variant %q as a runtime — got=%v", variant, keys(got)) + } + } } func retiredRuntimeNamesForTest() []string { @@ -127,6 +160,13 @@ func retiredRuntimeNamesForTest() []string { } } +// templateVariantNamesForTest are template slugs that must NOT be exposed as +// standalone runtime identifiers. They are selected via the `template` field +// (or manifest runtime mapping) and resolve to a base runtime at create time. +func templateVariantNamesForTest() []string { + return []string{"seo-agent"} +} + func keys(m map[string]struct{}) []string { out := make([]string, 0, len(m)) for k := range m { diff --git a/workspace-server/internal/handlers/workspace_crud.go b/workspace-server/internal/handlers/workspace_crud.go index 030519dde..94e198081 100644 --- a/workspace-server/internal/handlers/workspace_crud.go +++ b/workspace-server/internal/handlers/workspace_crud.go @@ -248,6 +248,27 @@ func (h *WorkspaceHandler) Update(c *gin.Context) { } needsRestart := false if runtime, ok := body["runtime"]; ok { + // Reject non-string or unrecognized runtime values before the model- + // compatibility check. Prevents template slugs such as "seo-agent" + // (a claude-code template variant) from being persisted as a runtime, + // which wedges the workspace on the next boot because no adapter + // recognizes the pseudo-runtime. Matches the create-boundary's + // knownRuntimes gate (workspace.go:427). + runtimeStr, typeOK := runtime.(string) + if !typeOK { + log.Printf("Update: PATCH runtime on %s REJECTED (not a string)", id) + c.JSON(http.StatusBadRequest, gin.H{"error": "runtime must be a string"}) + return + } + if !isKnownRuntime(runtimeStr) { + log.Printf("Update: PATCH runtime=%q on %s REJECTED (unknown runtime)", runtimeStr, id) + c.JSON(http.StatusUnprocessableEntity, gin.H{ + "error": "unsupported workspace runtime", + "runtime": runtimeStr, + "code": "RUNTIME_UNSUPPORTED", + }) + return + } // (runtime, model) compatibility validation (the new field a PATCH can // change). model is NOT patchable per the body whitelist above, so the // post-PATCH model is the workspace's CURRENT model — fetched here @@ -266,9 +287,9 @@ func (h *WorkspaceHandler) Update(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to read current model for runtime compatibility check"}) return } - if ok, reason := validateRegisteredModelForRuntime(runtime.(string), currentModel.String); !ok { + if ok, reason := validateRegisteredModelForRuntime(runtimeStr, currentModel.String); !ok { log.Printf("Update: PATCH runtime=%q on %s REJECTED (model=%q is not registered for that runtime): %s", - runtime.(string), id, currentModel.String, reason) + runtimeStr, id, currentModel.String, reason) // 422 (Unprocessable Entity) matches the create-boundary's // validateRegisteredModelForRuntime path (secrets.go:942, 952 // + workspace_crud.go create) — both reviewers flagged the @@ -278,7 +299,7 @@ func (h *WorkspaceHandler) Update(c *gin.Context) { c.JSON(http.StatusUnprocessableEntity, gin.H{"error": reason}) return } - if _, err := db.DB.ExecContext(ctx, `UPDATE workspaces SET runtime = $2, updated_at = now() WHERE id = $1`, id, runtime); err != nil { + if _, err := db.DB.ExecContext(ctx, `UPDATE workspaces SET runtime = $2, updated_at = now() WHERE id = $1`, id, runtimeStr); err != nil { log.Printf("Update runtime error for %s: %v", id, err) c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to save runtime"}) return diff --git a/workspace-server/internal/handlers/workspace_crud_test.go b/workspace-server/internal/handlers/workspace_crud_test.go index af7dd63ca..5efc5c4e0 100644 --- a/workspace-server/internal/handlers/workspace_crud_test.go +++ b/workspace-server/internal/handlers/workspace_crud_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "github.com/DATA-DOG/go-sqlmock" @@ -676,3 +677,39 @@ func TestUpdate_Runtime_UnroutableModel_Fails422(t *testing.T) { // (mock has unconsumed expectations). _ = mock } + +// TestUpdate_Runtime_UnknownPseudoRuntime_Fails422 pins the runtime-identity +// gate on PATCH: a template variant slug such as "seo-agent" is NOT a runtime, +// so the PATCH must be rejected before the (runtime, model) compatibility +// check runs. Prevents the customer incident where a conversion wrote +// runtime="seo-agent" and the workspace failed to boot because no adapter +// recognizes the pseudo-runtime. +func TestUpdate_Runtime_UnknownPseudoRuntime_Fails422(t *testing.T) { + wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + mock, r := setupWorkspaceCrudTest(t) + h := newWorkspaceCrudHandler(t) + r.PATCH("/workspaces/:id", h.Update) + + mock.ExpectQuery(`SELECT EXISTS\(SELECT 1 FROM workspaces WHERE id = \$1\)`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1)) + // The model compatibility query is intentionally NOT mocked: the + // unknown-runtime rejection must happen before that DB read. + + body := map[string]interface{}{"runtime": "seo-agent"} + b, _ := json.Marshal(body) + req, _ := http.NewRequest("PATCH", "/workspaces/"+wsID, 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 (unknown runtime), got %d: %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "unsupported workspace runtime") { + t.Errorf("expected 'unsupported workspace runtime' reason, got %s", w.Body.String()) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet expectations: %v", err) + } +} -- 2.52.0