feat(backend): wire PR engine into the daemon#87
Open
Pritom14 wants to merge 2 commits into
Open
Conversation
Contributor
Greptile SummaryThis PR wires the previously-built but dormant PR observation subsystem end-to-end into the running daemon: the lifecycle manager now receives a real runtime messenger instead of
Confidence Score: 5/5Safe to merge — wires a previously-built, tested subsystem into the daemon with correct shutdown ordering and graceful token-absent degradation. The change activates a path that was already fully unit- and integration-tested. Token resolution, per-session error isolation, and shutdown sequencing are all handled correctly. No new defects were found beyond the two diagnostic-quality observations already raised in prior threads. No files require special attention. The two pre-existing observations in the review thread (prpoller.go log levels, repo_resolver.go stderr capture) are worth tracking but do not block the merge. Important Files Changed
Sequence DiagramsequenceDiagram
participant D as daemon.go
participant PP as prpoller.Poller
participant RR as gitRepoResolver
participant GH as github.Provider
participant PRM as pr.Manager
participant LCM as lifecycle.Manager
participant MSG as runtimeMessenger
D->>D: startPRPoller: probe token chain
D->>PP: New + Start(ctx)
loop every 30s
PP->>PP: ListAllSessions
loop per non-terminated session with branch
PP->>RR: RepoIdent(projectID)
RR-->>PP: owner, repo
PP->>GH: FindPRForBranch(owner,repo,branch)
GH-->>PP: prURL, found
alt found
PP->>GH: Observe(prURL)
GH-->>PP: PRObservation
PP->>PRM: ApplyObservation(sessionID,obs)
PRM->>LCM: ApplyPRObservation(sessionID,obs)
alt nudge condition
LCM->>MSG: Send(sessionID,message)
end
end
end
end
D->>D: cancel ctx
D->>PP: prStack.Stop()
D->>LCM: lcStack.Stop()
Reviews (2): Last reviewed commit: "test(backend): add end-to-end functional..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+161
to
+176
| prURL, found, err := p.discoverer.FindPRForBranch(ctx, ident.owner, ident.repo, sess.Metadata.Branch) | ||
| if err != nil { | ||
| p.logger.Debug("prpoller: PR discovery failed", | ||
| "session", sess.ID, "branch", sess.Metadata.Branch, "err", err) | ||
| return | ||
| } | ||
| if !found { | ||
| return | ||
| } | ||
|
|
||
| obs, err := p.observer.Observe(ctx, prURL) | ||
| if err != nil { | ||
| p.logger.Debug("prpoller: PR observation failed", | ||
| "session", sess.ID, "pr", prURL, "err", err) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
Debug level, so they are invisible under default Info-level production logging. GitHub rate-limit errors (HTTP 429/403), expired tokens, or network failures would all be silently swallowed, making it very difficult to diagnose why agents stop receiving CI nudges in production.
Suggested change
| prURL, found, err := p.discoverer.FindPRForBranch(ctx, ident.owner, ident.repo, sess.Metadata.Branch) | |
| if err != nil { | |
| p.logger.Debug("prpoller: PR discovery failed", | |
| "session", sess.ID, "branch", sess.Metadata.Branch, "err", err) | |
| return | |
| } | |
| if !found { | |
| return | |
| } | |
| obs, err := p.observer.Observe(ctx, prURL) | |
| if err != nil { | |
| p.logger.Debug("prpoller: PR observation failed", | |
| "session", sess.ID, "pr", prURL, "err", err) | |
| return | |
| } | |
| prURL, found, err := p.discoverer.FindPRForBranch(ctx, ident.owner, ident.repo, sess.Metadata.Branch) | |
| if err != nil { | |
| p.logger.Warn("prpoller: PR discovery failed", | |
| "session", sess.ID, "branch", sess.Metadata.Branch, "err", err) | |
| return | |
| } | |
| if !found { | |
| return | |
| } | |
| obs, err := p.observer.Observe(ctx, prURL) | |
| if err != nil { | |
| p.logger.Warn("prpoller: PR observation failed", | |
| "session", sess.ID, "pr", prURL, "err", err) | |
| return | |
| } |
Comment on lines
+49
to
+55
| func gitOriginURL(ctx context.Context, repoPath string) (string, error) { | ||
| out, err := exec.CommandContext(ctx, "git", "-C", repoPath, "remote", "get-url", "origin").Output() | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| return string(out), nil | ||
| } |
Contributor
There was a problem hiding this comment.
exec.CommandContext(...).Output() only captures stdout; when git exits non-zero the error is just exit status 128 with no diagnostic text. Capturing ExitError.Stderr would surface the actual git message, making resolver failures much easier to diagnose in production.
Suggested change
| func gitOriginURL(ctx context.Context, repoPath string) (string, error) { | |
| out, err := exec.CommandContext(ctx, "git", "-C", repoPath, "remote", "get-url", "origin").Output() | |
| if err != nil { | |
| return "", err | |
| } | |
| return string(out), nil | |
| } | |
| func gitOriginURL(ctx context.Context, repoPath string) (string, error) { | |
| cmd := exec.CommandContext(ctx, "git", "-C", repoPath, "remote", "get-url", "origin") | |
| out, err := cmd.Output() | |
| if err != nil { | |
| if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 { | |
| return "", fmt.Errorf("%w: %s", err, strings.TrimSpace(string(ee.Stderr))) | |
| } | |
| return "", err | |
| } | |
| return string(out), nil | |
| } |
The PR observation subsystem was fully built and unit tested but never wired into the running daemon: the LCM was constructed with a nil messenger and a noopMessenger, and nothing produced PR observations. This makes the engine live end to end via branch self-discovery, with graceful degradation when no GitHub token is configured. - Real runtime messenger resolves a session's runtime handle and pastes via the zellij runtime, backing both human /send and lifecycle nudges. - FindPRForBranch discovery primitive on the GitHub provider. - PR poller that discovers, observes, persists, and drives nudges. - Daemon wiring: provider + PR service + poller goroutine with lifecycle-managed shutdown; disabled with a warning when no token. Closes #86 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Drive the wired poller against a fake GitHub (real github.Provider for discovery and observation), a real sqlite store, a real Lifecycle Manager, and the real PR service. Asserts a CI-failing PR flows discovery -> observe -> persist -> derived session status -> lifecycle nudge, and that a branch with no open PR stays quiet. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pritom14
force-pushed
the
feat/pr-engine-wiring
branch
from
664f7a1 to
5037b3c
Compare
This was referenced Jun 5, 2026
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
2 participants
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.
Closes #86
Summary
The PR observation subsystem (
service/pr.Manager,lifecycle/reactions.go,scm/github.Provider) was fully built and unit tested, but it was never wired into the running daemon. The daemon constructed the Lifecycle Manager with a nil messenger plus anoopMessenger, and nothing produced PR observations, so the entire path was dark in production. This PR makes the engine live end to end.Changes made
daemon/lifecycle_wiring.go,daemon.go). feat(cli): add minimal ao send #83 (now on main) added the runtime messenger that pastes a message into a session's zellij pane and wired it intostartSessionfor human/send. This PR threads that same messenger intostartLifecyclesolifecycle.New(store, messenger)gets a real sender instead of nil, which is what makes PR nudges actually reach the agent's pane. The messenger is built before the LCM so both consumers share one instance.scm/github/discover.go).FindPRForBranch(owner, repo, branch)queries the open PR whose head is the session's branch. Afound=falseresult with a nil error is the normal pre-PR state, not a failure.observe/prpoller/). A reaper-style timer (30s). Each tick lists non-terminated sessions that have a branch, resolves owner/repo per project, discovers the PR, observes it, and feeds the observation into the PR service. Per-session errors are logged and skipped so one bad session cannot stall the loop.daemon/pr_wiring.go,daemon/repo_resolver.go,daemon.go). Builds the provider (token chain: AO_GITHUB_TOKEN, then GITHUB_TOKEN, thengh auth token), the PR service over the shared store and LCM, a git-backed owner/repo resolver that parses the project's origin remote, and the poller goroutine with lifecycle-managed shutdown.Why
Without this, a session could open a PR and have CI fail, yet the agent was never nudged because no component produced observations and the messenger was a no-op. The branch self-discovery approach is used because
domain.SessionMetadataalready storesBranch, so the poller can find a session's PR via the GitHub API without a separate hook-based capture path.Graceful degradation: when no GitHub token resolves, the poller is disabled with a warning and the daemon still starts normally.
Flow diagram (textual)
Test plan
internal/integration/prpoller_functional_test.go): drives the wired poller against a fake GitHub with the realgithub.Provider, a real sqlite store, a real Lifecycle Manager, and the real PR service. Asserts a CI-failing PR flows discovery, observe, persist, derivedci_failedstatus, and a lifecycle nudge carrying the failing log tail. A second case asserts a branch with no open PR stays quiet.This branch was rebased onto main to pick up #83 (
ao send), which independently introduced the runtime messenger. The duplicate messenger this PR originally carried was dropped in favor of #83's, and only the lifecycle-wiring delta (passing the messenger into the LCM) plus the poller wiring remain.Note:
internal/terminalreal-zellij testTestSessionStreamsRealZellijPanefails locally on macOS due to the$TMPDIRIPC socket path exceeding zellij's 103-byte limit. It is unrelated to this change (the terminal package is untouched).