Skip to content

fix: detect pre-#100 legacy model-override IDs in opencode.json#110

Merged
iceglober merged 1 commit into
mainfrom
fix/pilot-agents-provider-model-not-found
Apr 26, 2026
Merged

fix: detect pre-#100 legacy model-override IDs in opencode.json#110
iceglober merged 1 commit into
mainfrom
fix/pilot-agents-provider-model-not-found

Conversation

@iceglober

Copy link
Copy Markdown
Owner

Detect legacy model-override IDs that stomp plugin defaults

Goal

Add a plugin-side safety net that detects pre-#100 legacy bedrock/claude-<model> / vertex/<…> IDs set by users in opencode.json's models block and warns visibly at runtime and in doctor. Root cause: resolveHarnessModels() in src/config-hook.ts applies user overrides unconditionally without validation, so stale IDs from pre-#100 installs (like bedrock/claude-opus-4 — missing anthropic. prefix, missing version digit) silently stomp the plugin's correct Catwalk IDs, producing ProviderModelNotFoundError at subagent invocation. The PRIME's session that produced this plan repaired one user's opencode.json out-of-band; this plan ships the general fix so other pre-#100 users self-heal.

Constraints

  • Zero user-filesystem-writes invariant at plugin startup. The validator emits warnings only — it MUST NOT auto-rewrite opencode.json.
  • Prompt files read via readFileSync, never static import.
  • Changeset required (patch bump — defensive warning, no behavior change to happy paths).
  • Must not change resolveHarnessModels's precedence (per-agent > per-tier > default still wins).
  • Validator must be conservative: flag ONLY confidently-broken patterns, stay silent on unknown/unclear forms (e.g., AWS CRIS global.anthropic.* prefix).
  • Tests: bun test must pass; new tests must cover valid + invalid cases.
  • Build + typecheck: bun run build && bun run typecheck must pass.

Acceptance criteria

  • src/model-validator.ts exports validateModelOverride(id: string): { valid: boolean; reason?: string; suggestion?: string } with conservative regex-based detection.
  • Validator returns { valid: false, reason, suggestion } for every pre-fix: correct agent modes, model IDs, and add Catwalk-powered installer #100 legacy form: bedrock/claude-opus-4, bedrock/claude-sonnet-4, bedrock/claude-haiku-4, bedrock/claude-opus, vertex/claude-opus-4, vertexai/claude-opus-4.
  • Validator returns { valid: true } for current Catwalk-canonical forms: anthropic/claude-opus-4-7, anthropic/claude-sonnet-4-6, anthropic/claude-haiku-4-5-20251001, anthropic/claude-haiku-4-5, bedrock/anthropic.claude-opus-4-7, bedrock/anthropic.claude-sonnet-4-6, bedrock/anthropic.claude-haiku-4-5-20251001-v1:0, vertexai/claude-opus-4-7@20250610.
  • Validator returns { valid: true } for unknown-but-plausible forms (e.g. global.anthropic.claude-opus-4-7, openai/gpt-5, arbitrary Catwalk-like IDs we don't recognize). Only the specific legacy <bedrock|vertex|vertexai>/claude-<...> malformed pattern is flagged.
  • resolveHarnessModels() in src/config-hook.ts calls the validator on every override value it applies (both per-agent and per-tier paths, both pluginOptions.models and legacy config.harness.models sources). On invalid, it emits a single console.warn per unique bad ID per call, format: [@glrs-dev/harness-opencode] Warning: invalid model override "<id>" (from <source>). <suggestion>. Run \bunx @glrs-dev/harness-opencode doctor` for details.whereis e.g.models.deepormodels.pilot-planner`.
  • Resolver still mutates agentCfg.model to the bad value (preserves user intent; warning is advisory only). Never aborts startup.
  • doctor() in src/cli/doctor.ts adds a new check that reads opencode.json's plugin-tuple options.models (and legacy top-level harness.models fallback), runs every string value through the validator, and prints a red X line for each invalid entry with the key path, the offending value, the validator's suggestion, and the remediation hint "remove this key or replace with <suggestion>".
  • test/model-validator.test.ts exists, covers valid + invalid + unknown cases, passes.
  • test/config-hook.test.ts (new or extended) asserts the warn is emitted exactly once per unique bad ID even when multiple agents hit the same bad tier override, and that agentCfg.model still gets set to the bad value.
  • test/doctor.test.ts (new or extended) runs doctor against a fixture opencode.json with bad model IDs and asserts the red-X line + suggestion text appear in stdout.
  • bun run build && bun run typecheck && bun test all pass.
  • npm publish --dry-run shows the new dist/model-validator.js (and .d.ts) ship in the tarball.
  • A changeset file exists at .changeset/<slug>.md with patch bump and a clear description.

File-level changes

src/model-validator.ts (NEW)

  • Change: Create the pure validator module. Export validateModelOverride and the internal pattern constants (for test import). Core detection logic: any ID matching ^(bedrock|vertex|vertexai)/claude-(opus|sonnet|haiku)(-\d+)?$ (no anthropic. subpath, no @YYYYMMDD vertex suffix, no -YYYYMMDD-v1:0 bedrock suffix) is flagged as pre-fix: correct agent modes, model IDs, and add Catwalk-powered installer #100 legacy. Provide a suggestion that maps the legacy ID to its Catwalk-preset equivalent (look up via shared MODEL_PRESETS — import from src/cli/install.ts if shape allows, otherwise duplicate the small mapping inline to avoid a CLI→runtime dependency).
  • Why: Central, testable, pure detection — reusable by both resolveHarnessModels (runtime) and doctor (CLI).
  • Risk: low. Pure function, no I/O, no side effects.

src/config-hook.ts

  • Change: Import validateModelOverride from ../model-validator.js. In resolveHarnessModels, after each agentCfg.model = ... mutation, call the validator on the chosen value. Maintain a Set<string> of already-warned IDs scoped to the single resolveHarnessModels invocation. For each invalid ID not yet warned, emit console.warn(...) with the source string (models.<agentName> for per-agent, models.<tier> for per-tier). Never abort.
  • Why: Put the warn at the exact point the bad ID lands on an agent — the user sees the warn before any subagent invocation attempts and fails.
  • Risk: low. Only adds a call + warn; precedence, mutation semantics, and return value unchanged. Existing test/agents.test.ts and any config-hook test should still pass untouched.

src/cli/doctor.ts

  • Change: Add a new check after the plugin-in-opencode.json check. If the config parse succeeded, extract the plugin tuple's options.models block (also check legacy config.harness.models). For every string or first-element-of-string-array value, call validateModelOverride. Accumulate invalid entries; print a red-X line per entry with key path + value + suggestion + remediation hint ("To fix: remove this key, or replace with <suggestion>"). If no invalid entries, print a green-check "all model overrides valid" line (only when a models block exists — stay silent when there's none, to avoid clutter on default installs).
  • Why: Gives users a direct, actionable diagnosis with a fix path when they bunx @glrs-dev/harness-opencode doctor after hitting the error in TUI.
  • Risk: low. Doctor always exits 0; new check is pure read + print.

