feat(credentials): agent-initiated oauth credential reconnect#5488
feat(credentials): agent-initiated oauth credential reconnect#5488j15z wants to merge 18 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview
Chat credential chips parse Display-name logic moves to Reviewed by Cursor Bugbot for commit d1dd5c0. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@cursor review |
|
Greptile SummaryThis PR adds agent-initiated OAuth credential reconnect: the
Confidence Score: 5/5Safe 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
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
%%{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
Reviews (3): Last reviewed commit: "fix(credentials): address reconnect revi..." | Re-trigger Greptile |
Greptile SummaryThis PR adds agent-initiated OAuth credential reconnect: the
Confidence Score: 4/5The 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
Reviews (2): Last reviewed commit: "feat(credentials): agent-initiated oauth..." | Re-trigger Greptile |
…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.
91cec7f to
24ef2cf
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ 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.
Summary
oauth_get_auth_linkaccepts an optionalcredentialId(tool param + prompts land in the companion mothership PR)/api/auth/oauth2/authorizeacceptscredentialId: 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 existinghandleReconnectCredentialpath rebinds the samecredential.idto the fresh account rowcredentialId(or null) on both the insert and conflict paths — a plain connect can never inherit a stale reconnect draft and silently rebind an existing credentiallib/credentials/display-name.tsand 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 alogger.warncredentialId(derived from the URL, not from model-emitted tag fields)ensureWorkspaceAccessnow returns the resolvedWorkspaceAccess(previously discarded) so the tool handler reuses it for the credential-admin lookup instead of re-queryingMerge order: this PR merges before the mothership companion — the new tool param crosses the tool-catalog codegen boundary.
Type of Change
Testing
bun run lintclean,bun run check:api-validation:strictpassed, tsc cleancredential.id, swaps the account row, deletes the orphaned account, dependent workflow still runsChecklist
Screenshots/Videos
🤖 Generated with Claude Code
Companion PRs