Skip to content

feat(cloud-agent): associated PR tracking for cloud-agent-next sessions#2903

Open
kilo-code-bot[bot] wants to merge 1 commit intomainfrom
convoy/associated-pr-for-cloud-agent-next-sessi/dbccdbdf/head
Open

feat(cloud-agent): associated PR tracking for cloud-agent-next sessions#2903
kilo-code-bot[bot] wants to merge 1 commit intomainfrom
convoy/associated-pr-for-cloud-agent-next-sessi/dbccdbdf/head

Conversation

@kilo-code-bot
Copy link
Copy Markdown
Contributor

@kilo-code-bot kilo-code-bot Bot commented Apr 29, 2026

Summary

Adds infrastructure to associate a GitHub pull request with a cloud-agent-next CLI session.

  • New cli_session_pull_requests side table (PK = session_id, FK → cli_sessions_v2.session_id with ON DELETE CASCADE) storing PR number, url, state, title, head sha, and last-synced timestamp.
  • Required unique index UQ_cli_sessions_v2_session_id on cli_sessions_v2.session_id so the FK has a unique target (the base table uses a composite PK (session_id, kilo_user_id)).
  • New composite index on (git_url, git_branch) to support branch → session lookups.
  • New fetchPullRequestForBranch helper in the GitHub adapter that looks up the most relevant PR for a (owner, repo, branch) triple via an installation token. Prefers open PRs, maps merged_at"merged" state, returns null on 404, and throws a dedicated GitHubRateLimitError (carrying resetAt) for rate/secondary-rate-limit responses while passing through genuine 403 permission failures unchanged.
  • Mock adapter in apps/web/src/tests/setup/__mocks__/ updated to mirror the new export surface.

Verification

  • Reviewed the generated migration (0106_gorgeous_iron_patriot) — matches the schema diff, no hand edits.
  • Reviewed fetch-pull-request-for-branch.test.ts (10 cases): single open PR, open-over-closed preference, merged state, fallback to newest, empty result, 404, 403 + rate-limit headers, 429, x-ratelimit-remaining: 0 on other statuses, secondary rate-limit message, permission-denied 403 passthrough, generic error passthrough.

Visual Changes

N/A

Reviewer Notes

  • The 403 disambiguation (rate limit vs. permission denied) relies on message-substring matching. GitHub's wording is stable today but may drift; the tests pin the current phrasing.
  • The unique index on cli_sessions_v2.session_id is required for the FK, even though the base PK is composite — be aware this enforces session_id uniqueness across the whole table going forward.
  • No caller is wired up yet; this PR only lands the data model + adapter helper + mocks. The tRPC mutation and webhook integration will land in a follow-up.

@kilo-code-bot kilo-code-bot Bot force-pushed the convoy/associated-pr-for-cloud-agent-next-sessi/dbccdbdf/head branch from 3e32e34 to cb82fe9 Compare April 29, 2026 14:07
Comment thread packages/db/src/migrations/0109_rare_vin_gonzales.sql
Comment thread packages/db/src/migrations/0107_dashing_mockingbird.sql
@kilo-code-bot
Copy link
Copy Markdown
Contributor Author

kilo-code-bot Bot commented Apr 29, 2026

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 0
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

CRITICAL

File Line Issue
packages/db/src/migrations/0109_rare_vin_gonzales.sql 13 Migration adds the foreign key before creating the unique index required by the referenced cli_sessions_v2(session_id) column.
Other Observations (not in diff)

Issues found in unchanged code that cannot receive inline comments:

File Line Issue
N/A N/A None
Files Reviewed (4 files)
  • packages/db/src/migrations/0109_rare_vin_gonzales.sql - 1 issue
  • packages/db/src/migrations/meta/0109_snapshot.json - 0 issues
  • packages/db/src/migrations/meta/_journal.json - 0 issues
  • packages/db/src/schema.ts - 0 issues

Reviewed by gpt-5.5-2026-04-23 · 1,099,450 tokens

Comment thread apps/web/src/routers/cli-sessions-v2-router.ts
kilo-code-bot Bot pushed a commit that referenced this pull request Apr 29, 2026
- Add associatedPr to mobile FetchedSessionData so mobile-session-manager
  matches the shared type definition. This unblocks the CI typecheck
  failure on apps/mobile.

- refreshAssociatedPullRequest: move ensureOrganizationAccess BEFORE
  the throttle short-circuit for org-scoped sessions. Previously a
  removed org member with a stale cli_sessions_v2 row could receive
  cached PR metadata via the throttle path without a current
  membership check. Adds a regression test covering the fresh-sentinel
  case where the throttle previously would have bypassed the check.

- upsertCliSessionPullRequestsFromWebhook: introduce
  WebhookInstallationOwner and require the caller (webhook router) to
  pass the integration owner. The session SELECT now constrains by
  organization_id OR kilo_user_id so a webhook from one tenant's
  installation cannot upsert PR metadata onto a session owned by
  another tenant that happens to share the same (git_url, git_branch).
  Adds cross-tenant isolation tests for both org and user ownership,
  including the slow-path normalization branch.
kilo-code-bot Bot pushed a commit that referenced this pull request Apr 30, 2026
- Add associatedPr to mobile FetchedSessionData so mobile-session-manager
  matches the shared type definition. This unblocks the CI typecheck
  failure on apps/mobile.

- refreshAssociatedPullRequest: move ensureOrganizationAccess BEFORE
  the throttle short-circuit for org-scoped sessions. Previously a
  removed org member with a stale cli_sessions_v2 row could receive
  cached PR metadata via the throttle path without a current
  membership check. Adds a regression test covering the fresh-sentinel
  case where the throttle previously would have bypassed the check.

- upsertCliSessionPullRequestsFromWebhook: introduce
  WebhookInstallationOwner and require the caller (webhook router) to
  pass the integration owner. The session SELECT now constrains by
  organization_id OR kilo_user_id so a webhook from one tenant's
  installation cannot upsert PR metadata onto a session owned by
  another tenant that happens to share the same (git_url, git_branch).
  Adds cross-tenant isolation tests for both org and user ownership,
  including the slow-path normalization branch.
@kilo-code-bot kilo-code-bot Bot force-pushed the convoy/associated-pr-for-cloud-agent-next-sessi/dbccdbdf/head branch from 124c0b4 to 084f755 Compare April 30, 2026 19:12
Surfaces the GitHub PR associated with a cloud-agent-next session in
ChatHeader and SessionInfoDialog, fed primarily by pull_request webhooks
with a throttled manual refresh fallback.

Data model
- New cli_session_pull_requests side table storing PR summary
  (url/number/state/title/last_synced_at) keyed by session_id, avoiding
  sparse nullable columns on cli_sessions_v2.
- Adds cli_sessions_v2 indexes on (git_url, git_branch), git_branch,
  and a unique session_id for the FK. Migration 0108 regenerated after
  rebase onto main.

Webhook-primary path
- pull_request events (opened/reopened/edited/synchronize/closed) fan
  out to all sessions matching normalized (git_url, git_branch) via a
  single bulk upsert. Closed events now flow through the upsert before
  the code-review short-circuit. Dedup via logWebhook runs first so
  redelivered events cannot stomp terminal state, and the ON CONFLICT
  clause refuses to demote closed/merged back to open (reopened is
  exempt so legitimate closed -> open transitions apply).
- Requires the integration owner (org or user) so cross-tenant
  installations sharing (git_url, git_branch) cannot mutate each
  other's sessions.

Manual refresh
- fetchPullRequestForBranch helper (Octokit) resolves the most recently
  updated PR for a branch, prefers open, maps merged_at to 'merged',
  and distinguishes real rate limits (429, secondary/abuse, or
  x-ratelimit-remaining=0) from permission 403s.
- refreshAssociatedPullRequest tRPC mutation runs ensureOrganizationAccess
  up front (before the 10s per-session throttle), resolves the GitHub
  App installation, and upserts a sentinel row on negative refreshes so
  pr_last_synced_at still throttles empty lookups. Rate-limit errors
  map to TOO_MANY_REQUESTS; other failures to INTERNAL_SERVER_ERROR to
  avoid leaking Octokit internals.

UI
- getWithRuntimeState LEFT JOINs the side table and returns associatedPr
  without ever calling GitHub.
- ChatHeader 'Open in GitHub' links to the PR when known (with state
  badge), falling back to the compare URL or plain repo browse URL.
  Adds a 'Refresh PR info' action with toast feedback.
- SessionInfoDialog gains a Pull Request row with state badge and
  'Last checked ...' subtitle.
- Refresh handler captures session id at call time and reads the latest
  FetchedSessionData via ref so a session switch during an in-flight
  refresh cannot overwrite the new session's snapshot. Mobile
  FetchedSessionData updated to match.

Tests
- Unit tests for the pure helpers (github-pr-link, normalizeGitUrl,
  parseGitHubOwnerRepo) and for fetchPullRequestForBranch including
  the 403-permission-vs-rate-limit split.
- webhook-handler tests drive handleGitHubWebhook end-to-end for
  closed+merged, closed+unmerged, redelivery-after-close dedup,
  closed -> reopened, cross-tenant isolation, and the monotonic
  pr_state guard.
- refreshAssociatedPullRequest tests cover org-access rejection (incl.
  the fresh-sentinel throttle-bypass regression), negative-refresh
  throttle, and sentinel upsert.
- Switches fetchPullRequestForBranch test mocking from jest.spyOn to
  jest.mock factory (SWC compiles ESM exports non-configurable) and
  adds universal-github-app-jwt to transformIgnorePatterns.

Co-authored-by: Maple (gastown) <Maple@gastown.local>
Co-authored-by: Toast (gastown) <Toast@gastown.local>
@eshurakov eshurakov force-pushed the convoy/associated-pr-for-cloud-agent-next-sessi/dbccdbdf/head branch from 084f755 to 13ee5a0 Compare May 4, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant