Add platform (built-in) OAuth integrations - #1303
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds operator-provisioned platform OAuth apps. It updates storage, lane-aware integration handling, OAuth connection and refresh flows, MCP capabilities, sandbox authentication, client behavior, tests, and architecture documentation. ChangesPlatform OAuth integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
kentcdodds
marked this pull request as ready for review
August 7, 2026 23:48
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/src/mcp/tools/search.node.test.ts (1)
30-51: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a platform-lane integration test case.
createJoinedIntegrationonly createslane: 'user', but the search output maps everyuserIntegrationRowsentry throughtoJoinedIntegrationConfig, which branches onlaneand builds a platform-specific integration config. Add alane: 'platform'case with the platformappshape such ashasClientSecret.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/tools/search.node.test.ts` around lines 30 - 51, Extend createJoinedIntegration to accept and construct a platform-lane fixture, including the platform app shape required by toJoinedIntegrationConfig such as hasClientSecret. Add a search test case using this platform fixture through userIntegrationRows and assert the platform-specific integration configuration is produced.
🧹 Nitpick comments (11)
packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-list.ts (1)
17-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
includeDisableddefault in the schema.The handler defaults
includeDisabledtotrue, butlistPlatformOauthAppsdefaults it tofalsewhen omitted. A model caller reads only the input schema, so it cannot know that disabled apps appear by default. Add a.describe()that states the default.♻️ Proposed schema description
const inputSchema = z .object({ - includeDisabled: z.boolean().optional(), + includeDisabled: z + .boolean() + .optional() + .describe('Include disabled apps. Defaults to true for admins.'), }) .strict()Also applies to: 57-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-list.ts` around lines 17 - 21, Update the includeDisabled field in inputSchema to add a .describe() stating that it defaults to true, matching the handler’s behavior when the field is omitted.packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts (1)
18-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueValidate
allowedScopesagainstdefaultScopesat the schema boundary.
upsertPlatformOauthAppmergesdefaultScopesintoallowedScopes, so a default scope that is absent from the allowlist is added silently. An operator who mistypes a default scope widens the allowlist without a warning. Add a.refine()that rejects adefaultScopesentry that is absent from a non-emptyallowedScopes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts` around lines 18 - 56, Update the inputSchema definition to add a refine validation after the object schema: when allowedScopes is non-empty, reject any defaultScopes entry not present in it, while allowing omitted or empty allowedScopes and preserving the existing schema validation.packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts (1)
104-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the
includeDisabled: falsefilter.Test 3 disables an app but never lists it. No test asserts that
includeDisabled: falsehides a disabled app, so a regression in the filter default at line 57 ofadmin-platform-oauth-app-list.tswould not fail the suite. Add a list call withincludeDisabled: falseafter the disable step and assert an emptyappsarray.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts` around lines 104 - 128, The tests do not cover that disabled apps are excluded when listing with includeDisabled: false. Extend the test covering app disabling, using adminPlatformOauthAppListCapability.handler with includeDisabled: false after the disable step, and assert that the returned apps array is empty.packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-delete.ts (1)
38-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLog the canonical slug in the audit reason.
deletePlatformOauthAppcanonicalizes the slug throughcanonicalIntegrationNamebefore it deletes the row (seepackages/worker/src/integrations/platform-apps.ts:233). The audit reason usesargs.slug.trim(), so the recorded value can differ from the deleted slug when the caller passes a non-canonical form. Return the canonical slug from the handler result and derive the reason from it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-delete.ts` around lines 38 - 52, The handler around deletePlatformOauthApp currently audits args.slug.trim() instead of the canonical slug being deleted. Capture the canonical slug in the handler result using the same canonicalization as deletePlatformOauthApp, return it alongside deleted, and derive successReason from that returned canonical value.packages/worker/src/mcp/execute-modules/kody-runtime-utils.node.test.ts (2)
650-675: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the missing-access-token-secret retry on the platform lane.
createAuthenticatedFetchreachesretryAuthorizationHeaderthrough two paths: the 401 response path and theisMissingAccessTokenSecretErrorcatch path at lines 163-168 ofkody-runtime-utils.ts. This test covers only the 401 path for a platform integration.Add a case where the first fetch rejects with
Secret "githubAccessToken" was not found.and assert thatintegration_token_refreshruns and the retry carries the placeholder header.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/execute-modules/kody-runtime-utils.node.test.ts` around lines 650 - 675, Add a platform-integration test for createAuthenticatedFetch that makes the initial fetch reject with the missing githubAccessToken secret error, then succeeds on retry. Assert integration_token_refresh is invoked and the retried request uses the placeholder authorization header, covering the isMissingAccessTokenSecretError catch path alongside the existing 401 test.
716-731: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake the interceptor listener async instead of using an unawaited IIFE.
FetchInterceptoruses synchronous request listeners, so the current IIFE runs outside the listener flow. Make the listenerasync, thencontroller.respondWith(...)without the IIFE.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/execute-modules/kody-runtime-utils.node.test.ts` around lines 716 - 731, Update the FetchInterceptor request listener to be an async callback, remove the unawaited IIFE wrapper, and retain the existing try/catch flow so it directly awaits controller.respondWith(...) and calls controller.errorWith(error) on failure.packages/worker/src/mcp/execute-modules/kody-runtime-utils.ts (1)
750-768: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe prelude string duplicates the TypeScript implementation.
__kodyRefreshAccessToken,__kodyRefreshPlatformIntegrationTokens, and theretryAuthorizationHeaderclosure restate the logic at lines 98-176 in a template literal. The two copies must stay identical, and no compiler checks the string copy.The error text at line 763 and line 105 is duplicated verbatim, so a wording change must be applied twice. Extract the shared message into a constant that both the TypeScript code and the template literal interpolate.
The declaration order is safe:
__kodyRefreshAccessTokenWithIntegrationat line 768 is aconst, and__kodyRefreshAccessTokenat line 759 only calls it at invocation time.Also applies to: 847-860, 876-883
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/execute-modules/kody-runtime-utils.ts` around lines 750 - 768, The duplicated platform-integration error message in __kodyRefreshPlatformIntegrationTokens and __kodyRefreshAccessToken must come from one shared constant. Define the constant in the surrounding TypeScript implementation and interpolate its value wherever the prelude template literal emits the same message, including the retryAuthorizationHeader-related duplicate locations, so wording changes require only one update while preserving existing behavior.packages/worker/src/integrations/platform-apps.ts (1)
251-264: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCanonicalize the slug in
countConnectionsForPlatformApp.
deletePlatformOauthAppcanonicalizes the slug before it calls this function, so the current production path is correct. This function is exported, though, and it bindsinput.slugunchanged. A caller that passes a non-canonical slug gets0and can conclude that no connections exist. ApplycanonicalIntegrationNamehere so the guard cannot be bypassed by input shape.♻️ Proposed fix
export async function countConnectionsForPlatformApp(input: { db: D1Database slug: string }): Promise<number> { + const slug = canonicalIntegrationName(input.slug) + if (!slug) return 0 const row = await input.db .prepare( `SELECT count(*) AS count FROM user_integrations WHERE platform_app_slug = ?`, ) - .bind(input.slug) + .bind(slug) .first<{ count: number }>() return Number(row?.count ?? 0) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/integrations/platform-apps.ts` around lines 251 - 264, Update countConnectionsForPlatformApp to canonicalize input.slug with canonicalIntegrationName before binding it in the query, ensuring direct callers receive the correct connection count regardless of slug format.packages/worker/src/mcp/secrets/crypto.ts (1)
84-123: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider sharing the encrypt/decrypt bodies with the secret-store functions.
encryptPlatformOauthClientSecretandencryptSecretValuediffer only in the purpose constant. The same applies to the decrypt pair. Two purpose-parameterized private helpers would keep the payload format in one place, so a future format change cannot drift between the two stores. The purpose separation stays intact.♻️ Proposed shared helpers
+async function encryptWithPurpose( + env: Pick<Env, 'SECRET_STORE_KEY'>, + purpose: string, + value: string, +) { + const key = await deriveEncryptionKey(env.SECRET_STORE_KEY, purpose) + const iv = crypto.getRandomValues(new Uint8Array(ivBytes)) + const ciphertext = await crypto.subtle.encrypt( + { name: 'AES-GCM', iv }, + key, + textEncoder.encode(value), + ) + return `${bytesToBase64Url(iv)}.${bytesToBase64Url(new Uint8Array(ciphertext))}` +} + export async function encryptPlatformOauthClientSecret( env: Pick<Env, 'SECRET_STORE_KEY'>, value: string, ) { - const key = await deriveEncryptionKey( - env.SECRET_STORE_KEY, - platformOauthClientSecretPurpose, - ) - const iv = crypto.getRandomValues(new Uint8Array(ivBytes)) - const ciphertext = await crypto.subtle.encrypt( - { name: 'AES-GCM', iv }, - key, - textEncoder.encode(value), - ) - return `${bytesToBase64Url(iv)}.${bytesToBase64Url(new Uint8Array(ciphertext))}` + return encryptWithPurpose(env, platformOauthClientSecretPurpose, value) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/secrets/crypto.ts` around lines 84 - 123, Refactor encryptPlatformOauthClientSecret and its decrypt counterpart to delegate their shared payload handling and crypto logic to private purpose-parameterized helpers, reusing the existing encryptSecretValue/decryptSecretValue behavior. Pass platformOauthClientSecretPurpose explicitly so purpose separation remains intact, and keep the public function signatures unchanged.packages/worker/src/integrations/platform-apps.node.test.ts (1)
147-167: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winNeither new test file exercises a second user. The platform lane adds user-scoped rows in
user_integrationsand a user-scoped refresh entry point, but both new test suites use a single user id. The shared root cause is missing cross-user coverage for the new lane.
packages/worker/src/integrations/platform-apps.node.test.ts#L147-L167: insert connections for two user ids against one platform app. Assert that each user reads only its own row and thatcountConnectionsForPlatformAppreturns 2.packages/worker/src/integrations/token-refresh.node.test.ts#L63-L98: callrefreshIntegrationTokensas a second user with the same connectionname. Assert that it rejects with the not-found error.As per coding guidelines: "user-scoped tests should exercise both the 'happy' path and a cross-user denial path."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/integrations/platform-apps.node.test.ts` around lines 147 - 167, Expand packages/worker/src/integrations/platform-apps.node.test.ts:147-167 to create connections for two user IDs, assert each user reads only its own row, and verify countConnectionsForPlatformApp returns 2. Expand packages/worker/src/integrations/token-refresh.node.test.ts:63-98 to call refreshIntegrationTokens for a second user using the same connection name and assert the expected not-found rejection.Source: Coding guidelines
packages/worker/src/integrations/token-refresh.node.test.ts (1)
99-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the
token_refreshed_atwrite and cover the provider failure paths.The
UPDATE user_integrations SET token_refreshed_at = ?write intoken-refresh.tsat lines 180 to 186 is not asserted anywhere.result.refreshedAtis returned from an in-memory value, so it passes even if theUPDATEmatches no row. Read the row back and compare it toresult.refreshedAt. Also add two cases withvi.stubGlobal('fetch', ...): a non-OK provider response, and a 200 response with noaccess_token. Both throw in the implementation and both are currently untested.💚 Proposed additional assertion
expect(refresh.found && refresh.value).toBe('rotated-refresh-token') + + const row = await env.APP_DB.prepare( + 'SELECT token_refreshed_at FROM user_integrations WHERE user_id = ? AND name = ?', + ) + .bind(userId, 'github') + .first<{ token_refreshed_at: string | null }>() + expect(row?.token_refreshed_at).toBe(result.refreshedAt) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/integrations/token-refresh.node.test.ts` around lines 99 - 129, Extend the token refresh test around the existing result assertions to read the user_integrations row back and verify its token_refreshed_at value matches result.refreshedAt, rather than relying only on the in-memory result. Add separate tests using vi.stubGlobal('fetch', ...) for a non-OK provider response and for a 200 response missing access_token, asserting both failure paths throw and restoring the stub between cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/migrations/0004-platform-oauth-apps.sql`:
- Around line 87-93: Recreate the migration-defined composite index
idx_user_integrations_user_app_slug after renaming user_integrations_next to
user_integrations, using columns user_id and app_slug with the predicate
requiring both to be non-null. Keep the existing
idx_user_integrations_platform_app creation unchanged.
In `@packages/worker/src/app/handlers/account-secrets.ts`:
- Around line 697-699: Update the parameter normalization near platformClientId
so platform-lane exchanges remove any caller-supplied client_secret from params
after parsing paramsRaw. Preserve the existing client_id override, ensuring
buildOAuthTokenExchangeRequest cannot forward caller-provided secrets for
platform PKCE form flows.
In `@packages/worker/src/integrations/service.ts`:
- Around line 360-370: Update the scope validation in the integration flow
around allowedScopes, scopes, and disallowed so every requested scope must exist
in allowedScopes, including when the allowlist is empty; remove the
allowedScopes.size guard. Apply this same validation before authorization or
token exchange in the account handler, before token values are saved or
upsertPlatformIntegration is called.
In `@packages/worker/src/integrations/token-refresh.ts`:
- Around line 130-134: Update the token request in the token-refresh flow around
the provider fetch to include an AbortSignal with an explicit timeout, ensuring
stalled provider calls terminate within the integration’s intended limit.
Preserve the existing POST method, headers, and request body, and ensure the
timeout applies to every invocation including createAuthenticatedFetch retry
handling.
- Around line 115-134: Update refreshIntegrationTokens() to validate
app.tokenUrl’s host against the user integration host allowlist before sending
credentials; for platform integrations, first normalize app.requiredHosts
together with app.tokenUrl and validate the normalized hosts. Reject disallowed
token URLs before the fetch call while preserving the existing request
construction for allowed URLs.
In `@packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts`:
- Around line 86-108: Preserve omitted optional platform OAuth app fields during
partial saves. In
packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts
lines 86-108, forward undefined for the listed optional fields and update
upsertPlatformOauthApp to retain existing values when those fields are omitted,
matching client_secret_encrypted handling; in
packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts
lines 130-154, assert the partial update preserves allowedScopes, defaultScopes,
and requiredHosts.
In
`@packages/worker/src/mcp/capabilities/integrations/integration-token-refresh.ts`:
- Around line 37-39: Update the capability metadata for refreshIntegrationTokens
to set idempotent to false, since the rotating refresh-token operation cannot
safely be retried or repeated.
In `@packages/worker/src/mcp/execute-modules/kody-runtime-utils.ts`:
- Around line 111-122: Update refreshPlatformIntegrationTokens and the
corresponding __kodyRefreshPlatformIntegrationTokens prelude helper to retain
the host refresh result, validate its ok status, and throw an error naming
providerName when refresh fails; preserve the existing unavailable-capability
error and successful refresh behavior.
---
Outside diff comments:
In `@packages/worker/src/mcp/tools/search.node.test.ts`:
- Around line 30-51: Extend createJoinedIntegration to accept and construct a
platform-lane fixture, including the platform app shape required by
toJoinedIntegrationConfig such as hasClientSecret. Add a search test case using
this platform fixture through userIntegrationRows and assert the
platform-specific integration configuration is produced.
---
Nitpick comments:
In `@packages/worker/src/integrations/platform-apps.node.test.ts`:
- Around line 147-167: Expand
packages/worker/src/integrations/platform-apps.node.test.ts:147-167 to create
connections for two user IDs, assert each user reads only its own row, and
verify countConnectionsForPlatformApp returns 2. Expand
packages/worker/src/integrations/token-refresh.node.test.ts:63-98 to call
refreshIntegrationTokens for a second user using the same connection name and
assert the expected not-found rejection.
In `@packages/worker/src/integrations/platform-apps.ts`:
- Around line 251-264: Update countConnectionsForPlatformApp to canonicalize
input.slug with canonicalIntegrationName before binding it in the query,
ensuring direct callers receive the correct connection count regardless of slug
format.
In `@packages/worker/src/integrations/token-refresh.node.test.ts`:
- Around line 99-129: Extend the token refresh test around the existing result
assertions to read the user_integrations row back and verify its
token_refreshed_at value matches result.refreshedAt, rather than relying only on
the in-memory result. Add separate tests using vi.stubGlobal('fetch', ...) for a
non-OK provider response and for a 200 response missing access_token, asserting
both failure paths throw and restoring the stub between cases.
In
`@packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts`:
- Around line 104-128: The tests do not cover that disabled apps are excluded
when listing with includeDisabled: false. Extend the test covering app
disabling, using adminPlatformOauthAppListCapability.handler with
includeDisabled: false after the disable step, and assert that the returned apps
array is empty.
In
`@packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-delete.ts`:
- Around line 38-52: The handler around deletePlatformOauthApp currently audits
args.slug.trim() instead of the canonical slug being deleted. Capture the
canonical slug in the handler result using the same canonicalization as
deletePlatformOauthApp, return it alongside deleted, and derive successReason
from that returned canonical value.
In `@packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-list.ts`:
- Around line 17-21: Update the includeDisabled field in inputSchema to add a
.describe() stating that it defaults to true, matching the handler’s behavior
when the field is omitted.
In `@packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts`:
- Around line 18-56: Update the inputSchema definition to add a refine
validation after the object schema: when allowedScopes is non-empty, reject any
defaultScopes entry not present in it, while allowing omitted or empty
allowedScopes and preserving the existing schema validation.
In `@packages/worker/src/mcp/execute-modules/kody-runtime-utils.node.test.ts`:
- Around line 650-675: Add a platform-integration test for
createAuthenticatedFetch that makes the initial fetch reject with the missing
githubAccessToken secret error, then succeeds on retry. Assert
integration_token_refresh is invoked and the retried request uses the
placeholder authorization header, covering the isMissingAccessTokenSecretError
catch path alongside the existing 401 test.
- Around line 716-731: Update the FetchInterceptor request listener to be an
async callback, remove the unawaited IIFE wrapper, and retain the existing
try/catch flow so it directly awaits controller.respondWith(...) and calls
controller.errorWith(error) on failure.
In `@packages/worker/src/mcp/execute-modules/kody-runtime-utils.ts`:
- Around line 750-768: The duplicated platform-integration error message in
__kodyRefreshPlatformIntegrationTokens and __kodyRefreshAccessToken must come
from one shared constant. Define the constant in the surrounding TypeScript
implementation and interpolate its value wherever the prelude template literal
emits the same message, including the retryAuthorizationHeader-related duplicate
locations, so wording changes require only one update while preserving existing
behavior.
In `@packages/worker/src/mcp/secrets/crypto.ts`:
- Around line 84-123: Refactor encryptPlatformOauthClientSecret and its decrypt
counterpart to delegate their shared payload handling and crypto logic to
private purpose-parameterized helpers, reusing the existing
encryptSecretValue/decryptSecretValue behavior. Pass
platformOauthClientSecretPurpose explicitly so purpose separation remains
intact, and keep the public function signatures unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 110cbe94-32a5-4313-a078-e3b8d1e5c928
📒 Files selected for processing (49)
docs/contributing/architecture/data-storage.mddocs/contributing/architecture/index.mddocs/contributing/architecture/integrations.mddocs/contributing/architecture/primitives.yamldocs/guides/oauth.mddocs/use/secrets-and-values.mdpackages/worker/client/routes/connect-oauth-config.tspackages/worker/client/routes/connect-oauth.tsxpackages/worker/migrations/0004-platform-oauth-apps.sqlpackages/worker/src/app/account-integrations-data.tspackages/worker/src/app/handlers/account-integrations.node.test.tspackages/worker/src/app/handlers/account-secrets.node.test.tspackages/worker/src/app/handlers/account-secrets.tspackages/worker/src/app/loader-data.tspackages/worker/src/integrations/oauth-token-exchange.node.test.tspackages/worker/src/integrations/oauth-token-exchange.tspackages/worker/src/integrations/platform-apps.node.test.tspackages/worker/src/integrations/platform-apps.tspackages/worker/src/integrations/repo.tspackages/worker/src/integrations/service.node.test.tspackages/worker/src/integrations/service.tspackages/worker/src/integrations/token-refresh.node.test.tspackages/worker/src/integrations/token-refresh.tspackages/worker/src/integrations/types.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-delete.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-list.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.tspackages/worker/src/mcp/capabilities/admin/domain.tspackages/worker/src/mcp/capabilities/integrations/domain.tspackages/worker/src/mcp/capabilities/integrations/integration-discover.tspackages/worker/src/mcp/capabilities/integrations/integration-oauth-app-list.tspackages/worker/src/mcp/capabilities/integrations/integration-oauth-app-rotate-credentials.tspackages/worker/src/mcp/capabilities/integrations/integration-platform-app-list.tspackages/worker/src/mcp/capabilities/integrations/integration-shared.tspackages/worker/src/mcp/capabilities/integrations/integration-token-refresh.tspackages/worker/src/mcp/capabilities/integrations/platform-app-shared.tspackages/worker/src/mcp/execute-modules/kody-runtime-utils.node.test.tspackages/worker/src/mcp/execute-modules/kody-runtime-utils.tspackages/worker/src/mcp/instructions/execute-tool-description.tspackages/worker/src/mcp/run-kody-registry.node.test.tspackages/worker/src/mcp/secrets/crypto.tspackages/worker/src/mcp/tools/search-detail.node.test.tspackages/worker/src/mcp/tools/search-detail.tspackages/worker/src/mcp/tools/search-entity-plugins/integration.tspackages/worker/src/mcp/tools/search-entity-registry.node.test.tspackages/worker/src/mcp/tools/search-handler.node.test.tspackages/worker/src/mcp/tools/search.node.test.tstools/migration-ledger.json
Comment on lines
+87
to
+93
| DROP TABLE user_integrations; | ||
|
|
||
| ALTER TABLE user_integrations_next RENAME TO user_integrations; | ||
|
|
||
| CREATE INDEX idx_user_integrations_platform_app | ||
| ON user_integrations(platform_app_slug) | ||
| WHERE platform_app_slug IS NOT NULL; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: List indexes and triggers defined on user_integrations across migrations.
set -euo pipefail
rg -n -i 'user_integrations' packages/worker/migrations --glob '*.sql' -C 2Repository: kentcdodds/kody
Length of output: 3336
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Migration 0001 relevant user_integrations definitions:"
sed -n '570,615p;1119,1124p' packages/worker/migrations/0001-squashed-init.sql
echo
echo "Migration 0004 user_integrations definitions:"
sed -n '34,94p' packages/worker/migrations/0004-platform-oauth-apps.sql
echo
echo "All CREATE INDEX / DROP INDEX and trigger definitions referencing user_integrations:"
rg -n -i 'CREATE INDEX|DROP INDEX|CREATE TRIGGER|DROP TRIGGER|user_integrations|user_integrations_next' packages/worker/migrations --glob '*.sql'Repository: kentcdodds/kody
Length of output: 16740
Recreate idx_user_integrations_user_app_slug after the table rename.
DROP TABLE user_integrations removes the migration-defined idx_user_integrations_user_app_slug(user_id, app_slug) index. Preserve the composite index for populated app_slug rows by creating it from user_integrations_next as idx_user_integrations_user_app_slug(user_id, app_slug) WHERE user_id IS NOT NULL AND app_slug IS NOT NULL.
🧰 Tools
🪛 Squawk (2.61.0)
[warning] 87-87: Dropping a table may break existing clients.
(ban-drop-table)
[warning] 89-89: Renaming a table may break existing clients.
(renaming-table)
[warning] 91-93: During normal index creation, table updates are blocked, but reads are still allowed. Use concurrently to avoid blocking writes.
(require-concurrent-index-creation)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/migrations/0004-platform-oauth-apps.sql` around lines 87 -
93, Recreate the migration-defined composite index
idx_user_integrations_user_app_slug after renaming user_integrations_next to
user_integrations, using columns user_id and app_slug with the predicate
requiring both to be non-null. Keep the existing
idx_user_integrations_platform_app creation unchanged.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
|
🔎 Preview deployed: https://kody-pr-1303.kody-a99.workers.dev Worker: Mocks:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/mcp/execute-modules/kody-runtime-utils.node.test.ts`:
- Around line 703-707: Update the test assertions near tokenRefreshCalls to also
verify that fetchCalls[0] has the expected authorization header, “Bearer
{{secret:githubAccessToken|scope=user}}”, while preserving the existing
fetchCalls[1] retry assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b46dac77-2b9a-4b2c-b07b-332f56036cc9
📒 Files selected for processing (49)
docs/contributing/architecture/data-storage.mddocs/contributing/architecture/index.mddocs/contributing/architecture/integrations.mddocs/contributing/architecture/primitives.yamldocs/guides/oauth.mddocs/use/secrets-and-values.mdpackages/worker/client/routes/connect-oauth-config.tspackages/worker/client/routes/connect-oauth.tsxpackages/worker/migrations/0004-platform-oauth-apps.sqlpackages/worker/src/app/account-integrations-data.tspackages/worker/src/app/handlers/account-integrations.node.test.tspackages/worker/src/app/handlers/account-secrets.node.test.tspackages/worker/src/app/handlers/account-secrets.tspackages/worker/src/integrations/oauth-token-exchange.node.test.tspackages/worker/src/integrations/oauth-token-exchange.tspackages/worker/src/integrations/platform-apps.node.test.tspackages/worker/src/integrations/platform-apps.tspackages/worker/src/integrations/repo.tspackages/worker/src/integrations/service.node.test.tspackages/worker/src/integrations/service.tspackages/worker/src/integrations/token-refresh.node.test.tspackages/worker/src/integrations/token-refresh.tspackages/worker/src/integrations/types.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-delete.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-list.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.tspackages/worker/src/mcp/capabilities/admin/domain.tspackages/worker/src/mcp/capabilities/integrations/domain.tspackages/worker/src/mcp/capabilities/integrations/integration-discover.tspackages/worker/src/mcp/capabilities/integrations/integration-oauth-app-list.tspackages/worker/src/mcp/capabilities/integrations/integration-oauth-app-rotate-credentials.tspackages/worker/src/mcp/capabilities/integrations/integration-platform-app-list.tspackages/worker/src/mcp/capabilities/integrations/integration-shared.tspackages/worker/src/mcp/capabilities/integrations/integration-token-refresh.tspackages/worker/src/mcp/capabilities/integrations/platform-app-shared.tspackages/worker/src/mcp/execute-modules/kody-runtime-utils.node.test.tspackages/worker/src/mcp/execute-modules/kody-runtime-utils.tspackages/worker/src/mcp/instructions/execute-tool-description.tspackages/worker/src/mcp/run-kody-registry.node.test.tspackages/worker/src/mcp/secrets/crypto.tspackages/worker/src/mcp/tools/search-detail.node.test.tspackages/worker/src/mcp/tools/search-detail.tspackages/worker/src/mcp/tools/search-entity-plugins/integration.tspackages/worker/src/mcp/tools/search-entity-registry.node.test.tspackages/worker/src/mcp/tools/search-handler.node.test.tspackages/worker/src/mcp/tools/search.node.test.tspackages/worker/universal/loader-data.tstools/migration-ledger.json
🚧 Files skipped from review as they are similar to previous changes (44)
- packages/worker/src/mcp/tools/search.node.test.ts
- packages/worker/src/mcp/capabilities/integrations/integration-token-refresh.ts
- tools/migration-ledger.json
- packages/worker/src/mcp/capabilities/admin/domain.ts
- packages/worker/client/routes/connect-oauth.tsx
- packages/worker/src/mcp/capabilities/integrations/platform-app-shared.ts
- packages/worker/src/mcp/tools/search-detail.node.test.ts
- packages/worker/src/app/handlers/account-integrations.node.test.ts
- packages/worker/src/mcp/tools/search-handler.node.test.ts
- packages/worker/src/mcp/instructions/execute-tool-description.ts
- docs/contributing/architecture/index.md
- packages/worker/src/mcp/tools/search-entity-plugins/integration.ts
- packages/worker/src/integrations/platform-apps.node.test.ts
- docs/guides/oauth.md
- docs/use/secrets-and-values.md
- packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-delete.ts
- packages/worker/src/mcp/tools/search-entity-registry.node.test.ts
- packages/worker/src/mcp/capabilities/integrations/integration-platform-app-list.ts
- packages/worker/src/integrations/token-refresh.node.test.ts
- packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-list.ts
- packages/worker/src/mcp/capabilities/integrations/integration-oauth-app-rotate-credentials.ts
- packages/worker/src/integrations/oauth-token-exchange.node.test.ts
- packages/worker/src/mcp/run-kody-registry.node.test.ts
- packages/worker/src/mcp/capabilities/integrations/integration-discover.ts
- packages/worker/src/app/account-integrations-data.ts
- packages/worker/src/integrations/service.node.test.ts
- packages/worker/src/app/handlers/account-secrets.ts
- packages/worker/src/mcp/tools/search-detail.ts
- packages/worker/client/routes/connect-oauth-config.ts
- docs/contributing/architecture/primitives.yaml
- packages/worker/src/mcp/capabilities/integrations/integration-oauth-app-list.ts
- packages/worker/src/integrations/types.ts
- packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts
- packages/worker/src/mcp/capabilities/integrations/integration-shared.ts
- packages/worker/src/mcp/capabilities/integrations/domain.ts
- docs/contributing/architecture/integrations.md
- packages/worker/src/integrations/oauth-token-exchange.ts
- packages/worker/src/integrations/token-refresh.ts
- packages/worker/src/mcp/execute-modules/kody-runtime-utils.ts
- docs/contributing/architecture/data-storage.md
- packages/worker/src/app/handlers/account-secrets.node.test.ts
- packages/worker/src/integrations/platform-apps.ts
- packages/worker/src/integrations/service.ts
- packages/worker/src/integrations/repo.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/worker/client/routes/connect-oauth-config.ts (1)
405-408: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPreserve account-scoped token-secret names for platform connections.
Platform app prefill and connected-token payloads need stable
accessTokenSecretName/refreshTokenSecretNamevalues for reconnect and provider-key fallbacks. Fallback names should match the user/account-connection namespace, not a global provider namespace, so another user cannot pre-create a secret name and steal token credentials.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/client/routes/connect-oauth-config.ts` around lines 405 - 408, Update the platform-connection fallback logic for accessTokenSecretName and refreshTokenSecretName to use stable user/account-connection-scoped secret names rather than global provider-scoped names. Preserve these names in platform-app prefill and connected-token payloads so reconnect and provider-key fallbacks resolve consistently, while keeping clientSecretSecretName server-side behavior unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/client/routes/connect-oauth-config.ts`:
- Around line 359-370: Update mergeConnectOauthConfig and the
authorization/token-exchange flow to derive platform app metadata and
platformAllowedScopes from authenticated server-side state, including the
initial unconnected platform flow. Apply the approved-scope clamp before
authorization regardless of browser-provided or saved session configuration, and
use the server-derived client-secret metadata instead of accepting null or
browser-controlled values.
In `@packages/worker/src/mcp/capabilities/integrations/integration-save.ts`:
- Around line 53-60: Update the platform-connection guard in the integration
save flow to build the reconnect URL from existing.platformAppSlug rather than
existing.name, preserving the displayed integration name in the message. Add a
regression test covering a platform connection with a custom name whose
reconnect provider must use the stored platform app slug.
---
Outside diff comments:
In `@packages/worker/client/routes/connect-oauth-config.ts`:
- Around line 405-408: Update the platform-connection fallback logic for
accessTokenSecretName and refreshTokenSecretName to use stable
user/account-connection-scoped secret names rather than global provider-scoped
names. Preserve these names in platform-app prefill and connected-token payloads
so reconnect and provider-key fallbacks resolve consistently, while keeping
clientSecretSecretName server-side behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3761c62e-7727-4cbe-aebc-7abf38443b6d
📒 Files selected for processing (15)
packages/worker/client/routes/connect-oauth-config.tspackages/worker/migrations/0004-platform-oauth-apps.sqlpackages/worker/src/app/handlers/account-secrets.node.test.tspackages/worker/src/app/handlers/account-secrets.tspackages/worker/src/integrations/platform-apps.node.test.tspackages/worker/src/integrations/platform-apps.tspackages/worker/src/integrations/service.node.test.tspackages/worker/src/integrations/service.tspackages/worker/src/integrations/token-refresh.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.tspackages/worker/src/mcp/capabilities/integrations/integration-save.node.test.tspackages/worker/src/mcp/capabilities/integrations/integration-save.tspackages/worker/src/mcp/capabilities/integrations/integration-token-refresh.tspackages/worker/src/mcp/execute-modules/kody-runtime-utils.tstools/migration-ledger.json
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/worker/src/mcp/capabilities/integrations/integration-token-refresh.ts
- tools/migration-ledger.json
- packages/worker/src/integrations/platform-apps.node.test.ts
- packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts
- packages/worker/src/app/handlers/account-secrets.ts
- packages/worker/src/integrations/token-refresh.ts
- packages/worker/src/app/handlers/account-secrets.node.test.ts
- packages/worker/src/mcp/execute-modules/kody-runtime-utils.ts
- packages/worker/src/integrations/platform-apps.ts
- packages/worker/src/integrations/service.ts
- packages/worker/src/integrations/service.node.test.ts
Comment on lines
+53
to
+60
| // A partial merge onto a platform (built-in) connection would | ||
| // silently convert it to a user-lane app and break host-side | ||
| // refresh; keep platform connections managed by the connect flow. | ||
| if (existing?.platform === true) { | ||
| throw new McpCallerError( | ||
| `Integration "${args.name}" is a platform (built-in) connection managed at /connect/oauth?provider=${encodeURIComponent(existing.name)}. Reconnect there to change scopes, or integration_delete it first to replace it with your own OAuth app.`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect whether getIntegration exposes the stored platform app slug and find
# all reconnect URL construction sites.
rg -n -P -C 4 \
'function\s+getIntegration\b|export\s+async\s+function\s+getIntegration\b|platformAppSlug|platform_app_slug|/connect/oauth\?provider=' \
packages/worker/src/integrations \
packages/worker/src/mcp/capabilities/integrationsRepository: kentcdodds/kody
Length of output: 27946
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the read projection, platform-app list provider field, and save guard in context.
sed -n '160,190p;200,235p' packages/worker/src/integrations/service.ts
sed -n '1,60p' packages/worker/src/mcp/capabilities/integrations/integration-platform-app-list.ts
sed -n '45,95p' packages/worker/src/integrations/service.node.test.ts
sed -n '468,492p' packages/worker/src/mcp/capabilities/integrations/integration-platform-app-list.ts
sed -n '1,80p' packages/worker/src/mcp/capabilities/integrations/integration-save.node.test.ts
sed -n '40,68p' packages/worker/src/mcp/capabilities/integrations/integration-save.tsRepository: kentcdodds/kody
Length of output: 8882
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate and inspect the public platform-app schema and mapping.
rg -n -C 8 'toPlatformOauthAppPublic|platformOauthAppPublicSchema|platformAppSlug|slug' packages/worker/src/integrations packages/worker/src/mcp/capabilities/integrations/platform-app-shared.ts
sed -n '1,180p' packages/worker/src/mcp/capabilities/integrations/platform-app-shared.tsRepository: kentcdodds/kody
Length of output: 50372
Use existing.platformAppSlug in the reconnect URL.
upsertPlatformIntegration stores the provider slug separately from name, and /connect/oauth?provider= expects that slug. A platform integration named github-work currently produces /connect/oauth?provider=github-work, which does not match the github app.
Use the existing provider slug for this guard, which getIntegration already projects as connection.platformAppSlug, and add a regression test with a custom platform connection name.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/worker/src/mcp/capabilities/integrations/integration-save.ts` around
lines 53 - 60, Update the platform-connection guard in the integration save flow
to build the reconnect URL from existing.platformAppSlug rather than
existing.name, preserving the displayed integration name in the message. Add a
regression test covering a platform connection with a custom name whose
reconnect provider must use the stored platform app slug.
Operator-provisioned OAuth apps every user can connect through
/connect/oauth without registering their own provider app:
- platform_oauth_apps D1 table; client secret encrypted (AES-GCM keyed
off SECRET_STORE_KEY, dedicated purpose) outside the user secret
store, so no {{secret:...}} placeholder can reference it
- user_integrations rebuilt: nullable app_slug + platform_app_slug FK,
CHECK exactly one lane per connection; JoinedIntegration is now a
lane-discriminated union
- host-side token refresh (integration_token_refresh) persists rotated
tokens server-side and returns metadata only; platform connections in
the sandbox refresh through it and retry with placeholder headers, so
raw tokens never enter the sandbox heap; refreshAccessToken throws
for platform integrations
- admin_platform_oauth_app_save/_list/_delete provisioning capabilities
and integration_platform_app_list for users/agents
- /connect/oauth prefills from enabled platform apps and skips the
client-credential setup step; oauth_exchange and connect_oauth take
every exchange input from the platform app row, never the body
- oauth-token-exchange moved from #app to #worker/integrations so the
MCP refresh capability respects import boundaries
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Security-review finding: integration_token_refresh materialized the user's refresh token and client secret and POSTed them to the connection's tokenUrl without consulting those secrets' allowed_hosts — the containment the fetch gateway enforces for the equivalent placeholder-based sandbox path. A sandboxed package could repoint tokenUrl via integration_save and exfiltrate both secrets. User-lane refresh now asserts each materialized secret's allowed_hosts contains the token host before the provider request, failing closed with the approval path in the error. Platform-lane destinations are operator-pinned rows, so no user-secret allowlist applies there. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
- Recreate idx_user_integrations_user_app_slug dropped by the 0004 table rebuild (ledger hash updated) - Validate platform scopes before persisting token secrets so a rejected scope set leaves no orphan secrets; share the assertion between the connect handler and upsertPlatformIntegration - Treat an empty allowed-scope menu as a strict allowlist (only scope-less connects pass) - integration_save refuses platform (built-in) connections instead of silently converting them to a user-lane app - Strip caller-supplied client_secret from platform-lane exchange params (client_id was already pinned) - Retain-on-omit semantics for every optional admin save field so a partial save cannot clear the scope menu, hosts, or stored secret - 30s timeout on the host-side refresh's provider request; mark integration_token_refresh non-idempotent (refresh-token rotation) - Sandbox platform-refresh helper verifies the capability's ok result - Client clamps requested platform scopes to the operator menu before building the authorize URL Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Bugbot: mergeConnectOauthConfig clamps platform scopes to the stored menu, but parseStoredIntegrationConfig never loaded the menu from JSON, so parsed platform configs authorized with no scopes. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
cursor
Bot
force-pushed
the
cursor/platform-oauth-integrations-c0a2
branch
from
August 9, 2026 06:01
9bd9b88 to
37f2d05
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/migrations/0004-platform-oauth-apps.sql`:
- Line 7: Update the platform_oauth_apps schema definition for slug to
explicitly declare it NOT NULL alongside PRIMARY KEY, ensuring every platform
app has a non-null slug.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a7e25f97-deb5-4f3f-a528-b9f9ada7e36b
📒 Files selected for processing (51)
docs/contributing/architecture/data-storage.mddocs/contributing/architecture/index.mddocs/contributing/architecture/integrations.mddocs/contributing/architecture/primitives.yamldocs/guides/oauth.mddocs/use/secrets-and-values.mdpackages/worker/client/routes/connect-oauth-config.tspackages/worker/client/routes/connect-oauth.tsxpackages/worker/migrations/0004-platform-oauth-apps.sqlpackages/worker/src/app/account-integrations-data.tspackages/worker/src/app/handlers/account-integrations.node.test.tspackages/worker/src/app/handlers/account-secrets.node.test.tspackages/worker/src/app/handlers/account-secrets.tspackages/worker/src/integrations/oauth-token-exchange.node.test.tspackages/worker/src/integrations/oauth-token-exchange.tspackages/worker/src/integrations/platform-apps.node.test.tspackages/worker/src/integrations/platform-apps.tspackages/worker/src/integrations/repo.tspackages/worker/src/integrations/service.node.test.tspackages/worker/src/integrations/service.tspackages/worker/src/integrations/token-refresh.node.test.tspackages/worker/src/integrations/token-refresh.tspackages/worker/src/integrations/types.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-delete.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-list.tspackages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.tspackages/worker/src/mcp/capabilities/admin/domain.tspackages/worker/src/mcp/capabilities/integrations/domain.tspackages/worker/src/mcp/capabilities/integrations/integration-discover.tspackages/worker/src/mcp/capabilities/integrations/integration-oauth-app-list.tspackages/worker/src/mcp/capabilities/integrations/integration-oauth-app-rotate-credentials.tspackages/worker/src/mcp/capabilities/integrations/integration-platform-app-list.tspackages/worker/src/mcp/capabilities/integrations/integration-save.node.test.tspackages/worker/src/mcp/capabilities/integrations/integration-save.tspackages/worker/src/mcp/capabilities/integrations/integration-shared.tspackages/worker/src/mcp/capabilities/integrations/integration-token-refresh.tspackages/worker/src/mcp/capabilities/integrations/platform-app-shared.tspackages/worker/src/mcp/execute-modules/kody-runtime-utils.node.test.tspackages/worker/src/mcp/execute-modules/kody-runtime-utils.tspackages/worker/src/mcp/instructions/execute-tool-description.tspackages/worker/src/mcp/run-kody-registry.node.test.tspackages/worker/src/mcp/secrets/crypto.tspackages/worker/src/mcp/tools/search-detail.node.test.tspackages/worker/src/mcp/tools/search-detail.tspackages/worker/src/mcp/tools/search-entity-plugins/integration.tspackages/worker/src/mcp/tools/search-entity-registry.node.test.tspackages/worker/src/mcp/tools/search-handler.node.test.tspackages/worker/src/mcp/tools/search.node.test.tspackages/worker/universal/loader-data.tstools/migration-ledger.json
🚧 Files skipped from review as they are similar to previous changes (49)
- tools/migration-ledger.json
- packages/worker/src/mcp/tools/search-handler.node.test.ts
- packages/worker/src/mcp/tools/search.node.test.ts
- packages/worker/src/mcp/tools/search-detail.ts
- packages/worker/src/mcp/tools/search-detail.node.test.ts
- packages/worker/src/mcp/instructions/execute-tool-description.ts
- packages/worker/src/mcp/tools/search-entity-plugins/integration.ts
- packages/worker/src/mcp/capabilities/integrations/integration-oauth-app-list.ts
- docs/contributing/architecture/index.md
- packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-list.ts
- packages/worker/src/mcp/capabilities/integrations/domain.ts
- packages/worker/src/mcp/capabilities/integrations/integration-save.node.test.ts
- packages/worker/src/mcp/capabilities/integrations/integration-discover.ts
- packages/worker/src/mcp/tools/search-entity-registry.node.test.ts
- packages/worker/universal/loader-data.ts
- docs/use/secrets-and-values.md
- packages/worker/src/mcp/secrets/crypto.ts
- packages/worker/src/mcp/capabilities/integrations/integration-shared.ts
- packages/worker/src/mcp/capabilities/integrations/integration-oauth-app-rotate-credentials.ts
- packages/worker/src/mcp/capabilities/integrations/integration-token-refresh.ts
- packages/worker/src/mcp/execute-modules/kody-runtime-utils.ts
- packages/worker/src/mcp/capabilities/integrations/integration-platform-app-list.ts
- packages/worker/client/routes/connect-oauth.tsx
- packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-capabilities.node.test.ts
- packages/worker/src/integrations/platform-apps.node.test.ts
- packages/worker/src/integrations/oauth-token-exchange.node.test.ts
- packages/worker/src/mcp/capabilities/admin/domain.ts
- docs/contributing/architecture/primitives.yaml
- packages/worker/src/mcp/capabilities/integrations/platform-app-shared.ts
- docs/contributing/architecture/data-storage.md
- packages/worker/src/app/handlers/account-integrations.node.test.ts
- packages/worker/src/integrations/token-refresh.node.test.ts
- packages/worker/client/routes/connect-oauth-config.ts
- docs/guides/oauth.md
- packages/worker/src/app/handlers/account-secrets.node.test.ts
- packages/worker/src/integrations/token-refresh.ts
- packages/worker/src/mcp/run-kody-registry.node.test.ts
- packages/worker/src/integrations/types.ts
- packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-delete.ts
- packages/worker/src/app/account-integrations-data.ts
- packages/worker/src/integrations/service.node.test.ts
- docs/contributing/architecture/integrations.md
- packages/worker/src/app/handlers/account-secrets.ts
- packages/worker/src/integrations/platform-apps.ts
- packages/worker/src/integrations/service.ts
- packages/worker/src/integrations/oauth-token-exchange.ts
- packages/worker/src/integrations/repo.ts
- packages/worker/src/mcp/capabilities/admin/admin-platform-oauth-app-save.ts
- packages/worker/src/mcp/capabilities/integrations/integration-save.ts
… both lanes
User-lane (BYO) connections now refresh through integration_token_refresh
exactly like platform connections: on 401 the sandbox helper triggers a
host-side refresh and retries with a {{secret:...}} placeholder header,
so raw tokens never enter the sandbox heap on this path for any lane.
Behavior-equivalent for working connections: the old in-sandbox path
resolved placeholders through the fetch gateway, which enforced each
secret's allowed_hosts — the host-side path enforces the same allowlist.
Semantic change: package code triggering refresh via
createAuthenticatedFetch no longer needs a secret-write grant (the
system persists rotated tokens host-side; packages never see values).
refreshAccessToken stays as the legacy raw-token helper for auth that
cannot use an Authorization header; it is unchanged for user-lane
integrations and still throws for platform ones. Repo-wide audit found
nothing relying on the old in-sandbox refresh traffic.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
- logo_key / logo_content_type columns (migration 0005); assets live in
the COMMUNITY_ASSETS R2 bucket under content-hashed
platform-oauth-app-logos/{slug}/ keys with immutable cache headers
- uploads through admin_platform_oauth_app_save logoBase64 (null clears,
omit retains); SVG input is sanitized and rasterized to PNG via the
community-icon pipeline so an active image format is never stored or
served; raster formats are dimension-validated
- public /integrations/logos/:integrationSlug serving route (nosniff,
immutable, ETag); projections expose a relative logoPath with a
content-hash cache-busting tag; the connect page renders the logo
- app deletes clean up the logo asset; app upserts never touch logo
columns
- platform_oauth_apps registered as an explicit operator-owned D1
surface in account data targets
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 751a5e3. Configure here.
- platform_oauth_apps.slug explicitly NOT NULL (SQLite allows NULL in plain TEXT PRIMARY KEY); ledger re-hashed - parseStoredIntegrationConfig loads platformLogoPath so restored configs keep the operator logo - parsePlatformLogoPath accepts only one clean slug segment plus an optional cache tag, so a tampered session snapshot cannot point the img at other same-origin paths Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
This was referenced Aug 9, 2026
This was referenced Aug 10, 2026
This was referenced Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Operator-provisioned OAuth apps every user can connect through
/connect/oauth?provider=<slug>without registering their own provider app. The BYO lane is unchanged and remains the escape hatch.What changed
platform_oauth_appstable (migration0004): global operator config (nouser_id, like feature flags). The shared client secret is stored encrypted (AES-GCM keyed offSECRET_STORE_KEYwith a dedicated purpose) outside the user secret store, so no{{secret:…}}placeholder can name it and sandboxed code has no resolution path to it.getPlatformOauthAppClientSecretis the only decrypt accessor.user_integrationsrebuild: nullableapp_slug+ newplatform_app_slugFK (ON DELETE RESTRICT),CHECKthat exactly one lane is set, both(user_id, app_slug)and platform-lane indexes recreated.JoinedIntegrationis now alane: 'user' | 'platform'discriminated union. Token secrets stay per-user in both lanes.integration_token_refresh): resolves refresh token + client secret server-side (user lane enforces each secret'sallowed_hostsagainst the token URL — the same containment the gateway applied in-sandbox), persists rotated tokens (refresh-first ordering, 30s provider timeout), returns metadata only.createAuthenticatedFetchrefreshes through it for both lanes and retries with a placeholderAuthorizationheader, so raw tokens never enter the sandbox heap on this path.refreshAccessTokenremains the legacy raw-token helper for non-header auth (user lane only; throws for platform integrations). Semantic note: package code triggering refresh viacreateAuthenticatedFetchno longer needs a secret-write grant — the system persists tokens host-side and packages never see values.admin_platform_oauth_app_save/_list/_delete(role-gated, audited; plaintextclientSecretinput encrypted at rest and never returned; retain-on-omit for every optional field so partial saves can't clear the scope menu or hosts; delete refuses while user connections reference the app)./connect/oauth?provider=<slug>prefills from an enabled platform app and skips the client-credentials setup step entirely.oauth_exchange/connect_oauthacceptplatformAppSlugand take every exchange input (token URL, flow, style, client id, secret) from the operator-provisioned row, never the request body; caller-suppliedclient_secretis stripped. Scope validation runs against the operator menu (strict allowlist; empty menu = scope-less only) before any token persists, and the client clamps requested scopes to the menu.integration_platform_app_listcapability;integration_getmarks platform configs withplatform: true;integration_saverefuses platform connections instead of silently converting them to the user lane.oauth-token-exchange.tsmoved from#appto#worker/integrationsso the MCP refresh capability respects import boundaries.Testing
npm run validategreen at every revision (format, lint, typecheck, full unit suites across node + workers pools, Playwright e2e, MCP tests, builds, primitives, migrations ledger, deploy guardrails, docs).refreshAccessTokenrefusal for platform).allowed_hosts) is fixed with fail-closed tests. All Bugbot/CodeRabbit findings addressed.System recap — extends the integrations primitive (medium risk)
Mode: recap · Base:
main@f84f38d6· Head:6c553665Classification: extends — the
integrationsprimitive gains a second app lane (operator-provisioned platform apps) and a unified host-side refresh path; no new primitive id,primitives.yamlsummary updated to match the reshaped meaning.Primitives touched
integrationsintegration_token_refreshfor all lanesd1-app-dbplatform_oauth_appstable;user_integrationsrebuilt with lane columnsmcp-servercapabilities-executeapp-uirbacSystem map
A user connects a built-in provider: the connect UI drives host-side token exchange against the operator's platform app row, tokens land in the user's secret store, and sandbox refresh for every lane goes through a host-side capability.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Before / after
Invariants
platform_oauth_appsholds no user data (global operator config, feature-flag-like); connections and token secrets stay keyed byuser_id.allowed_hostsagainst the token URL; sandbox retries use{{secret:…}}placeholders resolved at the fetch gateway.Summary by CodeRabbit
New Features
Documentation