test/model-validator.test.ts (NEW)

  • Change: Unit tests for validateModelOverride. Three buckets: (a) valid — includes current Catwalk IDs, Anthropic aliases, Bedrock dated, Vertex dated, CRIS-style global.*, unknown openai/gpt-5. (b) invalid — every pre-fix: correct agent modes, model IDs, and add Catwalk-powered installer #100 legacy form. (c) edge — empty string, whitespace-only, non-string types (pass-through — TS typing gates non-strings but test anyway with as any). Assert valid boolean and that suggestion maps to the expected Catwalk-preset ID.
  • Why: Lock the detection surface; prevent the validator from silently becoming too aggressive (flagging valid IDs) or too permissive (missing a legacy form).
  • Risk: none.

test/config-hook.test.ts (NEW or EXTEND)

  • Change: Check if file exists via ls test/. If exists, extend; else create. Test: call resolveHarnessModels with an agents map containing 3+ agents that all map to the same tier (e.g., all three to deep), and a pluginOptions.models.deep = ["bedrock/claude-opus-4"]. Spy on console.warn. Assert: (a) each agent's .model ends up as "bedrock/claude-opus-4", (b) console.warn called exactly once, (c) warn text includes "bedrock/claude-opus-4", "models.deep", and the suggestion.
  • Why: Verify dedup works (one warn per unique ID, not per agent) and that resolver semantics are otherwise unchanged.
  • Risk: low.

test/doctor.test.ts (NEW or EXTEND)

  • Change: Check if file exists. If exists, extend; else create with fixture-based pattern (see test/merge-config.test.ts for the fixture style — temp dir, write opencode.json, capture stdout). Run doctor() against a fixture with { plugin: [["@glrs-dev/harness-opencode", { models: { deep: ["bedrock/claude-opus-4"] } }]] }. Assert stdout contains a red-X-prefixed line with models.deep, bedrock/claude-opus-4, and the suggestion. Second fixture: valid overrides → green check line.
  • Why: End-to-end doctor check — proves the user's terminal experience on bunx doctor.
  • Risk: low. If doctor() writes directly to console.log (it does), stdout capture via spyOn(console, "log") is the pattern.

