Skip to content

feat(credentials): agent-initiated oauth credential reconnect#5488

Open
j15z wants to merge 18 commits into
devfrom
feat/agent-reconnect-credential
Open

feat(credentials): agent-initiated oauth credential reconnect#5488
j15z wants to merge 18 commits into
devfrom
feat/agent-reconnect-credential

Conversation

@j15z

@j15z j15z commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The agent can now generate a Reconnect link for an existing OAuth credential: oauth_get_auth_link accepts an optional credentialId (tool param + prompts land in the companion mothership PR)
  • /api/auth/oauth2/authorize accepts credentialId: requires credential-admin access (same check as the draft POST route), rejects cross-workspace / non-oauth / provider-mismatched ids, and threads the id into the pending credential draft so the existing handleReconnectCredential path rebinds the same credential.id to the fresh account row
  • Draft upsert writes credentialId (or null) on both the insert and conflict paths — a plain connect can never inherit a stale reconnect draft and silently rebind an existing credential
  • Tool handler validates the credential at link-generation time so failures surface in the tool result (agent-visible) instead of a silent browser redirect
  • Trello/Shopify reconnect is rejected on BOTH the tool and the authorize route (their custom authorize flows bypass this endpoint, so a reconnect draft written here would linger and could be consumed by their token-store callbacks) — users are pointed to the integrations page, where reconnect for them works as before
  • Reconnect drafts carry the credential's actual display name so audit records stay accurate (page parity)
  • Extracted the connect modal's display-name collision numbering into lib/credentials/display-name.ts and applied it to copilot-path connects — agent-created credentials get "Justin's Gmail 2" instead of identical duplicate names; name-lookup failures degrade to the un-numbered default with a logger.warn
  • Chat credential chip renders "Reconnect {credential display name}" when the link URL carries a credentialId (derived from the URL, not from model-emitted tag fields)
  • ensureWorkspaceAccess now returns the resolved WorkspaceAccess (previously discarded) so the tool handler reuses it for the credential-admin lookup instead of re-querying

Merge order: this PR merges before the mothership companion — the new tool param crosses the tool-catalog codegen boundary.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

  • 37 new unit tests: authorize route (reconnect authz, provider mismatch, custom-flow provider rejection, draft cross-contamination, audit display name), tool handler validation, display-name behavior pinning, chip label; neighboring suites green (780 tests)
  • bun run lint clean, bun run check:api-validation:strict passed, tsc clean
  • Manual E2E: reconnect keeps credential.id, swaps the account row, deletes the orphaned account, dependent workflow still runs

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Screenshots/Videos

🤖 Generated with Claude Code

Companion PRs

  • simstudioai/mothership#344

@vercel

vercel Bot commented Jul 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 7, 2026 10:29pm

Request Review

@cursor

cursor Bot commented Jul 7, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes OAuth authorize and pending-draft semantics (credential rebinding); mitigated by credential-admin gates, explicit null on plain connect upserts, and upfront validation in the tool and route.

Overview
Adds in-place OAuth reconnect so the copilot and chat can re-authorize an existing credential instead of always creating a new one.

oauth_get_auth_link accepts optional credentialId, validates admin access and provider/workspace match at tool time (failures show in the tool result), and appends credentialId to /api/auth/oauth2/authorize. Trello/Shopify reconnect is blocked with a pointer to the integrations UI.

/api/auth/oauth2/authorize reads optional credentialId: reconnect paths require credential-admin checks, reject custom-flow providers and mismatches, and write credentialId into the pending draft (insert and upsert both set credentialId or null so a plain connect cannot inherit a stale reconnect draft). Connect drafts now use shared defaultCredentialDisplayName with workspace collision numbering.

Chat credential chips parse credentialId from the link URL and show Reconnect {display name} when present.

Display-name logic moves to lib/credentials/display-name.ts (connect modal + server drafts). Broad unit test coverage for authorize, copilot handler, display names, and chip labels.

Reviewed by Cursor Bugbot for commit d1dd5c0. Bugbot is set up for automated code reviews on this repo. Configure here.

@j15z

j15z commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

@github-actions github-actions Bot added the requires-mothership-merge Has a companion PR on the mothership/copilot side — merge in lockstep label Jul 7, 2026
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

⚠️ Cross-repo companion check

One or more companion PRs aren't merged into staging yet. Merging this without them will leave copilot and sim out of sync — merge them in lockstep.

  • simstudioai/mothership#344OPEN, not merged (targets dev) — feat(tools): reconnect support for oauth_get_auth_link via credentialId

