Skip to content

fix(dev): start the git-token worker and seed the credential envelope keypairs - #8576

Merged
iscekic merged 2 commits into
mainfrom
drain/dev-review-token-service-wiring
Oct 22, 2026
Merged

iscekic merged 2 commits into
mainfrom
drain/dev-review-token-service-wiring

Conversation

@iscekic

@iscekic iscekic commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

What changed

The PR review inbox listed nothing and showed the generic load error whenever the local web stack was started the documented way, even with a connected GitHub account. Two dev-stack gaps produced that, and both are fixed here.

  1. The web stack never started git-token-service. nextjs had no dependency on it, and the group is not always-on, so pnpm dev:start and pnpm dev:start core mobile left port 8802 dead. Every code-review read and write resolves the caller's provider credential through that Worker (apps/web/src/lib/integrations/platforms/{github,gitlab,bitbucket}/, apps/web/src/lib/provider-review/), so githubPrReview.listInbox answered BAD_GATEWAY — "GitHub authorization is temporarily unavailable" — while githubApps.getUserAuthorization still reported connected: true from its own database read. nextjs now depends on cloudflare-git-token-service, with the same reason comment the session-ingest dependency carries.
  2. A fresh pnpm dev:setup-env left the credential envelopes unconfigured. USER_GITHUB_APP_TOKEN_ACTIVE_KEY_ID/_PUBLIC_KEY stayed empty and no private half existed, so even a running Worker answered POST /internal/github-user-authorizations/token with 503 ("Active GitHub user-token private key is not configured"), and the dev GitHub token seed failed with 500 ("GitHub user token envelope encryption is not configured"). dev:setup-env now generates a local 4096-bit RSA keypair for each unconfigured envelope — the GitHub user-token one and the Bitbucket/GitLab platform-credential one — and writes the key ID plus the base64 SPKI public and base64 PKCS#8 private halves into .env.local; the existing passthrough copies the private halves into the Worker's .dev.vars, and the local Secrets Store picks up the Bitbucket key ID. Existing values are kept: the lookup reads .env.local first and the Worker's .dev.vars second and skips empty slots, so a rerun neither rotates a keypair the Worker already stored credentials under nor drops a half that lives only in the Worker file, and every status records the pair it decided on so the file rebuild cannot erase it.

Supporting changes: .env.local.example documents the two dev-only private-key slots and that setup generates the pairs, and DEVELOPMENT.md states that the dev stack starts the token Worker and what setup provisions.

The mobile side needed no change: classifyPrReviewQueryState already maps FORBIDDEN to the permission state, NOT_FOUND to not-found, PRECONDITION_FAILED/UNAUTHORIZED to reconnect and BAD_GATEWAY to the localized service-unavailable state, and the inbox renders the empty state when a provider answers with no rows.

End-to-end evidence

Environment: this work VM, local dev stack on localhost, Android emulator omp36 with the cached mobile-native-android-e6da57a618c0776d1d5e04375263c56c9991422c dev client, Metro on 8081, and the hermetic GitHub API stub (apps/mobile/e2e/github-stub.sh) seeded with an owned fake account. No production data, no real provider credentials.

  1. Deleted .env.local and ran CI=1 pnpm dev:setup-env --ci: it reported ✓ Generated github-user-token-local-v1 and ✓ Generated platform-credential-local-v1 and wrote the file.
  2. Ran pnpm dev:start --no-attach with no extra targets: the stack came up with cloudflare-git-token-service :8802 up in pnpm dev:status, without naming it on the command line. pnpm dev:env:mobile plus a stack restart served the app; services/git-token-service/.dev.vars carried all six envelope values.
  3. Signed in on the emulator, opened PR Review (kiloapp:///pr-review): the INBOX listed the three stub pull requests (kilo-stub/discussion-mixed#1, kilo-stub/discussion-conversation-only#2, kilo-stub/discussion-empty#3) in organization scope, and the same three after switching the account sheet to Personal.
  4. Revoked the seeded token row (user_github_app_tokens.revoked_at) and relaunched: the inbox showed the reconnect state with Check connection instead of a generic failure. Clearing revoked_at and tapping Check connection opened a stub pull request; Back returned to an inbox that listed all three rows again.
  5. Envelope preservation: with the private half removed from .env.local but still present in services/git-token-service/.dev.vars (the layout that file documents), a pnpm dev:setup-env rerun reported the envelope as incomplete and wrote the private half back into .env.local from the Worker file (PKCS#8 length 4364, unchanged hash); a second rerun reported the pair as kept and changed no value, and the Worker's .dev.vars still held both private halves.

Proof published in the comments below: the organization-scope inbox, and the personal-scope inbox after the reconnect round trip.

Unit checks: pnpm exec tsx --test dev/local/envelope-keys.test.ts dev/local/services.test.ts passes (49 tests). The dev/local suite (dev/local/*.test.ts, dev/local/env-sync/*.test.ts, dev/local/scripts/*.test.ts, dev/seed/lib/*.test.ts) passes 417 tests; the only failure is dev/local/tmux.test.ts ("decoy session must have an attached client"), which fails identically at main in a clean worktree because the test needs an attached tmux client, so it is unrelated to this change. oxlint and oxfmt --check are clean on every touched file.

Not covered

  • iOS was not exercised. The change touches dev-stack wiring, environment provisioning and docs only, with no iOS-specific code, so it was verified on Android.
  • Production and preview environments are untouched: they already provision both envelopes as real secrets, and the new .env.local.example slots are read only by local tooling.

Proxy answers

The owner's proxy gave these answers in the owner's place.

  1. Question: Fix locus: On current main the inbox renders fine once the local stack is wired by hand: the failure is that pnpm dev:start never starts git-token-service and a fresh dev:setup-env leaves the GitHub user-token envelope keypair empty, so every githubPrReview.listInbox read answers BAD_GATEWAY while the account still reads connected. Both reproduced here. Where should the fix land?
    Answer: Dev-stack wiring only (recommended)
    Confidence: 0.77 · Precedent: Fix the root cause and the class of the issue: session 7f3470e0ba6f, "fix class of issue preventing progress"; least work with no drop in quality: session 00eb2a1d3e4b, "skip work that doesn't need to be done, do not redo work that was already done" (fix(mobile): surface review service outages with their own localized state #8100 already added the mobile states)
  2. Question: Envelope keypair: How should the local GitHub user-token envelope keypair be provisioned?
    Answer: Generate into root .env.local (recommended)
    Confidence: 0.80 · Precedent: Speed and least work: "do the least amount of work possible, with no drop in quality" (session 7f3470e0ba6f). This option reuses the existing passthrough and the NEXTAUTH_SECRET generation pattern.
  3. Question: Regression coverage: Which permanent tests should the pull request carry?
    Answer: Both tests (recommended)
    Confidence: 0.70 · Precedent: Proof and verification: "we want to catch errors in PRs, as early as possible" (session 99e88001c4b4). Fix the class: "fix class of issue preventing progress" (session 7f3470e0ba6f).
  4. Question: Other providers: A fresh env also leaves BITBUCKET_OAUTH_CREDENTIAL_ACTIVE_KEY_ID / _PUBLIC_KEY empty. Include that keypair too?
    Answer: Both envelopes
    Confidence: 0.60 · Precedent: Fix the root cause and the class of the issue on every affected surface: session 00eb2a1d3e4b, "if anything repeats the same mistake over and over again, fix the class"; also session 89824a222897, "if you saw a gap, fix it"
  5. Question: Docs: Should DEVELOPMENT.md mention the new behavior?
    Answer: Update DEVELOPMENT.md (recommended)
    Confidence: 0.70 · Precedent: Trust the code over documents and update stale documents when you find them: session 05f959cc4baa, "if any of the docs are stale, we should update them"

The local web stack never started git-token-service, and a fresh
`pnpm dev:setup-env` left the provider credential envelopes unconfigured.
Both make every code-review read answer BAD_GATEWAY while the account still
reports the provider as connected, so the PR review inbox and overview show
a service outage with no rows.

- `nextjs` depends on `cloudflare-git-token-service`, so the web stack starts
  the worker the review reads resolve credentials through.
- `dev:setup-env` generates a local 4096-bit RSA keypair for each
  unconfigured envelope (GitHub user token, Bitbucket/GitLab platform
  credential) and writes the key ID plus both halves into `.env.local`; the
  existing passthrough copies the private halves into the worker's
  `.dev.vars`. A configured pair is kept as is, so a rerun never rotates a
  keypair the worker already stored credentials under, and a half-configured
  one is reported.
- `.env.local.example` documents the two dev-only private-key slots, and
  `DEVELOPMENT.md` names both behaviors.
@iscekic iscekic added drain Work item from the findings drain explorer:ios Finding surfaced by the iOS explorer labels Oct 22, 2026
@iscekic

iscekic commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Verified on the local stack with the Android dev client and the hermetic GitHub API stub, using an owned fake account and stub fixtures only.

After deleting .env.local and re-running pnpm dev:setup-env, a plain pnpm dev:start brings up git-token-service on 8802 without naming it, and the PR review inbox lists the stub pull requests in both scopes. Revoking the seeded token row switches the same screen to the reconnect state, and clearing it plus "Check connection" brings the rows back.

  • inbox-organization-scope.png: the PR review inbox in organization scope, listing the three stub pull requests.
  • inbox-personal-scope.png: the same inbox after switching to the personal scope and completing the reconnect round trip, listing the three stub pull requests again with one in recents.

PR review inbox in organization scope listing three pull requests

PR review inbox in personal scope after the reconnect round trip

Comment thread dev/local/setup-env.ts Resolved
@kilo-code-bot

kilo-code-bot Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The follow-up commit resolves the earlier half-configured-envelope finding: every provisioning status now records its keypair in collected, and the lookup skips empty slots so a private half living only in the Worker's .dev.vars is read back and preserved.

Assumptions: [INFERENCE] Review is scoped to the incremental diff 14b2b77..7b2ee7b (4 files, 45 changed lines); no end-to-end dev:setup-env run was performed in this read-only review. One hardening edge remains unproven and is not a merge blocker: values are resolved per key, so mixing a key ID/public half from .env.local with a private half from .dev.vars belonging to a different on-disk pair would be reported as keep; this requires deliberately mismatched local files.

Files Reviewed (4 files)
  • DEVELOPMENT.md - no issues
  • dev/local/envelope-keys.test.ts - no issues
  • dev/local/envelope-keys.ts - no issues
  • dev/local/setup-env.ts - no issues
Previous Review Summary (commit 14b2b77)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 14b2b77)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The dev-stack wiring and keypair generation are sound, but dev:setup-env reports that a half-configured credential envelope is left unchanged while the file rewrite actually deletes its values.

Assumptions: [INFERENCE] The impact of the erased partial envelope assumes a developer can legitimately keep the private half only in services/git-token-service/.dev.vars, which that Worker's .dev.vars.example documents; the destructive rewrite itself was verified in dev/local/setup-env.ts Step 6 and dev/local/env-sync/output.ts apply path. No end-to-end run was performed in this read-only review.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
dev/local/setup-env.ts 323 The incomplete-envelope branch says "leaving it unchanged", but Step 6 rebuilds .env.local from .env.local.example and only writes back collected keys, so the existing key ID/public key are dropped (and can then be blanked in the Worker's .dev.vars by the next pnpm dev:env). Preserve the partial values in collected, or correct the message and the matching DEVELOPMENT.md sentence.
Files Reviewed (7 files)
  • .env.local.example - no issues
  • DEVELOPMENT.md - no issues (its "left untouched" sentence is covered by the setup-env.ts finding)
  • dev/local/envelope-keys.test.ts - no issues
  • dev/local/envelope-keys.ts - no issues
  • dev/local/services.test.ts - no issues
  • dev/local/services.ts - no issues
  • dev/local/setup-env.ts - 1 issue

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 41K · Output: 7K · Cached: 393.5K

Review guidance: REVIEW.md from base branch main

The setup write path rebuilds `.env.local` from the example and writes back
only the keys it collected, so an envelope whose half was configured outside
that file was dropped: the warning said the values stayed while the write
erased them, and the next `pnpm dev:env` emptied the Worker's copy too.

Every provisioning status now records the pair it decided on, and the lookup
skips empty slots, so a private half that lives only in the Worker's
`.dev.vars` is read back into `.env.local` instead of being lost. An
incomplete pair keeps exactly its own values and reports the gap.
@iscekic iscekic added the merge:human Substantive functional risk requires human review. label Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drain Work item from the findings drain explorer:ios Finding surfaced by the iOS explorer merge:human Substantive functional risk requires human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants