From 3b9547c61269aaef561ab6784f484d18b3813001 Mon Sep 17 00:00:00 2001 From: maxtanh Date: Sun, 17 May 2026 21:25:27 -0700 Subject: [PATCH] feat(orchestrator): add single-cycle PR revision on CHANGES_REQUESTED review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a human submits a CHANGES_REQUESTED review on a symphony-go PR, the orchestrator now detects it via polling and runs a single revision cycle on the existing branch/worktree: 1. PollPRRevisions (new tick step) scans pr_ready jobs for fresh CHANGES_REQUESTED reviews submitted after the job's UpdatedAt. 2. runPRRevision drives the implementation agent with a revision prompt containing the original issue, approved plan, and review feedback. 3. The agent's changes are committed and pushed to the same branch, updating the PR in place (no close/reopen cycle). 4. Job.RevisionAttempted is set to true — only one revision cycle runs per PR, subsequent reviews are left for human follow-up. Changes: - internal/github: add PRReview type, ListPRReviews to Client interface - internal/github/fake: add SeedPRReview + ListPRReviews - internal/types: add RevisionAttempted field to Job - internal/orchestrator/loop: call PollPRRevisions in tick - internal/orchestrator/revision.go (new): polling + execution logic - internal/orchestrator/revision_test.go (new): 3 test cases --- internal/github/fake.go | 56 ++++-- internal/github/github.go | 51 +++++ internal/orchestrator/loop.go | 3 + internal/orchestrator/revision.go | 202 ++++++++++++++++++++ internal/orchestrator/revision_test.go | 251 +++++++++++++++++++++++++ internal/types/types.go | 9 +- 6 files changed, 559 insertions(+), 13 deletions(-) create mode 100644 internal/orchestrator/revision.go create mode 100644 internal/orchestrator/revision_test.go diff --git a/internal/github/fake.go b/internal/github/fake.go index 92c6b70..aee7d21 100644 --- a/internal/github/fake.go +++ b/internal/github/fake.go @@ -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 @@ -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, } } @@ -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 { diff --git a/internal/github/github.go b/internal/github/github.go index 9b9cf33..9ff9cf1 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -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 @@ -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 @@ -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) @@ -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 +} diff --git a/internal/orchestrator/loop.go b/internal/orchestrator/loop.go index f49e1e7..fcddfa2 100644 --- a/internal/orchestrator/loop.go +++ b/internal/orchestrator/loop.go @@ -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 diff --git a/internal/orchestrator/revision.go b/internal/orchestrator/revision.go new file mode 100644 index 0000000..ead4e13 --- /dev/null +++ b/internal/orchestrator/revision.go @@ -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 +} diff --git a/internal/orchestrator/revision_test.go b/internal/orchestrator/revision_test.go new file mode 100644 index 0000000..b8c6c91 --- /dev/null +++ b/internal/orchestrator/revision_test.go @@ -0,0 +1,251 @@ +package orchestrator + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + "time" + + gh "github.com/logosc/symphony-go/internal/github" + "github.com/logosc/symphony-go/internal/types" +) + +// installPRReadyJob seeds an issue at the pr-ready label and persists a +// matching local job carrying a non-zero PRNumber. The PR number is also +// seeded as a fake "issue" so PostIssueComment(ctx, prNumber, ...) works +// (real GitHub treats PRs as issues for comment purposes; the fake needs +// an explicit seed). +// +// updatedAt sets Job.UpdatedAt; pass time.Time{} to leave it zero (which +// disables the staleness filter on reviews). Returns the seeded job. +func installPRReadyJob(t *testing.T, h *testHarness, issueNum, prNumber int, updatedAt time.Time) *types.Job { + t.Helper() + iss := types.Issue{ + Number: issueNum, Title: "feature", Description: "context", State: "open", + Labels: []string{h.cfg.Labels.PRReady}, + } + h.gh.SeedIssue(iss, false) + // Seed the PR number as an issue too so issue-comments API calls + // targeting the PR number succeed in the fake. + h.gh.SeedIssue(types.Issue{ + Number: prNumber, Title: "[agent] feature", State: "open", + }, true) + job := &types.Job{ + IssueNumber: issueNum, + Repo: h.cfg.Repo.FullName, + Status: types.StatusPRReady, + WorkspaceRoot: t.TempDir(), + RepoPath: h.repo, + Branch: fmt.Sprintf("symphony/issue-%d-feature", issueNum), + PRNumber: prNumber, + PlanText: canonicalPlan([]string{"a.txt"}), + UpdatedAt: updatedAt, + } + if err := h.state.Save(job); err != nil { + t.Fatalf("save: %v", err) + } + return job +} + +// TestPollPRRevisions_HappyPath runs the full pipeline to pr-ready, then +// posts a CHANGES_REQUESTED review and asserts the revision cycle runs: +// the agent is invoked with the revision prompt, the diff is committed +// and pushed, RevisionAttempted is set, and a confirmation comment is +// posted on the PR. +func TestPollPRRevisions_HappyPath(t *testing.T) { + h := newTestHarness(t) + h.cfg.Approval.Mode = "handoff" + o := h.newOrch(t, "x", false) + + h.runner.Responses[types.PhasePlanning] = types.RunResult{ + Success: true, Text: canonicalPlan([]string{"a.txt"}), + } + implWriter(h.runner, []string{"a.txt"}) + + iss := h.seedReadyIssue(501, "fix things") + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + if err := o.ProcessIssue(ctx, iss); err != nil { + t.Fatalf("ProcessIssue: %v", err) + } + job, err := h.state.Load(501) + if err != nil { + t.Fatalf("load: %v", err) + } + if job.Status != types.StatusPRReady { + t.Fatalf("expected pr_ready before revision, got %q", job.Status) + } + prNumber := job.PRNumber + if prNumber == 0 { + t.Fatalf("expected non-zero PRNumber after ProcessIssue") + } + // Seed the PR number as an issue in the fake so PostIssueComment on + // the PR succeeds (the orchestrator targets the PR for revision + // comments, mirroring real GitHub where PRs are issues). + h.gh.SeedIssue(types.Issue{ + Number: prNumber, Title: "[agent] fix things", State: "open", + }, true) + + // Swap OnRun to a revision-aware writer. We assert on req.Prompt to + // confirm the orchestrator wired in the issue title, plan, and review + // body. The writer drops a fresh file so we can detect the agent's + // effect on the worktree. + var capturedPrompt string + h.runner.OnRun = func(ctx context.Context, req types.RunRequest) (types.RunResult, error) { + capturedPrompt = req.Prompt + if err := os.WriteFile(filepath.Join(req.RepoPath, "fix.txt"), + []byte("revision change\n"), 0o644); err != nil { + return types.RunResult{}, err + } + return types.RunResult{Success: true, Text: "done"}, nil + } + // Reset call log so we can assert "exactly one runner call during + // revision" without counting the prior planning + impl invocations. + h.runner.Reset() + + // Give the review a SubmittedAt that's unambiguously after job.UpdatedAt + // on coarse-clock systems. + time.Sleep(5 * time.Millisecond) + h.gh.SeedPRReview(prNumber, gh.PRReview{ + User: "alice", + State: "CHANGES_REQUESTED", + Body: "Please rename the helper to follow our naming convention.", + SubmittedAt: time.Now().UTC(), + }) + + if err := o.PollPRRevisions(ctx); err != nil { + t.Fatalf("PollPRRevisions: %v", err) + } + + // Job: RevisionAttempted=true, status unchanged. + got, err := h.state.Load(501) + if err != nil { + t.Fatalf("load after revision: %v", err) + } + if !got.RevisionAttempted { + t.Errorf("expected RevisionAttempted=true") + } + if got.Status != types.StatusPRReady { + t.Errorf("status = %q, want pr_ready", got.Status) + } + + // Runner: exactly one revision call. + calls := h.runner.Calls() + if len(calls) != 1 { + t.Fatalf("runner calls during revision = %d, want 1: %+v", len(calls), calls) + } + if calls[0].Phase != types.PhaseImplementation { + t.Errorf("revision call phase = %q, want implementation", calls[0].Phase) + } + + // Prompt: contains review body, issue title, and plan text. + if !strings.Contains(capturedPrompt, "Please rename the helper") { + t.Errorf("revision prompt missing review body; got:\n%s", capturedPrompt) + } + if !strings.Contains(capturedPrompt, "fix things") { + t.Errorf("revision prompt missing issue title; got:\n%s", capturedPrompt) + } + if !strings.Contains(capturedPrompt, "files_touched") { + t.Errorf("revision prompt missing plan text; got:\n%s", capturedPrompt) + } + if !strings.Contains(capturedPrompt, "REVISION phase") { + t.Errorf("revision prompt missing REVISION marker; got:\n%s", capturedPrompt) + } + + // Worktree: agent's file was committed (no uncommitted changes). + if _, statErr := os.Stat(filepath.Join(got.RepoPath, "fix.txt")); statErr != nil { + t.Errorf("fix.txt not present in worktree: %v", statErr) + } + statusOut, _ := gitStatusPorcelain(ctx, got.RepoPath) + if strings.TrimSpace(statusOut) != "" { + t.Errorf("expected clean worktree after revision, got:\n%s", statusOut) + } + + // Comment: confirmation posted on the PR. + comments, err := h.gh.ListIssueComments(ctx, prNumber, time.Time{}) + if err != nil { + t.Fatalf("ListIssueComments(pr): %v", err) + } + if !containsBody(comments, "pushed revision addressing review feedback") { + t.Errorf("expected confirmation comment on PR #%d; got %+v", prNumber, comments) + } +} + +// TestPollPRRevisions_RevisionAttemptedSkips verifies that a job with +// RevisionAttempted=true is skipped even when a fresh CHANGES_REQUESTED +// review is present. +func TestPollPRRevisions_RevisionAttemptedSkips(t *testing.T) { + h := newTestHarness(t) + o := h.newOrch(t, "x", false) + + job := installPRReadyJob(t, h, 502, 1502, time.Now().UTC()) + job.RevisionAttempted = true + if err := h.state.Save(job); err != nil { + t.Fatalf("save: %v", err) + } + + time.Sleep(5 * time.Millisecond) + h.gh.SeedPRReview(1502, gh.PRReview{ + User: "alice", State: "CHANGES_REQUESTED", + Body: "more changes", SubmittedAt: time.Now().UTC(), + }) + + if err := o.PollPRRevisions(context.Background()); err != nil { + t.Fatalf("PollPRRevisions: %v", err) + } + + if calls := h.runner.Calls(); len(calls) != 0 { + t.Errorf("expected runner not called (RevisionAttempted skip), got %d calls", len(calls)) + } + got, _ := h.state.Load(502) + if !got.RevisionAttempted { + t.Errorf("RevisionAttempted should remain true") + } + // No revision comment should have been posted on the PR. + comments, _ := h.gh.ListIssueComments(context.Background(), 1502, time.Time{}) + if containsBody(comments, "pushed revision") || containsBody(comments, "revision agent") { + t.Errorf("unexpected revision comment posted: %+v", comments) + } +} + +// TestPollPRRevisions_StaleReviewIgnored verifies that a review whose +// SubmittedAt is before the job's UpdatedAt is treated as stale and the +// revision cycle is not triggered. +func TestPollPRRevisions_StaleReviewIgnored(t *testing.T) { + h := newTestHarness(t) + o := h.newOrch(t, "x", false) + + jobUpdatedAt := time.Now().UTC() + installPRReadyJob(t, h, 503, 1503, jobUpdatedAt) + + // Review submitted one hour BEFORE the job last advanced. + h.gh.SeedPRReview(1503, gh.PRReview{ + User: "alice", State: "CHANGES_REQUESTED", + Body: "old feedback", SubmittedAt: jobUpdatedAt.Add(-1 * time.Hour), + }) + + if err := o.PollPRRevisions(context.Background()); err != nil { + t.Fatalf("PollPRRevisions: %v", err) + } + + if calls := h.runner.Calls(); len(calls) != 0 { + t.Errorf("expected runner not called for stale review, got %d calls", len(calls)) + } + got, _ := h.state.Load(503) + if got.RevisionAttempted { + t.Errorf("RevisionAttempted should remain false for stale-only reviews") + } +} + +// containsBody reports whether any comment's body contains substr. +func containsBody(comments []types.IssueComment, substr string) bool { + for _, c := range comments { + if strings.Contains(c.Body, substr) { + return true + } + } + return false +} diff --git a/internal/types/types.go b/internal/types/types.go index c916928..d04aba6 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -108,8 +108,13 @@ type Job struct { ApprovalToken string `json:"approval_token,omitempty"` ReviewerDecision *ReviewerDecision `json:"reviewer_decision,omitempty"` PRNumber int `json:"pr_number,omitempty"` - Attempt int `json:"attempt"` - UpdatedAt time.Time `json:"updated_at"` + // RevisionAttempted is set to true once a single PR-revision cycle has + // run on this job (triggered by a CHANGES_REQUESTED review). The + // orchestrator only revises a PR once — subsequent CHANGES_REQUESTED + // reviews are ignored. + RevisionAttempted bool `json:"revision_attempted,omitempty"` + Attempt int `json:"attempt"` + UpdatedAt time.Time `json:"updated_at"` // AxisKey is the per-axis label this job was frozen against at claim // time. "default" when no per-axis map is configured or no concrete // label matched. Empty on jobs persisted before the per-axis feature