forked from molecule-ai/molecule-core
refactor(models): consolidate per-runtime model defaults to SSOT (RFC #2873 iter 1)
Two call sites — workspace_provision.go:537 and org_import.go:54 — duplicated the same `if runtime == "claude-code"` branch deciding the default model when the operator/agent didn't supply one. They were copy-pasted; nothing prevented them from drifting silently. Extract to `models.DefaultModel(runtime string) string`. Both call sites now route through the helper. New runtimes need one entry in DefaultModel + one assertion in TestDefaultModel — pre-fix it required two source edits + an audit. Foundation for the future `RuntimeConfig` interface (RFC #2873 + task #231): once we add `ProvisioningTimeout()`, `CapabilitiesSupported()` etc., the helper expands to per-runtime structs and `DefaultModel` becomes one method on the interface. ## Coverage 15 unit tests pinning the exact contract: - claude-code → "sonnet" - 9 other known runtimes → universal default - empty + unknown → universal default (matches pre-refactor fallthrough) - case-sensitivity preserved (CLAUDE-CODE → universal default) Plus invariant test: `DefaultModel` never returns "" — protects against a future "return early on unknown" regression that would silently break workspace creation. ## Verification - go build ./... clean - 15 model unit tests pass - existing handler tests untouched (no behavior change at call sites) - identical output to pre-refactor for every input First iteration of the OSS-shape refactor program. Each PR meets all 7 bars (plugin/abstract/modular/SSOT/coverage/cleanup/file-split). Refs RFC #2873.
This commit is contained in:
parent
ec574f3d4b
commit
26e2e97006
@ -51,11 +51,10 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, absX
|
||||
model = defaults.Model
|
||||
}
|
||||
if model == "" {
|
||||
if runtime == "claude-code" {
|
||||
model = "sonnet"
|
||||
} else {
|
||||
model = "anthropic:claude-opus-4-7"
|
||||
}
|
||||
// SSOT: per-runtime defaults live in models/runtime_defaults.go
|
||||
// (see RFC #2873). Consolidated from a duplicate of the same
|
||||
// branch in workspace_provision.go.
|
||||
model = models.DefaultModel(runtime)
|
||||
}
|
||||
tier := ws.Tier
|
||||
if tier == 0 {
|
||||
|
||||
@ -534,11 +534,10 @@ func (h *WorkspaceHandler) ensureDefaultConfig(workspaceID string, payload model
|
||||
// Generate a minimal config.yaml
|
||||
model := payload.Model
|
||||
if model == "" {
|
||||
if runtime == "claude-code" {
|
||||
model = "sonnet"
|
||||
} else {
|
||||
model = "anthropic:claude-opus-4-7"
|
||||
}
|
||||
// SSOT: per-runtime defaults live in models/runtime_defaults.go
|
||||
// (see RFC #2873). Was previously duplicated here AND in
|
||||
// org_import.go; consolidating prevents silent drift.
|
||||
model = models.DefaultModel(runtime)
|
||||
}
|
||||
|
||||
// Sanitize name/role/model for YAML safety — always double-quote so
|
||||
|
||||
39
workspace-server/internal/models/runtime_defaults.go
Normal file
39
workspace-server/internal/models/runtime_defaults.go
Normal file
@ -0,0 +1,39 @@
|
||||
package models
|
||||
|
||||
// runtime_defaults.go — single source of truth for per-runtime defaults
|
||||
// the platform applies when the operator/agent didn't supply a value.
|
||||
//
|
||||
// Why this lives in models/ (not handlers/): default selection is a
|
||||
// pure data fact about the runtime, not handler logic. Multiple
|
||||
// callers (Create-workspace handler, org-import handler, future
|
||||
// auto-provision paths) need the same answer; concentrating the
|
||||
// rule here means one edit when a runtime's default changes.
|
||||
//
|
||||
// Related work (RFC #2873): this is the seed for a future
|
||||
// `RuntimeConfig` interface that will also expose `ProvisioningTimeout()`,
|
||||
// `CapabilitiesSupported()`, and other per-runtime facts. For now the
|
||||
// surface is one helper — extracted from the duplicate branch in
|
||||
// workspace_provision.go:537 and org_import.go:54 that diverged silently
|
||||
// during refactors before this consolidation.
|
||||
|
||||
// DefaultModel returns the model slug to use when a workspace is
|
||||
// created without an explicit model and the runtime can't infer one
|
||||
// from its own config.
|
||||
//
|
||||
// - claude-code: "sonnet" — Anthropic's CLI accepts the short
|
||||
// name and resolves it via the operator's anthropic-oauth or
|
||||
// ANTHROPIC_API_KEY chain.
|
||||
// - everything else (hermes, langgraph, crewai, autogen, deepagents,
|
||||
// codex, openclaw, gemini-cli, external, ""): a fully-qualified
|
||||
// vendor:model slug that the universal MODEL_PROVIDER chain in
|
||||
// molecule-core PR #247 can route via per-vendor required_env.
|
||||
//
|
||||
// The function never returns an empty string; an unknown runtime
|
||||
// gets the universal default rather than failing closed (matches the
|
||||
// pre-refactor behavior — both call sites used the same fallback).
|
||||
func DefaultModel(runtime string) string {
|
||||
if runtime == "claude-code" {
|
||||
return "sonnet"
|
||||
}
|
||||
return "anthropic:claude-opus-4-7"
|
||||
}
|
||||
62
workspace-server/internal/models/runtime_defaults_test.go
Normal file
62
workspace-server/internal/models/runtime_defaults_test.go
Normal file
@ -0,0 +1,62 @@
|
||||
package models
|
||||
|
||||
import "testing"
|
||||
|
||||
// TestDefaultModel pins the contract: known runtimes return their
|
||||
// expected default; unknowns and the empty string fall through to the
|
||||
// universal default. Add new runtimes here as `case` entries — pre-fix
|
||||
// adding a runtime required two source edits + an audit; post-SSOT it
|
||||
// requires one entry in DefaultModel + one assertion here.
|
||||
func TestDefaultModel(t *testing.T) {
|
||||
cases := []struct {
|
||||
runtime string
|
||||
want string
|
||||
}{
|
||||
// Known runtimes.
|
||||
{"claude-code", "sonnet"},
|
||||
|
||||
// Universal fallback for everything else. Each runtime is named
|
||||
// explicitly so a future drift (e.g., adding a hermes-specific
|
||||
// branch) shows up as a failure on the runtime that drifted, not
|
||||
// as a generic "unknown" failure.
|
||||
{"hermes", "anthropic:claude-opus-4-7"},
|
||||
{"langgraph", "anthropic:claude-opus-4-7"},
|
||||
{"crewai", "anthropic:claude-opus-4-7"},
|
||||
{"autogen", "anthropic:claude-opus-4-7"},
|
||||
{"deepagents", "anthropic:claude-opus-4-7"},
|
||||
{"codex", "anthropic:claude-opus-4-7"},
|
||||
{"openclaw", "anthropic:claude-opus-4-7"},
|
||||
{"gemini-cli", "anthropic:claude-opus-4-7"},
|
||||
{"external", "anthropic:claude-opus-4-7"},
|
||||
|
||||
// Unknown / empty — fall through to universal default rather
|
||||
// than failing closed. Pre-refactor both call sites also fell
|
||||
// through; pinning the existing behavior, not changing it.
|
||||
{"", "anthropic:claude-opus-4-7"},
|
||||
{"some-future-runtime", "anthropic:claude-opus-4-7"},
|
||||
{"CLAUDE-CODE", "anthropic:claude-opus-4-7"}, // case-sensitive — matches prior behavior
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.runtime, func(t *testing.T) {
|
||||
got := DefaultModel(tc.runtime)
|
||||
if got != tc.want {
|
||||
t.Errorf("DefaultModel(%q) = %q, want %q", tc.runtime, got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestDefaultModel_NeverEmpty — invariant: no input produces an empty
|
||||
// string. The handlers that consume this would write empty into
|
||||
// config.yaml, which the runtime then can't dispatch — pinning the
|
||||
// non-empty contract here protects against a future "return early on
|
||||
// unknown runtime" change that would silently break workspace creation.
|
||||
func TestDefaultModel_NeverEmpty(t *testing.T) {
|
||||
for _, runtime := range []string{
|
||||
"", "claude-code", "hermes", "unknown-runtime",
|
||||
} {
|
||||
if got := DefaultModel(runtime); got == "" {
|
||||
t.Errorf("DefaultModel(%q) returned empty string", runtime)
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user