@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds agent-initiated OAuth credential reconnect: the oauth_get_auth_link tool and /api/auth/oauth2/authorize route now accept an optional credentialId that, when present, rebinds an existing credential to a freshly authorized account rather than creating a new one.

  • Route handler: /api/auth/oauth2/authorize validates credentialId (admin access, workspace membership, OAuth type, provider match) and threads it into the draft so the existing handleReconnectCredential path picks it up; the draft upsert explicitly writes credentialId: null on both insert and conflict paths for plain connects, preventing stale reconnect drafts from leaking.
  • Tool handler: oauth.ts now returns the WorkspaceAccess from ensureWorkspaceAccess and reuses it in getCredentialActorContext to avoid a double fetch; validation in the tool path surfaces errors agent-side before the user ever opens a URL.
  • Display-name logic extracted from the connect modal into lib/credentials/display-name.ts and reused across the route handler and modal; reconnect drafts skip collision numbering and use the credential's actual name for audit accuracy.

Confidence Score: 5/5

Safe to merge — all three layers of the reconnect flow (tool handler, authorize route, draft upsert) are correctly validated and the previously identified issues have been addressed.

The reconnect path is validated at two independent checkpoints (tool handler before URL generation, route handler before draft creation). The draft upsert explicitly clears credentialId on the conflict path for plain connects, closing the stale-draft cross-contamination window. The audit log display-name issue noted in previous review threads is fixed by using the credential's actual displayName directly for reconnect drafts, bypassing collision numbering. Thirty-one new unit tests cover authz edge cases, provider mismatches, and display-name behavior. No logic gaps found.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/api/auth/oauth2/authorize/route.ts Core reconnect logic: adds credentialId validation (admin, workspace, type, provider), threads reconnect display name through to audit, and explicitly nulls credentialId in the upsert conflict set. Well-structured and correct.
apps/sim/lib/copilot/tools/handlers/oauth.ts Tool handler now returns WorkspaceAccess from ensureWorkspaceAccess and threads it to getCredentialActorContext, eliminating the previously-flagged double-fetch. Reconnect validation mirrors the route handler, surfacing errors agent-side before click.
apps/sim/lib/copilot/tools/handlers/access.ts Return type changed from void to WorkspaceAccess; all existing branches updated. Minimal, correct change.
apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx CredentialDisplay split into CredentialLinkDisplay (link type) + remaining; reconnect credentialId derived from URL searchParams and used to show 'Reconnect {displayName}' label. Hook ordering is correct.
apps/sim/lib/credentials/display-name.ts New shared utility extracted from the connect modal. Logic is unchanged; exports DISPLAY_NAME_MAX_LENGTH for tests. Clean extraction.
apps/sim/lib/api/contracts/oauth-connections.ts Adds optional credentialId (min length 1) to authorizeOAuth2QuerySchema. Correct and minimal.
apps/sim/app/workspace/[workspaceId]/components/connect-oauth-modal/connect-oauth-modal.tsx Removes the inlined defaultDisplayName and replaces it with the shared defaultCredentialDisplayName import. Behavior is identical.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Agent as Copilot Agent
    participant Tool as oauth.ts (Tool Handler)
    participant Route as /api/auth/oauth2/authorize
    participant DB as DB (pendingCredentialDraft)
    participant Provider as OAuth Provider

    Agent->>Tool: "oauth_get_auth_link({ providerName, credentialId })"
    Tool->>Tool: ensureWorkspaceAccess(workspaceId, userId, 'write')
    Tool->>Tool: "getCredentialActorContext(credentialId, userId, { workspaceAccess })"
    Note over Tool: Validate: admin, workspace, type, provider match
    Tool-->>Agent: "{ oauth_url: /authorize?...&credentialId=cred-1 }"

    Agent-->>User: Renders "Reconnect Justin's Gmail" chip
    User->>Route: "GET /api/auth/oauth2/authorize?credentialId=cred-1"
    Route->>Route: checkWorkspaceAccess
    Route->>Route: getCredentialActorContext(credentialId) → isAdmin check
    Route->>Route: validate workspace / type / provider
    Route->>DB: "INSERT draft { credentialId: 'cred-1', displayName: credential.displayName }"
    Note over DB: onConflict → SET credentialId explicitly (nulls on plain connect)
    Route->>Provider: redirect to OAuth provider
    Provider-->>Route: OAuth callback
    Route->>DB: handleReconnectCredential → rebind existing credential.id to new account
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Agent as Copilot Agent
    participant Tool as oauth.ts (Tool Handler)
    participant Route as /api/auth/oauth2/authorize
    participant DB as DB (pendingCredentialDraft)
    participant Provider as OAuth Provider

    Agent->>Tool: "oauth_get_auth_link({ providerName, credentialId })"
    Tool->>Tool: ensureWorkspaceAccess(workspaceId, userId, 'write')
    Tool->>Tool: "getCredentialActorContext(credentialId, userId, { workspaceAccess })"
    Note over Tool: Validate: admin, workspace, type, provider match
    Tool-->>Agent: "{ oauth_url: /authorize?...&credentialId=cred-1 }"

    Agent-->>User: Renders "Reconnect Justin's Gmail" chip
    User->>Route: "GET /api/auth/oauth2/authorize?credentialId=cred-1"
    Route->>Route: checkWorkspaceAccess
    Route->>Route: getCredentialActorContext(credentialId) → isAdmin check
    Route->>Route: validate workspace / type / provider
    Route->>DB: "INSERT draft { credentialId: 'cred-1', displayName: credential.displayName }"
    Note over DB: onConflict → SET credentialId explicitly (nulls on plain connect)
    Route->>Provider: redirect to OAuth provider
    Provider-->>Route: OAuth callback
    Route->>DB: handleReconnectCredential → rebind existing credential.id to new account