.changeset/.md (NEW)

  • Change: Run bunx changeset → choose patch → write description: "Detect pre-fix: correct agent modes, model IDs, and add Catwalk-powered installer #100 legacy model-override IDs (e.g. bedrock/claude-opus-4) in opencode.json and warn at plugin startup and in doctor. These invalid IDs were silently stomping plugin defaults and causing ProviderModelNotFoundError at subagent invocation. The plugin now emits a one-line warn with the offending ID, its source key, and a Catwalk-preset suggestion. Run bunx @glrs-dev/harness-opencode doctor for full detail. No auto-rewrite of user config."
  • Why: Repo release contract requires changesets for every user-visible change.
  • Risk: none.

Test plan

  1. Unit: bun test test/model-validator.test.ts — all cases green.
  2. Integration (resolver): bun test test/config-hook.test.ts — warn dedup + mutation semantics green.
  3. Integration (doctor): bun test test/doctor.test.ts — red-X fixture + green-check fixture green.
  4. Existing: bun test — entire suite still passes. In particular test/agents.test.ts and test/merge-config.test.ts untouched.
  5. Build: bun run build — dist/model-validator.js emitted, dist/cli.js and dist/index.js both include the new warn path.
  6. Typecheck: bun run typecheck — no errors.
  7. Package: npm publish --dry-rundist/model-validator.js + .d.ts present in tarball. Size delta < 5 KB.
  8. Manual smoke: After building, node -e "const { validateModelOverride } = require('./dist/model-validator.js'); console.log(validateModelOverride('bedrock/claude-opus-4'))" returns { valid: false, reason: '...', suggestion: 'bedrock/anthropic.claude-opus-4-6' } (or similar).
  9. End-to-end doctor: copy a bad-IDs fixture over ~/.config/opencode/opencode.json in a throwaway shell, run bunx @glrs-dev/harness-opencode doctor, observe the red-X line. Restore. (Skip if CI sandbox; unit+integration tests cover it.)

Out of scope

  • Auto-rewriting the user's opencode.json. The plugin stays read-only on user config; only the CLI's install subcommand mutates it (and only via non-destructive merge).
  • Online Catwalk validation at plugin-startup runtime. The validator is pattern-based and offline — Catwalk API calls are install-time only.
  • Changing resolveHarnessModels's precedence rules. Per-agent beats per-tier beats plugin default — unchanged.
  • One-time user opencode.json repair. PRIME already performed the repair out-of-band on the reporting user's machine (backup saved to ~/.config/opencode/opencode.json.bak.1777167218); this plan's code does not touch user config.
  • src/agents/prompts/code-searcher.md alias-vs-dated-form review. anthropic/claude-haiku-4-5 is a valid Anthropic API alias; not the current bug. Separate concern.
  • Changing AGENT_TIERS mapping.
  • Changing the installer flow (src/cli/install.ts). The installer's presets are already correct post-fix: correct agent modes, model IDs, and add Catwalk-powered installer #100; this plan does not touch it.

Open questions

  • None — the validator surface, the warn format, the doctor integration, and the test locations are all specified above. Implementation may uncover small decisions (e.g., exact regex anchors, exact suggestion text) but nothing that requires user input.

Users who installed before PR #100 keep stale tier overrides like
`models.deep = ["bedrock/claude-opus-4"]` in their opencode.json.
resolveHarnessModels applied them unconditionally, stomping the
plugin's correct Catwalk-canonical defaults (`anthropic/claude-opus-4-7`
etc.) and crashing every subagent invocation with ProviderModelNotFoundError
— pilot-planner and qa-reviewer hit first because they're typically
delegated first.

Add a pure offline pattern validator (src/model-validator.ts) that flags
the specific legacy form `<bedrock|vertex|vertexai>/claude-<opus|sonnet|haiku>(-\d+)?`
and maps each form to the Catwalk-preset replacement. Conservative:
unknown or CRIS-prefixed IDs stay silent.

resolveHarnessModels now warns once per unique bad ID (deduped per call)
to stderr, naming the offending key and suggesting the fix. The bad
value is still written to agentCfg.model so the user's intent is
preserved — the warn is advisory, not corrective.

`bunx … doctor` gains a model-overrides check that walks both plugin
options.models and legacy top-level harness.models, prints a red-X line
with the reason and remediation hint per invalid entry, and a green
check when a models block exists with all valid entries. Silent on
default installs.

No user config is ever auto-rewritten.

Plan: ~/.glorious/opencode/glorious-opencode/plans/pilot-agents-provider-model-not-found.md
@iceglober iceglober merged commit 467df1d into main Apr 26, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant