ai/byre
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

review error

Browse Source
This commit is contained in:
Ian Gulliver
2026-10-17 07:43:33 -07:00
parent 4612aece55
commit 216f3dd737
13 changed files with 327 additions and 235 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/agents/leadtools.go
+13
View File
@@ -341,6 +341,19 @@ func (m *Manager) leadTools(l *live) []mcp.Tool {
return fmt.Sprintf("Merge of %s started.", task.Id), nil
}),
tool("take_snapshot", takeSnapshotDescription+" Refused while the task is merging or being discarded.", taskOnly,
func(ctx context.Context, user api.User, raw json.RawMessage) (string, error) {
task, _, err := m.taskArg(ctx, raw)
if err != nil {
return "", err
}
err = m.RequestSnapshot(ctx, task.Id)
if err != nil {
return "", err
}
return fmt.Sprintf("Snapshot of %s requested; its review shows in the task's transcript and Reviews tab, and findings go to its agent.", task.Id), nil
}),
tool("discard_task",
"Discard a task: its agent gets one turn to write the task summary, then byre stops it and deletes its worktree and branch without merging. This destroys its unmerged work. Refused for a task with active subtasks. Only when the human asks. "+
"If the agent is mid-turn, the summary turn starts when that turn ends; no other turn runs in between.",
internal/agents/manager.go
+2 -2
View File
@@ -688,7 +688,7 @@ func (m *Manager) startLive(ctx context.Context, sess api.AgentSession, task api
if err != nil {
return nil, err
}
tools := append([]mcp.Tool{m.askTool(l), m.cancelQuestionTool(l), m.setMergeTitleTool(l), m.stopMergeTool(l), m.setTaskSummaryTool(l), m.searchClosedTool(l), m.exposePortTool(l)}, m.subtaskTools(l)...)
tools := append([]mcp.Tool{m.askTool(l), m.cancelQuestionTool(l), m.setMergeTitleTool(l), m.stopMergeTool(l), m.setTaskSummaryTool(l), m.searchClosedTool(l), m.exposePortTool(l), m.takeSnapshotTool(l)}, m.subtaskTools(l)...)
if sess.Role == api.Implementer {
g += longTitleGuidance(task)
notes, err := m.notesText(ctx, task.CreatedBy, task.ProjectId)
@@ -1028,7 +1028,7 @@ func (m *Manager) turnEnded(ctx context.Context, l *live, res endResult) error {
return err
}
if snapshot {
m.snapshotSoon(l, api.SnapshotTriggerTurnEnd, 0, review)
m.snapshotSoon(liveTarget(l), api.SnapshotTriggerTurnEnd, 0, review, false)
}
switch next {
internal/agents/review.go
+5 -19
View File
@@ -24,7 +24,7 @@ var reviewModels = map[api.AgentSessionVendor]string{api.Claude: "claude-opus-5-
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 the change it just made, and a human may read your review in byre's web UI.
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 web UI.
- 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.
@@ -213,35 +213,22 @@ func (m *Manager) startReviewer(ctx context.Context, task api.Task, snap api.Sna
}
func (m *Manager) reviewPrompt(ctx context.Context, task api.Task, project api.Project, snap api.Snapshot) (string, error) {
previous := ""
if snap.N > 1 {
prev, err := m.store.GetSnapshot(ctx, task.Id, snap.N-1)
if err != nil {
return "", err
}
previous = prev.CommitSha
}
changes, err := git.Compare(project.RepoPath, project.DefaultBranch, snap.CommitSha, snap.TreeSha, previous)
changes, err := git.Compare(project.RepoPath, project.DefaultBranch, snap.CommitSha, snap.TreeSha, "")
if err != nil {
return "", err
}
previous = changes.Base
if len(changes.Files) == 0 {
return "", nil
}
whole, 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 change the agent working on the byre task %q just made: 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 `git diff %s %s`; %d files changed:\n",
snap.N, snap.CommitSha, previous, snap.CommitSha, len(changes.Files))
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)
@@ -249,7 +236,6 @@ func (m *Manager) reviewPrompt(ctx context.Context, task api.Task, project api.P
}
fmt.Fprintf(b, "- %s %s (+%d −%d)\n", f.Status, f.Path, f.Additions, f.Deletions)
}
fmt.Fprintf(b, "\nFor context, all of the task's work so far is `git diff %s %s` (from the merge-base with %s, %d files).\n", whole.Base, snap.CommitSha, project.DefaultBranch, len(whole.Files))
if brief != "" {
fmt.Fprintf(b, "\nThe task's brief, the first message its agent got:\n\n%s\n", quote(clip(brief, 4000)))
}
internal/agents/snapshots.go
+102 -25
View File
@@ -12,6 +12,7 @@ import (
"g.fc.run/misc/byre/internal/api"
"g.fc.run/misc/byre/internal/claude"
"g.fc.run/misc/byre/internal/git"
"g.fc.run/misc/byre/internal/mcp"
"g.fc.run/misc/byre/internal/store"
)
@@ -19,6 +20,20 @@ type snapRequest struct {
trigger api.SnapshotTrigger
seq int64
review bool
force bool
}
type snapTarget struct {
taskID string
projectID string
sessionID string
dir string
gitName string
gitEmail string
}
func liveTarget(l *live) snapTarget {
return snapTarget{taskID: l.taskID, projectID: l.projectID, sessionID: l.id, dir: l.dir, gitName: l.gitName, gitEmail: l.gitEmail}
}
type snapState struct {
@@ -37,44 +52,48 @@ func (m *Manager) snapStateLocked(taskID string) *snapState {
return st
}
func (m *Manager) snapshotSoon(l *live, trigger api.SnapshotTrigger, seq int64, review bool) {
func (m *Manager) snapshotSoon(t snapTarget, trigger api.SnapshotTrigger, seq int64, review, force bool) {
m.snapMu.Lock()
defer m.snapMu.Unlock()
st := m.snapStateLocked(l.taskID)
st := m.snapStateLocked(t.taskID)
if st.closed {
m.dropRequest(l.taskID, &snapRequest{review: review})
m.dropRequest(t.taskID, &snapRequest{review: review})
return
}
req := &snapRequest{trigger: trigger, seq: seq, review: review}
req := &snapRequest{trigger: trigger, seq: seq, review: review, force: force}
if st.running {
m.dropRequest(l.taskID, st.pending)
if p := st.pending; p != nil && p.force {
req.force = true
req.trigger = p.trigger
}
m.dropRequest(t.taskID, st.pending)
st.pending = req
return
}
st.running = true
go m.runSnapshots(l, st, req)
go m.runSnapshots(t, st, req)
}
func (m *Manager) runSnapshots(l *live, st *snapState, req *snapRequest) {
func (m *Manager) runSnapshots(t snapTarget, st *snapState, req *snapRequest) {
for {
st.mu.Lock()
m.snapMu.Lock()
closed := st.closed
m.snapMu.Unlock()
if !closed {
snap, err := m.takeSnapshot(context.Background(), l, req.trigger, req.seq)
snap, err := m.takeSnapshot(context.Background(), t, req.trigger, req.seq, req.force)
if err != nil {
slog.Error("take snapshot", "task", l.taskID, "trigger", req.trigger, "err", err)
slog.Error("take snapshot", "task", t.taskID, "trigger", req.trigger, "err", err)
}
if req.review {
go func() {
m.afterSnapshot(snap)
m.endReviewRequest(l.taskID)
m.endReviewRequest(t.taskID)
}()
}
} else {
m.dropRequest(l.taskID, req)
m.dropRequest(t.taskID, req)
}
st.mu.Unlock()
@@ -82,7 +101,7 @@ func (m *Manager) runSnapshots(l *live, st *snapState, req *snapRequest) {
req = st.pending
st.pending = nil
if req == nil || st.closed {
m.dropRequest(l.taskID, req)
m.dropRequest(t.taskID, req)
st.running = false
m.snapMu.Unlock()
return
@@ -104,34 +123,34 @@ func (m *Manager) endReviewRequest(taskID string) {
}
}
func (m *Manager) takeSnapshot(ctx context.Context, l *live, trigger api.SnapshotTrigger, seq int64) (*api.Snapshot, error) {
func (m *Manager) takeSnapshot(ctx context.Context, t snapTarget, trigger api.SnapshotTrigger, seq int64, force bool) (*api.Snapshot, error) {
if seq == 0 {
var err error
seq, err = m.store.LastTaskSeq(ctx, l.taskID)
seq, err = m.store.LastTaskSeq(ctx, t.taskID)
if err != nil {
return nil, err
}
}
tree, err := git.Snapshot(l.dir)
tree, err := git.Snapshot(t.dir)
if err != nil {
return nil, err
}
last, err := m.store.LastSnapshot(ctx, l.taskID)
last, err := m.store.LastSnapshot(ctx, t.taskID)
if err != nil && !errors.Is(err, store.ErrNotFound) {
return nil, err
}
if err == nil && last.TreeSha == tree {
if err == nil && last.TreeSha == tree && !force {
return nil, nil
}
n := last.N + 1
commit, err := git.CommitSnapshot(l.dir, tree, git.SnapshotRef(l.taskID, n), fmt.Sprintf("byre snapshot %d (%s)", n, trigger), []string{
"GIT_AUTHOR_NAME=" + l.gitName,
"GIT_AUTHOR_EMAIL=" + l.gitEmail,
"GIT_COMMITTER_NAME=" + l.gitName,
"GIT_COMMITTER_EMAIL=" + l.gitEmail,
commit, err := git.CommitSnapshot(t.dir, tree, git.SnapshotRef(t.taskID, n), fmt.Sprintf("byre snapshot %d (%s)", n, trigger), []string{
"GIT_AUTHOR_NAME=" + t.gitName,
"GIT_AUTHOR_EMAIL=" + t.gitEmail,
"GIT_COMMITTER_NAME=" + t.gitName,
"GIT_COMMITTER_EMAIL=" + t.gitEmail,
})
if err != nil {
return nil, err
@@ -139,8 +158,8 @@ func (m *Manager) takeSnapshot(ctx context.Context, l *live, trigger api.Snapsho
snap := api.Snapshot{
Id: store.NewID(),
TaskId: l.taskID,
AgentSessionId: l.id,
TaskId: t.taskID,
AgentSessionId: t.sessionID,
N: n,
CommitSha: commit,
TreeSha: tree,
@@ -148,13 +167,71 @@ func (m *Manager) takeSnapshot(ctx context.Context, l *live, trigger api.Snapsho
Trigger: trigger,
CreatedAt: time.Now().UTC(),
}
err = m.store.CreateSnapshot(ctx, snap, l.projectID)
err = m.store.CreateSnapshot(ctx, snap, t.projectID)
if err != nil {
return nil, err
}
return &snap, nil
}
func (m *Manager) RequestSnapshot(ctx context.Context, taskID string) error {
task, project, err := m.activeTask(ctx, taskID)
if err != nil {
return err
}
discard, err := m.store.GetDiscard(ctx, task.Id)
if err != nil {
return err
}
if discard.State != store.DiscardNone {
return fmt.Errorf("%w: task %s is being discarded", ErrConflict, task.Id)
}
mergingBy, err := m.store.MergingBy(ctx, task.Id)
if err != nil {
return err
}
if mergingBy != "" {
return fmt.Errorf("%w: task %s is merging", ErrConflict, task.Id)
}
sessions, err := m.store.ListSessions(ctx, task.Id)
if err != nil {
return err
}
if len(sessions) == 0 {
return fmt.Errorf("%w: task %s has no agent session to attribute a snapshot to", ErrConflict, task.Id)
}
sess := sessions[len(sessions)-1]
creator, err := m.store.GetUser(ctx, sess.CreatedBy)
if err != nil {
return err
}
err = m.store.BeginReviewRequest(ctx, task.Id)
if err != nil {
return err
}
t := snapTarget{taskID: task.Id, projectID: project.Id, sessionID: sess.Id, dir: task.WorktreePath, gitName: creator.GitName, gitEmail: creator.GitEmail}
m.snapshotSoon(t, api.SnapshotTriggerRequested, 0, true, true)
return nil
}
func (m *Manager) takeSnapshotTool(l *live) mcp.Tool {
return mcp.Tool{
Name: "take_snapshot",
Description: takeSnapshotDescription + " It snapshots your own task's worktree.",
InputSchema: json.RawMessage(`{"type": "object", "properties": {}}`),
Handler: func(ctx context.Context, args json.RawMessage) (string, error) {
err := m.RequestSnapshot(ctx, l.taskID)
if err != nil {
return "", err
}
return "Snapshot requested. A reviewer reviews it against the default branch, and any findings come to you as a message from byre.", nil
},
}
}
const takeSnapshotDescription = "Snapshot a task's worktree now, uncommitted work included, and start a review of it: a reviewer on the other vendor reviews all of the task's work against the default branch. " +
"byre already does this at the end of every turn that changes files; call this to review work mid-turn, or to retry a review that failed (it snapshots even when nothing changed since the last snapshot)."
func (m *Manager) dropSnapshots(repo, taskID string) error {
m.snapMu.Lock()
st := m.snapStateLocked(taskID)