feat(session): support multiple PRs per session#230
Conversation
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
A session can now own several pull requests (a root plus stacked
children) instead of being capped at one. The SQLite schema was already
1-session->many-PR (pr.url PK, session_id a plain FK), so this is a
behavioural change across the observe -> persist -> derive -> react
pipeline, not a migration.
- observe: the SCM observer discovers every open PR whose source branch
matches a session branch or descends from it ("branch/..." stacking),
attributing each to the owning session; the longest matching branch
wins so a child session claims its own stacked PRs.
- derive: session status is a worst-wins aggregate over all owned PRs,
with a stack model (B is a child of A iff B.target == A.source and A is
open) exposed via prs[] on every session read DTO.
- react: per-PR reactions; a stacked child blocked by an open parent is
exempt from the rebase/merge-conflict nudge (only the bottom of the
stack is eligible), and the session completes only when no PR is open
and at least one merged.
- tests: unit coverage across stack/status/observer/lifecycle, a
real-SQLite ListPRFactsForSession test for the stacked-PR read path,
and a functional end-to-end integration test driving the real store +
lifecycle + observer through attribution, completion, and stacked-child
nudge suppression.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
f866099 to
0005563
Compare
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
illegalcall
left a comment
There was a problem hiding this comment.
Two correctness issues to address before merging:
-
backend/internal/lifecycle/reactions.go: session completion should account for PRs discovered in the same poll before terminating. If a terminal PR observation is processed before a newly discovered sibling/child is persisted, the session can terminate while an open PR from that same poll is still pending. -
backend/internal/adapters/scm/github/observer_provider.go: branch-prefix attribution should ignore forked PR heads.head.refalone can misattribute a fork PR whose branch name matches an AO session branch; carry/filter by head repo owner/full name so only PRs from the project repo are auto-claimed.
…siblings before completion Branch-prefix attribution now requires a discovered PR's head branch to live in the project repo. A fork PR can reuse a session's branch name while its commits live in the fork, so the previous code could auto-claim foreign work. Carry head repo full_name from the REST list response and skip any PR whose head repo is not the base repo. discoverNewPRs also writes each newly discovered PR as an open baseline row before the refresh/lifecycle pass runs. A session can own several PRs, and a terminal observation triggers a completion check that reads all of the session's PRs from the store. Without the early write, an open sibling found in the same poll was not yet durable and the session could terminate while that PR was still open. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
addressed |
illegalcall
left a comment
There was a problem hiding this comment.
Two remaining review notes:
-
backend/internal/service/session/status.go: stack-blocked child PRs are excluded from status aggregation entirely. That hides actionable child signals like failing CI, draft state, requested changes, or unresolved comments. The stack exemption should only suppress mergeability/conflict readiness for blocked children, not CI/review/draft signals. -
backend/internal/session_manager/manager.go: the worker prompt says independent PRs can “branch off your base branch as usual,” but auto-attribution only matches the session branch orsession-branch/...descendants. Please clarify that independent PRs may target the base branch, but their source branches still need to stay under the session namespace.
The previous same-poll completion and fork-attribution issues look fixed in the latest commit.
…n; clarify worker prompt Status aggregation previously dropped any open PR blocked by an open parent, hiding actionable child signals (failing CI, draft, requested changes, unresolved comments) behind the parent's status. A blocked child still cannot merge, so its readiness signals (mergeable/approved/review-pending/open) stay suppressed, but its problem signals now contribute to the worst-wins aggregate. The all-blocked fallback is preserved so a session never goes dark. The worker multi-PR prompt said independent PRs could branch off the base branch as usual, which conflicts with branch-prefix attribution. Clarify that a PR may target the base branch, but its source branch must stay under the session branch namespace for AO to track it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
addressed |
Summary
Lets a single session own several pull requests (a root plus stacked children) instead of being capped at one. The SQLite schema was already 1-session→many-PR (
pr.urlPK,session_ida plain FK withidx_pr_session, no UNIQUE onsession_id), so this is a behavioural change across the observe → persist → derive → react pipeline, not a migration.State & user-action flow
The session's display status is never stored — it is derived at read time from durable PR facts. One full loop:
feat/x), optionally stacked children on thefeat/x/...convention.observe/scm): each poll lists the repo's open PRs, attributes every PR whose source branch equals the session branch or descends from it to the owning session (longest-prefix wins), and persists theprrow including the source/target branch pair.service/session/status.go, read-time): session status = worst-wins aggregate over the open PRs, by severityci_failed > changes_requested > draft > review_pending > pr_open > approved > mergeable. A stacked child whose parent is still open is excluded from the aggregate (it cannot merge until the parent does). The stack model + per-PR facts are surfaced asprs[]on every session DTO, so the dashboard shows each PR's state.lifecycle/reactions.go): per-PR nudges to the agent — CI failing → "fix CI" (with log tail); review changes/unresolved comments → "address feedback"; merge conflict → "rebase", but only the bottom of the stack; a stacked child blocked by an open parent is suppressed. Dedup signature is persisted per PR so a daemon restart never re-fires.prs[]the user/agent fixes CI, addresses review, or rebases and pushes; the next poll re-observes and re-derives. No status is ever written back — only PR facts are.merged) only when no PR is open and at least one merged. A single PR merging while another stays open keeps the session alive.What changed, by layer
observe/scm/observer.go): one refresh subject per open tracked PR;discoverNewPRs+matchSessiondo the branch-prefix attribution (longest match wins so a child session claims its own stacked PRs).service/session/stack.go,status.go): worst-wins aggregation + stack model (B is a child of A iffB.target_branch == A.source_branchand A is open), surfaced asprs[].lifecycle/reactions.go): per-PR reactions, stacked-child nudge suppression, andsessionComplete(all closed/merged + ≥1 merged).httpd/controllers/dto.go,sessions.go, regeneratedopenapi.yaml+frontend/src/api/schema.ts):prs[]on the session views viaSessionView.Tests
stack,status, observer multi-subject discovery, and lifecycle per-PR reactions/completion.store/pr_facts_test.go) for the stacked-PR read path:ListPRFactsForSessionreturns every owned PR newest-first with state flags and branch pair preserved (the data the stack aggregator builds on).integration/scm_observer_test.go,TestSCMObserverMultiPREndToEnd) drives the realsqlite.Store+lifecycle.Manager+observe/scm.Observer(canned provider only) through: (1) one session owning its root and stacked child from a single repo list with branch pairs persisted; (2) staying alive while a stacked PR is open and terminating only once all merge; (3) a stacked child blocked by an open parent suppressed while the stack bottom is nudged (asserted down to the persisted dedup signature).Test plan
go build ./...cleango vet ./...cleango test ./...green (53 packages, exit 0)TestSCMObserverMultiPREndToEndpasses (attribution / completion / nudge suppression)🤖 Generated with Claude Code