Skip to content

fix(settings): show resolved model in Local CLI tests - #6183

Closed
MumuTW wants to merge 1 commit into
nexu-io:mainfrom
MumuTW:agent/show-resolved-cli-model
Closed

fix(settings): show resolved model in Local CLI tests#6183
MumuTW wants to merge 1 commit into
nexu-io:mainfrom
MumuTW:agent/show-resolved-cli-model

Conversation

@MumuTW

@MumuTW MumuTW commented Jul 28, 2026

Copy link
Copy Markdown

Why

When I selected a rolling Claude Code alias such as opus in Settings and clicked Test, Open Design only reported that Claude Code replied successfully. That proved connectivity, but not which concrete model actually handled the request.

Claude Code already includes the concrete model in its system/init stream event, and Open Design already parses that value. The connection-test sink discarded the status event, however, so the response preserved only the requested slot (opus or default) and the UI could not show what the alias resolved to.

This addresses the resolved-model transparency portion of #6180. The stale Claude fallback catalog is intentionally not changed here because #6095 already refreshes that list and includes Fable 5/current-generation coverage.

What users will see

A successful Local CLI connection test now appends the concrete model reported by the agent when it differs from the selected alias/default slot. For example:

Claude Code replied in 42 ms — 'ok' Model: claude-opus-5

Exact pinned selections are not repeated when the reported model equals the selected model.

Surface area

  • UI — the existing Settings → Local CLI test-result message shows the resolved model
  • Keyboard shortcut — new or changed
  • CLI / env var — new od subcommand or flag, new tools-dev / tools-pack / tools-pr flag, or new OD_* env var
  • API / contract — adds the optional, backwards-compatible ConnectionTestResponse.resolvedModel field
  • Extension point — new entry under skills/, design-systems/, design-templates/, or craft/, or change to the skills protocol
  • i18n keys — no new key; the message reuses the existing localized settings.model label
  • New top-level dependency
  • Default behavior change — successful alias/default tests show one additional model detail
  • None

CLI parity

This does not add a new connection-test capability or endpoint; it preserves metadata in an existing Settings-only test flow. There is currently no od connection-test command to extend. Adding a new CLI command would be a separate user-facing capability and outside this focused bug fix. Any future CLI consumer of /api/test/connection will receive the same optional resolvedModel field from the shared contract.

Screenshots

The visible change is a single appended detail in the existing Local CLI test status row:

Before: Claude Code replied in 42 ms — 'ok'
After:  Claude Code replied in 42 ms — 'ok' Model: claude-opus-5

The rendered behavior is covered by apps/web/tests/components/SettingsDialog.execution.test.tsx. This draft does not include an uploaded screenshot yet.

Bug fix verification

  • Daemon red spec: apps/daemon/tests/connection-test.test.tsreports the concrete model resolved from a Claude alias
  • UI red spec: apps/web/tests/components/SettingsDialog.execution.test.tsxshows the concrete model reported by a Local CLI connection test
  • Red on main, green on this branch? Yes. On main, the daemon result lacks resolvedModel, and Settings omits the model from the success message. Both focused specs pass after the fix.

Validation

Passed:

  • pnpm guard
  • pnpm typecheck (full workspace)
  • pnpm --filter @open-design/contracts typecheck
  • pnpm --filter @open-design/daemon typecheck
  • pnpm --filter @open-design/web typecheck
  • Focused daemon regression: 1 passed
  • Focused web regression: 1 passed
  • Full web suite: 441 files passed; 4,866 tests passed, 7 skipped

Full daemon suite result on this machine:

  • 480 files passed; 6,098 tests passed
  • 4 files / 12 tests failed in unrelated environment-sensitive areas: AMR session-resume integration (8), Langfuse/Vela endpoint selection (2), and xAI no-credential expectations (2)
  • The two Langfuse failures are the same main-baseline failures already documented on fix(daemon): refresh Claude Code fallback model list to the current generation #6095. The AMR/xAI failures do not import or exercise the changed contract, Claude connection-test sink, or Settings result rendering. The directly affected connection-test regression is green.

Implementation

  • Capture the existing Claude status / initializing model in the connection-test sink.
  • Preserve both meanings in the response: model remains the requested alias/default slot; resolvedModel contains the concrete model reported by the CLI.
  • Render the concrete model only for successful Local CLI tests and avoid duplicate output for exact pinned IDs.
  • Keep the shared contract additive and optional for backwards compatibility.

@lefarcen
lefarcen requested a review from mrcfps July 28, 2026 16:31
@lefarcen lefarcen added size/M PR changes 100-300 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix labels Jul 28, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

Thanks @MumuTW — the scope here is clear. Since this is still a draft, we'll hold off on review until you mark it ready for review.

@mrcfps mrcfps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MumuTW I verified the additive resolvedModel contract, the Claude system/init model capture in the connection-test sink, and the Settings success-message rendering across every changed range. The requested alias remains preserved in model, exact pinned IDs are not repeated, and the focused daemon/web regressions, affected-package typechecks, and repository guard all pass locally. Thanks for the clear, well-scoped fix and solid regression coverage — nicely done! 🙌

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@lefarcen lefarcen added the needs-validation Runtime change detected; needs human or /explore agent validation. label Jul 28, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we'll loop QA in now that it's ready for that handoff.

@lefarcen
lefarcen requested a review from AmyShang-alt July 28, 2026 18:44
@AmyShang-alt AmyShang-alt added the needs-product-review Feature PR awaiting product sign-off before merge (see roadmap) label Jul 29, 2026
@AmyShang-alt

Copy link
Copy Markdown
Contributor

QA Accepted.\n\nAutomated validation passed:\n- Claude alias resolved-model daemon regression: passed\n- Settings resolved-model UI regression: passed\n- Contracts/daemon/web typechecks: passed\n- Related web test file: 150 passed\n\nTwo unrelated OpenCode timeout assertions remain environment-sensitive and are outside this PR scope.\n\nReal-device Electron + authenticated Claude Code verification is handed off to Product for acceptance.

@lefarcen lefarcen added validated Runtime change validated (via /explore Pass or manual QA). and removed needs-validation Runtime change detected; needs human or /explore agent validation. labels Jul 29, 2026
@AmyShang-alt AmyShang-alt removed the needs-product-review Feature PR awaiting product sign-off before merge (see roadmap) label Jul 29, 2026
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

@MumuTW friendly reminder: this PR appears to be waiting on author action (outstanding trusted reviewer or maintainer feedback, failing CI) and has had no human activity for more than 3 days.

When you have a chance, please reply here or push an update. To keep the queue manageable, PRs with no human activity for more than 5 days may be closed automatically, but they can be reopened when work resumes.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Closing this PR for now because it appears to be waiting on author action (outstanding trusted reviewer or maintainer feedback, failing CI) and has had no human activity for more than 5 days.

This is only a queue-management step, not a rejection of the work. If you would like to continue, please leave a comment or push an update and reopen the PR when ready.

@MumuTW

MumuTW commented Aug 3, 2026

Copy link
Copy Markdown
Author

Superseded by #6386 (same branch, rebased onto current main). GitHub blocks reopening PRs whose head branch was force-pushed after close, so this was re-submitted fresh — full context in the new PR body. The diff is unchanged since the approvals.

@lefarcen

lefarcen commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Thanks for the heads-up, @MumuTW — understood. We'll treat #6386 as the active continuation of this work and keep discussion/review there.

1 similar comment
@lefarcen

lefarcen commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Thanks for the heads-up, @MumuTW — understood. We'll treat #6386 as the active continuation of this work and keep discussion/review there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/M PR changes 100-300 lines type/bugfix Bug fix validated Runtime change validated (via /explore Pass or manual QA).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants