Skip to content

fix(mothership): fix superagent credentials#4185

Merged
Sg312 merged 3 commits intostagingfrom
fix/superagent-credentials
Apr 15, 2026
Merged

fix(mothership): fix superagent credentials#4185
Sg312 merged 3 commits intostagingfrom
fix/superagent-credentials

Conversation

@Sg312
Copy link
Copy Markdown
Collaborator

@Sg312 Sg312 commented Apr 15, 2026

Summary

Fix credential injection for superagent

Type of Change

  • Bug fix

Testing

Manual

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)

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 15, 2026

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Apr 15, 2026 7:48pm

Request Review

@Sg312 Sg312 merged commit f0285ad into staging Apr 15, 2026
13 checks passed
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 15, 2026

Greptile Summary

This PR fixes credential injection for Copilot/Superagent tool execution via two changes: (1) createUserToolSchema now adds credentialId as a required field only when surface === 'copilot', so the LLM is explicitly told to provide it; and (2) new helpers in executeTool normalize credentialIdcredential and throw a descriptive error if an OAuth tool is called without any credential selector. A second commit replaces the Ajv-based stream event validator (which required eval/new Function and violated CSP) with a hand-written structural validator in contract.ts.

Confidence Score: 5/5

Safe to merge — both findings are P2 style/consistency issues that do not affect runtime correctness.

The core credential-injection fix is logically sound: the schema correctly marks credentialId as required for copilot surface, normalization copies it to the canonical credential field, and enforceCopilotCredentialSelection provides a fast-fail safety net. The Ajv→structural-validator swap is well-tested with full event-type coverage. The two P2 findings (required: false in VFS docs; missing scope guard on normalizeCopilotCredentialParams) are minor inconsistencies with no impact on the happy path.

apps/sim/lib/copilot/vfs/serializers.ts (credentialId still says required: false) and apps/sim/tools/index.ts (normalizeCopilotCredentialParams lacks scope guard).

Important Files Changed

Filename Overview
apps/sim/tools/index.ts Adds three new helpers for copilot credential injection: readExplicitCredentialSelector, normalizeCopilotCredentialParams (copies credentialIdcredential), and enforceCopilotCredentialSelection (throws for copilot OAuth tools missing a credential). normalizeCopilotCredentialParams lacks the scope guard present on the sibling normalizeCopilotFileParams.
apps/sim/tools/params.ts Adds surface option to createUserToolSchema; credentialId is now only injected into the schema (and marked required) for surface === 'copilot', which is the correct fix.
apps/sim/lib/copilot/vfs/serializers.ts Updates credentialId description to "pass this explicitly", but leaves required: false, inconsistent with the new runtime enforcement and the params.ts copilot schema.
apps/sim/lib/copilot/request/session/contract.ts Replaces Ajv runtime schema compilation (which uses eval/new Function, violating CSP) with a hand-written structural validator; intentionally lightweight per comment, well-covered by new tests.
apps/sim/lib/copilot/request/session/contract.test.ts New tests covering all known event types (text, session, tool, span, resource, run, error, complete, synthetic file preview) plus rejection of invalid/unknown events and invalid JSON.
apps/sim/tools/index.test.ts Adds Copilot OAuth Credential Enforcement test suite verifying the fast-fail path when copilot executes an OAuth tool without an explicit credential selector.
apps/sim/tools/params.test.ts Adds tests asserting credentialId is absent in default-surface schemas and present (and required) only for copilot-surface OAuth tool schemas.
apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts Switches stream event parsing to use the new CSP-safe parsePersistedStreamEventEnvelope / parsePersistedStreamEventEnvelopeJson from the rewritten contract module; no logic changes to the chat hook itself.

Sequence Diagram

sequenceDiagram
    participant LLM as Copilot / Superagent LLM
    participant Schema as createUserToolSchema surface=copilot
    participant Exec as executeTool
    participant Norm as normalizeCopilotCredentialParams
    participant Enforce as enforceCopilotCredentialSelection
    participant Token as OAuth Token API

    LLM->>Schema: request tool definitions
    Schema-->>LLM: schema with credentialId required

    LLM->>Exec: tool call with credentialId and params
    Exec->>Norm: normalize params unconditional
    Norm-->>Exec: params.credential set from credentialId
    Exec->>Enforce: check copilotToolExecution and oauth.required
    alt no credential selector found
        Enforce-->>Exec: throw Error must pass credentialId
        Exec-->>LLM: success false with error message
    else credential found
        Enforce-->>Exec: ok
        Exec->>Token: fetch access token for credential
        Token-->>Exec: accessToken
        Exec-->>LLM: success true with output
    end
Loading

Comments Outside Diff (1)

  1. apps/sim/lib/copilot/vfs/serializers.ts, line 750-757 (link)

    P2 credentialId still marked required: false in VFS documentation

    serializeIntegrationSchema is what the Copilot agent reads from the virtual filesystem to understand the tool API. With required: false here, the agent may conclude it can omit credentialId — even though createUserToolSchema (copilot surface) now places it in the JSON Schema required array and enforceCopilotCredentialSelection throws at runtime if it's missing. Consider updating to required: true to align the documentation with the enforcement.

Reviews (1): Last reviewed commit: "Lint" | Re-trigger Greptile

Comment thread apps/sim/tools/index.ts
Comment on lines +178 to +183
function normalizeCopilotCredentialParams(params: Record<string, unknown>): void {
const credentialId = typeof params.credentialId === 'string' ? params.credentialId.trim() : ''
if (credentialId && !params.credential && !params.oauthCredential) {
params.credential = credentialId
}
}
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.

P2 Missing scope guard, inconsistent with sibling function

normalizeCopilotFileParams guards with if (!scope.copilotToolExecution) return before doing anything. normalizeCopilotCredentialParams has no such guard and runs unconditionally for every tool execution. In practice this is a no-op for non-copilot paths (since credentialId won't be in contextParams), but the inconsistency is a readability and maintenance hazard.

Suggested change
function normalizeCopilotCredentialParams(params: Record<string, unknown>): void {
const credentialId = typeof params.credentialId === 'string' ? params.credentialId.trim() : ''
if (credentialId && !params.credential && !params.oauthCredential) {
params.credential = credentialId
}
}
function normalizeCopilotCredentialParams(params: Record<string, unknown>, scope: ToolExecutionScope): void {
if (!scope.copilotToolExecution) {
return
}
const credentialId = typeof params.credentialId === 'string' ? params.credentialId.trim() : ''
if (credentialId && !params.credential && !params.oauthCredential) {
params.credential = credentialId
}
}

And update the call site to pass scope as a second argument.

Sg312 added a commit that referenced this pull request Apr 15, 2026
…mat, logs performance improvements

fix(csp): add missing analytics domains, remove unsafe-eval, fix workspace CSP gap (#4179)
fix(landing): return 404 for invalid dynamic route slugs (#4182)
improvement(seo): optimize sitemaps, robots.txt, and core web vitals across sim and docs (#4170)
fix(gemini): support structured output with tools on Gemini 3 models (#4184)
feat(brightdata): add Bright Data integration with 8 tools (#4183)
fix(mothership): fix superagent credentials (#4185)
fix(logs): close sidebar when selected log disappears from filtered list; cleanup (#4186)
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