docs(review): add PR 3024 findings

CODE_QUALITY.md

@@ -0,0 +1,66 @@
+# Code Quality Review
+
+## Findings
+
+### High: GitHub bot support is wired only to the standard GitHub App despite lite handling elsewhere
+
+- **Affected files/functions:** `apps/web/src/lib/bot.ts` (`githubAdapter` initialization), `apps/web/src/app/api/webhooks/github/route.ts`, `apps/web/src/app/api/webhooks/github-lite/route.ts`, `apps/web/src/app/github/link/route.ts`, `apps/web/src/app/api/integrations/github/callback/route.ts`
+- **Issue:** The bot adapter is always created from `getGitHubAppCredentials('standard')`, and only `/api/webhooks/github` forwards requests to `bot.webhooks.github`. Account linking explicitly supports `integration.github_app_type ?? 'standard'`, including `lite`.
+- **Risk:** Users can link through a lite integration, but lite webhooks never reach the chat adapter and the adapter has no lite credentials. Mention ingestion and linking have inconsistent app-type support.
+- **Recommended fix:** Either make GitHub bot support standard-only and block/link-message lite integrations, or instantiate/route a separate GitHub adapter for lite credentials. Mirror standard bot routing in `apps/web/src/app/api/webhooks/github-lite/route.ts` or add an adapter-selection layer keyed by `github_app_type`.
+
+### Medium: GitHub context API failures can fail the entire bot run
+
+- **Affected files/functions:** `apps/web/src/lib/bot/conversation-context.ts` (`getGitHubConversationContext`), `apps/web/src/lib/bot/agent-runner.ts` (`buildSystemPrompt`)
+- **Issue:** `getGitHubConversationContext` calls `issues.get`, `issues.listComments`, and `pulls.listReviewComments` inside a single `Promise.all` without local error handling.
+- **Risk:** A GitHub outage, rate limit, deleted issue/PR, missing permission, or malformed thread ID can reject prompt construction and prevent any bot response, even though reduced context would be enough to answer.
+- **Recommended fix:** Treat GitHub context as best-effort. Use `Promise.allSettled` or local catches, capture errors with repository/thread metadata, and return partial context or an empty string.
+
+### Medium: Webhook handler imports initialize the full bot and adapters as a side effect
+
+- **Affected files/functions:** `apps/web/src/lib/integrations/platforms/github/webhook-handlers/installation-handler.ts`, `apps/web/src/lib/integrations/platforms/github/webhook-handler.ts`, `apps/web/src/lib/bot.ts`
+- **Issue:** `installation-handler.ts` imports `bot` to call `bot.initialize()` and access bot state for `unlinkTeamKiloUsers`. Importing the generic GitHub webhook handler now initializes the full bot, Slack adapter, GitHub adapter, and standard GitHub credentials.
+- **Risk:** This couples integration cleanup to chat runtime initialization and can break webhook handling if bot configuration is missing. It also increases side effects and dependency-cycle risk.
+- **Recommended fix:** Avoid importing `bot` from integration handlers. `unlinkTeamKiloUsers` only needs a state adapter; create or inject chat state from a small state module, or move cleanup into a bot-owned path.
+
+### Medium: All standard GitHub webhooks are forwarded to the chat adapter
+
+- **Affected files/functions:** `apps/web/src/app/api/webhooks/github/route.ts`
+- **Issue:** The route schedules `bot.webhooks.github(...)` for every standard GitHub webhook before the legacy handler determines event type/action. Tests lock in forwarding `installation` and `pull_request` events, even though the bot needs mention-capable comment events.
+- **Risk:** Unnecessary work, noisy logs, doubled signature/body parsing, and broader future behavior if the adapter starts handling more event types.
+- **Recommended fix:** Filter before forwarding. At minimum gate on `x-github-event` for `issue_comment` and `pull_request_review_comment`, and ideally require `action === 'created'`.
+
+### Medium: GitHub link routes duplicate unsafe HTML response construction
+
+- **Affected files/functions:** `apps/web/src/app/github/link/route.ts` (`errorPage`), `apps/web/src/app/api/integrations/github/callback/route.ts` (`htmlPage`, `handleGitHubBotLinkCallback`)
+- **Issue:** Two routes manually build full HTML pages with duplicated inline styles and string interpolation. The callback interpolates `githubUser.login` directly into HTML.
+- **Risk:** GitHub logins are constrained today, but the helper is general-purpose and easy to reuse unsafely later. Duplicated page construction makes escaping and response behavior harder to keep consistent.
+- **Recommended fix:** Centralize minimal HTML responses in a shared helper that escapes interpolated text by default and only allows explicitly trusted HTML fragments.
+
+### Low: Token/state verification relies on `as` casts instead of schema validation
+
+- **Affected files/functions:** `apps/web/src/lib/bot/github-link-token.ts` (`verifyGitHubLinkToken`), `apps/web/src/lib/bot/github-link-state.ts` (`verifyGitHubBotLinkState`)
+- **Issue:** Both verifiers parse JSON as `Partial<...>` using `as`, then manually check fields. This conflicts with repo guidance to avoid casts when possible.
+- **Risk:** Manual validation can drift from payload types as fields are added.
+- **Recommended fix:** Define Zod schemas for both payloads and parse decoded JSON with `safeParse`, returning `null` on validation failure.
+
+### Low: Slack-specific documentation URL is returned for GitHub
+
+- **Affected files/functions:** `apps/web/src/lib/bot/platform-helpers.ts` (`getBotDocumentationUrl`), `apps/web/src/lib/bot/platform-helpers.test.ts`
+- **Issue:** `getBotDocumentationUrl(PLATFORM.GITHUB)` returns the Slack documentation URL, and the test locks in that behavior.
+- **Risk:** GitHub users asking for help receive Slack-specific docs.
+- **Recommended fix:** Add a GitHub-specific docs URL or a platform-neutral bot docs URL. Update the test to assert the intended fallback.
+
+### Low: GitHub platform identity extraction remains coupled to Slack-specific cast patterns
+
+- **Affected files/functions:** `apps/web/src/lib/bot/platform-helpers.ts` (`getPlatformIdentity`)
+- **Issue:** The Slack branch still casts `message as Message<SlackEvent>` to call `getSlackTeamId`. This helper is now the cross-platform identity boundary.
+- **Risk:** As more adapters are added, it becomes easier to access raw payloads with the wrong shape.
+- **Recommended fix:** Add type guards or delegate to adapter-specific identity extractors with typed inputs.
+
+### Low: Manual GitHub thread ID parsing is tightly coupled to adapter internals
+
+- **Affected files/functions:** `apps/web/src/lib/bot/conversation-context.ts` (`parseGitHubThreadId`, `getGitHubConversationContext`)
+- **Issue:** GitHub context depends on hard-coded thread ID formats such as `github:owner/repo:issue:number` and `github:owner/repo:number:rc:id`.
+- **Risk:** If `@chat-adapter/github` changes thread ID format, context silently disappears or fetches wrong coordinates.
+- **Recommended fix:** Prefer adapter-provided metadata/raw payload fields. If manual parsing is unavoidable, isolate it in a compatibility module with explicit tests and documentation.

CONCURRENCY.md

@@ -0,0 +1,67 @@
+# Concurrency Review
+
+## Overall Behavior
+
+GitHub webhooks now take two paths from `apps/web/src/app/api/webhooks/github/route.ts`:
+
+1. The request body is read once and cloned.
+2. `bot.webhooks.github(...)` is scheduled in `after(...)` for the Chat SDK GitHub adapter.
+3. The existing `handleGitHubWebhook(...)` path still runs and returns the HTTP response.
+4. The Chat SDK adapter turns created GitHub issue comments, PR comments, or review comments into `Message`s and calls `chatBot.onNewMention(...)` in `apps/web/src/lib/bot.ts`.
+5. The bot resolves installation/user/integration state, creates a `bot_requests` row, refetches live GitHub context, runs the AI agent, and posts a reply.
+
+Shared state includes Chat SDK Redis/memory state, GitHub adapter installation cache, Kilo user links, `bot_requests`, live GitHub comments, and the legacy auto-fix state for PR review comments.
+
+## Findings
+
+### High: Rapid mentions in the same GitHub thread are silently dropped
+
+- **Affected files/functions:** `apps/web/src/lib/bot.ts`, `apps/web/src/app/api/webhooks/github/route.ts`, `apps/web/src/lib/bot/run.ts`
+- **Scenario:** Several users add `@kilocode-bot ...` comments quickly on the same issue, PR, or review thread. The Chat SDK `Chat` instance has no explicit concurrency option, so it appears to use the SDK default strategy: `drop`. The first message gets the per-thread lock; later messages on the same thread fail while the long AI run is active.
+- **Impact:** Later GitHub mentions are not queued and may receive no visible reply. GitHub still gets a successful webhook response, so users see a delivered comment but the bot appears to ignore them.
+- **Recommended fix:** Configure explicit queueing for GitHub bot work, with bounded queue size and TTL. Add tests simulating multiple comments on the same thread and assert later comments are queued, coalesced, or clearly acknowledged rather than silently dropped.
+
+### High: Different review comment threads on the same PR can run concurrently against the same branch
+
+- **Affected files/functions:** `apps/web/src/lib/bot.ts`, `apps/web/src/lib/bot/agent-runner.ts`, `apps/web/src/lib/bot/conversation-context.ts`, GitHub adapter thread IDs like `github:{owner}/{repo}:{prNumber}:rc:{commentId}`
+- **Scenario:** A reviewer mentions the bot on multiple line comments in the same PR. Each root review comment becomes a different Chat SDK thread ID because the ID includes the review comment ID. The SDK lock is per thread, so all runs can proceed concurrently.
+- **Impact:** Multiple Cloud Agent sessions may edit the same repository/PR branch at the same time, producing conflicting pushes, out-of-order replies, and confusing workflows.
+- **Recommended fix:** Add an application-level mutex/queue keyed by GitHub PR coordinates, such as `github-pr:{owner}/{repo}:{prNumber}`. Serialize code-changing runs per PR while optionally allowing read-only answers to proceed independently.
+
+### High: Chat SDK lock TTL can expire while a long AI run is still active
+
+- **Affected files/functions:** `apps/web/src/lib/bot.ts`, `apps/web/src/lib/bot/run.ts`, `apps/web/src/lib/bot/agent-runner.ts`
+- **Scenario:** The first GitHub mention starts a long AI/tool run. If the Chat SDK lock TTL expires before the handler finishes, a later mention can start a second run on the same thread.
+- **Impact:** Replies may be out of order, and multiple Cloud Agent sessions can run for what users perceive as one conversation.
+- **Recommended fix:** Add a long-lived application-level lock renewed until `processMessage(...)` finishes, or store an explicit active-run record that later comments queue behind or append to. Add a slow-handler test around the lock TTL boundary.
+
+### Medium: Live context fetch can include comments created after the triggering mention
+
+- **Affected files/functions:** `apps/web/src/lib/bot/conversation-context.ts`, `apps/web/src/lib/bot/agent-runner.ts`
+- **Scenario:** Comment A mentions the bot. Before A's run builds context, comments B and C are added. `getGitHubConversationContext(...)` refetches current issue/review comments and only filters out the trigger message by ID.
+- **Impact:** The run triggered by A can see B/C as prior context even though they happened later. If B/C were dropped by concurrency handling, they may still influence the prompt without being acknowledged.
+- **Recommended fix:** Filter fetched comments to `created_at <= triggerMessage.metadata.dateSent`. If queueing is enabled, pass queued/skipped comments explicitly as messages received while the previous run was active.
+
+### Medium: GitHub review-comment webhooks can be handled by two independent systems
+
+- **Affected files/functions:** `apps/web/src/app/api/webhooks/github/route.ts`, `apps/web/src/lib/integrations/platforms/github/webhook-handler.ts`, `apps/web/src/lib/integrations/platforms/github/webhook-handlers/pr-review-comment-handler.ts`, `apps/web/src/lib/auto-fix/application/webhook/review-comment-webhook-processor.ts`
+- **Scenario:** Every GitHub webhook is sent to the Chat SDK adapter in `after(...)`. Created `pull_request_review_comment` events are also routed to legacy auto-fix, which looks for `@kilo` plus fix/patch language.
+- **Impact:** A single review comment can spawn both a Chat bot run and an auto-fix ticket if mention aliases overlap or users include both command styles.
+- **Recommended fix:** Decide one owner for review-comment fix commands. Gate the Chat SDK path away from legacy auto-fix comments, or migrate legacy behavior behind the Chat adapter with shared idempotency.
+
+### Medium: `bot_requests` logging is not idempotent for duplicate platform message processing
+
+- **Affected files/functions:** `apps/web/src/lib/bot/request-logging.ts`, `packages/db/src/schema.ts`, `apps/web/src/lib/bot/run.ts`
+- **Scenario:** GitHub retries, serverless duplication, lock expiry, or process restart causes the same GitHub comment to be processed more than once outside the SDK dedupe window. `createBotRequest(...)` inserts a new row every time.
+- **Impact:** Duplicate bot runs and admin records can be created for one GitHub comment. If Redis is unavailable and memory state is used, dedupe is process-local and easier to bypass.
+- **Recommended fix:** Add a unique idempotency key for inbound bot messages, likely `(platform, platform_thread_id, platform_message_id)`, and use `insert ... on conflict` behavior.
+
+## Expected User Experience
+
+- Multiple mentions in the same issue/PR conversation are likely to process only the first mention while later mentions are dropped.
+- Multiple line-level review mentions on the same PR can run concurrently and race on the same branch.
+- Replies can be out of order when long runs overlap lock expiry or independent review-comment threads.
+- Context can include comments added after the trigger, so the bot may appear to respond to the wrong slice of the conversation.
+- Users will not get a clear queued/already-working response unless the handler implements one.
+
+The safest model is explicit idempotent queueing, with serialization at least per PR for code-changing work and clear treatment of skipped or coalesced comments.

GENERAL.md

@@ -0,0 +1,39 @@
+# General Review
+
+## Findings
+
+### High: GitHub review-comment mentions can be processed twice
+
+- **Affected files/functions:** `apps/web/src/app/api/webhooks/github/route.ts`, `apps/web/src/lib/integrations/platforms/github/webhook-handler.ts`
+- **Scenario/impact:** The route forwards every standard GitHub webhook to the Chat SDK adapter in `after(...)`, then also calls the legacy GitHub webhook handler. For `pull_request_review_comment.created`, the legacy path still calls `handlePRReviewComment(...)`, while the Chat SDK adapter may also turn the same mention into `onNewMention`. A single review-comment mention can trigger both legacy auto-fix and the new bot flow, causing duplicate agent runs, duplicate comments, conflicting side effects, and doubled model/API usage.
+- **Recommended fix:** Route each event to exactly one bot path. Either exclude `pull_request_review_comment` events from the Chat SDK adapter while legacy auto-fix remains active, or migrate that event fully to the Chat SDK adapter and disable the legacy mention path. Add a regression test that a review comment with a bot mention invokes only one processing path.
+
+### High: Lite GitHub app webhooks are not connected to the new bot adapter
+
+- **Affected files/functions:** `apps/web/src/app/api/webhooks/github-lite/route.ts`, `apps/web/src/lib/bot.ts`
+- **Scenario/impact:** The new GitHub adapter is configured only with standard app credentials, and only `/api/webhooks/github` forwards deliveries to `bot.webhooks.github(...)`. Lite installations still post to `/api/webhooks/github-lite`, which only calls the legacy integration handler. Mentions from repos installed through `github_app_type: 'lite'` will not reach the new bot handler, even though account-linking and context code try to support lite integrations.
+- **Recommended fix:** Add bot adapter handling for the lite webhook endpoint with lite credentials, or create a routing layer that can verify and process both standard and lite app deliveries. Add tests for `/api/webhooks/github-lite` proving an `issue_comment.created` mention reaches the bot adapter.
+
+### High: GitHub Cloud Agent session links may be invisible to users
+
+- **Affected files/functions:** `apps/web/src/lib/bot/agent-runner.ts`, `apps/web/src/lib/bot/run.ts`
+- **Scenario/impact:** When the bot starts a Cloud Agent session, `processMessage` suppresses the normal public final reply, and `postSessionLinkEphemeral(...)` is the only place that posts the session URL. That helper always uses `thread.postEphemeral(...)`, which is Slack-oriented and may be unsupported or ineffective for GitHub. Failures are caught and only logged, so a GitHub user can ask Kilo to start work, get no visible response, and have no way to open the created session from GitHub.
+- **Recommended fix:** Branch on `thread.adapter.name`: keep ephemeral cards for Slack, but post a normal GitHub reply with the session link or a safe handoff message for GitHub. If `postEphemeral` fails on platforms without ephemeral support, fall back to `thread.post(...)`. Add a GitHub-specific test for the session-start path.
+
+### Medium: GitHub context tags are not covered by the prompt-injection warning
+
+- **Affected files/functions:** `apps/web/src/lib/bot/conversation-context.ts`, `apps/web/src/lib/bot/agent-runner.ts`
+- **Scenario/impact:** GitHub issue descriptions, comments, review comments, and diff hunks are user/repository-controlled content, but they are inserted into tags the system prompt does not explicitly identify as untrusted. Malicious issue comments or PR descriptions can contain prompt-injection text that the model may treat as instructions.
+- **Recommended fix:** Update the system prompt to mark all GitHub context tags as untrusted, or wrap GitHub user-controlled content in the existing untrusted convention. Separate trusted metadata from untrusted bodies and add tests for the generated prompt warning.
+
+### Medium: GitHub API context fetch failures abort the entire bot run
+
+- **Affected files/functions:** `apps/web/src/lib/bot/conversation-context.ts`, `apps/web/src/lib/bot/agent-runner.ts`
+- **Scenario/impact:** `getGitHubConversationContext(...)` fetches the issue, issue comments, and review comments with `Promise.all(...)` and no local error handling. A transient GitHub error, rate limit, missing permission, deleted issue/PR, or unsupported thread ID shape can reject prompt construction and make the bot fail instead of answering with reduced context.
+- **Recommended fix:** Make GitHub context best-effort. Catch context-fetch errors, capture them with useful tags, and return minimal context containing the trigger message and thread coordinates. Prefer `Promise.allSettled(...)` so one failed auxiliary call does not discard all available context.
+
+### Medium: Public GitHub account-link links are not bound to the triggering GitHub user
+
+- **Affected files/functions:** `apps/web/src/lib/bot/link-account.tsx`, `apps/web/src/lib/bot/github-link-token.ts`, `apps/web/src/app/github/link/route.ts`
+- **Scenario/impact:** The public link posted in an issue/PR contains only the platform integration and installation ID. Any Kilo user who is a member of the owning organization can click someone else's visible link and complete OAuth for their own GitHub account. This likely does not hijack the original commenter because the callback links the OAuth-authenticated GitHub user ID, but it is an unintended enrollment path and can make public prompts misleading.
+- **Recommended fix:** Human-verify the intended policy. If the link should only be usable by the triggering GitHub user, include that GitHub user ID in the signed token/state and verify it against the OAuth-authenticated GitHub user. If any org member may self-link from any prompt, update copy and tests to make that explicit.