Skip to content

feat(github): per-repo import connections with scoped OAuth tokens#3484

Closed
guitavano wants to merge 1 commit into
mainfrom
feat/github-per-repo-import
Closed

feat(github): per-repo import connections with scoped OAuth tokens#3484
guitavano wants to merge 1 commit into
mainfrom
feat/github-per-repo-import

Conversation

@guitavano
Copy link
Copy Markdown
Contributor

@guitavano guitavano commented May 26, 2026

Summary

  • Create a dedicated GitHub MCP connection per import session instead of reusing a global org connection
  • After repo selection, scope the downstream OAuth token to that repository and persist repositoryId in connection metadata
  • Pass repository_id on token refresh and add a dev override (VITE_GITHUB_MCP_URL) for local GitHub MCP testing

Depends on

Test plan

  • Deploy or run local GitHub MCP with GITHUB_SCOPE_TOKEN
  • Import GitHub repo in Studio: OAuth → pick installation → pick repo
  • Confirm connection title becomes GitHub — owner/repo and metadata stores repositoryId
  • Confirm scoped token call succeeds (no 401/422)
  • Confirm token refresh still works after scoped import
  • bun test apps/mesh/src/shared/github-connection.test.ts
  • bun run check

Summary by cubic

Create a per-import GitHub MCP connection and scope the OAuth token to the selected repository. Refresh now includes the repository ID, and a dev override lets Studio target a local MCP.

  • New Features

    • Create a new GitHub connection for each import; set title to GitHub — owner/repo and store metadata.githubRepo with repositoryId.
    • Scope the token via GITHUB_SCOPE_TOKEN and persist it; downstream refresh adds repository_id from connection metadata.
    • Add VITE_GITHUB_MCP_URL to point to a local MCP in development (supports localhost and /api/mcp).
  • Dependencies

Written for commit 3cb3a21. Summary will update on new commits. Review in cubic

Create a dedicated GitHub MCP connection per import session, scope the user
token to the selected repository, and refresh downstream tokens with
repository_id from connection metadata. Adds a local MCP URL override for dev.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Benchmark

Should we run the Virtual MCP strategy benchmark for this PR?

React with 👍 to run the benchmark.

Reaction Action
👍 Run quick benchmark (10 & 128 tools)

Benchmark will run on the next push after you react.

@github-actions
Copy link
Copy Markdown
Contributor

Release Options

Suggested: Minor (2.354.0) — based on feat: prefix

React with an emoji to override the release type:

Reaction Type Next Version
👍 Prerelease 2.353.1-alpha.1
🎉 Patch 2.353.1
❤️ Minor 2.354.0
🚀 Major 3.0.0

Current version: 2.353.0

Note: If multiple reactions exist, the smallest bump wins. If no reactions, the suggested bump is used (default: patch).

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

5 issues found across 17 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/mesh/src/shared/github-clone-info.ts">

<violation number="1" location="apps/mesh/src/shared/github-clone-info.ts:63">
P2: Validate that the requested `owner/name` matches the connection’s scoped repo before using `repositoryId` for refresh, otherwise token scope and clone target can diverge.</violation>
</file>

<file name="packages/runtime/src/tools.ts">

<violation number="1" location="packages/runtime/src/tools.ts:376">
P2: `state` is not a valid OAuth 2.0 token exchange parameter (RFC 6749 Section 4.1.3). Adding it to the `OAuthParams` interface, documented specifically as "Token Exchange Parameters", is spec-inconsistent and could lead implementers of `exchangeCode` to incorrectly include it in the token endpoint request body.</violation>
</file>

<file name="apps/mesh/src/tools/github/list-user-orgs.ts">

<violation number="1" location="apps/mesh/src/tools/github/list-user-orgs.ts:49">
P1: Validate that the org-scoped connection exists before continuing. Without this check, the tool can still operate using a token row keyed only by `connectionId` even when the connection lookup failed.</violation>
</file>

<file name="apps/mesh/src/web/lib/github-oauth.ts">

<violation number="1" location="apps/mesh/src/web/lib/github-oauth.ts:57">
P2: Handle `isError` MCP tool responses before attempting to parse token payloads.</violation>

<violation number="2" location="apps/mesh/src/web/lib/github-oauth.ts:110">
P1: Do not persist scoped tokens without required refresh metadata, or this can null out stored refresh credentials and break token refresh.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment on lines +49 to +52
const connection = await ctx.storage.connections.findById(
input.connectionId,
organizationId,
);
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.

P1: Validate that the org-scoped connection exists before continuing. Without this check, the tool can still operate using a token row keyed only by connectionId even when the connection lookup failed.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/tools/github/list-user-orgs.ts, line 49:

<comment>Validate that the org-scoped connection exists before continuing. Without this check, the tool can still operate using a token row keyed only by `connectionId` even when the connection lookup failed.</comment>

<file context>
@@ -40,6 +41,22 @@ export const GITHUB_LIST_USER_ORGS = defineTool({
+      throw new Error("Organization context required");
+    }
+
+    const connection = await ctx.storage.connections.findById(
+      input.connectionId,
+      organizationId,
</file context>
Suggested change
const connection = await ctx.storage.connections.findById(
input.connectionId,
organizationId,
);
const connection = await ctx.storage.connections.findById(
input.connectionId,
organizationId,
);
if (!connection) {
throw new Error("Connection not found");
}


const parsed = parseScopeTokenResult(result);

await persistDownstreamOAuthToken({
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.

P1: Do not persist scoped tokens without required refresh metadata, or this can null out stored refresh credentials and break token refresh.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/web/lib/github-oauth.ts, line 110:

<comment>Do not persist scoped tokens without required refresh metadata, or this can null out stored refresh credentials and break token refresh.</comment>

<file context>
@@ -0,0 +1,129 @@
+
+  const parsed = parseScopeTokenResult(result);
+
+  await persistDownstreamOAuthToken({
+    orgSlug: params.orgSlug,
+    connectionId: params.connectionId,
</file context>

Comment on lines +63 to +65
const refreshOptions = repoScope
? { repositoryId: repoScope.repositoryId }
: undefined;
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: Validate that the requested owner/name matches the connection’s scoped repo before using repositoryId for refresh, otherwise token scope and clone target can diverge.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/shared/github-clone-info.ts, line 63:

<comment>Validate that the requested `owner/name` matches the connection’s scoped repo before using `repositoryId` for refresh, otherwise token scope and clone target can diverge.</comment>

<file context>
@@ -51,6 +52,18 @@ export async function buildCloneInfo(
+  const repoScope = getGithubConnectionRepoScope(
+    (connection?.metadata ?? null) as Record<string, unknown> | null,
+  );
+  const refreshOptions = repoScope
+    ? { repositoryId: repoScope.repositoryId }
+    : undefined;
</file context>
Suggested change
const refreshOptions = repoScope
? { repositoryId: repoScope.repositoryId }
: undefined;
const refreshOptions =
repoScope && repoScope.owner === owner && repoScope.name === name
? { repositoryId: repoScope.repositoryId }
: undefined;

* MUST be identical if included in the authorization request
*/
redirect_uri?: string;
/** OPTIONAL - MCP client state echoed from the authorization request */
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: state is not a valid OAuth 2.0 token exchange parameter (RFC 6749 Section 4.1.3). Adding it to the OAuthParams interface, documented specifically as "Token Exchange Parameters", is spec-inconsistent and could lead implementers of exchangeCode to incorrectly include it in the token endpoint request body.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/runtime/src/tools.ts, line 376:

<comment>`state` is not a valid OAuth 2.0 token exchange parameter (RFC 6749 Section 4.1.3). Adding it to the `OAuthParams` interface, documented specifically as "Token Exchange Parameters", is spec-inconsistent and could lead implementers of `exchangeCode` to incorrectly include it in the token endpoint request body.</comment>

<file context>
@@ -373,6 +373,8 @@ export interface OAuthParams {
    * MUST be identical if included in the authorization request
    */
   redirect_uri?: string;
+  /** OPTIONAL - MCP client state echoed from the authorization request */
+  state?: string;
 }
</file context>

access_token: string;
expires_in?: number;
} {
const typed = result as {
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: Handle isError MCP tool responses before attempting to parse token payloads.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/web/lib/github-oauth.ts, line 57:

<comment>Handle `isError` MCP tool responses before attempting to parse token payloads.</comment>

<file context>
@@ -0,0 +1,129 @@
+  access_token: string;
+  expires_in?: number;
+} {
+  const typed = result as {
+    structuredContent?: { access_token?: string; expires_in?: number };
+    content?: Array<{ text?: string }>;
</file context>

@guitavano guitavano closed this May 27, 2026
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