feat(cloud-agent): associated PR tracking for cloud-agent-next sessions#2903
Open
kilo-code-bot[bot] wants to merge 1 commit intomainfrom
Open
feat(cloud-agent): associated PR tracking for cloud-agent-next sessions#2903kilo-code-bot[bot] wants to merge 1 commit intomainfrom
kilo-code-bot[bot] wants to merge 1 commit intomainfrom
Conversation
3e32e34 to
cb82fe9
Compare
Contributor
Author
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)CRITICAL
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (4 files)
Reviewed by gpt-5.5-2026-04-23 · 1,099,450 tokens |
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.
124c0b4 to
084f755
Compare
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>
084f755 to
13ee5a0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds infrastructure to associate a GitHub pull request with a cloud-agent-next CLI session.
cli_session_pull_requestsside table (PK =session_id, FK →cli_sessions_v2.session_idwithON DELETE CASCADE) storing PR number, url, state, title, head sha, and last-synced timestamp.UQ_cli_sessions_v2_session_idoncli_sessions_v2.session_idso the FK has a unique target (the base table uses a composite PK(session_id, kilo_user_id)).(git_url, git_branch)to support branch → session lookups.fetchPullRequestForBranchhelper in the GitHub adapter that looks up the most relevant PR for a(owner, repo, branch)triple via an installation token. PrefersopenPRs, mapsmerged_at→"merged"state, returnsnullon 404, and throws a dedicatedGitHubRateLimitError(carryingresetAt) for rate/secondary-rate-limit responses while passing through genuine 403 permission failures unchanged.apps/web/src/tests/setup/__mocks__/updated to mirror the new export surface.Verification
0106_gorgeous_iron_patriot) — matches the schema diff, no hand edits.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: 0on other statuses, secondary rate-limit message, permission-denied 403 passthrough, generic error passthrough.Visual Changes
N/A
Reviewer Notes
cli_sessions_v2.session_idis required for the FK, even though the base PK is composite — be aware this enforcessession_iduniqueness across the whole table going forward.