From 284012a768abd10af549939e2e558dea5ab9c515 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sat, 2 May 2026 19:48:37 -0700 Subject: [PATCH 1/2] =?UTF-8?q?test(workspace-server):=20AST=20drift=20gat?= =?UTF-8?q?e=20for=20derive-provider.sh=20=E2=86=94=20Go=20port?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #2535 added a Go port of derive-provider.sh (deriveProviderFromModelSlug) so workspace-server can persist LLM_PROVIDER into workspace_secrets at provision time. This created two sources of truth — if a future PR adds a provider prefix to one without the other, the platform's persisted LLM_PROVIDER silently disagrees with what the container's derive-provider.sh produces at boot, with no test going red. This adds a hermetic drift gate that: 1. Parses workspace-configs-templates/hermes/scripts/derive-provider.sh with regex (handling both single-line `pat/*) PROVIDER="x" ;;` clauses and multi-line conditional clauses) to build a map[prefix]provider. 2. Walks workspace_provision.go's AST with go/ast, finds deriveProviderFromModelSlug, and extracts every case-clause prefix → return-string-literal pair. 3. Cross-checks both directions and accepts only the two documented divergences (nousresearch/* and openai/* both → "openrouter" at provision time because derive-provider.sh's runtime-env checks aren't loaded yet) via a hardcoded acceptedDivergences map. 4. Fails with an actionable message that names both files and suggests the exact fix (add the case OR add to divergence list with a comment). Pattern: behavior-based AST gate from PR #2367 / memory feedback — pin the invariant by what the function maps, not by what it's named. Stdlib-only (go/ast, go/parser, go/token, regexp); no network, no DB, no docker — reads two monorepo files in-process. A second sanity-check test pins anchor prefixes the regex must find, so a future shell-syntax change can't silently produce an empty map and trivially pass the main gate. Closes task #242. --- .../handlers/derive_provider_drift_test.go | 476 ++++++++++++++++++ 1 file changed, 476 insertions(+) create mode 100644 workspace-server/internal/handlers/derive_provider_drift_test.go diff --git a/workspace-server/internal/handlers/derive_provider_drift_test.go b/workspace-server/internal/handlers/derive_provider_drift_test.go new file mode 100644 index 00000000..ca53584a --- /dev/null +++ b/workspace-server/internal/handlers/derive_provider_drift_test.go @@ -0,0 +1,476 @@ +package handlers + +// derive_provider_drift_test.go — behavior-based AST/text drift gate. +// +// Why this exists: PR #2535 introduced a Go port of derive-provider.sh +// (see deriveProviderFromModelSlug in workspace_provision.go) so the +// workspace-server can persist LLM_PROVIDER into workspace_secrets at +// provision time. That created two sources of truth: +// +// 1. workspace-configs-templates/hermes/scripts/derive-provider.sh — +// runs inside the container at boot, has the final say on which +// provider hermes targets (writes ~/.hermes/config.yaml's +// model.provider field). +// 2. workspace-server/internal/handlers/workspace_provision.go's +// deriveProviderFromModelSlug — runs at provision time on the +// platform side so LLM_PROVIDER lands in workspace_secrets and +// survives Save+Restart. +// +// If a future PR adds a new provider prefix to one but not the other, +// the workspace-server's persisted LLM_PROVIDER silently disagrees +// with what the container's derive-provider.sh produces. The container +// wins (it writes the actual config.yaml), so the workspace-server's +// persisted value becomes stale and misleading without anything +// flipping red in CI. +// +// This gate pins the invariant that the *prefix set* the two functions +// know about is identical, modulo a small hardcoded acceptedDivergences +// map for the two intentional differences documented in +// deriveProviderFromModelSlug's doc comment (nousresearch/* and +// openai/* both fall back to "openrouter" at provision time because +// the runtime env that picks "nous" / "custom" isn't available yet). +// +// Pattern: the "behavior-based AST gate" from PR #2367 / memory +// feedback_behavior_based_ast_gates — pin invariants by what a +// function maps, not by what it's named. Walks the actual Go AST of +// deriveProviderFromModelSlug's switch statement so a rename or a +// duplicate function in another file can't sneak past the gate. +// +// Task: #242. Companion to the table-driven mapping test in +// workspace_provision_shared_test.go (TestDeriveProviderFromModelSlug) +// which pins the *values*; this test pins the *coverage* of the +// prefix set itself. +// +// Hermetic: only reads two files from the monorepo + parses them +// in-process. No network, no docker, no DB. + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "regexp" + "runtime" + "sort" + "strconv" + "strings" + "testing" +) + +// acceptedDivergences pins the prefixes where the Go port intentionally +// differs from derive-provider.sh. Each entry's value is the provider +// the Go function returns; the shell would (at runtime, with the right +// env keys present) return something else. Documented in +// deriveProviderFromModelSlug's doc comment in workspace_provision.go. +// +// If a NEW divergence appears, this test fails and the engineer must +// either (a) align the Go function with the shell, or (b) add the +// prefix here with a comment explaining why the divergence is +// intentional and safe at provision time. +var acceptedDivergences = map[string]string{ + // Shell: "nous" if HERMES_API_KEY/NOUS_API_KEY set, else "openrouter". + // Go: "openrouter" unconditionally — runtime keys aren't loaded at + // provision time. derive-provider.sh upgrades to "nous" at boot + // when the keys are present. + "nousresearch": "openrouter", + // Shell: "custom" if OPENAI_API_KEY set, "openrouter" if OPENROUTER_API_KEY + // set, else "openrouter" as a no-key fallback. + // Go: "openrouter" unconditionally — same reason as nousresearch/*. + // derive-provider.sh upgrades to "custom" at boot when + // OPENAI_API_KEY is present. + "openai": "openrouter", +} + +// TestDeriveProviderDrift_ShellAndGoStayInSync is the drift gate. +// It extracts the prefix→provider mapping from both sources and +// asserts: +// +// 1. Every prefix the shell knows about, the Go function also handles +// (returning either the same provider OR the value pinned in +// acceptedDivergences for that prefix). +// 2. Every prefix the Go function handles (extracted from its switch +// statement via go/ast), the shell case statement also lists. +func TestDeriveProviderDrift_ShellAndGoStayInSync(t *testing.T) { + t.Parallel() + + shellMap := loadShellPrefixMap(t) + goMap := loadGoPrefixMap(t) + + if len(shellMap) == 0 { + t.Fatalf("parsed zero prefixes from derive-provider.sh — regex likely broke; rebuild parser before trusting this gate") + } + if len(goMap) == 0 { + t.Fatalf("parsed zero prefixes from deriveProviderFromModelSlug — AST walk likely broke; rebuild parser before trusting this gate") + } + + // Direction 1: every shell prefix must be in the Go map (with the + // same provider value, or with the documented divergence). + for prefix, shellProvider := range shellMap { + goProvider, ok := goMap[prefix] + if !ok { + t.Errorf( + "DRIFT: derive-provider.sh has prefix %q -> %q but deriveProviderFromModelSlug doesn't handle it.\n"+ + "Fix: either add a case for %q to deriveProviderFromModelSlug in "+ + "workspace-server/internal/handlers/workspace_provision.go (returning %q to match the shell), "+ + "OR if this prefix is intentionally provision-time-divergent, add it to acceptedDivergences{} "+ + "in this test with a comment explaining why.", + prefix, shellProvider, prefix, shellProvider, + ) + continue + } + if goProvider == shellProvider { + continue + } + // Mismatch — only acceptable if it's on the explicit divergence list + // AND the Go side returns exactly the documented value. + expected, divergenceAllowed := acceptedDivergences[prefix] + if !divergenceAllowed { + t.Errorf( + "DRIFT: prefix %q maps to %q in derive-provider.sh but %q in deriveProviderFromModelSlug.\n"+ + "Fix: align the Go function with the shell (preferred — they should agree), "+ + "OR if the divergence is intentional and safe at provision time, "+ + "add %q: %q to acceptedDivergences{} in this test with a comment explaining why.", + prefix, shellProvider, goProvider, prefix, goProvider, + ) + continue + } + if goProvider != expected { + t.Errorf( + "DRIFT: prefix %q is on the acceptedDivergences list with expected Go value %q but "+ + "deriveProviderFromModelSlug now returns %q.\n"+ + "Fix: update acceptedDivergences[%q] in this test to %q (and update its comment), "+ + "OR revert the Go function to return %q.", + prefix, expected, goProvider, prefix, goProvider, expected, + ) + } + } + + // Direction 2: every Go prefix must be in the shell map. Drift in + // this direction is rarer (someone added a Go case without touching + // the shell) but produces the same broken state — provision-time + // LLM_PROVIDER disagrees with what the container actually uses. + for prefix, goProvider := range goMap { + if _, ok := shellMap[prefix]; ok { + continue + } + t.Errorf( + "DRIFT: deriveProviderFromModelSlug handles prefix %q -> %q but derive-provider.sh doesn't list it.\n"+ + "Fix: add a `%s/*) PROVIDER=%q ;;` case to "+ + "workspace-configs-templates/hermes/scripts/derive-provider.sh — the Go provision-time hint "+ + "is meaningless if the container's runtime script doesn't recognize the same prefix.", + prefix, goProvider, prefix, goProvider, + ) + } + + // Belt-and-braces: every entry in acceptedDivergences must actually + // appear in BOTH maps. A stale divergence entry (prefix removed from + // either source) silently weakens the gate. + for prefix := range acceptedDivergences { + if _, ok := shellMap[prefix]; !ok { + t.Errorf( + "acceptedDivergences contains prefix %q but derive-provider.sh no longer lists it. "+ + "Remove the entry from acceptedDivergences{} in this test.", + prefix, + ) + } + if _, ok := goMap[prefix]; !ok { + t.Errorf( + "acceptedDivergences contains prefix %q but deriveProviderFromModelSlug no longer lists it. "+ + "Remove the entry from acceptedDivergences{} in this test.", + prefix, + ) + } + } +} + +// monorepoRoot resolves the absolute path of the molecule-monorepo +// root by walking up from this test file's directory. Avoids relying +// on a fixed CWD or env var. +func monorepoRoot(t *testing.T) string { + t.Helper() + _, thisFile, _, ok := runtime.Caller(0) + if !ok { + t.Fatalf("runtime.Caller failed — cannot locate test file path") + } + // .../workspace-server/internal/handlers/derive_provider_drift_test.go + dir := filepath.Dir(thisFile) + for i := 0; i < 6; i++ { + if _, err := os.Stat(filepath.Join(dir, "workspace-configs-templates")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + break + } + dir = parent + } + t.Fatalf("could not find monorepo root (looked for workspace-configs-templates/) walking up from %s", thisFile) + return "" +} + +// loadShellPrefixMap parses derive-provider.sh and returns a +// map[prefix]provider for every case clause. Aliases inside a single +// `pat1/*|pat2/*)` clause expand to one map entry per alias, both +// pointing at the same provider. +// +// Stops at the first `*)` (the catch-all) and ignores it — the +// catch-all maps to PROVIDER="auto" which has no Go counterpart by +// design (deriveProviderFromModelSlug returns "" for unknowns and +// lets the shell's *=auto branch decide at runtime). +// +// Ambiguity: case clauses whose body branches on env vars (openai/*, +// nousresearch/*) are still extracted as the FIRST PROVIDER= literal +// inside the body. The shell's full conditional logic is documented +// via the acceptedDivergences map in this file rather than re-encoded +// in the parser, because re-encoding sh `if` semantics in regex is a +// fool's errand — the divergences are stable and small enough to +// hardcode. +func loadShellPrefixMap(t *testing.T) map[string]string { + t.Helper() + root := monorepoRoot(t) + shellPath := filepath.Join(root, "workspace-configs-templates", "hermes", "scripts", "derive-provider.sh") + raw, err := os.ReadFile(shellPath) + if err != nil { + t.Fatalf("read %s: %v", shellPath, err) + } + + // Locate the case statement body so we don't accidentally match + // PROVIDER= assignments above the case (the HERMES_INFERENCE_PROVIDER + // override + HERMES_DEFAULT_MODEL empty fallback both write PROVIDER= + // before the case). + caseStart := regexp.MustCompile(`(?m)^case\s+"\$\{HERMES_DEFAULT_MODEL\}"\s+in\s*$`) + startLoc := caseStart.FindIndex(raw) + if startLoc == nil { + t.Fatalf("could not locate `case \"${HERMES_DEFAULT_MODEL}\" in` in %s — shell file shape changed; rebuild parser", shellPath) + } + caseEnd := regexp.MustCompile(`(?m)^esac\s*$`) + endLoc := caseEnd.FindIndex(raw[startLoc[1]:]) + if endLoc == nil { + t.Fatalf("could not locate `esac` after the case statement in %s — shell file shape changed", shellPath) + } + body := string(raw[startLoc[1] : startLoc[1]+endLoc[0]]) + + out := map[string]string{} + + // Pattern A: single-line clauses like + // minimax-cn/*) PROVIDER="minimax-cn" ;; + // alibaba/*|dashscope/*|qwen/*) PROVIDER="alibaba" ;; + // Capture group 1 is the patterns (e.g. `minimax-cn/*` or + // `alibaba/*|dashscope/*|qwen/*`); group 2 is the provider literal. + singleLine := regexp.MustCompile(`(?m)^\s*([a-zA-Z0-9_./*|\-]+)\)\s*PROVIDER="([^"]+)"\s*;;`) + + // Pattern B: multi-line clauses like + // openai/*) + // if [ -n "${OPENAI_API_KEY:-}" ]; then + // PROVIDER="custom" + // ... + // We capture the patterns and the FIRST PROVIDER= that follows + // (before the next `;;`). The acceptedDivergences map handles the + // fact that the runtime branching can pick a different value. + multiLine := regexp.MustCompile(`(?ms)^\s*([a-zA-Z0-9_./*|\-]+)\)\s*\n(.*?);;`) + + addEntry := func(patterns, provider string) { + // Skip the `*)` catch-all — it has no Go counterpart by design. + if strings.TrimSpace(patterns) == "*" { + return + } + for _, alt := range strings.Split(patterns, "|") { + alt = strings.TrimSpace(alt) + // Each alternative is `/*` — strip the trailing `/*`. + alt = strings.TrimSuffix(alt, "/*") + if alt == "" { + continue + } + // First write wins — a single-line match outranks a multi-line + // fallback for the same patterns block (defensive; the regexes + // shouldn't overlap on the same line in practice). + if _, exists := out[alt]; !exists { + out[alt] = provider + } + } + } + + // Run single-line first so it claims its lines before the multi-line + // pass sees them. + consumed := map[int]bool{} + for _, m := range singleLine.FindAllStringSubmatchIndex(body, -1) { + addEntry(body[m[2]:m[3]], body[m[4]:m[5]]) + // Mark every line touched so multi-line pass can skip it. + for i := m[0]; i < m[1]; i++ { + consumed[i] = true + } + } + + for _, m := range multiLine.FindAllStringSubmatchIndex(body, -1) { + // Skip if the start of this match overlaps a single-line clause. + if consumed[m[0]] { + continue + } + patterns := body[m[2]:m[3]] + clauseBody := body[m[4]:m[5]] + // Extract the FIRST PROVIDER="..." from the clause body. + firstProvider := regexp.MustCompile(`PROVIDER="([^"]+)"`).FindStringSubmatch(clauseBody) + if firstProvider == nil { + t.Errorf("multi-line case clause for %q has no PROVIDER= literal — shell file shape changed; rebuild parser", patterns) + continue + } + addEntry(patterns, firstProvider[1]) + } + + return out +} + +// loadGoPrefixMap parses workspace_provision.go and walks the AST to +// extract the prefix→provider mapping from deriveProviderFromModelSlug's +// switch statement. +// +// Each case clause's string-literal labels become map keys, all +// pointing at the provider returned by that case body's `return "..."` +// statement. A clause like `case "alibaba", "dashscope", "qwen": +// return "alibaba"` produces three map entries. +// +// Skips the default clause (returns ""). Skips any case clause whose +// body's first statement isn't a single `return STRING_LITERAL` — those +// would need their own divergence handling and don't currently exist +// in the function. +func loadGoPrefixMap(t *testing.T) map[string]string { + t.Helper() + root := monorepoRoot(t) + goPath := filepath.Join(root, "workspace-server", "internal", "handlers", "workspace_provision.go") + + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, goPath, nil, parser.ParseComments) + if err != nil { + t.Fatalf("parse %s: %v", goPath, err) + } + + var fn *ast.FuncDecl + for _, decl := range file.Decls { + f, ok := decl.(*ast.FuncDecl) + if !ok { + continue + } + if f.Name.Name == "deriveProviderFromModelSlug" { + fn = f + break + } + } + if fn == nil { + t.Fatalf("could not find deriveProviderFromModelSlug in %s — function renamed/removed; this gate's invariant has been violated", goPath) + } + + // Walk the function body for the SwitchStmt. + var sw *ast.SwitchStmt + ast.Inspect(fn.Body, func(n ast.Node) bool { + if s, ok := n.(*ast.SwitchStmt); ok { + sw = s + return false + } + return true + }) + if sw == nil { + t.Fatalf("no switch statement found in deriveProviderFromModelSlug — function shape changed; rebuild parser") + } + + out := map[string]string{} + for _, stmt := range sw.Body.List { + clause, ok := stmt.(*ast.CaseClause) + if !ok { + continue + } + // Default clause has no list — skip. + if len(clause.List) == 0 { + continue + } + // Find the first return statement in the clause body. + var ret *ast.ReturnStmt + for _, bodyStmt := range clause.Body { + if r, ok := bodyStmt.(*ast.ReturnStmt); ok { + ret = r + break + } + } + if ret == nil || len(ret.Results) != 1 { + t.Errorf("case clause at %s has no single-value return — function shape changed; gate may be incomplete", + fset.Position(clause.Pos())) + continue + } + lit, ok := ret.Results[0].(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + t.Errorf("case clause at %s returns a non-literal — gate cannot extract provider value", + fset.Position(clause.Pos())) + continue + } + provider, err := strconv.Unquote(lit.Value) + if err != nil { + t.Errorf("case clause at %s has unparseable string literal %q: %v", + fset.Position(clause.Pos()), lit.Value, err) + continue + } + + for _, expr := range clause.List { + lbl, ok := expr.(*ast.BasicLit) + if !ok || lbl.Kind != token.STRING { + t.Errorf("case clause at %s has a non-string-literal label — gate cannot extract prefix", + fset.Position(clause.Pos())) + continue + } + prefix, err := strconv.Unquote(lbl.Value) + if err != nil { + t.Errorf("case clause at %s has unparseable label literal %q: %v", + fset.Position(clause.Pos()), lbl.Value, err) + continue + } + out[prefix] = provider + } + } + return out +} + +// TestDeriveProviderDrift_ShellParserIsSane is a guard test: the shell +// parser is regex-based, so we sanity-check that it actually finds the +// well-known prefixes documented in derive-provider.sh's header +// comment. If this test passes but the main drift test reports +// missing prefixes, the bug is almost certainly in the regex (not in +// the production code). +func TestDeriveProviderDrift_ShellParserIsSane(t *testing.T) { + t.Parallel() + shellMap := loadShellPrefixMap(t) + + // Anchor prefixes — these have lived in derive-provider.sh since it + // was first introduced. If the parser can't find them, it's broken. + mustHave := map[string]string{ + "anthropic": "anthropic", + "minimax": "minimax", + "minimax-cn": "minimax-cn", + "openrouter": "openrouter", + "custom": "custom", + "alibaba": "alibaba", // in an alias group with dashscope/qwen + "dashscope": "alibaba", // ditto + "qwen": "alibaba", // ditto + "openai": "custom", // multi-line; first PROVIDER= is "custom" + "nousresearch": "nous", // multi-line; first PROVIDER= is "nous" + } + + missing := []string{} + wrong := []string{} + for prefix, want := range mustHave { + got, ok := shellMap[prefix] + if !ok { + missing = append(missing, prefix) + continue + } + if got != want { + wrong = append(wrong, prefix+" got="+got+" want="+want) + } + } + sort.Strings(missing) + sort.Strings(wrong) + if len(missing) > 0 { + t.Errorf("shell parser failed to extract anchor prefixes: %v", missing) + } + if len(wrong) > 0 { + t.Errorf("shell parser extracted wrong values for anchor prefixes: %v", wrong) + } +} From dfeefb0accca74264dde40e0941526eb6cabd852 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sat, 2 May 2026 23:51:20 -0700 Subject: [PATCH 2/2] fix(workspace-server): vendor upstream derive-provider.sh + close 12-prefix drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drift gate's monorepoRoot walk-up looked for workspace-configs-templates/ which is gitignored locally and doesn't exist in this repo at all (the canonical script lives in molecule-ai-workspace-template-hermes). Test failed on CI from day one with "could not find monorepo root". Two layered fixes in one PR: 1. Vendor upstream derive-provider.sh as testdata/ + drop monorepoRoot. The vendored copy has a header pointing operators at the upstream source and a one-line cp command for refresh. Test now reads two files (vendored shell + workspace_provision.go) via package-relative paths — Go test sets cwd to the package dir, so this is hermetic without any walk-up gymnastics. 2. Update the case-statement regex to match upstream's renamed variable (${_HERMES_MODEL} since v0.12.0, the resolved value of HERMES_INFERENCE_MODEL with a HERMES_DEFAULT_MODEL legacy fallback). Regex now accepts either spelling so a future rename fails loudly on the parser-sanity check rather than silently returning empty. Vendoring upstream surfaced real drift the gate was designed to catch: upstream v0.12.0 added 12 provider prefixes that deriveProviderFromModelSlug didn't handle (xai/grok, bedrock/aws, tencent/tencent-tokenhub, gmi, qwen-oauth, lmstudio/lm-studio, minimax-oauth, alibaba-coding-plan, google-gemini-cli, openai-codex, copilot-acp, copilot). Without these, Save+Restart on a workspace using one of those prefixes would persist LLM_PROVIDER="" and the next boot would fall back to derive-provider.sh's runtime *=auto branch — losing the user's explicit choice on every restart. Added all 12 case clauses + 16 new table-driven test cases (covering both canonical and aliased forms). Drift gate now passes; future upstream additions will fail loudly with a "DRIFT: ..." message pointing the engineer at the missing case. Task: #242 --- .../handlers/derive_provider_drift_test.go | 76 ++++----- .../handlers/testdata/derive-provider.sh | 150 ++++++++++++++++++ .../internal/handlers/workspace_provision.go | 28 ++++ .../workspace_provision_shared_test.go | 19 +++ 4 files changed, 229 insertions(+), 44 deletions(-) create mode 100755 workspace-server/internal/handlers/testdata/derive-provider.sh diff --git a/workspace-server/internal/handlers/derive_provider_drift_test.go b/workspace-server/internal/handlers/derive_provider_drift_test.go index ca53584a..4bd21127 100644 --- a/workspace-server/internal/handlers/derive_provider_drift_test.go +++ b/workspace-server/internal/handlers/derive_provider_drift_test.go @@ -7,10 +7,12 @@ package handlers // workspace-server can persist LLM_PROVIDER into workspace_secrets at // provision time. That created two sources of truth: // -// 1. workspace-configs-templates/hermes/scripts/derive-provider.sh — +// 1. molecule-ai-workspace-template-hermes/scripts/derive-provider.sh — // runs inside the container at boot, has the final say on which // provider hermes targets (writes ~/.hermes/config.yaml's -// model.provider field). +// model.provider field). The shell script lives in a separate +// OSS repo, so we vendor a snapshot at testdata/derive-provider.sh +// to keep this gate hermetic. // 2. workspace-server/internal/handlers/workspace_provision.go's // deriveProviderFromModelSlug — runs at provision time on the // platform side so LLM_PROVIDER lands in workspace_secrets and @@ -41,17 +43,19 @@ package handlers // which pins the *values*; this test pins the *coverage* of the // prefix set itself. // -// Hermetic: only reads two files from the monorepo + parses them -// in-process. No network, no docker, no DB. +// Hermetic: reads two files (vendored shell script + Go source) from +// paths relative to the test package directory and parses them +// in-process. No network, no docker, no DB. The vendored shell script +// at testdata/derive-provider.sh is a snapshot of the upstream OSS +// template repo's script — refresh it via the cp command in that file's +// header when upstream changes. import ( "go/ast" "go/parser" "go/token" "os" - "path/filepath" "regexp" - "runtime" "sort" "strconv" "strings" @@ -184,30 +188,15 @@ func TestDeriveProviderDrift_ShellAndGoStayInSync(t *testing.T) { } } -// monorepoRoot resolves the absolute path of the molecule-monorepo -// root by walking up from this test file's directory. Avoids relying -// on a fixed CWD or env var. -func monorepoRoot(t *testing.T) string { - t.Helper() - _, thisFile, _, ok := runtime.Caller(0) - if !ok { - t.Fatalf("runtime.Caller failed — cannot locate test file path") - } - // .../workspace-server/internal/handlers/derive_provider_drift_test.go - dir := filepath.Dir(thisFile) - for i := 0; i < 6; i++ { - if _, err := os.Stat(filepath.Join(dir, "workspace-configs-templates")); err == nil { - return dir - } - parent := filepath.Dir(dir) - if parent == dir { - break - } - dir = parent - } - t.Fatalf("could not find monorepo root (looked for workspace-configs-templates/) walking up from %s", thisFile) - return "" -} +// vendoredShellPath is the testdata snapshot of upstream +// derive-provider.sh. The path is relative to the test package +// directory (which is what `go test` sets as cwd). See the file's +// header for the refresh procedure when upstream changes. +const vendoredShellPath = "testdata/derive-provider.sh" + +// goSourcePath is the file containing deriveProviderFromModelSlug. +// Relative to the test package directory. +const goSourcePath = "workspace_provision.go" // loadShellPrefixMap parses derive-provider.sh and returns a // map[prefix]provider for every case clause. Aliases inside a single @@ -228,26 +217,27 @@ func monorepoRoot(t *testing.T) string { // hardcode. func loadShellPrefixMap(t *testing.T) map[string]string { t.Helper() - root := monorepoRoot(t) - shellPath := filepath.Join(root, "workspace-configs-templates", "hermes", "scripts", "derive-provider.sh") - raw, err := os.ReadFile(shellPath) + raw, err := os.ReadFile(vendoredShellPath) if err != nil { - t.Fatalf("read %s: %v", shellPath, err) + t.Fatalf("read %s: %v (refresh from upstream — see file header)", vendoredShellPath, err) } // Locate the case statement body so we don't accidentally match // PROVIDER= assignments above the case (the HERMES_INFERENCE_PROVIDER - // override + HERMES_DEFAULT_MODEL empty fallback both write PROVIDER= - // before the case). - caseStart := regexp.MustCompile(`(?m)^case\s+"\$\{HERMES_DEFAULT_MODEL\}"\s+in\s*$`) + // override + the empty-model fallback both write PROVIDER= before + // the case). Upstream renamed the case variable to ${_HERMES_MODEL} + // in v0.12.0 (the resolved value of HERMES_INFERENCE_MODEL with a + // HERMES_DEFAULT_MODEL legacy fallback); accept either spelling so + // this test survives a future rename. + caseStart := regexp.MustCompile(`(?m)^case\s+"\$\{(_?HERMES(?:_DEFAULT|_INFERENCE)?_MODEL)\}"\s+in\s*$`) startLoc := caseStart.FindIndex(raw) if startLoc == nil { - t.Fatalf("could not locate `case \"${HERMES_DEFAULT_MODEL}\" in` in %s — shell file shape changed; rebuild parser", shellPath) + t.Fatalf("could not locate `case \"${...HERMES...MODEL}\" in` in %s — shell file shape changed; rebuild parser", vendoredShellPath) } caseEnd := regexp.MustCompile(`(?m)^esac\s*$`) endLoc := caseEnd.FindIndex(raw[startLoc[1]:]) if endLoc == nil { - t.Fatalf("could not locate `esac` after the case statement in %s — shell file shape changed", shellPath) + t.Fatalf("could not locate `esac` after the case statement in %s — shell file shape changed", vendoredShellPath) } body := string(raw[startLoc[1] : startLoc[1]+endLoc[0]]) @@ -336,13 +326,11 @@ func loadShellPrefixMap(t *testing.T) map[string]string { // in the function. func loadGoPrefixMap(t *testing.T) map[string]string { t.Helper() - root := monorepoRoot(t) - goPath := filepath.Join(root, "workspace-server", "internal", "handlers", "workspace_provision.go") fset := token.NewFileSet() - file, err := parser.ParseFile(fset, goPath, nil, parser.ParseComments) + file, err := parser.ParseFile(fset, goSourcePath, nil, parser.ParseComments) if err != nil { - t.Fatalf("parse %s: %v", goPath, err) + t.Fatalf("parse %s: %v", goSourcePath, err) } var fn *ast.FuncDecl @@ -357,7 +345,7 @@ func loadGoPrefixMap(t *testing.T) map[string]string { } } if fn == nil { - t.Fatalf("could not find deriveProviderFromModelSlug in %s — function renamed/removed; this gate's invariant has been violated", goPath) + t.Fatalf("could not find deriveProviderFromModelSlug in %s — function renamed/removed; this gate's invariant has been violated", goSourcePath) } // Walk the function body for the SwitchStmt. diff --git a/workspace-server/internal/handlers/testdata/derive-provider.sh b/workspace-server/internal/handlers/testdata/derive-provider.sh new file mode 100755 index 00000000..e82c0938 --- /dev/null +++ b/workspace-server/internal/handlers/testdata/derive-provider.sh @@ -0,0 +1,150 @@ +#!/usr/bin/env bash +# VENDORED COPY — DO NOT EDIT THIS FILE BY HAND. +# +# Source of truth: +# github.com/Molecule-AI/molecule-ai-workspace-template-hermes +# scripts/derive-provider.sh +# +# This snapshot is read by derive_provider_drift_test.go so the AST +# drift gate stays hermetic (no network, no submodule, no path-walk). +# When upstream changes, refresh via: +# +# cp ~/path/to/molecule-ai-workspace-template-hermes/scripts/derive-provider.sh \ +# workspace-server/internal/handlers/testdata/derive-provider.sh +# +# (and re-add the VENDORED COPY header below.) The drift test will +# fail loudly if upstream adds prefixes that deriveProviderFromModelSlug +# doesn't handle — fix it by adding the missing case to the Go function, +# not by silently widening acceptedDivergences. +# +# derive-provider.sh — map a hermes-agent model slug to its provider +# name. Sourced by both install.sh (SaaS bare-host path) and start.sh +# (Docker path) so the two entry-points stay consistent. +# +# Contract: +# Reads: $HERMES_INFERENCE_PROVIDER (if already set, we respect it) +# $HERMES_INFERENCE_MODEL (preferred — matches upstream env name) +# $HERMES_DEFAULT_MODEL (legacy fallback — name we invented before +# 2026-05; workspace-server still writes +# it during the migration window) +# $HERMES_API_KEY / $NOUS_API_KEY (affect the nousresearch/* branch) +# Writes: $PROVIDER — the derived provider name, or "auto" if unknown. +# +# Upstream's actual env var is $HERMES_INFERENCE_MODEL (see +# website/docs/reference/environment-variables.md in NousResearch/hermes-agent). +# We accept both for one release cycle so workspaces booting under the legacy +# control-plane don't break — drop $HERMES_DEFAULT_MODEL once workspace-server +# is updated to write the upstream name. +# +# Why the per-template sub-script (vs doing this in CP): every runtime +# has its own provider taxonomy. Keeping the logic inside the template +# repo means CP stays runtime-agnostic and adding a new runtime with +# different provider semantics doesn't require a CP edit. +# +# Hermes-specific quirks encoded here: +# - `openai/...` routes through `openrouter` (hermes has no direct +# openai provider; openai-codex is OAuth-only for Codex models) +# - `nousresearch/...` prefers direct `nous` if HERMES_API_KEY is +# set, else falls back to `openrouter` (which also serves Hermes 3) +# - chinese-region variants (minimax-cn, kimi-coding-cn) keep their +# full prefix as the provider name +# +# See molecule-controlplane/docs/canary-tenants.md and the hermes-agent +# providers.md docs for the full taxonomy. + +# Honour an explicit override. +if [ -n "${HERMES_INFERENCE_PROVIDER:-}" ]; then + PROVIDER="${HERMES_INFERENCE_PROVIDER}" + return 0 2>/dev/null || exit 0 +fi + +# Resolve the model slug — prefer the upstream env name, fall back to legacy. +_HERMES_MODEL="${HERMES_INFERENCE_MODEL:-${HERMES_DEFAULT_MODEL:-}}" + +if [ -z "${_HERMES_MODEL}" ]; then + PROVIDER="auto" + return 0 2>/dev/null || exit 0 +fi + +case "${_HERMES_MODEL}" in + # Keep full CN-suffix as provider so chinese-region keys route right + minimax-cn/*) PROVIDER="minimax-cn" ;; + kimi-coding-cn/*) PROVIDER="kimi-coding-cn" ;; + + # Direct-SDK providers (clean 1:1 prefix→provider mapping) + minimax/*) PROVIDER="minimax" ;; + anthropic/*) PROVIDER="anthropic" ;; + gemini/*) PROVIDER="gemini" ;; + deepseek/*) PROVIDER="deepseek" ;; + zai/*) PROVIDER="zai" ;; + kimi-coding/*) PROVIDER="kimi-coding" ;; + alibaba/*|dashscope/*|qwen/*) PROVIDER="alibaba" ;; + xiaomi/*|mimo/*) PROVIDER="xiaomi" ;; + arcee/*|arcee-ai/*) PROVIDER="arcee" ;; + nvidia/*|nim/*) PROVIDER="nvidia" ;; + ollama-cloud/*) PROVIDER="ollama-cloud" ;; + huggingface/*|hf/*) PROVIDER="huggingface" ;; + ai-gateway/*|aigateway/*) PROVIDER="ai-gateway" ;; + kilocode/*) PROVIDER="kilocode" ;; + opencode-zen/*) PROVIDER="opencode-zen" ;; + opencode-go/*) PROVIDER="opencode-go" ;; + + # Hermes-specific routing quirks. `openai/*` has two valid targets: + # 1. hermes's "custom" provider pointed at api.openai.com — requires + # OPENAI_API_KEY. install.sh sees this case and auto-populates + # HERMES_CUSTOM_{BASE_URL,API_KEY} so the direct-OpenAI path works + # without the user having to set HERMES_CUSTOM_* explicitly. + # 2. OpenRouter (hermes's built-in path — requires OPENROUTER_API_KEY). + # + # Priority: prefer **custom** (direct OpenAI) when OPENAI_API_KEY is set. + # The operator supplying OPENAI_API_KEY for an openai/* model is an + # explicit intent signal to hit OpenAI directly. The previous "prefer + # OR if any OR key exists" rule silently hijacked that intent whenever + # a tenant-global OPENROUTER_API_KEY was present (even if stale/empty + # enough to 401), which is exactly what bit the 2026-04-23 E2E (surfaced + # as OpenRouter's `401 Missing Authentication header` in the agent reply). + # + # To explicitly route openai/* through OR, set HERMES_INFERENCE_PROVIDER=openrouter + # (handled at the top of this file) or use an openrouter/* model slug. + openai/*) + if [ -n "${OPENAI_API_KEY:-}" ]; then + PROVIDER="custom" + elif [ -n "${OPENROUTER_API_KEY:-}" ]; then + PROVIDER="openrouter" + else + PROVIDER="openrouter" # no-key fallback — hermes will error clearly + fi + ;; + nousresearch/*) + # Prefer direct Nous Portal if Nous credentials present, else OR. + if [ -n "${HERMES_API_KEY:-}" ] || [ -n "${NOUS_API_KEY:-}" ]; then + PROVIDER="nous" + else + PROVIDER="openrouter" + fi + ;; + + # Explicit catch-alls + openrouter/*) PROVIDER="openrouter" ;; + custom/*) PROVIDER="custom" ;; + + # Additional 1:1 prefix→provider mappings — kept aligned with upstream's + # HERMES_INFERENCE_PROVIDER list (website/docs/reference/environment-variables.md + # in NousResearch/hermes-agent, v0.12.0 / 2026-04-30). Place these BEFORE the + # catch-all so they win. + xai/*|grok/*) PROVIDER="xai" ;; + bedrock/*|aws/*) PROVIDER="bedrock" ;; + tencent/*|tencent-tokenhub/*) PROVIDER="tencent-tokenhub" ;; + gmi/*) PROVIDER="gmi" ;; + qwen-oauth/*) PROVIDER="qwen-oauth" ;; + lmstudio/*|lm-studio/*) PROVIDER="lmstudio" ;; + minimax-oauth/*) PROVIDER="minimax-oauth" ;; + alibaba-coding-plan/*) PROVIDER="alibaba-coding-plan" ;; + google-gemini-cli/*) PROVIDER="google-gemini-cli" ;; + openai-codex/*) PROVIDER="openai-codex" ;; + copilot-acp/*) PROVIDER="copilot-acp" ;; + copilot/*) PROVIDER="copilot" ;; + + # Unknown prefix → let hermes auto-detect + *) PROVIDER="auto" ;; +esac diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index 561860f9..e34ea315 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -662,6 +662,34 @@ func deriveProviderFromModelSlug(model string) string { // extra config) and let the script upgrade to nous/custom at runtime. case "nousresearch", "openai": return "openrouter" + // Additional 1:1 prefix→provider mappings — kept aligned with upstream's + // HERMES_INFERENCE_PROVIDER list (NousResearch/hermes-agent v0.12.0, + // 2026-04-30) and the additional case clauses in derive-provider.sh. + // The drift gate in derive_provider_drift_test.go enforces parity. + case "xai", "grok": + return "xai" + case "bedrock", "aws": + return "bedrock" + case "tencent", "tencent-tokenhub": + return "tencent-tokenhub" + case "gmi": + return "gmi" + case "qwen-oauth": + return "qwen-oauth" + case "lmstudio", "lm-studio": + return "lmstudio" + case "minimax-oauth": + return "minimax-oauth" + case "alibaba-coding-plan": + return "alibaba-coding-plan" + case "google-gemini-cli": + return "google-gemini-cli" + case "openai-codex": + return "openai-codex" + case "copilot-acp": + return "copilot-acp" + case "copilot": + return "copilot" } // Unknown prefix → don't persist a guess. derive-provider.sh's // *=auto fallback handles it at runtime. diff --git a/workspace-server/internal/handlers/workspace_provision_shared_test.go b/workspace-server/internal/handlers/workspace_provision_shared_test.go index 2166cb23..51391c93 100644 --- a/workspace-server/internal/handlers/workspace_provision_shared_test.go +++ b/workspace-server/internal/handlers/workspace_provision_shared_test.go @@ -437,6 +437,25 @@ func TestDeriveProviderFromModelSlug(t *testing.T) { // boot if HERMES_API_KEY/OPENAI_API_KEY are present). {"nousresearch defaults to openrouter at provision time", "nousresearch/hermes-4-70b", "openrouter"}, {"openai defaults to openrouter at provision time", "openai/gpt-5", "openrouter"}, + // hermes-agent v0.12.0 / 2026-04-30 provider list — the drift gate + // in derive_provider_drift_test.go pins parity with the shell case + // statement. + {"xai", "xai/grok-4", "xai"}, + {"xai via grok alias", "grok/grok-4", "xai"}, + {"bedrock", "bedrock/anthropic.claude-sonnet-4-6", "bedrock"}, + {"bedrock via aws alias", "aws/anthropic.claude-sonnet-4-6", "bedrock"}, + {"tencent", "tencent/hunyuan-coder", "tencent-tokenhub"}, + {"tencent-tokenhub passthrough", "tencent-tokenhub/hunyuan-coder", "tencent-tokenhub"}, + {"gmi", "gmi/gmi-coder-1", "gmi"}, + {"qwen-oauth", "qwen-oauth/qwen3-coder", "qwen-oauth"}, + {"lmstudio", "lmstudio/qwen3-coder", "lmstudio"}, + {"lmstudio via lm-studio alias", "lm-studio/qwen3-coder", "lmstudio"}, + {"minimax-oauth", "minimax-oauth/MiniMax-M2.7", "minimax-oauth"}, + {"alibaba-coding-plan", "alibaba-coding-plan/qwen3-coder", "alibaba-coding-plan"}, + {"google-gemini-cli", "google-gemini-cli/gemini-2.5-pro", "google-gemini-cli"}, + {"openai-codex", "openai-codex/gpt-5-codex", "openai-codex"}, + {"copilot-acp", "copilot-acp/claude-sonnet-4-6", "copilot-acp"}, + {"copilot", "copilot/claude-sonnet-4-6", "copilot"}, // Unknowns return "" so the caller skips the LLM_PROVIDER write // and lets derive-provider.sh's *=auto branch decide at runtime. {"unknown prefix returns empty", "totally-unknown-model/foo", ""},