feat(cloud-agent-next): MVP-3 session-prepare token selection + identity-aware author config + env-var gate#3201
Conversation
| @@ -0,0 +1,138 @@ | |||
| import type { NextRequest } from 'next/server'; | |||
| import { NextResponse } from 'next/server'; | |||
| import { getUserFromAuth } from '@/lib/user.server'; | |||
There was a problem hiding this comment.
WARNING: getUserFromAuth is imported but never used anywhere in this file. Remove the unused import.
| import { getUserFromAuth } from '@/lib/user.server'; |
There was a problem hiding this comment.
These unused imports (, ) are not present in the current branch — they appear to have been removed in a prior commit. The current file only imports from and does not import .
| import { eq } from 'drizzle-orm'; | ||
| import { USER_GH_APP_TOKEN_ENCRYPTION_KEY } from '@/lib/config.server'; | ||
| import { encryptWithSymmetricKey } from '@/lib/encryption'; | ||
| import { captureException, captureMessage } from '@sentry/nextjs'; |
There was a problem hiding this comment.
WARNING: captureMessage is imported from @sentry/nextjs but is never called. Only captureException is used. Remove the unused named import.
| import { captureException, captureMessage } from '@sentry/nextjs'; | |
| import { captureException } from '@sentry/nextjs'; |
There was a problem hiding this comment.
These unused imports are not present in the current branch. The file was already cleaned up in prior work.
| } | ||
|
|
||
| // App-token path (original behaviour). | ||
| if (metadata.githubInstallationId) { |
There was a problem hiding this comment.
WARNING: After the user-token fallback (lines 2461–2466 downgrade identity to 'app'), the app-token path here only fires when metadata.githubInstallationId is set. However, sessions prepared via the user-token path will have githubInstallationId === undefined — it is only populated when the app-installation path succeeds (see runPreparationAsync line 1136). After the downgrade, this condition is false and the function falls through to return metadata.githubToken (line 2475), which returns the original expired/revoked user token.
The mid-session fallback to the bot will silently hand a stale token to the execution, rather than a fresh installation token. Consider also resolving the app token via resolveGitHubTokenForRepo when githubInstallationId is absent, or storing installationId on the session at prepare time even when the user-token path wins.
There was a problem hiding this comment.
Fixed. When githubInstallationId is absent (user-token path session that has been downgraded to app identity), we now call resolveGitHubTokenForRepo to obtain a fresh installation token on demand, rather than falling through to the stale metadata.githubToken. Added in commit 9474ce2.
Code Review SummaryStatus: 1 Issue Remaining | Recommendation: Address before merge Overview
Resolved ✅The following issues from previous reviews have been fixed:
Remaining Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files ReviewedNew in this round (incremental diff):
Carried forward from previous reviews (unchanged):
Fix these issues in Kilo Cloud Reviewed by claude-sonnet-4.6 · 2,250,202 tokens |
There was a problem hiding this comment.
Review findings — changes required before merge:
1. getUserFromAuth unused import — apps/web/src/app/api/integrations/github/user-connect/callback/route.ts line 3
getUserFromAuth is imported but never called. The user identity comes from the OAuth state payload (decoded.kilo_user_id). Remove this import — it will cause a lint failure.
2. captureMessage unused import — apps/web/src/app/api/integrations/github/user-connect/callback/route.ts line 13
captureMessage is imported alongside captureException but is never called anywhere in the file. Remove it to fix the lint error.
3. Missing eq import — apps/web/src/routers/github-apps-router.test.ts line 114
The disconnectUserIdentity test calls .where(eq(user_github_app_tokens.kilo_user_id, 'user-123')) but eq is never imported. The import block only brings in { beforeEach, describe, expect, test }, { db, cleanupDbForTest }, { githubAppsRouter }, { user_github_app_tokens }, and { encryptWithSymmetricKey }. Add import { eq } from 'drizzle-orm'; — without it the test throws ReferenceError: eq is not defined at runtime.
4. as cast on caught error — services/git-token-service/src/user-github-token-service.ts line 118
const status = (error as { status?: number }).status;This is a cast on a caught error from Octokit — untrusted external data. The coding standards explicitly prohibit this: "Avoid broad casts that hide real uncertainty, especially on external/untrusted data". Replace with a type guard:
function getOctokitStatus(e: unknown): number | undefined {
return e != null &&
typeof e === 'object' &&
'status' in e &&
typeof (e as Record<string, unknown>).status === 'number'
? (e as { status: number }).status
: undefined;
}
// ...
const status = getOctokitStatus(error);The rest of the implementation looks correct: the GitIdentity discriminated union, buildGitAuthor, token-selection gate with silent fallback, DO state persistence of identityKind/identityKiloUserId, refreshGitHubToken mid-session helper, and env-var gating are all well-structured and consistent with the spec.
…t author, env-var gate - Add GitIdentity discriminated union (app | user) in types/git-identity.ts - Add buildGitAuthor() to workspace.ts for identity-aware git user.name/email - Update cloneGitHubRepo() to accept GitIdentity or legacy env-obj - Gate user-token preference behind ENABLE_GITHUB_USER_TOKENS env var - session-prepare.ts: try getUserTokenForRepo first when gate=true, fall back to app token - async-preparation.ts: same token-selection logic for autoInitiate (async) path - Persist identityKind + identityKiloUserId in DO session state (schemas, types, prepare()) - CloudAgentSession.refreshGitHubToken: mid-session user-token renewal; on failure falls back to app installation token, emits one-time warning, downgrades persisted identityKind - Both startExecutionV2 token paths (initiatePrepared + followup) use refreshGitHubToken - Add ENABLE_GITHUB_USER_TOKENS to wrangler.jsonc (default false) and .dev.vars.example - Add getUserTokenForRepo to GitTokenService type in types.ts - Tests: buildGitAuthor snapshots, token-selection with gate on/off, fallback cases
1b2ee01 to
2d3377a
Compare
|
Rebased onto current convoy head; dropped the duplicate MVP-2 commits and the WIP eviction-save commit. The squash-merge of #3203 means MVP-2 changes are already present on the base. |
- Replace as-cast on caught Octokit error with type guard in user-github-token-service - Fix mid-session app-token fallback when githubInstallationId is absent (sessions prepared via user-token path don't store installationId; after identity downgrade to 'app', resolve a fresh installation token via resolveGitHubTokenForRepo)
7afb8d9
into
convoy/mvp-commit-as-user-via-github-app-user-t/db234b74/head
…ity-aware author config + env-var gate (#3201) * feat(cloud-agent-next): MVP-3 user-token selection, identity-aware git author, env-var gate - Add GitIdentity discriminated union (app | user) in types/git-identity.ts - Add buildGitAuthor() to workspace.ts for identity-aware git user.name/email - Update cloneGitHubRepo() to accept GitIdentity or legacy env-obj - Gate user-token preference behind ENABLE_GITHUB_USER_TOKENS env var - session-prepare.ts: try getUserTokenForRepo first when gate=true, fall back to app token - async-preparation.ts: same token-selection logic for autoInitiate (async) path - Persist identityKind + identityKiloUserId in DO session state (schemas, types, prepare()) - CloudAgentSession.refreshGitHubToken: mid-session user-token renewal; on failure falls back to app installation token, emits one-time warning, downgrades persisted identityKind - Both startExecutionV2 token paths (initiatePrepared + followup) use refreshGitHubToken - Add ENABLE_GITHUB_USER_TOKENS to wrangler.jsonc (default false) and .dev.vars.example - Add getUserTokenForRepo to GitTokenService type in types.ts - Tests: buildGitAuthor snapshots, token-selection with gate on/off, fallback cases * fix(cloud-agent-next): address PR review feedback - Replace as-cast on caught Octokit error with type guard in user-github-token-service - Fix mid-session app-token fallback when githubInstallationId is absent (sessions prepared via user-token path don't store installationId; after identity downgrade to 'app', resolve a fresh installation token via resolveGitHubTokenForRepo) --------- Co-authored-by: Toast (gastown) <Toast@gastown.local> Co-authored-by: Maple (gastown) <Maple@gastown.local>
…ity-aware author config + env-var gate (#3201) * feat(cloud-agent-next): MVP-3 user-token selection, identity-aware git author, env-var gate - Add GitIdentity discriminated union (app | user) in types/git-identity.ts - Add buildGitAuthor() to workspace.ts for identity-aware git user.name/email - Update cloneGitHubRepo() to accept GitIdentity or legacy env-obj - Gate user-token preference behind ENABLE_GITHUB_USER_TOKENS env var - session-prepare.ts: try getUserTokenForRepo first when gate=true, fall back to app token - async-preparation.ts: same token-selection logic for autoInitiate (async) path - Persist identityKind + identityKiloUserId in DO session state (schemas, types, prepare()) - CloudAgentSession.refreshGitHubToken: mid-session user-token renewal; on failure falls back to app installation token, emits one-time warning, downgrades persisted identityKind - Both startExecutionV2 token paths (initiatePrepared + followup) use refreshGitHubToken - Add ENABLE_GITHUB_USER_TOKENS to wrangler.jsonc (default false) and .dev.vars.example - Add getUserTokenForRepo to GitTokenService type in types.ts - Tests: buildGitAuthor snapshots, token-selection with gate on/off, fallback cases * fix(cloud-agent-next): address PR review feedback - Replace as-cast on caught Octokit error with type guard in user-github-token-service - Fix mid-session app-token fallback when githubInstallationId is absent (sessions prepared via user-token path don't store installationId; after identity downgrade to 'app', resolve a fresh installation token via resolveGitHubTokenForRepo) --------- Co-authored-by: Toast (gastown) <Toast@gastown.local> Co-authored-by: Maple (gastown) <Maple@gastown.local>
…ity-aware author config + env-var gate (#3201) * feat(cloud-agent-next): MVP-3 user-token selection, identity-aware git author, env-var gate - Add GitIdentity discriminated union (app | user) in types/git-identity.ts - Add buildGitAuthor() to workspace.ts for identity-aware git user.name/email - Update cloneGitHubRepo() to accept GitIdentity or legacy env-obj - Gate user-token preference behind ENABLE_GITHUB_USER_TOKENS env var - session-prepare.ts: try getUserTokenForRepo first when gate=true, fall back to app token - async-preparation.ts: same token-selection logic for autoInitiate (async) path - Persist identityKind + identityKiloUserId in DO session state (schemas, types, prepare()) - CloudAgentSession.refreshGitHubToken: mid-session user-token renewal; on failure falls back to app installation token, emits one-time warning, downgrades persisted identityKind - Both startExecutionV2 token paths (initiatePrepared + followup) use refreshGitHubToken - Add ENABLE_GITHUB_USER_TOKENS to wrangler.jsonc (default false) and .dev.vars.example - Add getUserTokenForRepo to GitTokenService type in types.ts - Tests: buildGitAuthor snapshots, token-selection with gate on/off, fallback cases * fix(cloud-agent-next): address PR review feedback - Replace as-cast on caught Octokit error with type guard in user-github-token-service - Fix mid-session app-token fallback when githubInstallationId is absent (sessions prepared via user-token path don't store installationId; after identity downgrade to 'app', resolve a fresh installation token via resolveGitHubTokenForRepo) --------- Co-authored-by: Toast (gastown) <Toast@gastown.local> Co-authored-by: Maple (gastown) <Maple@gastown.local>
…ity-aware author config + env-var gate (#3201) * feat(cloud-agent-next): MVP-3 user-token selection, identity-aware git author, env-var gate - Add GitIdentity discriminated union (app | user) in types/git-identity.ts - Add buildGitAuthor() to workspace.ts for identity-aware git user.name/email - Update cloneGitHubRepo() to accept GitIdentity or legacy env-obj - Gate user-token preference behind ENABLE_GITHUB_USER_TOKENS env var - session-prepare.ts: try getUserTokenForRepo first when gate=true, fall back to app token - async-preparation.ts: same token-selection logic for autoInitiate (async) path - Persist identityKind + identityKiloUserId in DO session state (schemas, types, prepare()) - CloudAgentSession.refreshGitHubToken: mid-session user-token renewal; on failure falls back to app installation token, emits one-time warning, downgrades persisted identityKind - Both startExecutionV2 token paths (initiatePrepared + followup) use refreshGitHubToken - Add ENABLE_GITHUB_USER_TOKENS to wrangler.jsonc (default false) and .dev.vars.example - Add getUserTokenForRepo to GitTokenService type in types.ts - Tests: buildGitAuthor snapshots, token-selection with gate on/off, fallback cases * fix(cloud-agent-next): address PR review feedback - Replace as-cast on caught Octokit error with type guard in user-github-token-service - Fix mid-session app-token fallback when githubInstallationId is absent (sessions prepared via user-token path don't store installationId; after identity downgrade to 'app', resolve a fresh installation token via resolveGitHubTokenForRepo) --------- Co-authored-by: Toast (gastown) <Toast@gastown.local> Co-authored-by: Maple (gastown) <Maple@gastown.local>
…ity-aware author config + env-var gate (#3201) * feat(cloud-agent-next): MVP-3 user-token selection, identity-aware git author, env-var gate - Add GitIdentity discriminated union (app | user) in types/git-identity.ts - Add buildGitAuthor() to workspace.ts for identity-aware git user.name/email - Update cloneGitHubRepo() to accept GitIdentity or legacy env-obj - Gate user-token preference behind ENABLE_GITHUB_USER_TOKENS env var - session-prepare.ts: try getUserTokenForRepo first when gate=true, fall back to app token - async-preparation.ts: same token-selection logic for autoInitiate (async) path - Persist identityKind + identityKiloUserId in DO session state (schemas, types, prepare()) - CloudAgentSession.refreshGitHubToken: mid-session user-token renewal; on failure falls back to app installation token, emits one-time warning, downgrades persisted identityKind - Both startExecutionV2 token paths (initiatePrepared + followup) use refreshGitHubToken - Add ENABLE_GITHUB_USER_TOKENS to wrangler.jsonc (default false) and .dev.vars.example - Add getUserTokenForRepo to GitTokenService type in types.ts - Tests: buildGitAuthor snapshots, token-selection with gate on/off, fallback cases * fix(cloud-agent-next): address PR review feedback - Replace as-cast on caught Octokit error with type guard in user-github-token-service - Fix mid-session app-token fallback when githubInstallationId is absent (sessions prepared via user-token path don't store installationId; after identity downgrade to 'app', resolve a fresh installation token via resolveGitHubTokenForRepo) --------- Co-authored-by: Toast (gastown) <Toast@gastown.local> Co-authored-by: Maple (gastown) <Maple@gastown.local>
…ity-aware author config + env-var gate (#3201) * feat(cloud-agent-next): MVP-3 user-token selection, identity-aware git author, env-var gate - Add GitIdentity discriminated union (app | user) in types/git-identity.ts - Add buildGitAuthor() to workspace.ts for identity-aware git user.name/email - Update cloneGitHubRepo() to accept GitIdentity or legacy env-obj - Gate user-token preference behind ENABLE_GITHUB_USER_TOKENS env var - session-prepare.ts: try getUserTokenForRepo first when gate=true, fall back to app token - async-preparation.ts: same token-selection logic for autoInitiate (async) path - Persist identityKind + identityKiloUserId in DO session state (schemas, types, prepare()) - CloudAgentSession.refreshGitHubToken: mid-session user-token renewal; on failure falls back to app installation token, emits one-time warning, downgrades persisted identityKind - Both startExecutionV2 token paths (initiatePrepared + followup) use refreshGitHubToken - Add ENABLE_GITHUB_USER_TOKENS to wrangler.jsonc (default false) and .dev.vars.example - Add getUserTokenForRepo to GitTokenService type in types.ts - Tests: buildGitAuthor snapshots, token-selection with gate on/off, fallback cases * fix(cloud-agent-next): address PR review feedback - Replace as-cast on caught Octokit error with type guard in user-github-token-service - Fix mid-session app-token fallback when githubInstallationId is absent (sessions prepared via user-token path don't store installationId; after identity downgrade to 'app', resolve a fresh installation token via resolveGitHubTokenForRepo) --------- Co-authored-by: Toast (gastown) <Toast@gastown.local> Co-authored-by: Maple (gastown) <Maple@gastown.local>
…ity-aware author config + env-var gate (#3201) * feat(cloud-agent-next): MVP-3 user-token selection, identity-aware git author, env-var gate - Add GitIdentity discriminated union (app | user) in types/git-identity.ts - Add buildGitAuthor() to workspace.ts for identity-aware git user.name/email - Update cloneGitHubRepo() to accept GitIdentity or legacy env-obj - Gate user-token preference behind ENABLE_GITHUB_USER_TOKENS env var - session-prepare.ts: try getUserTokenForRepo first when gate=true, fall back to app token - async-preparation.ts: same token-selection logic for autoInitiate (async) path - Persist identityKind + identityKiloUserId in DO session state (schemas, types, prepare()) - CloudAgentSession.refreshGitHubToken: mid-session user-token renewal; on failure falls back to app installation token, emits one-time warning, downgrades persisted identityKind - Both startExecutionV2 token paths (initiatePrepared + followup) use refreshGitHubToken - Add ENABLE_GITHUB_USER_TOKENS to wrangler.jsonc (default false) and .dev.vars.example - Add getUserTokenForRepo to GitTokenService type in types.ts - Tests: buildGitAuthor snapshots, token-selection with gate on/off, fallback cases * fix(cloud-agent-next): address PR review feedback - Replace as-cast on caught Octokit error with type guard in user-github-token-service - Fix mid-session app-token fallback when githubInstallationId is absent (sessions prepared via user-token path don't store installationId; after identity downgrade to 'app', resolve a fresh installation token via resolveGitHubTokenForRepo) --------- Co-authored-by: Toast (gastown) <Toast@gastown.local> Co-authored-by: Maple (gastown) <Maple@gastown.local>
Summary
services/cloud-agent-next/src/types/git-identity.tswithapp | userdiscriminated union.session-prepare.tsandasync-preparation.tsattemptgetUserTokenForRepofirst whenENABLE_GITHUB_USER_TOKENS=true; silently fall back to the app installation token on any failure.buildGitAuthor(identity)inworkspace.tsderivesuser.name/user.emailfrom the resolved identity.cloneGitHubRepoaccepts either aGitIdentity(new) or the legacy env-var object (backward compat).identityKind(app | user) andidentityKiloUserIdstored in session metadata andPreparationInputso subsequent renewals know which token path to use.CloudAgentSession.refreshGitHubTokenreplaces the two rawGIT_TOKEN_SERVICE.getTokencalls instartExecutionV2. Forkind=user, it callsgetUserTokenForRepo; onok:false, falls back to the installation token, emits a one-timestatuswarning to the user ("GitHub user token expired — pushing as the bot for the rest of this session..."), and downgradesidentityKindto'app'in DO storage.ENABLE_GITHUB_USER_TOKENS=falseadded towrangler.jsonc(prod + dev) and.dev.vars.example. Default off — byte-identical to pre-MVP-3 when unset.convoy/.../gt/toast/4b8b7e18):getUserTokenForRepoRPC, OAuth callback, settings UI,user_github_app_tokenstable integration.Verification
ENABLE_GITHUB_USER_TOKENS=false(default): run a session against a GitHub repo. Commits showkiloconnect[bot]. No calls togetUserTokenForRepo.ENABLE_GITHUB_USER_TOKENS=true, user not connected: silent fallback to bot identity, session works.ENABLE_GITHUB_USER_TOKENS=true, user connected: commits show user's GitHub login + verified email.access_token_expires_atin past): session prepare falls back to app token.getUserTokenForReporeturningrevoked): warning emitted, bot token used, session continues.Visual Changes
N/A — no UI changes in this PR.
Reviewer Notes
identityKindis downgraded to'app', subsequent renewals stay on the app-token path for that session.statusstream event (not persisted, volatile only).cloneGitHubReposignature change is backward-compatible: the second overload accepts the legacy env-var object, detected by'kind' in identityOrEnv.test/unit/wrapper/lifecycle.test.tsfailures are unrelated to this PR.