900 lines
30 KiB
Go
900 lines
30 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.startReviews(ctx, task, *snap)
|
||
if err != nil {
|
||
slog.Error("start review", "task", task.Id, "snapshot", snap.N, "err", err)
|
||
}
|
||
}
|
||
|
||
func (m *Manager) reviewsSettled(ctx context.Context, reviews []api.Review) (bool, error) {
|
||
byStage := map[string]api.Review{}
|
||
for _, r := range reviews {
|
||
if r.State == api.ReviewStateRunning {
|
||
return false, nil
|
||
}
|
||
byStage[r.Stage] = r
|
||
}
|
||
for _, r := range reviews {
|
||
if r.State != api.ReviewStateDone {
|
||
continue
|
||
}
|
||
_, def, err := m.store.ReviewDefinition(ctx, r.Id)
|
||
if err != nil {
|
||
return false, err
|
||
}
|
||
if len(def.Steps) == 0 {
|
||
continue
|
||
}
|
||
for _, next := range workflow.Next(def, r.Stage) {
|
||
if _, found := byStage[next]; found {
|
||
continue
|
||
}
|
||
ready := true
|
||
for _, a := range workflow.After(def, next) {
|
||
ready = ready && byStage[a].State == api.ReviewStateDone
|
||
}
|
||
if ready {
|
||
return false, nil
|
||
}
|
||
}
|
||
}
|
||
return true, nil
|
||
}
|
||
|
||
func (m *Manager) snapshotReviews(ctx context.Context, taskID string, n int) ([]api.Review, error) {
|
||
reviews, err := m.store.ListReviews(ctx, taskID)
|
||
if err != nil {
|
||
return nil, err
|
||
}
|
||
out := []api.Review{}
|
||
for _, r := range reviews {
|
||
if r.SnapshotN == n {
|
||
out = append(out, r)
|
||
}
|
||
}
|
||
return out, nil
|
||
}
|
||
|
||
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) startReviews(ctx context.Context, task api.Task, snap api.Snapshot) error {
|
||
project, err := m.store.GetProject(ctx, task.ProjectId)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
defer m.failRunningReviews(ctx, task.Id, project, snap.N, fmt.Sprintf("superseded by snapshot %d", snap.N))
|
||
if project.ReviewWorkflow == "" {
|
||
slog.Info("review not started, the project has no review workflow", "task", task.Id, "snapshot", snap.N)
|
||
return nil
|
||
}
|
||
changes, err := git.Compare(project.RepoPath, project.DefaultBranch, snap.CommitSha, snap.TreeSha, "")
|
||
if err != nil {
|
||
return err
|
||
}
|
||
now := time.Now().UTC()
|
||
if len(changes.Files) == 0 {
|
||
slog.Info("review skipped, snapshot has no diff", "task", task.Id, "snapshot", snap.N)
|
||
return m.store.CreateReview(ctx, api.Review{Id: store.NewID(), TaskId: task.Id, SnapshotN: snap.N, State: api.ReviewStateSkipped, CreatedAt: now, FinishedAt: &now}, "")
|
||
}
|
||
|
||
frozen, err := m.store.FreezeWorkflow(ctx, project.ReviewWorkflow)
|
||
if err == nil {
|
||
err = reviewProblem(frozen.Definition)
|
||
}
|
||
if err != nil {
|
||
return m.store.CreateReview(ctx, api.Review{
|
||
Id: store.NewID(),
|
||
TaskId: task.Id,
|
||
SnapshotN: snap.N,
|
||
Stage: project.ReviewWorkflow,
|
||
State: api.ReviewStateFailed,
|
||
Error: fmt.Sprintf("project %s reviews with workflow %q, which can't run: %v", project.Name, project.ReviewWorkflow, err),
|
||
CreatedAt: now,
|
||
FinishedAt: &now,
|
||
}, "")
|
||
}
|
||
|
||
for _, stage := range workflow.Starts(frozen.Definition) {
|
||
err = m.startReviewer(ctx, task, project, snap, frozen.DefinitionID, frozen.Definition, stage, nil)
|
||
if err != nil {
|
||
slog.Error("start review stage", "task", task.Id, "snapshot", snap.N, "stage", stage, "err", err)
|
||
}
|
||
}
|
||
return nil
|
||
}
|
||
|
||
func (m *Manager) advanceReviews(ctx context.Context, r api.Review) bool {
|
||
if r.State != api.ReviewStateDone {
|
||
return false
|
||
}
|
||
m.reviewMu.Lock()
|
||
defer m.reviewMu.Unlock()
|
||
|
||
defID, def, err := m.store.ReviewDefinition(ctx, r.Id)
|
||
if err != nil {
|
||
slog.Error("load review definition", "review", r.Id, "err", err)
|
||
return false
|
||
}
|
||
if len(def.Steps) == 0 {
|
||
return false
|
||
}
|
||
task, project, err := m.activeTask(ctx, r.TaskId)
|
||
if err != nil {
|
||
return false
|
||
}
|
||
all, err := m.store.ListReviews(ctx, task.Id)
|
||
if err != nil {
|
||
slog.Error("list reviews to advance", "task", task.Id, "err", err)
|
||
return false
|
||
}
|
||
byStage := map[string]api.Review{}
|
||
for _, sr := range all {
|
||
if sr.SnapshotN > r.SnapshotN {
|
||
return false
|
||
}
|
||
if sr.SnapshotN == r.SnapshotN {
|
||
byStage[sr.Stage] = sr
|
||
}
|
||
}
|
||
snap, err := m.store.GetSnapshot(ctx, task.Id, r.SnapshotN)
|
||
if err != nil {
|
||
slog.Error("load snapshot to advance review", "task", task.Id, "snapshot", r.SnapshotN, "err", err)
|
||
return false
|
||
}
|
||
|
||
started := false
|
||
for _, next := range workflow.Next(def, r.Stage) {
|
||
if _, found := byStage[next]; found {
|
||
continue
|
||
}
|
||
inputs := []api.Review{}
|
||
for _, a := range workflow.After(def, next) {
|
||
if in, found := byStage[a]; found && in.State == api.ReviewStateDone {
|
||
inputs = append(inputs, in)
|
||
}
|
||
}
|
||
if len(inputs) != len(workflow.After(def, next)) {
|
||
continue
|
||
}
|
||
err = m.startReviewer(ctx, task, project, snap, defID, def, next, inputs)
|
||
if err != nil {
|
||
slog.Error("start review stage", "task", task.Id, "snapshot", r.SnapshotN, "stage", next, "err", err)
|
||
continue
|
||
}
|
||
started = true
|
||
}
|
||
return started
|
||
}
|
||
|
||
func (m *Manager) startReviewer(ctx context.Context, task api.Task, project api.Project, snap api.Snapshot, defID string, def api.WorkflowDefinition, stage string, inputs []api.Review) (err error) {
|
||
model := workflow.Get(def, stage).Model
|
||
vendor := workflow.VendorOf(model)
|
||
reviewID := store.NewID()
|
||
recorded := false
|
||
defer func() {
|
||
if err == nil || recorded {
|
||
return
|
||
}
|
||
now := time.Now().UTC()
|
||
rerr := m.store.CreateReview(ctx, api.Review{
|
||
Id: reviewID,
|
||
TaskId: task.Id,
|
||
SnapshotN: snap.N,
|
||
Stage: stage,
|
||
Vendor: string(vendor),
|
||
Model: model,
|
||
State: api.ReviewStateFailed,
|
||
Error: "the reviewer didn't start: " + err.Error(),
|
||
CreatedAt: now,
|
||
FinishedAt: &now,
|
||
}, defID)
|
||
if rerr != nil {
|
||
slog.Error("record review that didn't start", "task", task.Id, "stage", stage, "err", rerr)
|
||
return
|
||
}
|
||
go m.reviewFinished(reviewID, true)
|
||
}()
|
||
|
||
prompt, err := m.reviewPrompt(ctx, task, project, snap, def, stage, inputs)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
|
||
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,
|
||
Stage: stage,
|
||
AgentSessionId: sess.Id,
|
||
Vendor: string(vendor),
|
||
Model: model,
|
||
State: api.ReviewStateRunning,
|
||
CreatedAt: time.Now().UTC(),
|
||
}, defID)
|
||
if err != nil {
|
||
m.endReviewer(ctx, sess.Id, project, reviewID)
|
||
return err
|
||
}
|
||
recorded = true
|
||
|
||
_, 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, "stage", stage, "vendor", vendor, "model", model)
|
||
return nil
|
||
}
|
||
|
||
func (m *Manager) reviewPrompt(ctx context.Context, task api.Task, project api.Project, snap api.Snapshot, def api.WorkflowDefinition, stage string, inputs []api.Review) (string, error) {
|
||
changes, err := git.Compare(project.RepoPath, project.DefaultBranch, snap.CommitSha, snap.TreeSha, "")
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
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, as reviewer %q of the project's review workflow %q.\n\n%s\n\n", task.Title, stage, project.ReviewWorkflow, strings.TrimSpace(workflow.Get(def, stage).Prompt))
|
||
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)))
|
||
}
|
||
for _, in := range inputs {
|
||
fmt.Fprintf(b, "\n## From reviewer %q\n\n%s\n", in.Stage, reviewText(in))
|
||
}
|
||
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
|
||
}
|
||
go m.reviewFinished(l.reviewID, true)
|
||
return "Review recorded. End your turn now with one line.", nil
|
||
}
|
||
|
||
func (m *Manager) deliverFindings(ctx context.Context, r api.Review, reviews []api.Review) {
|
||
last := r
|
||
for _, sr := range reviews {
|
||
if sr.FinishedAt != nil && last.FinishedAt != nil && (sr.FinishedAt.After(*last.FinishedAt) || sr.FinishedAt.Equal(*last.FinishedAt) && sr.Id > last.Id) {
|
||
last = sr
|
||
}
|
||
}
|
||
if last.Id != r.Id {
|
||
return
|
||
}
|
||
latest, err := m.store.LastReviewable(ctx, r.TaskId)
|
||
if err != nil || latest.N != r.SnapshotN {
|
||
return
|
||
}
|
||
text := findingsMessage(r.SnapshotN, reviews)
|
||
if text == "" {
|
||
return
|
||
}
|
||
s, found, err := m.liveSession(ctx, r.TaskId)
|
||
if err != nil {
|
||
slog.Error("list sessions to deliver findings", "task", r.TaskId, "err", err)
|
||
return
|
||
}
|
||
if !found {
|
||
slog.Info("review findings not delivered, task has no live agent", "task", r.TaskId, "snapshot", r.SnapshotN)
|
||
return
|
||
}
|
||
_, _, err = m.Message(ctx, s.Id, "byre", text, "", "")
|
||
if err != nil {
|
||
slog.Error("deliver review findings", "task", r.TaskId, "snapshot", r.SnapshotN, "session", s.Id, "err", err)
|
||
return
|
||
}
|
||
slog.Info("review findings delivered", "task", r.TaskId, "snapshot", r.SnapshotN, "session", s.Id)
|
||
}
|
||
|
||
const findingsPreamble = "The review of snapshot #"
|
||
|
||
func isFindingsMessage(actor, text string) bool {
|
||
return actor == "byre" && strings.HasPrefix(text, findingsPreamble)
|
||
}
|
||
|
||
func findingsMessage(n int, reviews []api.Review) string {
|
||
b := &strings.Builder{}
|
||
for _, r := range reviews {
|
||
if r.State != api.ReviewStateDone || r.Result == nil {
|
||
continue
|
||
}
|
||
findings, _ := (*r.Result)["findings"].([]any)
|
||
if len(findings) == 0 {
|
||
continue
|
||
}
|
||
fmt.Fprintf(b, "\n\n## Reviewer %q", r.Stage)
|
||
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"])))
|
||
}
|
||
}
|
||
if b.Len() == 0 {
|
||
return ""
|
||
}
|
||
return fmt.Sprintf(findingsPreamble+"%d of your work has finished, and its reviewers 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, the reviewer's name 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.", n, n) + b.String()
|
||
}
|
||
|
||
type findingResponseArgs struct {
|
||
Snapshot int `json:"snapshot"`
|
||
Reviewer string `json:"reviewer"`
|
||
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."},
|
||
"reviewer": {"type": "string", "description": "The reviewer's name, as the findings message gives it."},
|
||
"finding": {"type": "integer", "description": "The finding's number in that reviewer's 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", "reviewer", "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.Stage == args.Reviewer && r.State == api.ReviewStateDone {
|
||
reviewID = r.Id
|
||
}
|
||
}
|
||
if reviewID == "" {
|
||
return "", fmt.Errorf("%w: snapshot %d has no finished review by reviewer %q", ErrInvalid, args.Snapshot, args.Reviewer)
|
||
}
|
||
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: reviewer %q's review of snapshot %d has no finding %d", ErrInvalid, args.Reviewer, args.Snapshot, args.Finding)
|
||
}
|
||
if err != nil {
|
||
return "", err
|
||
}
|
||
return fmt.Sprintf("Recorded: %s finding %d of reviewer %q's review of snapshot %d.", args.Response, args.Finding, args.Reviewer, 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, true)
|
||
}
|
||
|
||
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")
|
||
}
|
||
done, err := m.store.LatestDoneStages(ctx)
|
||
if err != nil {
|
||
return err
|
||
}
|
||
for _, id := range done {
|
||
go m.reviewFinished(id, false)
|
||
}
|
||
return nil
|
||
}
|
||
|
||
func reviewName(r api.Review) string {
|
||
if r.Stage == "" {
|
||
return fmt.Sprintf("review of snapshot %d", r.SnapshotN)
|
||
}
|
||
return fmt.Sprintf("%q review of snapshot %d", r.Stage, r.SnapshotN)
|
||
}
|
||
|
||
func reviewLine(r api.Review) string {
|
||
if r.State == api.ReviewStateFailed {
|
||
return fmt.Sprintf("%s failed: %s", reviewName(r), clip(r.Error, 300))
|
||
}
|
||
if r.State == api.ReviewStateSkipped {
|
||
return fmt.Sprintf("%s skipped: no changes", reviewName(r))
|
||
}
|
||
res := map[string]any{}
|
||
if r.Result != nil {
|
||
res = *r.Result
|
||
}
|
||
b := &strings.Builder{}
|
||
fmt.Fprintf(b, "%s: %s", reviewName(r), 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))
|
||
}
|
||
writeFindings(b, res, "\n ", 300)
|
||
return b.String()
|
||
}
|
||
|
||
func reviewText(r api.Review) string {
|
||
res := map[string]any{}
|
||
if r.Result != nil {
|
||
res = *r.Result
|
||
}
|
||
b := &strings.Builder{}
|
||
b.WriteString(strings.TrimSpace(fmt.Sprint(res["summary"])))
|
||
writeFindings(b, res, "\n", 4000)
|
||
return b.String()
|
||
}
|
||
|
||
func writeFindings(b *strings.Builder, res map[string]any, lead string, limit int) {
|
||
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, "%s- [%v] %v%s: %s", lead, fm["severity"], fm["title"], where, clip(strings.TrimSpace(fmt.Sprint(fm["detail"])), limit))
|
||
}
|
||
}
|