Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 45 additions & 11 deletions internal/github/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@ type InMemoryFake struct {
permissions map[string]string
prs []fakePR
reactions map[int64][]string
prReviews map[int][]PRReview

// next monotonic IDs for created comments / PRs.
nextCommentID int64
nextPRNumber int
// next monotonic IDs for created comments / PRs / reviews.
nextCommentID int64
nextPRNumber int
nextPRReviewID int64
}

// NewInMemoryFake constructs an empty InMemoryFake bound to the given
Expand All @@ -52,14 +54,16 @@ func NewInMemoryFake(fullName string) *InMemoryFake {
panic(err)
}
return &InMemoryFake{
owner: owner,
repo: repo,
issues: make(map[int]*fakeIssue),
comments: make(map[int][]types.IssueComment),
permissions: make(map[string]string),
reactions: make(map[int64][]string),
nextCommentID: 1,
nextPRNumber: 1000,
owner: owner,
repo: repo,
issues: make(map[int]*fakeIssue),
comments: make(map[int][]types.IssueComment),
permissions: make(map[string]string),
reactions: make(map[int64][]string),
prReviews: make(map[int][]PRReview),
nextCommentID: 1,
nextPRNumber: 1000,
nextPRReviewID: 1,
}
}

Expand Down Expand Up @@ -284,6 +288,36 @@ func (f *InMemoryFake) AddReaction(_ context.Context, commentID int64, reaction
return nil
}

// SeedPRReview appends a review to a PR. If r.ID is zero, a new ID is
// assigned. If r.SubmittedAt is zero, time.Now() is used.
func (f *InMemoryFake) SeedPRReview(prNumber int, r PRReview) PRReview {
f.mu.Lock()
defer f.mu.Unlock()
if r.ID == 0 {
r.ID = f.nextPRReviewID
f.nextPRReviewID++
} else if r.ID >= f.nextPRReviewID {
f.nextPRReviewID = r.ID + 1
}
if r.SubmittedAt.IsZero() {
r.SubmittedAt = time.Now().UTC()
}
f.prReviews[prNumber] = append(f.prReviews[prNumber], r)
return r
}

// ListPRReviews implements Client. Returned reviews are sorted by
// SubmittedAt ascending (oldest first).
func (f *InMemoryFake) ListPRReviews(_ context.Context, prNumber int) ([]PRReview, error) {
f.mu.Lock()
defer f.mu.Unlock()
rs := f.prReviews[prNumber]
out := make([]PRReview, len(rs))
copy(out, rs)
sort.Slice(out, func(i, j int) bool { return out[i].SubmittedAt.Before(out[j].SubmittedAt) })
return out, nil
}

// containsLabel does a case-insensitive lookup in a label slice. The
// slice is expected to already be lowercase.
func containsLabel(labels []string, want string) bool {
Expand Down
51 changes: 51 additions & 0 deletions internal/github/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ type PullRequest struct {
State string
}

// PRReview is a normalized PR review returned by Client.ListPRReviews.
// State is one of "APPROVED", "CHANGES_REQUESTED", "COMMENTED", "DISMISSED",
// or "PENDING" (mirroring GitHub's review states).
type PRReview struct {
ID int64
User string
State string
Body string
SubmittedAt time.Time
}

// Client is the minimal GitHub surface used by symphony-go.
type Client interface {
// ListReadyIssues returns open issues carrying readyLabel. Pull
Expand Down Expand Up @@ -61,6 +72,8 @@ type Client interface {
// FindPRsByHead lists open PRs whose head branch matches the given
// branch name (in the repo's owner namespace).
FindPRsByHead(ctx context.Context, headBranch string) ([]PullRequest, error)
// ListPRReviews returns every review submitted on a PR, oldest first.
ListPRReviews(ctx context.Context, prNumber int) ([]PRReview, error)
// AddReaction adds a reaction (e.g. "+1", "-1", "eyes") to an issue
// comment.
AddReaction(ctx context.Context, commentID int64, reaction string) error
Expand Down Expand Up @@ -311,6 +324,28 @@ func (r *realClient) FindPRsByHead(ctx context.Context, headBranch string) ([]Pu
return out, nil
}

func (r *realClient) ListPRReviews(ctx context.Context, prNumber int) ([]PRReview, error) {
opts := &gh.ListOptions{PerPage: 100}
var out []PRReview
for {
reviews, resp, err := r.c.PullRequests.ListReviews(ctx, r.owner, r.repo, prNumber, opts)
if err != nil {
return nil, fmt.Errorf("github: list PR reviews on #%d: %w", prNumber, err)
}
for _, rv := range reviews {
if rv == nil {
continue
}
out = append(out, normalizePRReview(rv))
}
if resp == nil || resp.NextPage == 0 {
break
}
opts.Page = resp.NextPage
}
return out, nil
}

func (r *realClient) AddReaction(ctx context.Context, commentID int64, reaction string) error {
if _, _, err := r.c.Reactions.CreateIssueCommentReaction(ctx, r.owner, r.repo, commentID, reaction); err != nil {
return fmt.Errorf("github: add reaction %q to comment %d: %w", reaction, commentID, err)
Expand Down Expand Up @@ -367,3 +402,19 @@ func normalizePR(p *gh.PullRequest) PullRequest {
State: p.GetState(),
}
}

// normalizePRReview converts a go-github *PullRequestReview into PRReview.
func normalizePRReview(rv *gh.PullRequestReview) PRReview {
out := PRReview{
ID: rv.GetID(),
Body: rv.GetBody(),
State: rv.GetState(),
}
if rv.User != nil && rv.User.Login != nil {
out.User = *rv.User.Login
}
if rv.SubmittedAt != nil {
out.SubmittedAt = rv.SubmittedAt.Time
}
return out
}
3 changes: 3 additions & 0 deletions internal/orchestrator/loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ func (o *Orchestrator) Run(ctx context.Context, once bool) error {
if err := o.PollApprovals(ctx); err != nil {
o.deps.Logger.Error("tick: PollApprovals", "err", err)
}
if err := o.PollPRRevisions(ctx); err != nil {
o.deps.Logger.Error("tick: PollPRRevisions", "err", err)
}
max := o.deps.Config.Orchestrator.MaxConcurrentJobs
if max < 1 {
max = 1
Expand Down
202 changes: 202 additions & 0 deletions internal/orchestrator/revision.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
package orchestrator

import (
"bytes"
"context"
"fmt"
"os"
"os/exec"
"strings"
"time"

"github.com/logosc/symphony-go/internal/config"
"github.com/logosc/symphony-go/internal/github"
"github.com/logosc/symphony-go/internal/types"
)

// revisionSuffixTmpl is appended to the rendered WORKFLOW.md prompt when
// the orchestrator drives a single revision cycle in response to a
// CHANGES_REQUESTED PR review. Format args (in order): issue title,
// issue description, approved plan text, review feedback body.
const revisionSuffixTmpl = `

---

You are in REVISION phase. A reviewer has requested changes on your pull request.

Original issue:
Title: %s
Description: %s

Your approved plan:
%s

Review feedback requesting changes:
%s

Address the reviewer's feedback by making the necessary code changes. Focus only on what the reviewer asked for.
`

// PollPRRevisions services pr_ready jobs whose PRs have received a
// CHANGES_REQUESTED review since the job was finalized. Each job is
// revised at most once; Job.RevisionAttempted guards a second pass. The
// job stays in pr_ready regardless of revision outcome.
func (o *Orchestrator) PollPRRevisions(ctx context.Context) error {
jobs, err := o.deps.State.List()
if err != nil {
return err
}
for _, job := range jobs {
if job.Status != types.StatusPRReady {
continue
}
if job.RevisionAttempted {
continue
}
if job.PRNumber == 0 {
continue
}
if err := o.servicePRRevision(ctx, job); err != nil {
o.deps.Logger.Error("revision: service", "issue", job.IssueNumber, "err", err)
}
}
return nil
}

// servicePRRevision looks for a CHANGES_REQUESTED review on job.PRNumber
// submitted after job.UpdatedAt, and if one is found, runs a single
// revision cycle under a claim.
func (o *Orchestrator) servicePRRevision(ctx context.Context, job *types.Job) error {
reviews, err := o.deps.GitHub.ListPRReviews(ctx, job.PRNumber)
if err != nil {
return err
}
since := job.UpdatedAt
var match *github.PRReview
for i := range reviews {
r := reviews[i]
if !strings.EqualFold(r.State, "CHANGES_REQUESTED") {
continue
}
if !since.IsZero() && !r.SubmittedAt.After(since) {
continue
}
if o.isIgnoredApprovalUser(r.User) {
continue
}
match = &r
break
}
if match == nil {
return nil
}
if !o.tryClaim(job.IssueNumber) {
return nil
}
defer o.release(job.IssueNumber)
return o.runPRRevision(ctx, job, *match)
}

// runPRRevision executes the revision cycle: render a revision prompt,
// drive the implementation agent on the existing worktree, commit any
// uncommitted changes, push the branch (which updates the existing PR in
// place), and post a status comment on the PR. Always sets
// RevisionAttempted=true and re-saves the job so the cycle never repeats.
// The job's status (pr_ready) and label are intentionally left alone.
func (o *Orchestrator) runPRRevision(ctx context.Context, job *types.Job, review github.PRReview) error {
cfg := o.deps.Config
log := o.deps.Logger.With("issue", job.IssueNumber, "pr", job.PRNumber)

finish := func(comment string) error {
if comment != "" {
_, _ = o.deps.GitHub.PostIssueComment(ctx, job.PRNumber, comment)
}
job.RevisionAttempted = true
return o.saveJob(job)
}

issue, err := o.deps.GitHub.GetIssue(ctx, job.IssueNumber)
if err != nil {
return finish(fmt.Sprintf("[symphony-go] revision: get issue failed: %v", err))
}

layout := o.layoutForJob(job)
if _, statErr := os.Stat(layout.RepoPath); statErr != nil {
log.Warn("revision_worktree_missing", "path", layout.RepoPath, "err", statErr)
return finish("[symphony-go] revision skipped: worktree no longer on disk.")
}

rendered, rerr := config.RenderPrompt(o.promptBodyFor(job.AxisKey), issue, job.Attempt)
if rerr != nil {
return finish(fmt.Sprintf("[symphony-go] revision: render prompt failed: %v", rerr))
}
revPrompt := rendered + fmt.Sprintf(revisionSuffixTmpl,
issue.Title, issue.Description, job.PlanText, review.Body)

headBefore, _ := gitRevParseHead(ctx, layout.RepoPath)

log.Info("revision_started", "reviewer", review.User, "review_id", review.ID)
revReq := types.RunRequest{
Issue: issue,
RepoPath: layout.RepoPath,
HomePath: layout.HomePath,
Prompt: revPrompt,
Phase: types.PhaseImplementation,
Timeout: time.Duration(cfg.Agent.TimeoutSeconds) * time.Second,
AxisKey: job.AxisKey,
}
revResult, turns, runErr := o.runImplementationAgent(ctx, log, job, revReq)
log.Info("revision_completed", "success", revResult.Success, "turns", turns, "err", runErr)

if runErr != nil {
return finish(fmt.Sprintf("[symphony-go] revision agent error; PR unchanged.\n\n%s",
truncate(summarizeRunFailure(revResult), 1500)))
}
if !revResult.Success {
return finish(fmt.Sprintf("[symphony-go] revision agent reported failure; PR unchanged.\n\n%s",
truncate(summarizeRunFailure(revResult), 1500)))
}

statusOut, sErr := gitStatusPorcelain(ctx, layout.RepoPath)
if sErr != nil {
return finish(fmt.Sprintf("[symphony-go] revision: git status failed: %v", sErr))
}
hasUncommitted := strings.TrimSpace(statusOut) != ""
headAfter, _ := gitRevParseHead(ctx, layout.RepoPath)
agentCommitted := headBefore != "" && headAfter != "" && headBefore != headAfter
if !hasUncommitted && !agentCommitted {
return finish("[symphony-go] revision agent produced no diff; PR unchanged.")
}

if hasUncommitted {
msg := fmt.Sprintf("Address review feedback on PR #%d", job.PRNumber)
if err := gitCommitAll(ctx, layout.RepoPath, msg); err != nil {
return finish(fmt.Sprintf("[symphony-go] revision: commit failed: %v", err))
}
}

pushToken, tokErr := o.resolveGitHubToken(ctx)
if tokErr != nil {
return finish(fmt.Sprintf("[symphony-go] revision: resolve token failed: %v", tokErr))
}
if err := o.deps.PushFunc(ctx, layout.RepoPath, job.Branch, pushToken); err != nil {
return finish(fmt.Sprintf("[symphony-go] revision: push failed: %v", err))
}

log.Info("revision_pushed")
return finish("[symphony-go] pushed revision addressing review feedback")
}

// gitRevParseHead returns the current HEAD SHA at repoPath. Returns the
// empty string and the underlying error on failure (callers can treat an
// empty result as "unknown" for diff-detection purposes).
func gitRevParseHead(ctx context.Context, repoPath string) (string, error) {
cmd := exec.CommandContext(ctx, "git", "-C", repoPath, "rev-parse", "HEAD")
var out, errb bytes.Buffer
cmd.Stdout = &out
cmd.Stderr = &errb
if err := cmd.Run(); err != nil {
return "", fmt.Errorf("%w: %s", err, errb.String())
}
return strings.TrimSpace(out.String()), nil
}
Loading