Skip to content

Show OMP sign-in guidance when OMP reports no model selected - #26649

Merged
brennanb2025 merged 17 commits into
mainfrom
brennanb2025/omp-no-model-signin
Oct 8, 2026
Merged

brennanb2025 merged 17 commits into
mainfrom
brennanb2025/omp-no-model-signin

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​10 $\color{#cf222e}{\Huge{\mathbf{−}}}$​5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5

ELI5

When OMP has no sign-in at all, it answers a message with "No model selected. Use /login…" instead of "No API key found…". Orca only recognized the second wording, so a fully signed-out OMP chat showed a generic failure line instead of the sign-in advice every other agent gets. Now both wordings count as "not signed in".

What Changed

Before: in a native chat with OMP 17.0.5 and nothing signed in, the chat opens normally, and the first message fails with OMP's own text ("No model selected. Use /login, set an API key environment variable, or create …/agent.db. Then use /model to select a model."). Orca treated it as an ordinary failure, without "Sign in to OMP." and with the generic retry wording.

After: the same failure is classified as not signed in, so the chat shows the same sign-in line as for OMP's missing-key case, with OMP's text kept as its detail. With #26544 underneath, the next send after signing in also starts a fresh OMP, which picks up the new login (a running OMP does not).

Mechanism: OMP's prompt check (agent-session.ts around line 8815 at tag v17.0.5) throws "No model selected." followed by "Use /login" when no model is usable, which is what happens with no credentials; it throws "No API key found for …" when the chosen model's provider has no key. Both reach Orca as an Agent Client Protocol internal error (-32603) with the text in data.details. The OMP check in omp-dialect.ts now accepts either prefix. The bare "No model selected" (no period, no sign-in advice) that OMP raises during compaction is not matched.

Why

The failure text is OMP's only signal; there is no typed error for it. Matching the full "No model selected.\n\nUse /login" prefix keeps it to OMP's sign-in branch and leaves the compaction error alone. This is the smallest change that gives a signed-out OMP user the right advice.

Limitation (intended): OMP also leaves no model selected when the user's own enabledModels allow-list excludes every model it has credentials for. Its prompt-time message is then the same "No model selected. Use /login…", so Orca's "Sign in to OMP." matches what OMP itself says, and OMP's text is shown beside it. With #26544, the next send starts a fresh OMP, which also picks up a corrected config. Orca cannot tell the two cases apart from the error alone.

Linked Issue

Stacked on #26544 (the OMP sign-in check this extends exists only there). Requested by @BrennanKB5.

Visual Proof

N/A: no new wording or surface. The chat shows the existing OMP sign-in line from #26544, now for one more OMP failure text.

Testing

  • I manually tested these changes locally (real OMP 17.0.5 run, see below)
  • Automated tests added/updated

Real OMP 17.0.5 over Agent Client Protocol, in a throwaway home folder against a local fake model server, with no real account: fully signed out, the first prompt returns {"code":-32603,"message":"Internal error","data":{"details":"No model selected.\n\nUse /login, set an API key environment variable, or create …/agent.db\n\nThen use /model to select a model."}}. That exact text is the new test case.

src/main/acp/acp-structured-sign-in.test.ts adds the signed-out "No model selected" text (classified as not signed in) and the bare compaction "No model selected" (not classified); 12 of 12 pass. Ablation: with the dialect change reverted, exactly the new signed-out case fails.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Host-side classification only: SSH and local chats use the same path, no wire change, no new user-facing strings.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…th-parity

# Conflicts:
#	src/main/claude/claude-structured-session-journal-control.ts
#	src/renderer/src/components/native-chat/NativeChatNoticeRow.tsx
#	src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx
…th-parity

# Conflicts:
#	mobile/src/session/mobile-structured-queued-message-cards.test.ts
#	mobile/src/session/mobile-structured-queued-message-cards.ts
#	mobile/src/session/use-mobile-native-chat-controller.ts
#	mobile/src/session/use-mobile-native-chat-send-error.test.ts
#	mobile/src/session/use-mobile-native-chat-send-error.ts
#	mobile/src/session/use-mobile-native-chat-session-lane.ts
#	mobile/src/session/use-mobile-structured-agent-session.ts
#	mobile/src/session/use-mobile-structured-queued-message-controls.ts
#	mobile/src/session/use-mobile-structured-send-with-outcome.ts
#	src/renderer/src/i18n/locales/en.json
#	src/renderer/src/i18n/locales/es.json
#	src/renderer/src/i18n/locales/fr.json
#	src/renderer/src/i18n/locales/ja.json
#	src/renderer/src/i18n/locales/ko.json
#	src/renderer/src/i18n/locales/zh.json
@brennanb2025
brennanb2025 changed the base branch from brennanb2025/agent-auth-parity to main October 8, 2026 21:39
# Conflicts:
#	src/main/acp/acp-dialects/omp-dialect.ts
#	src/main/acp/acp-structured-sign-in.test.ts
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 145b1c14-5fff-4400-b78a-b2205e96b138
📥 Commits

Reviewing files that changed from the base of the PR and between ff4a51c and 3819726.

📒 Files selected for processing (2)
  • src/main/acp/acp-dialects/omp-dialect.ts
  • src/main/acp/acp-structured-sign-in.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The OMP authentication check still requires error code -32603. It now recognizes details beginning with either the “No model selected” login-guidance prompt or the missing-provider-key prefix. Tests cover classification of the login-guidance prompt as notSignedIn and verify that the bare “No model selected” message is not classified.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 38197

The change recognizes OMP’s login-guidance prompt while retaining its model-selection details. No concrete merge-blocking risk is established by the available evidence.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: showing OMP sign-in guidance when OMP reports that no model is selected.
Description check ✅ Passed The description is complete and relevant. It explains the user impact, mechanism, rationale, limitation, linked issue context, testing, visual-proof status, compatibility considerations, and checklist…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brennanb2025
brennanb2025 merged commit 58a58f4 into main Oct 8, 2026
30 checks passed
@brennanb2025
brennanb2025 deleted the brennanb2025/omp-no-model-signin branch October 8, 2026 21:55
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