Skip to content

give the OpenCode reviewer a model dropdown and hide reviewers this machine can't run - #6078

Merged
atomantic merged 1 commit into
mainfrom
cos/task-mtln0anl/agent-6b8a0c46
Sep 3, 2026
Merged

give the OpenCode reviewer a model dropdown and hide reviewers this machine can't run#6078
atomantic merged 1 commit into
mainfrom
cos/task-mtln0anl/agent-6b8a0c46

Conversation

@atomantic

Copy link
Copy Markdown
Owner

Summary

  • OpenCode's Model cell is a dropdown, not a text box. Its options come from the seeded OpenCode Zen CLI/TUI records, whose namespaced opencode/* ids the Harnesses page refreshes from opencode models. The opencode-<local-backend> wrappers stay excluded — their ids resolve only under the OPENCODE_CONFIG_CONTENT a PortOS-spawned provider injects, and the reviewer runs a bare opencode against the user's own config. Still free-text underneath, so a config-declared namespace can be typed.
  • Each reviewer's options now union every provider record fronting its binary instead of reading one. Reading a single record is what showed the Claude reviewer the retired claude-sonnet-4-6 from claude-code while claude-code-tui already listed claude-sonnet-5. The CLI record still owns the shown default, since the reviewer is spawned non-interactively. Bedrock records are excluded (their us.anthropic.* ids resolve only in that environment).
  • The Add row hides reviewers this machine can't run — CLI missing, or every backing provider record switched off — behind a +N unavailable toggle. Hidden, not dropped: both checks are local-machine-only and the reviewer list is federation-wide config, so the toggle reveals them badged with which signal fired, and an already-selected reviewer always renders its row.
  • Migration 337 repairs the stale data. claude has no models subcommand, so nothing in the app can refresh a stale Claude record. It inserts claude-sonnet-5 after a retired claude-sonnet-4-6 even on a curated list, which migration 153's exact-match policy skipped.

Why the migration is additive

153/206 rewrote models only on a list matching the prior seed exactly, leaving a user who had appended one id stuck on the retired tier. 337 targets exactly those records, so it takes the opposite policy: claude-sonnet-4-6 and every tier pointer survive untouched. That id still resolves for the CLI, and the defect is the new tier being absent, not the old one being present. A record already listing claude-sonnet-5 is untouched, so it is a no-op on a seeded install and on a second run.

Test plan

  • client: 1952 tests pass across components/cos, hooks, components/settings.
  • New hook tests pin the union (Claude CLI+TUI, grok CLI+TUI), OpenCode's Zen-only sourcing, that a local-runtime wrapper's ids are never offered, OpenCode's deliberately-null default, and providerDisabled separating "all switched off" from "no records" and "fetch failed".
  • New picker tests pin the hide/reveal behavior, both badge reasons, that a revealed reviewer still adds normally, that an already-selected unavailable reviewer stays visible, and that nothing is hidden when neither signal was fetched.
  • New migration tests cover a curated list, each Bedrock record's region-qualified spelling, idempotency across two runs, non-target records left alone, and missing/malformed/non-array inputs.
  • Migration verified against the four seeded Claude ids; scripts/migrations + reviewerConfig + codeReview suites pass (2072 tests).

Follow-up filed as #6072: makeSeededProviderTierMigration exists to generalize this shape but no migration calls it, so 337 is the third hand-rolled copy of the same scaffolding.

…achine can't run

The reviewer picker's Model cell was a bare text input for OpenCode, and its
Add row offered every reviewer in the roster regardless of whether the machine
could run one.

- OpenCode's options now come from the seeded OpenCode Zen CLI/TUI records,
  whose namespaced `opencode/*` ids the Harnesses page refreshes from
  `opencode models`. The local-runtime wrappers stay excluded: their ids resolve
  only under the config a PortOS-spawned provider injects, and the reviewer runs
  a bare `opencode` against the user's own config.
- Each reviewer's options now UNION every provider record fronting its binary
  instead of reading one. Sourcing from a single record is what hid
  `claude-sonnet-5` behind the CLI record's retired `claude-sonnet-4-6` while
  `claude-code-tui` already listed the current tier.
- The Add row folds a reviewer whose CLI is missing, or whose provider records
  are all switched off, behind a `+N unavailable` toggle. Hidden rather than
  dropped: both checks are local-machine-only and the reviewer list is
  federation-wide config, so the toggle reveals them badged with which signal
  fired, and an already-selected reviewer always renders its row.

Migration 337 repairs the underlying data: `claude` has no `models` subcommand,
so nothing in the app can refresh a stale Claude record. It inserts
`claude-sonnet-5` after a retired `claude-sonnet-4-6` even on a curated list,
which migration 153's exact-match policy skipped. Additive on purpose — the
retired id and every tier pointer survive, since 4-6 still resolves and the
defect is the new tier being absent, not the old one being present.
@atomantic
atomantic merged commit 62ce5ea into main Sep 3, 2026
7 checks passed
@atomantic
atomantic deleted the cos/task-mtln0anl/agent-6b8a0c46 branch September 3, 2026 15:19
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