Test: AST drift gate for derive-provider.sh ↔ Go port - #2537
Merged
Merged
Conversation
HongmingWang-Rabbit
requested a review
from hongmingwang-moleculeai
as a code owner
May 3, 2026 02:49
HongmingWang-Rabbit
enabled auto-merge
May 3, 2026 02:49
added 2 commits
May 2, 2026 23:51
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.
…prefix drift
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
HongmingWang-Rabbit
force-pushed
the
test/derive-provider-drift-gate
branch
from
May 3, 2026 06:51
898a312 to
dfeefb0
Compare
Contributor
|
🔒 Auto-merge disabled — new commit ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PR #2535 introduced a Go port of
workspace-configs-templates/hermes/scripts/derive-provider.sh(deriveProviderFromModelSluginworkspace-server/internal/handlers/workspace_provision.go) so the platform can persistLLM_PROVIDERtoworkspace_secretsat provision time and survive Save+Restart. Independent code review flagged that the port created two sources of truth — if a future PR adds a provider prefix to one without the other, the platform's persistedLLM_PROVIDERsilently disagrees with what the container'sderive-provider.shproduces at boot. The container always wins (it writes~/.hermes/config.yaml), so the platform's value becomes stale and misleading without any test going red.This PR adds a hermetic drift gate (one new test file, no production-code changes) that pins the invariant: the prefix set the two functions know about must be identical, modulo a hardcoded
acceptedDivergencesmap for the two documented runtime-only differences (nousresearch/*andopenai/*both fall back toopenrouterat provision time becausederive-provider.sh'sHERMES_API_KEY/OPENAI_API_KEYchecks aren't available until container boot).Approach
Behavior-based AST gate, same pattern as PR #2367 (TeamHandler.Expand drift) — pin the invariant by what the function maps, not by what it's named:
derive-provider.shwith regex (handles both single-linepat/*) PROVIDER="x" ;;clauses and multi-line conditional clauses; firstPROVIDER=literal wins for the conditionals); skip the*)catch-all because itsautovalue has no Go counterpart by design.workspace_provision.gowithgo/ast, locatederiveProviderFromModelSlugby name, walk the*ast.SwitchStmt, extract every*ast.CaseClause's string-literal labels paired with the case body's firstreturn STRING_LITERAL.acceptedDivergencesstill appears in both maps (a stale entry silently weakens the gate).acceptedDivergences{}with a comment.A companion
TestDeriveProviderDrift_ShellParserIsSanepins anchor prefixes (anthropic,minimax-cn, theopenai/nousresearchmulti-line clauses, thedashscope/qwenalias group) so a future shell-syntax change can't silently produce an empty map and trivially pass the main gate.Constraints respected
go/ast,go/parser,go/token,regexp. No new dependencies.workspace_provision.go,derive-provider.sh) untouched.derive-provider.sh, not the standalone mirror atmolecule-ai-workspace-template-hermes— the monorepo copy ships in the same git SHA as the Go code; mirror drift is task fix(scheduler): recover from panics + add liveness watchdog (#85) #90's concern.Closes task #242. Companion to PR #2535's table-driven mapping test in
workspace_provision_shared_test.go(which pins the values; this gate pins the coverage of the prefix set itself).Test plan
go test ./workspace-server/internal/handlers/... -run "DeriveProviderDrift" -v— passes (29 prefixes extracted from each side, drift directions both green)go vet ./workspace-server/...— cleango build ./...— clean🤖 Generated with Claude Code