Repository navigation
Persist rotated OAuth refresh tokens in codemode helpers - #125
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant Execute as Execute Helper / Prelude
participant Codemode as CodeMode Capabilities
participant Provider as Token Provider (POST /token)
Execute->>Codemode: call __kodyRefreshAccessToken(refreshTokenSecretName,...)
Codemode->>Execute: returns stored refresh token template
Execute->>Provider: POST token request (body includes encoded refresh_token template)
Provider-->>Execute: returns payload { access_token, refresh_token? }
Execute->>Codemode: call __kodyPersistSecret(connector.accessTokenSecretName, access_token)
alt payload.refresh_token present
Execute->>Codemode: call __kodyPersistSecret(refreshTokenSecretName, refresh_token)
end
Execute->>Provider: subsequent authenticated API call with Authorization: Bearer <access_token>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
kentcdodds
marked this pull request as ready for review
March 31, 2026 23:38
Contributor
|
🔎 Preview deployed: https://kody-pr-125.kentcdodds.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/worker/src/mcp/skills/infer-codemode-capabilities.ts (1)
3-7: Move the execute-helper capability list to a shared constant.This is now a second hand-maintained copy of the allowlist from
packages/worker/src/mcp/execute-modules/codemode-utils.tsat Lines 39-43. If they drift, inference and the runtime helper surface will silently diverge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/skills/infer-codemode-capabilities.ts` around lines 3 - 7, The local const executeHelperCapabilities should be removed and replaced with a single shared exported constant to avoid duplication; export the allowlist from the existing codemode-utils module (e.g., add and export a descriptive name like EXECUTE_HELPER_CAPABILITIES in the module that currently defines the original list) and import that constant into infer-codemode-capabilities.ts, replacing the local executeHelperCapabilities usage so both inference and runtime use the same source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/src/mcp/execute-modules/codemode-utils.node.test.ts`:
- Around line 48-49: The test's fetchStub signature uses the unavailable DOM
type RequestInfo; update its parameter type to a Node-compatible union (e.g.
Request | string | URL) or use Parameters<typeof fetch>[0] to match the real
fetch signature; modify the fetchStub declaration (and the other occurrence
around the 149-151 area) to replace RequestInfo with the chosen type and keep
the init?: RequestInit as-is so Request construction (new Request(input, init))
still type-checks.
- Around line 142-152: The current cast of the newly constructed Function to a
signature referencing typeof codemode is self-referential and causes a
TypeScript error; fix it by introducing a separate type alias for the codemode
parameter (e.g., type Codemode = typeof codemodeModule) or by renaming the
parameter used in the Function signature so the cast no longer refers to the
parameter name itself, then update the cast on createSandboxHelpers to use that
alias (or the renamed symbol) and keep the returned shape for refreshAccessToken
and createAuthenticatedFetch intact while still referencing prelude,
refreshAccessToken, createAuthenticatedFetch and codemode in the Function body.
- Around line 27-45: The codemode test stub uses narrower parameter types
causing contravariance errors; update the stub so each method signature matches
the helper namespace types (use CodemodeNamespace or CapabilityArgs =
Record<string, unknown> for connector_get, value_get, secret_set, etc.), replace
any RequestInfo | URL occurrences with ExecuteRequestInput | URL, and remove the
self-referential cast using typeof codemode (use an explicit CodemodeNamespace
cast instead) so the stub can be passed to refreshAccessToken and
createAuthenticatedFetch without type errors.
---
Nitpick comments:
In `@packages/worker/src/mcp/skills/infer-codemode-capabilities.ts`:
- Around line 3-7: The local const executeHelperCapabilities should be removed
and replaced with a single shared exported constant to avoid duplication; export
the allowlist from the existing codemode-utils module (e.g., add and export a
descriptive name like EXECUTE_HELPER_CAPABILITIES in the module that currently
defines the original list) and import that constant into
infer-codemode-capabilities.ts, replacing the local executeHelperCapabilities
usage so both inference and runtime use the same source of truth.
🪄 Autofix (Beta)
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
Run ID: 18145c79-efd6-4d40-8325-e1a05ae2fece
📒 Files selected for processing (4)
packages/worker/src/mcp/execute-modules/codemode-utils.node.test.tspackages/worker/src/mcp/execute-modules/codemode-utils.tspackages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.tspackages/worker/src/mcp/skills/infer-codemode-capabilities.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/worker/src/mcp/execute-modules/codemode-utils.ts (2)
320-341: Prelude mirrors TypeScript implementation correctly.The
__kodyPersistSecretfunction and token persistence logic in__kodyRefreshAccessTokencorrectly replicate the TypeScript counterparts (persistSecretand lines 199-214).Note: This architecture creates duplicated logic between the TypeScript module and the string prelude. Consider adding a comment or test assertion to flag drift, or extracting shared logic patterns if this file grows. This is non-blocking given the current scope.
Also applies to: 434-447
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/execute-modules/codemode-utils.ts` around lines 320 - 341, The string prelude duplicates logic from the TypeScript module (persistSecret and the token logic in __kodyRefreshAccessToken); add a clear comment adjacent to __kodyPersistSecret (and similarly near __kodyRefreshAccessToken) stating that this mirrors the TypeScript implementations (persistSecret and lines ~199-214) and must be kept in sync, and add a TODO to either extract shared logic or add a test/assertion to detect drift; reference the functions by name (__kodyPersistSecret and __kodyRefreshAccessToken) so future maintainers know where the canonical logic lives and how to reduce duplication.
199-214: Token persistence logic is correct.The ordering (refresh token first, then access token) is appropriate for rotation scenarios where the old refresh token may be immediately invalidated by the provider.
One minor consideration: the
payload.refresh_tokencheck guards against empty strings but not whitespace-only values. If a provider somehow returns" "as a refresh token, it would be persisted. This is extremely unlikely in practice, but you could add.trim()to the length check for completeness.Optional: guard against whitespace-only tokens
-if (typeof payload.refresh_token === 'string' && payload.refresh_token.length > 0) { +if (typeof payload.refresh_token === 'string' && payload.refresh_token.trim().length > 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/execute-modules/codemode-utils.ts` around lines 199 - 214, The refresh token guard currently checks typeof payload.refresh_token === 'string' && payload.refresh_token.length > 0 but can allow whitespace-only values; update the condition in the block where persistSecret is called for refreshTokenSecretName to use payload.refresh_token.trim().length > 0 (keeping the typeof check) so only non-empty, non-whitespace refresh tokens are persisted; leave the access token persistence for connector.accessTokenSecretName unchanged and keep using persistSecret(codemode, providerName, refreshTokenSecretName, 'refresh token', payload.refresh_token) when the trimmed check passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/worker/src/mcp/execute-modules/codemode-utils.ts`:
- Around line 320-341: The string prelude duplicates logic from the TypeScript
module (persistSecret and the token logic in __kodyRefreshAccessToken); add a
clear comment adjacent to __kodyPersistSecret (and similarly near
__kodyRefreshAccessToken) stating that this mirrors the TypeScript
implementations (persistSecret and lines ~199-214) and must be kept in sync, and
add a TODO to either extract shared logic or add a test/assertion to detect
drift; reference the functions by name (__kodyPersistSecret and
__kodyRefreshAccessToken) so future maintainers know where the canonical logic
lives and how to reduce duplication.
- Around line 199-214: The refresh token guard currently checks typeof
payload.refresh_token === 'string' && payload.refresh_token.length > 0 but can
allow whitespace-only values; update the condition in the block where
persistSecret is called for refreshTokenSecretName to use
payload.refresh_token.trim().length > 0 (keeping the typeof check) so only
non-empty, non-whitespace refresh tokens are persisted; leave the access token
persistence for connector.accessTokenSecretName unchanged and keep using
persistSecret(codemode, providerName, refreshTokenSecretName, 'refresh token',
payload.refresh_token) when the trimmed check passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e85f0266-9221-438f-a696-4d9a2f0aa302
📒 Files selected for processing (3)
packages/worker/src/mcp/execute-modules/codemode-utils.node.test.tspackages/worker/src/mcp/execute-modules/codemode-utils.tspackages/worker/src/mcp/skills/infer-codemode-capabilities.ts
✅ Files skipped from review due to trivial changes (1)
- packages/worker/src/mcp/execute-modules/codemode-utils.node.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/worker/src/mcp/skills/infer-codemode-capabilities.ts
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
2 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.
Summary
codemode.secret_setafter connector token refreshesrefresh_token, in both the runtime helper and generated execute preludesecret_setin execute helper capability lists and align helper capability inference/tests with the new dependencyTesting
npm exec vitest run --project node-unit "packages/worker/src/mcp/execute-modules/codemode-utils.node.test.ts" "packages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.ts"Summary by CodeRabbit
New Features
Tests