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() }