feat(github): MVP commit-as-user via GitHub App user-to-server tokens

[kilo-code-bot]

Summary

• Adds user_github_app_tokens table to store encrypted GitHub user-to-server (OAuth) tokens, with GDPR soft-delete coverage.
• Implements the full OAuth connect/disconnect flow: HMAC-signed state with TTL, code exchange, identity resolution, encrypted storage, and a settings UI card gated behind ENABLE_GITHUB_USER_TOKENS.
• Adds getUserTokenForRepo RPC to git-token-service that decrypts the stored token, verifies repo access (with KV cache), and returns the token + user identity.
• Updates session-prepare (both in the router handler and async-preparation) to prefer the user token when the feature gate is enabled, falling back to the app installation token transparently.
• Introduces GitIdentity type and buildGitAuthor helper for identity-aware git commit attribution (user vs. app-bot).
• Adds refreshGitHubToken method on CloudAgentSession that handles mid-session user-token expiry with a graceful downgrade to the app token and a one-time user-visible warning.
• Adds redactGitHubTokens utility to worker-utils to prevent ghu_/ghr_ tokens from appearing in logs.

Verification

• Connect a GitHub identity via the settings UI → verify commits are attributed to your GitHub account.
• Revoke the token on GitHub and trigger a session → verify fallback to app token with warning message.
• Delete your Kilo account → verify user_github_app_tokens rows are deleted.
• Set ENABLE_GITHUB_USER_TOKENS=false (default) → verify no change in existing app-token behaviour.

Visual Changes

N/A (new card added to GitHub integrations settings page, gated behind feature flag).

Reviewer Notes

• Feature is off by default (ENABLE_GITHUB_USER_TOKENS=false in wrangler.jsonc and .dev.vars.example). Safe to merge without enabling.
• Refresh tokens are ignored in this MVP (comment in callback notes this explicitly); token expiry falls back to app token.
• disconnectUserIdentity calls GitHub's DELETE /applications/{client_id}/grant to revoke the authorization server-side; failures are logged but do not block the local row deletion.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental Update (commits since d8c7da9)

New commits since last review:

Commit	Change
e393c62	chore(db): renumber user_github_app_tokens migration to 0131 after rebase conflict with 0130 — PR 3209 migration renumbered
d24765ad	WIP: container eviction save — empty/unrelated commit
5d3c954	perf(kiloclaw): index-backed first-instance lookup for setup promo (#3260) — merged PR, out of scope
409ee51	fix(deploy): restore blocked production deployments (#3259) — merged PR, out of scope

PR 3209 files changed since d8c7da9:

File	Change
packages/db/src/migrations/0131_fast_jamie_braddock.sql	Renumbered from 0130_cloudy_mandrill → 0131_fast_jamie_braddock (SQL content unchanged)
packages/db/src/migrations/meta/_journal.json	Journal updated: new 0130_kiloclaw_instances_user_id_created_at_idx (#3260) inserted before PR 3209's migration; PR 3209's entry correctly becomes idx: 131
packages/db/src/migrations/meta/0131_snapshot.json	Regenerated snapshot (generated file, skipped)

Migration ordering: 0131_fast_jamie_braddock correctly follows 0130_kiloclaw_instances_user_id_created_at_idx. Journal and snapshot files are correctly regenerated.

All other PR 3209 files are unchanged since the previous review.

Previous Review Findings (all resolved)

All issues from prior reviews remain resolved:

Issue	Status
errorDescription log injection in route.ts	✅ Fixed — sanitized before logging
Double as cast in user-github-token-service.ts	✅ Fixed — reduced to single targeted cast
as { status: number } cast on line 11	✅ Acknowledged — single cast at known boundary, acceptable per coding standards
getUserConnectionStatus not gated	✅ Intentional — documented: gate only blocks new connections
Empty alt on avatar image	✅ Acknowledged as follow-up

Other Observations (not in diff)

• Migration ordering: 0131_fast_jamie_braddock.sql correctly follows 0130_kiloclaw_instances_user_id_created_at_idx.sql. Journal and snapshot files are all regenerated correctly.
• route.ts OAuth state/error ordering: Intentional and correct — state decoded first for a safe returnTo path.
• UserGitHubTokenService DB connection caching: Appropriate for the Cloudflare Worker lifecycle. No issue.
• Revocation error logging: console.error logs GitHub API response body, not the token itself. No issue.
• Feature-gate bypass in connectUserIdentity: tRPC gate + UI prop gating provides adequate defence-in-depth.
• identityKiloUserId in async-preparation path: Safe — arrives via internal API protected by internalApiProtectedProcedure. Forwarded in the autoInitiate fast-path.
• redactGitHubTokens in worker-utils: Added but not yet wired into all error-path logging. Low risk; worth tracking post-merge.
• sanitizeForLog lint suppression: The eslint-disable-next-line no-control-regex is appropriate — the regex is intentional.
• Remaining as { status: unknown } cast in user-github-token-service.ts:9: Single targeted cast after 'status' in e narrowing; acceptable per coding standards.

Files Reviewed (41 files)

• apps/web/.env.development.local.example
• apps/web/src/app/(app)/integrations/github/page.tsx
• apps/web/src/app/api/integrations/github/user-connect/callback/route.test.ts — ✅ test fixes applied
• apps/web/src/app/api/integrations/github/user-connect/callback/route.ts — ✅ all issues resolved
• apps/web/src/components/integrations/GitHubIntegrationDetails.tsx — suggestion acknowledged/deferred
• apps/web/src/lib/config.server.ts
• apps/web/src/lib/integrations/platforms/github/oauth-state.test.ts — ✅ unused import removed
• apps/web/src/lib/integrations/platforms/github/oauth-state.ts
• apps/web/src/lib/user.test.ts
• apps/web/src/lib/user.ts
• apps/web/src/routers/github-apps-router.test.ts — ✅ as any replaced, user inserted before FK
• apps/web/src/routers/github-apps-router.ts — ✅ unused imports removed; suggestion acknowledged as intentional
• packages/db/src/migrations/0131_fast_jamie_braddock.sql — ✅ renumbered to 0131 (was 0130 twice), SQL unchanged
• packages/db/src/migrations/meta/0131_snapshot.json — skipped (generated)
• packages/db/src/migrations/meta/_journal.json — skipped (generated)
• packages/db/src/schema.ts
• packages/worker-utils/src/index.ts
• packages/worker-utils/src/redact-headers.test.ts
• packages/worker-utils/src/redact-headers.ts
• services/cloud-agent-next/.dev.vars.example
• services/cloud-agent-next/src/git-identity.test.ts
• services/cloud-agent-next/src/persistence/CloudAgentSession.ts
• services/cloud-agent-next/src/persistence/async-preparation.test.ts
• services/cloud-agent-next/src/persistence/async-preparation.ts
• services/cloud-agent-next/src/persistence/schemas.ts
• services/cloud-agent-next/src/persistence/types.ts
• services/cloud-agent-next/src/router/handlers/session-prepare.ts — ✅ identityKind/identityKiloUserId forwarded in autoInitiate path
• services/cloud-agent-next/src/services/git-token-service-client.ts
• services/cloud-agent-next/src/session-prepare.test.ts — ✅ type cleanup: MockDOProcedure alias
• services/cloud-agent-next/src/types.ts
• services/cloud-agent-next/src/types/git-identity.ts
• services/cloud-agent-next/src/workspace.ts
• services/cloud-agent-next/wrangler.jsonc
• services/git-token-service/.dev.vars.example
• services/git-token-service/package.json
• services/git-token-service/src/index.ts — ✅ explicit named type imports
• services/git-token-service/src/user-github-token-service.ts — ✅ cast reduced to targeted single cast
• services/git-token-service/worker-configuration.d.ts
• services/git-token-service/wrangler.jsonc
• pnpm-lock.yaml — skipped (generated)

[kilo-code-bot]

Code Review

Bug (must fix): startPreparationAsync is missing identityKiloUserId and identityKind

In services/cloud-agent-next/src/router/handlers/session-prepare.ts, the startPreparationAsync call (the async fast-path when autoInitiate=true) does not forward identityKiloUserId or identityKind to the Durable Object's PreparationInput.

The user-token path inside executePreparationSteps (in async-preparation.ts) is guarded by:

if (env.ENABLE_GITHUB_USER_TOKENS === 'true' && input.identityKiloUserId) {

Because input.identityKiloUserId is undefined on the async path, user-token resolution is silently skipped and every session prepared via autoInitiate=true falls back to the app installation token — which is the typical production path. The feature is effectively broken on the async path even when ENABLE_GITHUB_USER_TOKENS=true.

Fix: Add identityKiloUserId: ctx.userId (and optionally identityKind: 'user') to the startPreparationAsync call block, mirroring what the sync prepare() call at line ~691 does. Since async-preparation.ts gates on both the env var and the field being present, passing userId unconditionally is safe — it will only be used when ENABLE_GITHUB_USER_TOKENS === 'true'.

---

Style (non-blocking): double as-cast in getOctokitStatus

In services/git-token-service/src/user-github-token-service.ts, getOctokitStatus uses two as-casts:\nts\nif (... && typeof (e as Record<string, unknown>).status === 'number') {\n return (e as { status: number }).status;\n}\n\n\nThis can be simplified to one cast with runtime validation:\nts\nfunction getOctokitStatus(e: unknown): number | undefined {\n if (e != null && typeof e === 'object' && 'status' in e) {\n const { status } = e as { status?: unknown };\n if (typeof status === 'number') return status;\n }\n return undefined;\n}\n

[kilo-code-bot]

One correctness bug must be fixed before merge (see inline comment above).

Bug: startPreparationAsync in session-prepare.ts does not pass identityKiloUserId to the Durable Object's PreparationInput. The user-token path in executePreparationSteps (async-preparation.ts) is gated on input.identityKiloUserId being set, so user-token resolution is silently skipped on every autoInitiate=true session (the normal production fast path). All sessions on the async path fall back to the app bot token even when ENABLE_GITHUB_USER_TOKENS=true.

Fix: Add identityKiloUserId: ctx.userId to the startPreparationAsync call block in session-prepare.ts (mirroring what the sync prepare() call already does at line ~691).

A secondary non-blocking style issue: the double as-cast in getOctokitStatus (user-github-token-service.ts) should use flow-sensitive typing with a single narrowing cast instead.

[kilo-code-bot]

Fixed both issues:

1. ** missing identityKiloUserId/identityKind**: Added both fields to the startPreparationAsync call in session-prepare.ts, forwarding the already-resolved resolvedGitIdentity (which is resolved before the autoInitiate branch). This mirrors what the sync prepare() call already does.

2. Test encryption key (35 bytes): The test fixtures in github-apps-router.test.ts and route.test.ts used a base64 key that decoded to 35 bytes, causing EncryptionConfigurationError: Encryption key must be exactly 32 bytes (256 bits). Replaced with a valid 32-byte key.

[kilo-code-bot]

Refinery code review passed. All quality gates pass.

Summary: The MVP commit-as-user implementation is well-structured and secure. The OAuth state is properly HMAC-signed with TTL, tokens are encrypted at rest with a dedicated key, the GDPR soft-delete is covered, and the feature is safely gated behind ENABLE_GITHUB_USER_TOKENS=false by default.

Minor observations (no changes required):

1. expired_state in the GitHubConnectError union type in callback/route.ts is dead code — it is never emitted. Expiry is currently surfaced as invalid_state (via verifyOAuthState returning null). Fine to clean up in a follow-up if desired.

2. The KV repo-access cache key (gh-user-repo-access:{userId}:{owner}:{repo}) does not include the GitHub user ID. If a user disconnects and reconnects with a different GitHub account, a stale negative-access entry could block the new account for up to 5 minutes. This is a known MVP trade-off and is acceptable at this stage.

Both are minor and do not affect correctness or security. Safe to merge.

[kilo-code-bot]

Refinery code review passed. All quality gates pass.

[kilo-code-bot]

Refinery code review passed. All quality gates pass.

Reviewed the full diff across all 40 changed files. The implementation is correct and well-structured:

Schema & Migration: user_github_app_tokens table is properly defined with FK cascade, unique index on (kilo_user_id, github_app_type), and the migration was generated by drizzle-kit (not hand-written). Migration number conflict was correctly resolved (0131).

Security: HMAC-SHA256 signed OAuth state with 10-minute TTL and timing-safe comparison; AES-256-GCM encrypted token storage; safeReturnTo correctly blocks open-redirect (protocol-relative, absolute URLs, javascript:); log injection mitigation via sanitizeForLog with eslint suppression comment; no tokens logged; redactGitHubTokens utility correctly covers ghu_/ghr_ patterns.

GDPR: softDeleteUser updated to delete user_github_app_tokens rows with test coverage in user.test.ts.

Feature gate: ENABLE_GITHUB_USER_TOKENS=false by default in wrangler.jsonc and .dev.vars.example — safe to merge without enabling.

Fallback logic: Token selection in both session-prepare.ts (sync path) and async-preparation.ts correctly tries user token first when gate is on, falls back transparently to app installation token. Mid-session expiry handled in refreshGitHubToken with one-time warning broadcast and identity downgrade.

Worker module-scope safety: UserGitHubTokenService correctly uses getWorkerDb per-call (not module scope), db is instance field guarded by lazy init — compliant with Workers DO rules.

Test coverage: Route callback, oauth-state, github-apps-router, session-prepare (gate on/off, fallback), git-identity builder, and async-preparation all have meaningful tests. Mocks are used only where truly necessary (external OAuth and Octokit APIs).

WIP commit (d24765ad): empty diff — just a checkpoint marker, no code impact.

One minor style note (non-blocking): the blank line between the import block and type declaration was removed in GitHubIntegrationDetails.tsx (import { useOrganizationWithMembers } from ... → type GitHubIntegrationDetailsProps). Not a correctness issue.

[kilo-code-bot]

Refinery Code Review

Overall the implementation is solid — schema, GDPR deletion, HMAC-signed state, AES-256-GCM encryption, feature gate, and tests are all well done. Three issues need addressing before merge.

---

🔴 Issue 1: GITHUB_OAUTH_STATE_SECRET missing from .env.development.local.example

File: apps/web/.env.development.local.example

GITHUB_OAUTH_STATE_SECRET is a new required server env var — it's read via getEnvVariable in config.server.ts, which throws at boot if absent. But only USER_GH_APP_TOKEN_ENCRYPTION_KEY was added to the example file. Any developer pulling this branch will get an immediate startup crash until they manually discover and add the missing var.

Fix: Add the missing entry alongside USER_GH_APP_TOKEN_ENCRYPTION_KEY:

# HMAC secret for signing GitHub OAuth user-connect state tokens (random 32+ bytes, base64)
GITHUB_OAUTH_STATE_SECRET=

---

🟡 Issue 2: Raw response.text() logged in disconnectUserIdentity

File: apps/web/src/routers/github-apps-router.ts (inside disconnectUserIdentity)

console.error('Failed to revoke GitHub authorization:', {
  status: response.status,
  body: await response.text(),   // ← problem
});

The GitHub API error response body for this endpoint (DELETE /applications/{client_id}/grant) can include OAuth error strings and potentially token fragments. Per the project's logging rules, token-bearing values must not appear in logs.

Fix: Drop the body or sanitize with redactGitHubTokens from @kilocode/worker-utils/redact-headers:

console.error('Failed to revoke GitHub authorization:', {
  status: response.status,
});

---

🟡 Issue 3: process.env.ENABLE_GITHUB_USER_TOKENS read directly in page.tsx instead of from config.server

File: apps/web/src/app/(app)/integrations/github/page.tsx

enableUserTokens={process.env.ENABLE_GITHUB_USER_TOKENS === 'true'}

ENABLE_GITHUB_USER_TOKENS was centrally exported from config.server.ts (as a boolean) but the page reads the raw process.env instead. This is a server component so the import is safe, and using the central export avoids the inconsistency.

Fix:

import { ENABLE_GITHUB_USER_TOKENS } from '@/lib/config.server';
// ...
enableUserTokens={ENABLE_GITHUB_USER_TOKENS}

[kilo-code-bot]

Refinery code review passed. All quality gates pass.

Reviewed the full diff across all 39 changed files. The implementation is well-structured:

• DB schema: table is correctly defined with cascade FK, unique index on , and proper enums. Migration was generated correctly and renumbered to 0131 after rebase.
• GDPR: correctly deletes token rows; test coverage added in .
• Security: OAuth state uses HMAC-SHA256 with nonce + TTL (10 min). guards open-redirect. Tokens are AES-256-GCM encrypted at rest with a dedicated key separate from other encryption keys. Log injection is prevented in the callback route. utility added for / patterns.
• Feature gate: by default; safe to merge without enabling.
• Token service: in follows the same DB-caching pattern as the existing (instance-level, not module-scope). KV cache for repo-access checks with 5-min TTL.
• Session flow: Token selection in both and tries user token first (when gate=true), falls back to app installation token. Mid-session fallback in downgrades identity to 'app' and emits one-time user warning.
• Tests: Comprehensive coverage for happy paths, error cases, state expiry, conflict upserts, and GDPR delete.

No issues found.

[kilo-code-bot]

Refinery code review passed. All quality gates pass.

All previously identified issues from prior review rounds have been resolved:

• Fixed: startPreparationAsync in session-prepare.ts now correctly forwards identityKind and identityKiloUserId to the Durable Object's PreparationInput (lines 433–434), ensuring user-token resolution works on the autoInitiate=true async fast path.
• Fixed: getOctokitStatus in user-github-token-service.ts uses a single targeted as { status: unknown } cast after the 'status' in e guard — no more double cast through Record<string, unknown>.
• Fixed: error_description log injection risk addressed via sanitizeForLog() helper that truncates and strips control characters.
• Documented: getUserConnectionStatus being ungated is intentional design (cleanup path still works when feature is off).

Implementation is correct and complete across all three MVPs.