Loading

Reviews (3): Last reviewed commit: "fix(credentials): address reconnect revi..." | Re-trigger Greptile

Comment thread apps/sim/app/api/auth/oauth2/authorize/route.ts Outdated
@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds agent-initiated OAuth credential reconnect: the oauth_get_auth_link tool and the /api/auth/oauth2/authorize route both accept an optional credentialId that threads through the pending credential draft so the existing handleReconnectCredential path rebinds the credential in place. It also extracts display-name collision-numbering into a shared lib/credentials/display-name.ts and updates the chat credential chip to show "Reconnect {name}" when the link URL carries a credentialId.

  • Authorization route (route.ts): validates credential ownership, workspace membership, OAuth type, provider match, and admin role before creating a reconnect draft; upsert explicitly sets credentialId: null on the conflict path so a stale reconnect draft cannot leak into a plain connect.
  • Tool handler (oauth.ts): validates the same constraints at link-generation time so failures surface in the tool result; Trello/Shopify reconnect is blocked with a pointer to the integrations page.
  • Chat chip (special-tags.tsx): derives reconnectCredentialId from the URL rather than model-emitted fields and resolves the display name via useWorkspaceCredential.

Confidence Score: 4/5

The reconnect authorization chain is well-guarded with redundant credential-admin checks at both the tool-handler and route layers. The main imprecision is in the audit log for reconnects, where the draft display name gets a collision suffix because the credential being reconnected is counted in takenNames.

Cross-workspace attacks, non-admin reconnects, type and provider mismatches, and draft cross-contamination are all explicitly checked and tested. The two issues found are the misleading audit log name during reconnects and a redundant workspace-access DB round-trip in the tool handler — neither affects credential data or user-facing behavior.

apps/sim/app/api/auth/oauth2/authorize/route.ts — the createConnectDraft call in the reconnect path should skip or narrow collision detection so the draft's displayName matches the existing credential name for correct audit logs.

Important Files Changed

Filename Overview
apps/sim/app/api/auth/oauth2/authorize/route.ts Adds credentialId query param support to the authorize GET route; validates credential ownership, workspace membership, and provider match before creating a reconnect draft. The draft's displayName is incorrectly collision-checked against all credentials (including the one being reconnected), producing a suffixed name in the audit log while leaving the actual credential name unchanged.
apps/sim/lib/copilot/tools/handlers/oauth.ts Extends the OAuth tool handler with reconnect validation (credential existence, workspace, type, provider, admin checks) before embedding credentialId in the authorize URL. Calls getCredentialActorContext without the pre-fetched workspace access, causing a redundant DB round-trip per reconnect link generation.
apps/sim/lib/credentials/display-name.ts New module extracting the defaultCredentialDisplayName helper from the connect modal into a shared library; clean extraction with unchanged logic and full unit test coverage.
apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/special-tags/special-tags.tsx Extracts link rendering into CredentialLinkDisplay; derives reconnect label from the URL's credentialId param via useWorkspaceCredential rather than trusting model-emitted fields, with safe URL parsing and fallback to provider ID while loading.
apps/sim/lib/api/contracts/oauth-connections.ts Adds optional credentialId field (min-length 1) to authorizeOAuth2QuerySchema; straightforward contract extension.

Reviews (2): Last reviewed commit: "feat(credentials): agent-initiated oauth..." | Re-trigger Greptile

