feat(cloud-agent): associated PR tracking for cloud-agent-next sessions#2903
Open
kilo-code-bot[bot] wants to merge 9 commits intomainfrom
Open
feat(cloud-agent): associated PR tracking for cloud-agent-next sessions#2903kilo-code-bot[bot] wants to merge 9 commits intomainfrom
kilo-code-bot[bot] wants to merge 9 commits intomainfrom
Conversation
kilo-code-bot
Bot
force-pushed
the
convoy/associated-pr-for-cloud-agent-next-sessi/dbccdbdf/head
branch
from
3e32e34 to
cb82fe9
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Author
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Reviewed by gpt-5.5-2026-04-23 · 1,987,260 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.
feat(db): add cli_session_pull_requests side table for associated PR tracking Adds a new side table to store GitHub PR metadata associated with cloud-agent-next sessions. Keeping this separate from cli_sessions_v2 avoids adding 6 sparse nullable columns to an already-wide table and allows webhook-driven PR writes to evolve independently. Also adds indexes on cli_sessions_v2 needed by upcoming beads: - (git_url, git_branch) for webhook session lookup - unique on session_id to support the FK from the new side table Bead b7954dba — convoy dbccdbdf. Co-authored-by: Toast (gastown) <Toast@gastown.local>
* feat(github): add fetchPullRequestForBranch helper
Adds a small Octokit helper that looks up the PR associated with a
(repo, branch) pair using an existing GitHub App installation token.
Used exclusively by the manual 'Refresh PR info' mutation — the common
case is webhook-driven. Intentionally no caching or in-flight dedup
since the caller is throttled to once per 10s per session.
- Returns most recently updated PR whose head ref matches, preferring
open PRs when multiple exist.
- Maps merged_at != null to a 'merged' state.
- Throws GitHubRateLimitError on 403/429 or x-ratelimit-remaining=0,
surfacing the parsed reset timestamp so callers can report it.
- Returns null on 404 (repo no longer accessible / deleted).
Bead 1 of 5 in the 'Associated PR for cloud-agent-next session' convoy.
* fix(github): only classify 403 as rate limit when message indicates it
Non-rate-limit 403 responses (e.g. installation missing pull request
permission) were being wrapped as GitHubRateLimitError, which would
incorrectly tell users to retry instead of surfacing the permission
problem. Now only treat 403 as rate-limited when the x-ratelimit
headers or the error message ('rate limit', 'secondary rate limit',
'abuse') indicate so. 429 remains unambiguously rate-limited, and
x-ratelimit-remaining: 0 still triggers the rate-limit path at any
status.
---------
Co-authored-by: Maple (gastown) <Maple@gastown.local>
…uest (#2911) * feat(cli-sessions-v2): expose associatedPr + refreshAssociatedPullRequest - getWithRuntimeState LEFT JOINs cli_session_pull_requests and returns associatedPr (nullable) without ever hitting GitHub, keeping the read path cheap. - Adds refreshAssociatedPullRequest mutation: resolves the session owner's GitHub installation, calls fetchPullRequestForBranch, and upserts or deletes the side-table row. 10s per-session throttle short-circuits repeat calls without calling GitHub. - Maps GitHubRateLimitError to TOO_MANY_REQUESTS; other failures become a generic INTERNAL_SERVER_ERROR so Octokit internals do not leak. - parseGitHubOwnerRepo is a pure helper covering https, ssh, and .git suffix variants; non-GitHub hosts bail out with BAD_REQUEST before any token resolution. * fix(cli-sessions-v2): address review comments on refreshAssociatedPullRequest - Re-check org membership via ensureOrganizationAccess before using the org GitHub installation. An old cli_sessions_v2 row carrying the caller's kilo_user_id is no longer sufficient authorization after the user has lost org access. - Persist a null-fields sentinel row on negative refreshes instead of deleting, so pr_last_synced_at still throttles repeated lookups for branches that have no PR. Migration regenerated with pr_url/pr_number/ pr_state now nullable. - Tests: add org-access rejection case and negative-refresh throttle case; update the prior 'delete on null' test to assert on the new sentinel-row behavior. --------- Co-authored-by: Maple (gastown) <Maple@gastown.local>
…ents (#2910) * feat(webhooks): upsert cli_session_pull_requests from pull_request events Add a pull_request webhook side-effect that finds cloud-agent-next sessions with matching (git_url, git_branch) and upserts the PR summary into the cli_session_pull_requests side table. Webhook-primary hot path for the Associated PR feature — zero GitHub API calls on the critical path. - Pure normalizeGitUrl helper (handles https/ssh/.git/case variants) with unit tests. - Handles opened, reopened, edited, synchronize, closed. For closed, state is derived from pull_request.merged. - Bulk upsert via Drizzle onConflictDoUpdate keyed on session_id — a single statement across all matching sessions. - Extends PullRequestPayloadSchema with head.repo.clone_url, head.repo.html_url and pull_request.merged. - Errors are caught and logged so the existing code-review side-effect is never blocked. * fix(github-webhooks): route closed PR events to cli-session upsert Review feedback on #2910: 1. Move upsertCliSessionPullRequestsFromWebhook out of pull-request-handler.ts and into webhook-handler.ts, above the closed early-return. Without this, closed/merged pull_request webhooks short-circuit before ever reaching the upsert, so pr_state never transitions to merged/closed in cli_session_pull_requests. 2. Add a branch-leading index (cli_sessions_v2_git_branch_idx) so the slow-path branch-only scan in findMatchingSessionIds does not force a seq scan for common branch names ('main', 'master', 'develop') on every PR webhook delivery. The existing composite (git_url, git_branch) index cannot be used when only git_branch is constrained. 3. Regenerate migration 0107 from the current schema (per AGENTS.md one-migration-per-branch policy) with the index added and the FK/unique-index ordering fix preserved. 4. Add webhook-handler.test.ts driving the real handleGitHubWebhook entry point with a closed+merged payload and asserting pr_state='merged' in the DB. Plus a closed+unmerged case. Closes the test gap noted in review — previously the upsert was tested in isolation so the dispatch regression was invisible. * fix(github-webhooks): dedupe pull_request events before cli-session PR upsert Moves logWebhook above upsertCliSessionPullRequestsFromWebhook so that duplicate deliveries (same x-github-delivery) are rejected before the upsert mutates pr_state. Without this, a redelivered older opened/synchronize event could stomp a later closed/merged terminal state back to open. closed events are now logged for dedup purposes too; the code-review pipeline still short-circuits on closed. Belt-and-braces: the upsert's ON CONFLICT DO UPDATE SET clause now refuses to demote pr_state from closed/merged back to open, so out-of-order deliveries from different delivery ids stay monotonic. New tests cover: - webhook-handler dedup rejects a redelivered opened after a closed+merged and leaves pr_state='merged' - upsert does not demote merged -> open or closed -> open - upsert still allows the legitimate closed(unmerged) -> merged transition * fix(github-webhooks): allow reopened PRs to exit closed state The monotonic pr_state guard in the cli-session PR upsert blocked every closed/merged -> open transition to defend against stale webhook redeliveries. But `reopened` webhooks carry a legitimate closed -> open transition, which the guard was trapping. Exempt `reopened` from the guard: when the action is reopened we apply excluded.pr_state directly. `opened`/`synchronize`/`edited` redeliveries remain blocked from demoting terminal state. Merged PRs cannot be reopened on GitHub, so merged -> open stays impossible either way. Adds a regression test covering closed -> reopened -> open. --------- Co-authored-by: Toast (gastown) <Toast@gastown.local>
…InfoDialog (#2917) * feat(cloud-agent-next): surface associated PR in ChatHeader + SessionInfoDialog Wires the new associatedPr field from getWithRuntimeState through the session manager into the chat UI. - ChatHeader: 'Open in GitHub' menu item now links to the PR when one exists (with an open/closed/merged state badge). Falls back to the existing compare URL or a plain repo browse URL when no PR is known or the git host isn't GitHub. Adds a 'Refresh PR info' action that calls refreshAssociatedPullRequest and shows a toast on TOO_MANY_REQUESTS / other errors. - SessionInfoDialog: new Pull Request row, with state badge and a 'Last checked ...' subtitle from pr_last_synced_at. - CloudChatPage: passes branch, gitUrl, associatedPr, and the refresh handler to ChatHeader (these props were previously missing, leaving the existing compare link dead). - FetchedSessionData: carries associatedPr so the UI updates in place after a manual refresh without re-fetching getWithRuntimeState. Adds unit tests for the pure link-resolution helper covering PR-open, PR-merged, PR-closed, PR-null, non-GitHub fallback, and PR-wins-over- non-GitHub-URL cases. * fix(cloud-agent-next): avoid stale fetched session data on PR refresh Reading fetchedSessionData from closure in handleRefreshPr meant a session switch during an in-flight refreshAssociatedPullRequest request would write the previous session's snapshot back into the shared atom when the mutation resolved. Capture the session id at call time, read the latest fetched data from a ref, and bail out if the active session changed while awaiting. --------- Co-authored-by: Maple (gastown) <Maple@gastown.local> Co-authored-by: Toast (gastown) <Toast@gastown.local>
- 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.
SWC compiles ESM exports as non-configurable, so jest.spyOn on the re-exported fetchPullRequestForBranch throws silently and returns undefined. Switch to jest.mock with a factory that swaps in a fresh jest.fn(), and update all call sites to drive the mock directly. Also add universal-github-app-jwt to transformIgnorePatterns so jest can load the @octokit/auth-app dependency chain when the webhook-handler test imports it transitively.
Main landed a 0107 migration (0107_classy_ultimo: kiloclaw tracked_image_tag), so this branch's migration was renumbered from 0107 to 0108. Migration contents are functionally equivalent to the pre-rebase 0107_clammy_cyclops — the UNIQUE INDEX on cli_sessions_v2.session_id is created before the FK on cli_session_pull_requests.session_id, preserving the fix from the superseded reorder commit.
kilo-code-bot
Bot
force-pushed
the
convoy/associated-pr-for-cloud-agent-next-sessi/dbccdbdf/head
branch
from
124c0b4 to
084f755
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.