703 lines
24 KiB
Go
703 lines
24 KiB
Go
package agents
|
||
|
||
import (
|
||
"context"
|
||
"encoding/json"
|
||
"errors"
|
||
"fmt"
|
||
"log/slog"
|
||
"math"
|
||
"net/http"
|
||
"os"
|
||
"path/filepath"
|
||
"strings"
|
||
"time"
|
||
|
||
"g.fc.run/ai/byre/internal/api"
|
||
"g.fc.run/ai/byre/internal/git"
|
||
"g.fc.run/ai/byre/internal/mcp"
|
||
"g.fc.run/ai/byre/internal/store"
|
||
"g.fc.run/ai/byre/internal/workflow"
|
||
)
|
||
|
||
const maxReviewFiles = 200
|
||
|
||
var severities = []string{"critical", "high", "medium", "low", "info"}
|
||
|
||
const reviewGuidance = `You are a reviewer running inside byre. Another agent is doing a task; you review all of its work so far, and a human may read your review in byre's app.
|
||
- Your working directory is a detached checkout of the snapshot under review. Read, build and run whatever you need there, but don't edit, commit or push: the checkout is discarded when you finish, and the task's own worktree belongs to its agent.
|
||
- Report with byre's submit_review tool, exactly once, then end your turn with one line. A review that isn't submitted is lost.
|
||
- Start the review with ` + "`change`" + `: what the change does, in a few sentences, for someone who hasn't read it.
|
||
- Each finding says what is wrong, where (path and line), why it matters and what to do about it. Leave out praise and nits. No findings is a fine result.
|
||
- With no findings, the summary is exactly "No significant findings." and nothing else. With findings, the summary is a bulleted Markdown list, one line per finding, and nothing else.
|
||
- Show rather than quote. Give a finding's file and lines and byre shows that part of the diff, so don't paste code. When a picture would explain something better than prose, such as a data flow, call graph, state machine or schema the change alters, draw it in a ` + "```mermaid" + ` block. When the change alters something you can see, you can run it (on any free port on 127.0.0.1) and attach screenshots taken with the Playwright tools. Most reviews need neither; leave out a diagram or screenshot that adds nothing to the text.`
|
||
|
||
func reviewSchema() json.RawMessage {
|
||
images := map[string]any{
|
||
"type": "array",
|
||
"description": "Screenshots, only when they show something the text can't.",
|
||
"items": map[string]any{
|
||
"type": "object",
|
||
"properties": map[string]any{
|
||
"path": map[string]any{"type": "string", "description": "A PNG, JPEG, GIF or WebP file of at most 5 MiB you saved: absolute, or relative to your working directory. byre keeps a copy."},
|
||
"caption": map[string]any{"type": "string", "description": "One line saying what it shows."},
|
||
},
|
||
"required": []any{"path"},
|
||
},
|
||
}
|
||
b, err := json.Marshal(map[string]any{
|
||
"type": "object",
|
||
"properties": map[string]any{
|
||
"change": map[string]any{"type": "string", "description": "What the change does, in a few sentences of Markdown, for someone who hasn't read it."},
|
||
"summary": map[string]any{"type": "string", "description": "Exactly \"No significant findings.\" when findings is empty; otherwise a bulleted Markdown list with one line per finding, and nothing else."},
|
||
"findings": map[string]any{
|
||
"type": "array",
|
||
"items": map[string]any{
|
||
"type": "object",
|
||
"properties": map[string]any{
|
||
"severity": map[string]any{"type": "string", "enum": severities},
|
||
"category": map[string]any{"type": "string", "description": "A short label, such as correctness, security or tests."},
|
||
"title": map[string]any{"type": "string", "description": "One line."},
|
||
"detail": map[string]any{"type": "string", "description": "What is wrong, why it matters and what to do, in Markdown."},
|
||
"path": map[string]any{"type": "string", "description": "Repo-relative path of the file, if the finding is about one."},
|
||
"line": map[string]any{"type": "integer", "description": "Line in the file as of this snapshot; byre shows the diff around it."},
|
||
"end_line": map[string]any{"type": "integer", "description": "Last line, when the finding covers a range."},
|
||
"images": images,
|
||
},
|
||
"required": []any{"severity", "title", "detail"},
|
||
},
|
||
},
|
||
"images": images,
|
||
},
|
||
"required": []any{"change", "summary", "findings"},
|
||
})
|
||
if err != nil {
|
||
panic(err)
|
||
}
|
||
return b
|
||
}
|
||
|
||
func (m *Manager) afterSnapshot(snap *api.Snapshot) {
|
||
if snap == nil {
|
||
return
|
||
}
|
||
ctx := context.Background()
|
||
m.reviewMu.Lock()
|
||
defer m.reviewMu.Unlock()
|
||
|
||
task, err := m.store.GetTask(ctx, snap.TaskId)
|
||
if err != nil {
|
||
slog.Error("load task to review", "task", snap.TaskId, "err", err)
|
||
return
|
||
}
|
||
if task.Status != api.TaskStatusActive {
|
||
return
|
||
}
|
||
reviews, err := m.store.ListReviews(ctx, task.Id)
|
||
if err != nil {
|
||
slog.Error("list reviews to review", "task", task.Id, "err", err)
|
||
return
|
||
}
|
||
for _, r := range reviews {
|
||
if r.SnapshotN > snap.N {
|
||
slog.Info("review not started, a newer snapshot has one", "task", task.Id, "snapshot", snap.N, "newer", r.SnapshotN)
|
||
return
|
||
}
|
||
}
|
||
err = m.startReviewer(ctx, task, *snap)
|
||
if err != nil {
|
||
slog.Error("start review", "task", task.Id, "snapshot", snap.N, "err", err)
|
||
}
|
||
}
|
||
|
||
func (m *Manager) reviewDir(projectID, reviewID string) string {
|
||
return filepath.Join(m.worktreeRoot, projectID, "review-"+reviewID)
|
||
}
|
||
|
||
func (m *Manager) reviewMediaDir(projectID, reviewID string) string {
|
||
return m.reviewDir(projectID, reviewID) + "-media"
|
||
}
|
||
|
||
func playwrightMCP(outputDir string) mcp.Stdio {
|
||
return mcp.Stdio{
|
||
Name: "playwright",
|
||
Command: "npx",
|
||
Args: []string{"--yes", "@playwright/mcp@0.0.83", "--headless", "--isolated", "--browser", "chrome", "--file-paths", "absolute", "--output-dir", outputDir},
|
||
}
|
||
}
|
||
|
||
func (m *Manager) reviewerModel(ctx context.Context, task api.Task) (string, error) {
|
||
creator, err := m.store.GetUser(ctx, task.CreatedBy)
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
sessions, err := m.store.ListSessions(ctx, task.Id)
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
if len(sessions) > 0 && sessions[len(sessions)-1].Model == creator.ReviewModel {
|
||
return creator.SecondReviewModel, nil
|
||
}
|
||
return creator.ReviewModel, nil
|
||
}
|
||
|
||
func (m *Manager) startReviewer(ctx context.Context, task api.Task, snap api.Snapshot) error {
|
||
project, err := m.store.GetProject(ctx, task.ProjectId)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
model, err := m.reviewerModel(ctx, task)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
vendor := workflow.VendorOf(model)
|
||
prompt, err := m.reviewPrompt(ctx, task, project, snap)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
if prompt == "" {
|
||
now := time.Now().UTC()
|
||
err = m.store.CreateReview(ctx, api.Review{
|
||
Id: store.NewID(),
|
||
TaskId: task.Id,
|
||
SnapshotN: snap.N,
|
||
State: api.ReviewStateSkipped,
|
||
CreatedAt: now,
|
||
FinishedAt: &now,
|
||
})
|
||
if err != nil {
|
||
return err
|
||
}
|
||
slog.Info("review skipped, snapshot has no diff", "task", task.Id, "snapshot", snap.N)
|
||
m.failRunningReviews(ctx, task.Id, project, snap.N, fmt.Sprintf("superseded by snapshot %d", snap.N))
|
||
return nil
|
||
}
|
||
|
||
reviewID := store.NewID()
|
||
dir := m.reviewDir(project.Id, reviewID)
|
||
err = git.AddDetachedWorktree(project.RepoPath, dir, snap.CommitSha)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
err = os.MkdirAll(m.reviewMediaDir(project.Id, reviewID), 0o755)
|
||
if err != nil {
|
||
m.removeReviewDir(project, reviewID)
|
||
return err
|
||
}
|
||
|
||
id := newUUID()
|
||
sess := api.AgentSession{
|
||
Id: id,
|
||
TaskId: task.Id,
|
||
Vendor: vendor,
|
||
Model: model,
|
||
VendorSessionId: initialVendorSessionID(vendor, id),
|
||
State: api.AgentSessionStateIdle,
|
||
Role: api.Reviewer,
|
||
CreatedBy: task.CreatedBy,
|
||
CreatedAt: time.Now().UTC(),
|
||
}
|
||
err = m.store.CreateSession(ctx, sess, project.Id)
|
||
if err != nil {
|
||
m.removeReviewDir(project, reviewID)
|
||
return err
|
||
}
|
||
err = m.store.CreateReview(ctx, api.Review{
|
||
Id: reviewID,
|
||
TaskId: task.Id,
|
||
SnapshotN: snap.N,
|
||
AgentSessionId: sess.Id,
|
||
Vendor: string(vendor),
|
||
Model: model,
|
||
State: api.ReviewStateRunning,
|
||
CreatedAt: time.Now().UTC(),
|
||
})
|
||
if err != nil {
|
||
m.endReviewer(ctx, sess.Id, project, reviewID)
|
||
return err
|
||
}
|
||
|
||
_, err = m.turn(ctx, sess, task, "byre", prompt, nil)
|
||
if err != nil {
|
||
m.endReviewer(ctx, sess.Id, project, reviewID)
|
||
m.failReview(ctx, reviewID, "the reviewer didn't start: "+err.Error())
|
||
return err
|
||
}
|
||
slog.Info("review started", "review", reviewID, "task", task.Id, "snapshot", snap.N, "vendor", vendor, "model", model)
|
||
m.failRunningReviews(ctx, task.Id, project, snap.N, fmt.Sprintf("superseded by snapshot %d", snap.N))
|
||
return nil
|
||
}
|
||
|
||
func (m *Manager) reviewPrompt(ctx context.Context, task api.Task, project api.Project, snap api.Snapshot) (string, error) {
|
||
changes, err := git.Compare(project.RepoPath, project.DefaultBranch, snap.CommitSha, snap.TreeSha, "")
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
if len(changes.Files) == 0 {
|
||
return "", nil
|
||
}
|
||
brief, err := m.store.FirstTaskMessage(ctx, task.Id)
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
|
||
b := &strings.Builder{}
|
||
fmt.Fprintf(b, "Review the work of the agent on the byre task %q: look for bugs, security problems, missing or wrong tests, and anything that doesn't do what the task asked.\n\n", task.Title)
|
||
fmt.Fprintf(b, "It is snapshot %d of the task, commit %s, checked out in your working directory. The change is all of the task's work, `git diff %s %s` (from the merge-base with %s); %d files changed:\n",
|
||
snap.N, snap.CommitSha, changes.Base, snap.CommitSha, project.DefaultBranch, len(changes.Files))
|
||
for i, f := range changes.Files {
|
||
if i == maxReviewFiles {
|
||
fmt.Fprintf(b, "- … and %d more\n", len(changes.Files)-maxReviewFiles)
|
||
break
|
||
}
|
||
fmt.Fprintf(b, "- %s %s (+%d −%d)\n", f.Status, f.Path, f.Additions, f.Deletions)
|
||
}
|
||
if brief != "" {
|
||
fmt.Fprintf(b, "\nThe task's brief, the first message its agent got:\n\n%s\n", quote(clip(brief, 4000)))
|
||
}
|
||
b.WriteString("\nWhen you're done, call submit_review once with your review, then end your turn.")
|
||
return b.String(), nil
|
||
}
|
||
|
||
func quote(s string) string {
|
||
lines := strings.Split(strings.TrimSpace(s), "\n")
|
||
for i, l := range lines {
|
||
lines[i] = "> " + l
|
||
}
|
||
return strings.Join(lines, "\n")
|
||
}
|
||
|
||
func (m *Manager) submitReviewTool(l *live) mcp.Tool {
|
||
return mcp.Tool{
|
||
Name: "submit_review",
|
||
Description: "Submit your review. Call it exactly once, when you're done; its arguments are the review the human reads.",
|
||
InputSchema: reviewSchema(),
|
||
Handler: func(ctx context.Context, args json.RawMessage) (string, error) {
|
||
return m.submitReview(ctx, l, args)
|
||
},
|
||
}
|
||
}
|
||
|
||
func (m *Manager) submitReview(ctx context.Context, l *live, raw json.RawMessage) (string, error) {
|
||
result := map[string]any{}
|
||
err := json.Unmarshal(raw, &result)
|
||
if err != nil {
|
||
return "", fmt.Errorf("submit_review takes a JSON object: %w", err)
|
||
}
|
||
if c, ok := result["change"].(string); !ok || strings.TrimSpace(c) == "" {
|
||
return "", errors.New("submit_review rejected, nothing was recorded: change must say what the change does")
|
||
}
|
||
if _, ok := result["summary"].(string); !ok {
|
||
return "", errors.New("submit_review rejected, nothing was recorded: summary must be a string")
|
||
}
|
||
findings, ok := result["findings"].([]any)
|
||
if !ok {
|
||
return "", errors.New("submit_review rejected, nothing was recorded: findings must be a list")
|
||
}
|
||
|
||
holders := []map[string]any{result}
|
||
for _, f := range findings {
|
||
fm, ok := f.(map[string]any)
|
||
if !ok {
|
||
return "", errors.New("submit_review rejected, nothing was recorded: each finding must be an object")
|
||
}
|
||
holders = append(holders, fm)
|
||
}
|
||
pending := []reviewImage{}
|
||
for _, holder := range holders {
|
||
imgs, err := m.readReviewImages(l, holder["images"])
|
||
if err != nil {
|
||
return "", fmt.Errorf("submit_review rejected, nothing was recorded: %w", err)
|
||
}
|
||
pending = append(pending, imgs...)
|
||
}
|
||
for _, img := range pending {
|
||
b, err := m.store.CreateBlob(ctx, img.contentType, img.data, agentActor)
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
img.entry["blob"] = b.Id
|
||
delete(img.entry, "path")
|
||
}
|
||
|
||
err = m.store.FinishReview(ctx, l.reviewID, api.ReviewStateDone, result, "")
|
||
if errors.Is(err, store.ErrNotFound) {
|
||
return "", errors.New("this review was already submitted or has ended")
|
||
}
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
if len(findings) > 0 {
|
||
go m.forwardFindings(l.taskID, l.reviewID)
|
||
}
|
||
go m.reviewFinished(l.reviewID)
|
||
return "Review recorded. End your turn now with one line.", nil
|
||
}
|
||
|
||
func (m *Manager) forwardFindings(taskID, reviewID string) {
|
||
ctx := context.Background()
|
||
r, err := m.store.GetReview(ctx, reviewID)
|
||
if err != nil {
|
||
slog.Error("load review to forward", "review", reviewID, "err", err)
|
||
return
|
||
}
|
||
s, found, err := m.liveSession(ctx, taskID)
|
||
if err != nil {
|
||
slog.Error("list sessions to forward review", "review", reviewID, "err", err)
|
||
return
|
||
}
|
||
if !found {
|
||
slog.Info("review findings not forwarded, task has no live agent", "review", reviewID, "task", taskID)
|
||
return
|
||
}
|
||
_, _, err = m.Message(ctx, s.Id, "byre", findingsMessage(r), "")
|
||
if err != nil {
|
||
slog.Error("forward review findings", "review", reviewID, "session", s.Id, "err", err)
|
||
return
|
||
}
|
||
slog.Info("review findings forwarded", "review", reviewID, "session", s.Id)
|
||
}
|
||
|
||
func findingsMessage(r api.Review) string {
|
||
res := map[string]any{}
|
||
if r.Result != nil {
|
||
res = *r.Result
|
||
}
|
||
b := &strings.Builder{}
|
||
fmt.Fprintf(b, "A reviewer looked at snapshot #%d of your work and raised the findings below. Reviewers can be wrong or miss context: check each one against the code and your brief. "+
|
||
"Before you change anything, answer each finding with respond_to_finding (snapshot %d and the finding's number): fixing if it holds up and you'll act on it, ignoring with your reason if you're leaving it. "+
|
||
"The human sees your answers on the findings and can overrule an ignore.", r.SnapshotN, r.SnapshotN)
|
||
findings, _ := res["findings"].([]any)
|
||
for i, f := range findings {
|
||
fm, _ := f.(map[string]any)
|
||
where := ""
|
||
if p, ok := fm["path"].(string); ok && p != "" {
|
||
where = p
|
||
if line, ok := fm["line"].(float64); ok && line > 0 {
|
||
where += fmt.Sprintf(":%d", int(line))
|
||
}
|
||
where = " (`" + where + "`)"
|
||
}
|
||
fmt.Fprintf(b, "\n\n**Finding %d: [%v] %v**%s\n\n%s", i+1, fm["severity"], fm["title"], where, strings.TrimSpace(fmt.Sprint(fm["detail"])))
|
||
}
|
||
return b.String()
|
||
}
|
||
|
||
type findingResponseArgs struct {
|
||
Snapshot int `json:"snapshot"`
|
||
Finding int `json:"finding"`
|
||
Response api.FindingResponseResponse `json:"response"`
|
||
Reason string `json:"reason"`
|
||
}
|
||
|
||
func (m *Manager) respondToFindingTool(l *live) mcp.Tool {
|
||
return mcp.Tool{
|
||
Name: "respond_to_finding",
|
||
Description: "Answer one finding of a review of your work, as soon as you've decided what to do about it: fixing when you'll act on it, ignoring when you're leaving it. " +
|
||
"The human sees your answer on the finding; an ignore keeps the human's fix button, so they can overrule you. Call it again to change your answer.",
|
||
InputSchema: json.RawMessage(`{
|
||
"type": "object",
|
||
"properties": {
|
||
"snapshot": {"type": "integer", "description": "The reviewed snapshot's number."},
|
||
"finding": {"type": "integer", "description": "The finding's number in the review, from 1."},
|
||
"response": {"type": "string", "enum": ["fixing", "ignoring"]},
|
||
"reason": {"type": "string", "description": "One line: what you'll do about it, or why you're leaving it."}
|
||
},
|
||
"required": ["snapshot", "finding", "response", "reason"]
|
||
}`),
|
||
Handler: func(ctx context.Context, raw json.RawMessage) (string, error) {
|
||
args := findingResponseArgs{}
|
||
err := json.Unmarshal(raw, &args)
|
||
if err != nil {
|
||
return "", fmt.Errorf("malformed respond_to_finding call: %w", err)
|
||
}
|
||
if !args.Response.Valid() {
|
||
return "", fmt.Errorf("%w: response must be fixing or ignoring", ErrInvalid)
|
||
}
|
||
if args.Finding < 1 {
|
||
return "", fmt.Errorf("%w: finding numbers start at 1", ErrInvalid)
|
||
}
|
||
reason := strings.TrimSpace(args.Reason)
|
||
if reason == "" {
|
||
return "", fmt.Errorf("%w: reason must not be empty", ErrInvalid)
|
||
}
|
||
reviews, err := m.store.ListReviews(ctx, l.taskID)
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
reviewID := ""
|
||
for _, r := range reviews {
|
||
if r.SnapshotN == args.Snapshot && r.State == api.ReviewStateDone {
|
||
reviewID = r.Id
|
||
}
|
||
}
|
||
if reviewID == "" {
|
||
return "", fmt.Errorf("%w: snapshot %d has no finished review", ErrInvalid, args.Snapshot)
|
||
}
|
||
err = m.store.SetFindingResponse(ctx, reviewID, args.Finding-1, api.FindingResponse{Response: args.Response, Reason: reason}, agentActor)
|
||
if errors.Is(err, store.ErrNotFound) {
|
||
return "", fmt.Errorf("%w: the review of snapshot %d has no finding %d", ErrInvalid, args.Snapshot, args.Finding)
|
||
}
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
return fmt.Sprintf("Recorded: %s finding %d of snapshot %d's review.", args.Response, args.Finding, args.Snapshot), nil
|
||
},
|
||
}
|
||
}
|
||
|
||
type reviewImage struct {
|
||
entry map[string]any
|
||
contentType string
|
||
data []byte
|
||
}
|
||
|
||
func (m *Manager) readReviewImages(l *live, raw any) ([]reviewImage, error) {
|
||
if raw == nil {
|
||
return nil, nil
|
||
}
|
||
list, ok := raw.([]any)
|
||
if !ok {
|
||
return nil, errors.New("images must be a list")
|
||
}
|
||
out := []reviewImage{}
|
||
for _, item := range list {
|
||
entry, ok := item.(map[string]any)
|
||
if !ok {
|
||
return nil, errors.New("each image must be an object with a path")
|
||
}
|
||
p, ok := entry["path"].(string)
|
||
if !ok || p == "" {
|
||
return nil, errors.New("each image needs a path")
|
||
}
|
||
if !filepath.IsAbs(p) {
|
||
p = filepath.Join(l.dir, p)
|
||
}
|
||
info, err := os.Stat(p)
|
||
if err != nil {
|
||
return nil, fmt.Errorf("image %s: %w", p, err)
|
||
}
|
||
if !info.Mode().IsRegular() {
|
||
return nil, fmt.Errorf("image %s is not a file", p)
|
||
}
|
||
if info.Size() > store.MaxBlobBytes {
|
||
return nil, fmt.Errorf("image %s is %d bytes; images are limited to %d MiB", p, info.Size(), store.MaxBlobBytes>>20)
|
||
}
|
||
data, err := os.ReadFile(p)
|
||
if err != nil {
|
||
return nil, fmt.Errorf("image %s: %w", p, err)
|
||
}
|
||
contentType := http.DetectContentType(data)
|
||
if !store.ImageTypes[contentType] {
|
||
return nil, fmt.Errorf("image %s is %s; use PNG, JPEG, GIF or WebP", p, contentType)
|
||
}
|
||
out = append(out, reviewImage{entry: entry, contentType: contentType, data: data})
|
||
}
|
||
return out, nil
|
||
}
|
||
|
||
func (m *Manager) reviewTurnEnded(ctx context.Context, l *live, res endResult) error {
|
||
m.mu.Lock()
|
||
interrupted := l.interrupted
|
||
m.mu.Unlock()
|
||
|
||
err := m.emit(ctx, l, agentActor, "turn.ended", res.payload)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
if res.window > 0 {
|
||
err = m.updateContext(ctx, l, l.contextUsed, res.window)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
}
|
||
|
||
reason := fmt.Sprintf("the reviewer ended its turn without calling submit_review: %s", clip(strings.TrimSpace(res.result), 500))
|
||
switch {
|
||
case interrupted:
|
||
reason = "the reviewer was interrupted"
|
||
case res.isError:
|
||
reason = fmt.Sprintf("the reviewer's turn ended with an error (%s): %s", res.subtype, clip(res.result, 500))
|
||
}
|
||
go m.finishReviewer(l, reason)
|
||
return nil
|
||
}
|
||
|
||
func (m *Manager) finishReviewer(l *live, reason string) {
|
||
ctx := context.Background()
|
||
project, err := m.store.GetProject(ctx, l.projectID)
|
||
if err != nil {
|
||
slog.Error("load project to end reviewer", "review", l.reviewID, "err", err)
|
||
return
|
||
}
|
||
m.endReviewer(ctx, l.id, project, l.reviewID)
|
||
m.failReview(ctx, l.reviewID, reason)
|
||
}
|
||
|
||
func (m *Manager) endReviewer(ctx context.Context, sessionID string, project api.Project, reviewID string) {
|
||
sess, err := m.store.GetSession(ctx, sessionID)
|
||
if err != nil {
|
||
slog.Error("load reviewer session", "session", sessionID, "err", err)
|
||
}
|
||
if err == nil && sess.State != api.AgentSessionStateCompleted {
|
||
err = m.retire(ctx, sess, "byre")
|
||
if err != nil {
|
||
slog.Error("retire reviewer session", "session", sessionID, "err", err)
|
||
}
|
||
}
|
||
m.removeReviewDir(project, reviewID)
|
||
}
|
||
|
||
func (m *Manager) removeReviewDir(project api.Project, reviewID string) {
|
||
err := git.RemoveDetachedWorktree(project.RepoPath, m.reviewDir(project.Id, reviewID))
|
||
if err != nil {
|
||
slog.Error("remove review worktree", "review", reviewID, "err", err)
|
||
}
|
||
err = os.RemoveAll(m.reviewMediaDir(project.Id, reviewID))
|
||
if err != nil {
|
||
slog.Error("remove review media", "review", reviewID, "err", err)
|
||
}
|
||
}
|
||
|
||
func (m *Manager) failReview(ctx context.Context, reviewID, reason string) {
|
||
err := m.store.FinishReview(ctx, reviewID, api.ReviewStateFailed, nil, reason)
|
||
if errors.Is(err, store.ErrNotFound) {
|
||
return
|
||
}
|
||
if err != nil {
|
||
slog.Error("record failed review", "review", reviewID, "err", err)
|
||
return
|
||
}
|
||
slog.Info("review failed", "review", reviewID, "reason", reason)
|
||
go m.reviewFinished(reviewID)
|
||
}
|
||
|
||
func (m *Manager) cancelReviews(ctx context.Context, task api.Task, project api.Project) {
|
||
m.reviewMu.Lock()
|
||
defer m.reviewMu.Unlock()
|
||
m.failRunningReviews(ctx, task.Id, project, math.MaxInt, "the task was closed")
|
||
}
|
||
|
||
func (m *Manager) failRunningReviews(ctx context.Context, taskID string, project api.Project, before int, reason string) {
|
||
reviews, err := m.store.ListReviews(ctx, taskID)
|
||
if err != nil {
|
||
slog.Error("list reviews to cancel", "task", taskID, "err", err)
|
||
return
|
||
}
|
||
for _, r := range reviews {
|
||
if r.State != api.ReviewStateRunning || r.SnapshotN >= before {
|
||
continue
|
||
}
|
||
m.endReviewer(ctx, r.AgentSessionId, project, r.Id)
|
||
m.failReview(ctx, r.Id, reason)
|
||
}
|
||
}
|
||
|
||
func (m *Manager) StopReview(ctx context.Context, reviewID, reason string) (api.Review, error) {
|
||
m.reviewMu.Lock()
|
||
defer m.reviewMu.Unlock()
|
||
r, err := m.store.GetReview(ctx, reviewID)
|
||
if err != nil {
|
||
return api.Review{}, err
|
||
}
|
||
if r.State != api.ReviewStateRunning {
|
||
return api.Review{}, fmt.Errorf("%w: review %s is %s, not running", ErrConflict, r.Id, r.State)
|
||
}
|
||
task, err := m.store.GetTask(ctx, r.TaskId)
|
||
if err != nil {
|
||
return api.Review{}, err
|
||
}
|
||
project, err := m.store.GetProject(ctx, task.ProjectId)
|
||
if err != nil {
|
||
return api.Review{}, err
|
||
}
|
||
m.endReviewer(ctx, r.AgentSessionId, project, r.Id)
|
||
m.failReview(ctx, r.Id, reason)
|
||
return m.store.GetReview(ctx, r.Id)
|
||
}
|
||
|
||
func (m *Manager) stopStuckReviewers(ctx context.Context, h Health) {
|
||
for _, p := range h.Processes {
|
||
if p.Health != api.HealthStuck {
|
||
continue
|
||
}
|
||
m.mu.Lock()
|
||
l := m.live[p.SessionId]
|
||
reviewID := ""
|
||
if l != nil {
|
||
reviewID = l.reviewID
|
||
}
|
||
m.mu.Unlock()
|
||
if reviewID == "" {
|
||
continue
|
||
}
|
||
_, err := m.StopReview(ctx, reviewID, fmt.Sprintf("the reviewer went %d minutes without an event", int(StuckAfter/time.Minute)))
|
||
if err != nil && !errors.Is(err, ErrConflict) {
|
||
slog.Error("stop stuck reviewer", "review", reviewID, "session", p.SessionId, "err", err)
|
||
}
|
||
}
|
||
}
|
||
|
||
func (m *Manager) recoverReviews(ctx context.Context) error {
|
||
err := m.store.ClearReviewRequests(ctx)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
running, err := m.store.RunningReviews(ctx)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
for _, r := range running {
|
||
sess, err := m.store.GetSession(ctx, r.AgentSessionId)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
if sess.State == api.AgentSessionStateRunning {
|
||
continue
|
||
}
|
||
task, err := m.store.GetTask(ctx, r.TaskId)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
project, err := m.store.GetProject(ctx, task.ProjectId)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
m.endReviewer(ctx, r.AgentSessionId, project, r.Id)
|
||
m.failReview(ctx, r.Id, "byre restarted before the reviewer finished")
|
||
}
|
||
return nil
|
||
}
|
||
|
||
func reviewLine(r api.Review) string {
|
||
if r.State == api.ReviewStateFailed {
|
||
return fmt.Sprintf("review of snapshot %d failed: %s", r.SnapshotN, clip(r.Error, 300))
|
||
}
|
||
if r.State == api.ReviewStateSkipped {
|
||
return fmt.Sprintf("review of snapshot %d skipped: no changes", r.SnapshotN)
|
||
}
|
||
res := map[string]any{}
|
||
if r.Result != nil {
|
||
res = *r.Result
|
||
}
|
||
b := &strings.Builder{}
|
||
fmt.Fprintf(b, "review of snapshot %d: %s", r.SnapshotN, clip(strings.TrimSpace(fmt.Sprint(res["summary"])), 600))
|
||
if c, ok := res["change"].(string); ok && c != "" {
|
||
fmt.Fprintf(b, "\n change: %s", clip(strings.TrimSpace(c), 600))
|
||
}
|
||
findings, _ := res["findings"].([]any)
|
||
for _, f := range findings {
|
||
fm, _ := f.(map[string]any)
|
||
where := ""
|
||
if p, ok := fm["path"].(string); ok && p != "" {
|
||
where = " (" + p
|
||
if line, ok := fm["line"].(float64); ok && line > 0 {
|
||
where += fmt.Sprintf(":%d", int(line))
|
||
}
|
||
where += ")"
|
||
}
|
||
fmt.Fprintf(b, "\n - [%v] %v%s: %s", fm["severity"], fm["title"], where, clip(strings.TrimSpace(fmt.Sprint(fm["detail"])), 300))
|
||
}
|
||
return b.String()
|
||
}
|