diff --git a/workspace-server/internal/db/migration_20260520_drop_runtime_image_pins_test.go b/workspace-server/internal/db/migration_20260520_drop_runtime_image_pins_test.go new file mode 100644 index 000000000..4db41cd04 --- /dev/null +++ b/workspace-server/internal/db/migration_20260520_drop_runtime_image_pins_test.go @@ -0,0 +1,143 @@ +package db + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// Regression pin for RFC internal#617 / task #335. +// +// The drop-runtime_image_pins migration MUST honor the care zone documented +// in the RFC: drop the `runtime_image_pins` table but PRESERVE the column +// `workspaces.runtime_image_digest` and its partial index +// `idx_workspaces_runtime_image_digest`. +// +// This is a static-file lint, not a DB-execution test. Running the actual +// migration is out of scope for unit tests (the migration test infra in +// postgres_schema_migrations_test.go already proves the apply mechanism +// works for any forward file). What we pin here is the *content shape* of +// the new migration: +// +// - up.sql DROPs runtime_image_pins (the dead table) +// - up.sql does NOT touch runtime_image_digest (the care-zone column) +// - up.sql does NOT touch idx_workspaces_runtime_image_digest (care-zone index) +// - down.sql recreates runtime_image_pins (idempotent rollback) +// +// If a future cleanup PR wants to also drop the column, it should be a +// separate migration with its own RFC — this test catches accidental +// scope creep at PR time, before it ships to tenant DBs. +func TestMigration20260520_DropsRuntimeImagePins_PreservesDigestColumn(t *testing.T) { + // Locate the migrations dir relative to this test file's package dir. + // /workspace-server/internal/db/ → ../../migrations/ + const migDir = "../../migrations" + const upFile = "20260520120000_drop_runtime_image_pins.up.sql" + const downFile = "20260520120000_drop_runtime_image_pins.down.sql" + + upPath := filepath.Join(migDir, upFile) + downPath := filepath.Join(migDir, downFile) + + upBytes, err := os.ReadFile(upPath) + if err != nil { + t.Fatalf("read %s: %v", upPath, err) + } + downBytes, err := os.ReadFile(downPath) + if err != nil { + t.Fatalf("read %s: %v", downPath, err) + } + + // Strip single-line SQL comments (`-- ...`) before assertion so the + // rationale prose in the migration headers can mention the care-zone + // column by name without tripping the DDL-touch guard. The guard is + // specifically about DDL statements that act on the column. + upDDL := stripSQLLineComments(strings.ToLower(string(upBytes))) + downDDL := stripSQLLineComments(strings.ToLower(string(downBytes))) + + // up.sql MUST drop the dead table. + if !strings.Contains(upDDL, "drop table") || !strings.Contains(upDDL, "runtime_image_pins") { + t.Errorf("up.sql must DROP TABLE runtime_image_pins; got DDL:\n%s\n(full file:\n%s)", upDDL, upBytes) + } + + // CARE ZONE: up.sql DDL MUST NOT touch the workspaces.runtime_image_digest + // column or its index. A DDL statement that references either name is a + // scope-creep defect — file a separate RFC. + if strings.Contains(upDDL, "runtime_image_digest") { + t.Errorf("up.sql DDL references runtime_image_digest — care-zone column must NOT be touched by this migration. See RFC internal#617 §3. DDL:\n%s\n(full file:\n%s)", upDDL, upBytes) + } + if strings.Contains(upDDL, "idx_workspaces_runtime_image_digest") { + t.Errorf("up.sql DDL references idx_workspaces_runtime_image_digest — care-zone index must NOT be touched by this migration. See RFC internal#617 §3. DDL:\n%s\n(full file:\n%s)", upDDL, upBytes) + } + + // down.sql MUST recreate the table (rollback path). + if !strings.Contains(downDDL, "create table") || !strings.Contains(downDDL, "runtime_image_pins") { + t.Errorf("down.sql must CREATE TABLE runtime_image_pins (rollback path); got DDL:\n%s\n(full file:\n%s)", downDDL, downBytes) + } + + // down.sql DDL also must not touch the care-zone column (symmetry — + // we never added the column in the up so we cannot drop or recreate it + // in the down either). + if strings.Contains(downDDL, "runtime_image_digest") { + t.Errorf("down.sql DDL references runtime_image_digest — should be a no-op for the care-zone column. DDL:\n%s\n(full file:\n%s)", downDDL, downBytes) + } +} + +// stripSQLLineComments removes `-- ...` line comments from a SQL string, +// leaving only DDL statements + whitespace. Used by the migration-content +// guards so descriptive prose in the migration header doesn't false-flag. +// +// This is intentionally minimal — does NOT handle `/* */` block comments +// (the migration files don't use them) or string-literal embedded `--` +// (DDL doesn't use that shape). Good enough for static-content lint. +func stripSQLLineComments(s string) string { + lines := strings.Split(s, "\n") + out := make([]string, 0, len(lines)) + for _, ln := range lines { + // Trim everything after the first `--`. Conservative — if a future + // migration genuinely needs `--` inside a string literal, that + // would require parsing. + if idx := strings.Index(ln, "--"); idx >= 0 { + ln = ln[:idx] + } + out = append(out, ln) + } + return strings.Join(out, "\n") +} + +// TestMigration20260520_PairExists is a belt-and-braces guard that the +// up + down files both exist and aren't empty. RunMigrations only consumes +// the up but a missing down breaks the dev-side rollback workflow silently. +func TestMigration20260520_PairExists(t *testing.T) { + const migDir = "../../migrations" + for _, f := range []string{ + "20260520120000_drop_runtime_image_pins.up.sql", + "20260520120000_drop_runtime_image_pins.down.sql", + } { + p := filepath.Join(migDir, f) + info, err := os.Stat(p) + if err != nil { + t.Errorf("expected migration file %s to exist: %v", p, err) + continue + } + if info.Size() < 50 { + t.Errorf("migration file %s is suspiciously small (%d bytes) — header comment missing?", p, info.Size()) + } + } +} + +// TestMigration20260520_DeadReaderIsGone pins the deletion of the dead +// runtime_image_pin.go reader. If anyone reintroduces it (e.g., a cherry- +// pick from a stale branch), this catches it in unit tests before it hits +// review. The reader is provably dead under CP-as-SSOT — re-adding it +// reopens the divergence the RFC closed. +func TestMigration20260520_DeadReaderIsGone(t *testing.T) { + const readerPath = "../handlers/runtime_image_pin.go" + if _, err := os.Stat(readerPath); err == nil { + t.Errorf("dead reader %s reappeared — RFC internal#617 retired it. If you really need a per-tenant pin path, file a follow-up RFC; do not just re-add the reader.", readerPath) + } + const testPath = "../handlers/runtime_image_pin_test.go" + if _, err := os.Stat(testPath); err == nil { + t.Errorf("dead reader test %s reappeared — should have been removed alongside the implementation.", testPath) + } +} + diff --git a/workspace-server/internal/handlers/handlers_test.go b/workspace-server/internal/handlers/handlers_test.go index 1f335c4d5..7ce01b239 100644 --- a/workspace-server/internal/handlers/handlers_test.go +++ b/workspace-server/internal/handlers/handlers_test.go @@ -416,10 +416,9 @@ func TestWorkspaceCreate(t *testing.T) { } func TestBuildProvisionerConfig_IncludesAwarenessSettings(t *testing.T) { - mock := setupTestDB(t) - mock.ExpectQuery(`SELECT digest FROM runtime_image_pins`). - WithArgs("claude-code"). - WillReturnError(sql.ErrNoRows) + setupTestDB(t) + // runtime_image_pins reader removed by RFC internal#617 / task #335 + // — CP is the SSOT for runtime image pins. No DB lookup here anymore. broadcaster := newTestBroadcaster() handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", "/tmp/configs") diff --git a/workspace-server/internal/handlers/runtime_image_pin.go b/workspace-server/internal/handlers/runtime_image_pin.go deleted file mode 100644 index 2cd90bd4a..000000000 --- a/workspace-server/internal/handlers/runtime_image_pin.go +++ /dev/null @@ -1,67 +0,0 @@ -package handlers - -import ( - "context" - "database/sql" - "errors" - "log" - "os" - "strings" - - "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" - "github.com/Molecule-AI/molecule-monorepo/platform/internal/provisioner" -) - -// resolveRuntimeImage returns the digest-pinned image ref for a runtime when -// an operator has promoted one via the runtime_image_pins table (#2272 layer 1), -// otherwise "" so the caller falls back to the legacy `:latest` lookup in -// provisioner.RuntimeImages. -// -// Policy: availability over pinning. Any DB hiccup (sql.ErrNoRows is the -// steady-state when nothing is pinned, but transient network blips, table -// missing post-rollback, etc.) returns "" and the provision continues on -// the moving tag — better one workspace on a slightly-newer image than a -// provision-blocked tenant. -// -// WORKSPACE_IMAGE_LOCAL_OVERRIDE=1 short-circuits the lookup entirely so a -// developer rebuilding template images locally gets their fresh build via -// `:latest` even when a remote digest is pinned for the same runtime. -func resolveRuntimeImage(ctx context.Context, runtime string) string { - if runtime == "" { - return "" - } - base, ok := provisioner.RuntimeImages[runtime] - if !ok { - // Unknown runtime — let provisioner.Start fall through to its own - // DefaultImage. Querying the pin table for a runtime that doesn't - // exist would only produce noise and a guaranteed ErrNoRows. - return "" - } - if os.Getenv("WORKSPACE_IMAGE_LOCAL_OVERRIDE") != "" { - return "" - } - if db.DB == nil { - return "" - } - - var digest string - err := db.DB.QueryRowContext(ctx, - `SELECT digest FROM runtime_image_pins WHERE template_name = $1`, runtime, - ).Scan(&digest) - if err != nil { - if !errors.Is(err, sql.ErrNoRows) { - log.Printf("resolveRuntimeImage: pin lookup for %q failed (%v) — falling back to :latest", runtime, err) - } - return "" - } - - // Strip the moving tag suffix (`:latest`, `:staging`) before appending - // the immutable digest. Docker treats `name:tag@sha256:...` as valid - // but the tag is ignored; dropping it keeps logs and admin diffs honest - // about what's actually being pulled. - pinned := base - if idx := strings.LastIndex(pinned, ":"); idx > strings.LastIndex(pinned, "/") { - pinned = pinned[:idx] - } - return pinned + "@" + digest -} diff --git a/workspace-server/internal/handlers/runtime_image_pin_test.go b/workspace-server/internal/handlers/runtime_image_pin_test.go deleted file mode 100644 index b7589bc6f..000000000 --- a/workspace-server/internal/handlers/runtime_image_pin_test.go +++ /dev/null @@ -1,138 +0,0 @@ -package handlers - -import ( - "context" - "database/sql" - "strings" - "testing" - - "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" - sqlmock "github.com/DATA-DOG/go-sqlmock" -) - -// TestResolveRuntimeImage_NoPin: lookup returns sql.ErrNoRows (the steady- -// state when an operator hasn't pinned this runtime). resolveRuntimeImage -// returns "" so the caller falls back to RuntimeImages[runtime] (legacy -// :latest behavior). This is the expected hot path until digest pinning -// is opted into per runtime. -func TestResolveRuntimeImage_NoPin(t *testing.T) { - mockDB, mock, err := sqlmock.New() - if err != nil { - t.Fatal(err) - } - defer mockDB.Close() - prev := db.DB - db.DB = mockDB - defer func() { db.DB = prev }() - - mock.ExpectQuery(`SELECT digest FROM runtime_image_pins WHERE template_name = \$1`). - WithArgs("claude-code"). - WillReturnError(sql.ErrNoRows) - - got := resolveRuntimeImage(context.Background(), "claude-code") - if got != "" { - t.Errorf("expected empty (no pin = fallback), got %q", got) - } - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("sqlmock expectations: %v", err) - } -} - -// TestResolveRuntimeImage_DBError: an unexpected DB failure (transient -// network blip, table missing post-rollback, etc.) must NOT block the -// provision — log + fall through to the legacy :latest path. This is -// the availability-over-pinning policy spelled out in resolveRuntimeImage's -// doc comment. -func TestResolveRuntimeImage_DBError(t *testing.T) { - mockDB, mock, err := sqlmock.New() - if err != nil { - t.Fatal(err) - } - defer mockDB.Close() - prev := db.DB - db.DB = mockDB - defer func() { db.DB = prev }() - - mock.ExpectQuery(`SELECT digest FROM runtime_image_pins WHERE template_name = \$1`). - WithArgs("claude-code"). - WillReturnError(sqlmock.ErrCancelled) - - got := resolveRuntimeImage(context.Background(), "claude-code") - if got != "" { - t.Errorf("expected empty on DB error (fallback), got %q", got) - } - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("sqlmock expectations: %v", err) - } -} - -// TestResolveRuntimeImage_WithPin returns image@sha256: when row exists. -func TestResolveRuntimeImage_WithPin(t *testing.T) { - mockDB, mock, err := sqlmock.New() - if err != nil { - t.Fatal(err) - } - defer mockDB.Close() - prev := db.DB - db.DB = mockDB - defer func() { db.DB = prev }() - - digest := "sha256:3d6761a97ed07d7d33cfc19a8fbab81175d9d9179618d493dbc00c5f7ef076a3" - mock.ExpectQuery(`SELECT digest FROM runtime_image_pins WHERE template_name = \$1`). - WithArgs("claude-code"). - WillReturnRows(sqlmock.NewRows([]string{"digest"}).AddRow(digest)) - - got := resolveRuntimeImage(context.Background(), "claude-code") - if !strings.HasSuffix(got, "@"+digest) { - t.Errorf("expected suffix @%s, got %q", digest, got) - } - if !strings.HasPrefix(got, "ghcr.io/molecule-ai/workspace-template-claude-code") { - t.Errorf("expected GHCR prefix preserved, got %q", got) - } - if strings.Contains(got, ":latest") { - t.Errorf("expected :latest stripped, got %q", got) - } -} - -// TestResolveRuntimeImage_EmptyRuntime short-circuits to "" without DB. -func TestResolveRuntimeImage_EmptyRuntime(t *testing.T) { - got := resolveRuntimeImage(context.Background(), "") - if got != "" { - t.Errorf("expected empty for empty runtime, got %q", got) - } -} - -// TestResolveRuntimeImage_UnknownRuntime returns "" without DB lookup. -func TestResolveRuntimeImage_UnknownRuntime(t *testing.T) { - got := resolveRuntimeImage(context.Background(), "no-such-runtime") - if got != "" { - t.Errorf("expected empty for unknown runtime, got %q", got) - } -} - -// TestResolveRuntimeImage_LocalOverride: when WORKSPACE_IMAGE_LOCAL_OVERRIDE -// is set, the pin lookup is short-circuited even with a row present — -// devs rebuild images locally and want the floating tag to resolve to -// their fresh build, not a remote-pinned digest. -func TestResolveRuntimeImage_LocalOverride(t *testing.T) { - t.Setenv("WORKSPACE_IMAGE_LOCAL_OVERRIDE", "1") - - mockDB, mock, err := sqlmock.New() - if err != nil { - t.Fatal(err) - } - defer mockDB.Close() - prev := db.DB - db.DB = mockDB - defer func() { db.DB = prev }() - - // No expectation set — if resolveRuntimeImage queries the DB despite - // the override, sqlmock fails the test via ExpectationsWereMet. - got := resolveRuntimeImage(context.Background(), "claude-code") - if got != "" { - t.Errorf("expected empty under WORKSPACE_IMAGE_LOCAL_OVERRIDE=1, got %q", got) - } - if err := mock.ExpectationsWereMet(); err != nil { - t.Errorf("DB queried despite override: %v", err) - } -} diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index 843a3cace..c30b9b299 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -261,7 +261,14 @@ func (h *WorkspaceHandler) buildProvisionerConfig( workspaceAccess := payload.WorkspaceAccess if (workspacePath == "" || workspaceAccess == "") && db.DB != nil { var dbDir, dbAccess string - if err := db.DB.QueryRow( + // QueryRowContext (not QueryRow) so the provision-timeout ctx + // propagates here too. Previously ctx flowed in only to be passed + // to resolveRuntimeImage; that dead reader was removed by + // RFC internal#617 / task #335. Wiring ctx into the surviving DB + // query keeps the parameter load-bearing and is a small correctness + // nudge (a 10s ProvisionTimeout now actually bounds this lookup). + if err := db.DB.QueryRowContext( + ctx, `SELECT COALESCE(workspace_dir, ''), COALESCE(workspace_access, 'none') FROM workspaces WHERE id = $1`, workspaceID, ).Scan(&dbDir, &dbAccess); err == nil { @@ -293,7 +300,15 @@ func (h *WorkspaceHandler) buildProvisionerConfig( PlatformURL: h.platformURL, AwarenessURL: os.Getenv("AWARENESS_URL"), AwarenessNamespace: awarenessNamespace, - Image: resolveRuntimeImage(ctx, payload.Runtime), + // Image left empty — molecule-core's runtime_image_pins table (mig + // 047, dead reader removed by RFC internal#617 / task #335) was an + // aspirational SSOT that never received a writer. CP's + // runtime_image_pins (CP migration 027) is the single SSOT; the + // pin is applied at CP's provisioner layer before this code path + // runs. Empty here means selectImage() falls back to the legacy + // RuntimeImages[Runtime] :latest lookup, which is what the dead + // reader's sql.ErrNoRows path was producing already. + Image: "", } } @@ -970,3 +985,4 @@ func (h *WorkspaceHandler) provisionWorkspaceCP(workspaceID, templatePath string log.Printf("CPProvisioner: workspace %s started as machine %s via control plane", workspaceID, machineID) } + diff --git a/workspace-server/internal/handlers/workspace_provision_test.go b/workspace-server/internal/handlers/workspace_provision_test.go index 9e783814c..a5e46d64a 100644 --- a/workspace-server/internal/handlers/workspace_provision_test.go +++ b/workspace-server/internal/handlers/workspace_provision_test.go @@ -749,9 +749,8 @@ func TestBuildProvisionerConfig_WorkspacePathFromEnv(t *testing.T) { mock.ExpectQuery(`SELECT COALESCE\(workspace_dir`). WithArgs("ws-env"). WillReturnError(sql.ErrNoRows) - mock.ExpectQuery(`SELECT digest FROM runtime_image_pins`). - WithArgs("claude-code"). - WillReturnError(sql.ErrNoRows) + // runtime_image_pins reader removed by RFC internal#617 / task #335 + // — CP is the SSOT for runtime image pins. No DB lookup here anymore. broadcaster := newTestBroadcaster() handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir()) diff --git a/workspace-server/internal/provisioner/provisioner.go b/workspace-server/internal/provisioner/provisioner.go index 6c895867e..164c951bf 100644 --- a/workspace-server/internal/provisioner/provisioner.go +++ b/workspace-server/internal/provisioner/provisioner.go @@ -105,19 +105,27 @@ type WorkspaceConfig struct { WorkspaceAccess string // #65: "none" (default), "read_only", or "read_write" ResetClaudeSession bool // #12: if true, discard the claude-sessions volume before start (fresh session dir) - // Image, when non-empty, overrides the runtime→image lookup. The handler - // layer sets this to the digest-pinned form (`@sha256:`) - // when an operator has promoted a specific runtime build via the - // runtime_image_pins table (#2272 layer 1). Empty = legacy behavior, - // fall back to RuntimeImages[Runtime] which resolves to the moving - // `:latest` tag. + // Image, when non-empty, overrides the runtime→image lookup. CP + // (molecule-controlplane) is the single SSOT for runtime image digest + // pins via its migrations/027_runtime_image_pins table — the pin is + // applied at CP's provisioner layer before the workspace-server even + // runs, so under the current architecture this field is always empty + // on the workspace-server side. Empty = fall back to RuntimeImages + // [Runtime] which resolves to the moving `:latest` tag. + // + // Historical note: molecule-core's own runtime_image_pins table + // (workspace-server/migrations 047) was the original aspirational + // design (#2272 layer 1) but never received a writer; RFC internal#617 / + // task #335 retired the dead reader + table in favor of CP-as-SSOT. Image string } // selectImage resolves the final Docker image ref for a workspace. The handler // layer is the source of truth — if it set cfg.Image (the digest-pinned form -// from runtime_image_pins, #2272), honor that. Otherwise fall back to the -// runtime→tag lookup in RuntimeImages (legacy `:latest` behavior). +// supplied by CP, the SSOT for runtime image pins; molecule-core's own +// runtime_image_pins reader retired by RFC internal#617 / task #335), honor +// that. Otherwise fall back to the runtime→tag lookup in RuntimeImages +// (legacy `:latest` behavior). // // Fail-closed contract (RFC internal#483 / security review 4269 / // feedback_platform_must_hardgate_base_contract): if the workspace NAMES a @@ -378,7 +386,7 @@ func (p *Provisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, e // + `docker build`s it locally. Replace the placeholder image ref with // the SHA-pinned tag of the freshly-built image before ContainerCreate. // - // Pinned overrides (cfg.Image set, e.g. via runtime_image_pins for + // Pinned overrides (cfg.Image set, e.g. via CP's runtime_image_pins for // production thin-AMI launches) bypass this path — they pin a digest // the operator chose explicitly. if cfg.Image == "" && cfg.Runtime != "" { @@ -1597,3 +1605,4 @@ func parseOCIPlatform(s string) *ocispec.Platform { } return &ocispec.Platform{OS: parts[0], Architecture: parts[1]} } + diff --git a/workspace-server/internal/provisioner/provisioner_test.go b/workspace-server/internal/provisioner/provisioner_test.go index 9d0067c33..941fdaef2 100644 --- a/workspace-server/internal/provisioner/provisioner_test.go +++ b/workspace-server/internal/provisioner/provisioner_test.go @@ -506,11 +506,12 @@ func TestWorkspaceConfig_ResetClaudeSessionFieldPresent(t *testing.T) { // ---------- selectImage (#2272 layer 1) ---------- -// TestSelectImage_PrefersExplicitImage: when the handler resolved a digest -// pin via runtime_image_pins, cfg.Image is set. selectImage must honor it -// and ignore the cfg.Runtime → :latest fallback. This is the load-bearing -// invariant for digest pinning — if it ever silently reverts to :latest, -// we lose the "one bad publish doesn't break every workspace" guarantee. +// TestSelectImage_PrefersExplicitImage: when CP (the SSOT for runtime image +// pins under RFC internal#617 / task #335) supplied a digest pin via +// cfg.Image, selectImage must honor it and ignore the cfg.Runtime → :latest +// fallback. This is the load-bearing invariant for digest pinning — if it +// ever silently reverts to :latest, we lose the "one bad publish doesn't +// break every workspace" guarantee. func TestSelectImage_PrefersExplicitImage(t *testing.T) { pinned := "ghcr.io/molecule-ai/workspace-template-claude-code@sha256:3d6761a97ed07d7d33cfc19a8fbab81175d9d9179618d493dbc00c5f7ef076a3" got, err := selectImage(WorkspaceConfig{Runtime: "claude-code", Image: pinned}) diff --git a/workspace-server/internal/provisioner/registry.go b/workspace-server/internal/provisioner/registry.go index e1c72a7a7..a6fa5d776 100644 --- a/workspace-server/internal/provisioner/registry.go +++ b/workspace-server/internal/provisioner/registry.go @@ -89,11 +89,13 @@ func RegistryHost() string { // RuntimeImage returns the canonical image reference for the given runtime, // using the current RegistryPrefix() and the moving `:latest` tag. // -// For SHA-pinned references (production thin-AMI launches), the -// runtime_image_pins lookup in handlers/runtime_image_pin.go strips the -// `:latest` suffix and appends an immutable `@sha256:` from the DB. -// That code path naturally inherits any RegistryPrefix() change because it -// reads from RuntimeImages[runtime] and only re-formats the tag suffix. +// SHA-pinned references for production thin-AMI launches are applied by CP +// (molecule-controlplane) at its provisioner layer using CP's +// migrations/027_runtime_image_pins table, which is the single SSOT for +// runtime image pins. The local digest-pin reader that previously lived at +// handlers/runtime_image_pin.go was retired by RFC internal#617 / task #335 +// (it never had a writer; the table was always empty so the reader hit +// sql.ErrNoRows and fell through to :latest on every provision). // // Returns the empty string for unknown runtimes; callers should fall through // to DefaultImage in that case (matching legacy behavior). @@ -117,3 +119,4 @@ func computeRuntimeImages() map[string]string { } return out } + diff --git a/workspace-server/migrations/20260520120000_drop_runtime_image_pins.down.sql b/workspace-server/migrations/20260520120000_drop_runtime_image_pins.down.sql new file mode 100644 index 000000000..22bf98e8b --- /dev/null +++ b/workspace-server/migrations/20260520120000_drop_runtime_image_pins.down.sql @@ -0,0 +1,15 @@ +-- Reverse of 20260520120000_drop_runtime_image_pins.up.sql. +-- +-- Recreates the runtime_image_pins table verbatim from migration 047 so a +-- down-cycle leaves the schema bit-identical to the state before the drop. +-- The `workspaces.runtime_image_digest` column is unaffected by both the +-- up and the down (we never touched it on the up side). + +CREATE TABLE IF NOT EXISTS runtime_image_pins ( + template_name TEXT PRIMARY KEY, + digest TEXT NOT NULL CHECK (digest ~ '^sha256:[a-f0-9]{64}$'), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_by TEXT, + notes TEXT +); + diff --git a/workspace-server/migrations/20260520120000_drop_runtime_image_pins.up.sql b/workspace-server/migrations/20260520120000_drop_runtime_image_pins.up.sql new file mode 100644 index 000000000..c3fe9ac46 --- /dev/null +++ b/workspace-server/migrations/20260520120000_drop_runtime_image_pins.up.sql @@ -0,0 +1,26 @@ +-- Task #335 / RFC internal#617 — drop molecule-core's dead runtime_image_pins +-- table. CP (molecule-controlplane migrations/027_runtime_image_pins.up.sql) +-- is the single SSOT for runtime image digest pins. +-- +-- Empirical state at the time of this migration (a6e3ff018 finding, +-- 2026-05-20): no code in any molecule-ai repo INSERTs or UPDATEs this +-- table. The reader in workspace-server/internal/handlers/runtime_image_pin.go +-- has been hitting sql.ErrNoRows on every single workspace provision since +-- mig 047 landed (PR #2276) — silently falling through to the legacy +-- :latest path. Functionally indistinguishable from removing the call entirely. +-- +-- CP's parallel-named table (CP mig 027) has the writer, reader, hard-gate +-- (RFC internal#541 Step 2), seeded post-suspension digests (CP mig 028), +-- and admin endpoints. CP is now the de-facto SSOT and this migration just +-- ratifies that reality by removing the unused copy. +-- +-- CARE ZONE: migration 047 ALSO added `workspaces.runtime_image_digest TEXT` +-- and `idx_workspaces_runtime_image_digest`. Per RFC internal#617 §3, that +-- column is earmarked for the canvas admin's stale-workspaces panel +-- (workspaces still on an old digest after a CP-side promotion). It has no +-- current consumer but the cost of keeping it is one nullable column + a +-- partial index, and dropping it is a separate decision out of scope here. +-- DO NOT touch the column or its index in this migration. + +DROP TABLE IF EXISTS runtime_image_pins; +