Comment thread apps/sim/app/api/auth/oauth2/authorize/route.ts Outdated
Comment thread apps/sim/lib/copilot/tools/handlers/oauth.ts Outdated
Comment thread apps/sim/app/api/auth/oauth2/authorize/route.ts Outdated
Sg312 and others added 15 commits July 7, 2026 13:57
…KB tool handlers

Mirrors mothership dev f90f9b05:
- regenerated tool-catalog/tool-schemas mirrors (search trigger replaces
  research + scout; QueryUserTable / SearchKnowledgeBase entries)
- queryUserTableServerTool / searchKnowledgeBaseServerTool: read-only
  wrappers delegating to the full user_table / knowledge_base handlers with
  hard operation allowlists (and outputPath export rejection on
  query_user_table)
- display maps: 'search' agent label/title/icon added; research + scout
  entries retained so historical transcripts keep rendering
- Search.id replaces Research.id in LONG_RUNNING_TOOL_IDS (it inherits
  research's long crawls)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mirrors mothership dev db60da94: run_code is the compute-only variant of
function_execute for the search agent — same sandbox and inputs, no
outputs.files / outputTable, so it cannot create or overwrite workspace
resources. Wrapper handler hard-rejects the write vectors and delegates to
executeFunctionExecute; run_code is deliberately absent from
OUTPUT_PATH_TOOLS and the table output post-processor, so the name gating
blocks writes even for leaked args. Added to LONG_RUNNING_TOOL_IDS,
display title/icon maps, and the regenerated catalog/schema mirrors.

Also removes two ineffective biome suppression comments in the docs
workflow-preview (the rule doesn't fire in the docs app config).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…data

A failed handler result that carried a defined-but-empty output (the
app-tool executor's 'Tool not found' ships output: {}) won the priority
race in getToolCallTerminalData, so the resume payload's data — the only
thing the model reads — was a bare {} with the error text dropped. The
search agent retried run_code 20+ times blind against a stale server
because every failure rendered as empty instead of 'Tool not found'.

Failed calls now always carry error in their terminal data: merged into
object outputs, wrapped alongside non-object outputs, preserved when the
output already has an error field.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…splay

Companion to mothership 8ae32e97 (user_memory tool removed — the feature no
longer exists). Regenerates the mothership contract mirrors via
generate-mship-contracts.ts, which also picks up the pending telemetry
contract additions (gen_ai.agent.name labels, llm.client.context_tokens,
llm.client.compactions, llm.request.compaction_trigger, llm.compaction.pause,
gen_ai.usage.context_tokens), and removes the user_memory display title.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…select types only

UI ordering: answering a question card no longer echoes a duplicate user
bubble. The combined answer still goes on the wire as a user message, but the
chat pairs it back to its card (strict 'Prompt — Answer' match, now uniform
for single questions too) and renders the card as the answered recap — the
card IS the user turn, and the next assistant message streams below it. The
pairing is derived from the transcript, so live and reloaded renders are
identical; a dismissed card followed by an unrelated typed message does not
match and renders normally. Messages ending with a question card also drop
the copy/thumbs actions row — the card is an input surface, not a reactable
assistant turn.

Question types are now single_select and multi_select only: text is removed
(the free-text 'Something else' row covers it) and confirm collapses into
single_select with Yes/No options. multi_select rows toggle with a check and
the free-text row's arrow submits the step; answers are comma-joined labels
plus any typed entry. Agent-supplied catch-all options ('Other', 'Something
else', 'None of the above') are stripped at parse — the card always provides
its own free-text row; a question left with no real options is invalid.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Removes multi_select (and its toggle/check UI). The card is one shape: pick
one option or type into the always-present 'Something else' row. Catch-all
stripping and the transcript pairing/recap behavior are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Re-adds multi_select with a reworked interaction: option rows carry real
checkboxes (emcn Checkbox chrome) instead of numbers and arrows, an
option-styled Submit row confirms the step, and the "Something else" row
reads as a plain option until clicked — then it becomes the focused text
box, auto-checks, and can be unchecked without losing the typed text
(blur with nothing typed reverts it). single_select behavior, catch-all
stripping, and the transcript pairing/recap format are unchanged;
multi_select answers are the checked labels comma-joined.
@j15z j15z force-pushed the feat/agent-reconnect-credential branch from 91cec7f to 24ef2cf Compare July 7, 2026 21:39
@j15z j15z changed the base branch from staging to dev July 7, 2026 21:39
@j15z

j15z commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 24ef2cf. Configure here.

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

Labels

requires-mothership-merge Has a companion PR on the mothership/copilot side — merge in lockstep

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants