Skip to content

feat(desktop): auto-connect OAuth catalog installs and render post_install notes - #66660

Open
joelbrilliant wants to merge 2 commits into
NousResearch:mainfrom
joelbrilliant:feat/desktop-connector-oauth-chain
Open

feat(desktop): auto-connect OAuth catalog installs and render post_install notes#66660
joelbrilliant wants to merge 2 commits into
NousResearch:mainfrom
joelbrilliant:feat/desktop-connector-oauth-chain

Conversation

@joelbrilliant

Copy link
Copy Markdown
Contributor

Two additive fixes to the desktop MCP catalog so an installed OAuth connector actually connects and shows its setup notes — both reusing machinery that already exists on main.\n>\n> 1. Install → OAuth auto-launch. Today install() in apps/desktop/src/app/skills/mcp-tab.tsx calls installMcpCatalogEntryPOST /api/mcp/catalog/install, which only writes config/env. For an OAuth-over-HTTP connector that mints no token, so the user has to go find the Servers tab and trigger auth separately. This chains straight into the existing desktop OAuth flow — the same completeMcpDesktopOAuth the Servers tab already uses for re-auth (mcp-tab.tsx:583) — when entry.auth_type === 'oauth' && entry.transport === 'http'. Best-effort: a cancelled/failed auth leaves the install intact and surfaces a "connect to finish" notification instead of crashing. No new OAuth code; success is persisted server-side and picked up by the existing onInstalled() refetch.\n>\n> 2. Render post_install. The desktop catalog card never showed entry.post_install even though the type carries it and the API returns it — the web dashboard already renders it (web/src/pages/McpPage.tsx:860). Added a truthy-guarded "Setup notes" block mirroring that render.\n>\n> Both changes are additive, guard every new field read for the version-skew case (Desktop newer than the runtime it talks to), and touch neither the install API contract nor the OAuth machinery. Complements the connector-catalog work in #66653; independent of the presentation PRs #59692 / #59872.\n>\n> Tests (vitest): oauth+http install chains into the OAuth launch (asserts completeMcpDesktopOAuth called with the entry's server name); non-oauth and oauth-on-non-http installs do not launch OAuth; an OAuth-launch failure leaves the install succeeded (no throw, routes to the connect-later notify, onInstalled still fires); post_install renders when present and is absent when empty. tsc -p apps/desktop --noEmit clean, eslint clean.

…stall

Installing an OAuth-over-HTTP connector from the desktop MCP catalog only
wrote config/env — it never minted a token, so the user had to separately
trigger auth from the Servers tab before the connector worked. Chain the
install straight into the existing desktop OAuth flow (the same
completeMcpDesktopOAuth machinery the Servers tab already uses for re-auth)
when entry.auth_type === 'oauth' && entry.transport === 'http'. The launch is
best-effort: a cancelled or failed auth leaves the install intact and surfaces
a connect-later notification rather than crashing.

Also render entry.post_install as a "Setup notes" block on the catalog card,
mirroring the web dashboard's McpPage. Guarded on truthiness so older runtimes
that omit the field don't break the newer desktop that reads it.

Both changes are additive and version-skew-guarded; no install API contract or
OAuth machinery was modified.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) tool/mcp MCP client and OAuth labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #66653 and #59692: this adds the desktop install-to-OAuth handoff and post-install notes, complementing connector catalog content and earlier catalog presentation rather than duplicating either.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Auto-chains OAuth launch on catalog install + renders post_install notes. Clean desktop changes:

  • OAuth launch on install() only when auth_type === 'oauth' && transport === 'http', mirroring existing completeMcpDesktopOAuth flow
  • Best-effort: cancelled/failed auth leaves install intact with a "connect to finish" notification
  • post_install rendered with truthy guard for version-skew
  • 172 new vitest test cases covering the full chain
  • TypeScript clean (tsc -p apps/desktop --noEmit)

No issues found. LGTM.


Reviewed by Hermes Agent

@teknium1 teknium1 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.

Thanks for extending the existing desktop OAuth path and matching the web catalog’s post_install presentation. The two reported gaps are present on current main: McpCatalog installs and immediately calls onInstalled() at apps/desktop/src/app/skills/mcp-tab.tsx:1373-1397, while the web catalog already renders setup notes at web/src/pages/McpPage.tsx:860-868.

Problems

  • apps/desktop/src/app/skills/mcp-tab.tsx:1409 awaits OAuth before onInstalled() at line 1427. The shared helper polls every second until approval/error (apps/desktop/src/lib/mcp-dashboard-oauth.ts:43-69); an abandoned browser flow remains pending until the backend’s 15-minute flow GC (hermes_cli/web_server.py:11329-11346). During that interval installing remains set, so the claimed best-effort cancellation path does not release the catalog UI.

Suggested changes

  • Refresh the installed catalog/config immediately after the install succeeds, and start OAuth as a non-blocking follow-up. Refresh again on OAuth success if the post-auth configuration needs reconciliation; use the connect-later notice for errors.
  • Add a test for a persistently authorization_required OAuth status, asserting installation completion is not held behind browser authorization.

Automated hermes-sweeper review.

Comment thread apps/desktop/src/app/skills/mcp-tab.tsx Outdated
@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 18, 2026
@joelbrilliant

Copy link
Copy Markdown
Contributor Author

Sweeper follow-up is pushed in ee1a98c2d.

  • A successful catalog install now refreshes installed catalog and config state immediately, then releases the row before starting OAuth as a non-blocking follow-up.
  • OAuth approval triggers a second refresh for the persisted auth config. OAuth errors keep the existing connect-later notice and cannot enter the install-failed path.
  • Added regression coverage for immediate refresh, post-approval refresh, and a persistently pending authorization_required flow.
  • Browser-verified the real component and OAuth helper against a persistent authorization-required response. After 10 status polls, the row was enabled, the button was back to Install, and the successful install refresh had already fired once.

Verification: 9 focused Vitest tests passed, the renderer TypeScript check passed, targeted ESLint passed, and the diff check passed.

@teknium1 teknium1 added the area/install-update Installer, updater, packaging, wheels, doctor label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/mcp MCP client and OAuth type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants