"reviewing"

This commit is contained in:
Ian Gulliver
2026-10-18 23:13:59 -07:00
parent 74def5c9e6
commit b6d351101d
2 changed files with 9 additions and 9 deletions
+3 -3
View File
@@ -67,7 +67,7 @@ Subagent count: Claude Code's subagents can run in the background past the `turn
`review_requests` covers the gap between a turn's end and its review starting. `turnEnded` emits `turn.ended` inside `EndTurn`'s transaction, which also sets the session idle and, when the turn's snapshot will be reviewed (no discard or merge pending, and no subagents running), adds one to `review_requests`, so the task goes from `running` to `review_pending` in the transaction that publishes the turn's summary. The snapshot is requested after that transaction. Each review request ends exactly once with `EndReviewRequest`, which subtracts one: after `afterSnapshot` returns (by then the review row is `running`, so the task is `reviewing`, or it was skipped or never started, so it is `idle`), or when the request is dropped because a newer one replaced it or the task's snapshots were closed. On startup `recoverReviews` zeroes every task's `review_requests` (`ClearReviewRequests`), since the requests in flight died with the process.
The UI's labels (see "Task state" under the task switcher) read `state` for **Working**; `task.state` reloads the project's tasks. `sqlite3 byre.db "SELECT id, title, state, review_requests FROM tasks WHERE status = 'active'"` shows them.
The UI's labels (see "Task state" under the task switcher) read `state` for **Working** and **Reviewing**; `task.state` reloads the project's tasks. `sqlite3 byre.db "SELECT id, title, state, review_requests FROM tasks WHERE status = 'active'"` shows them.
`POST /api/tasks/{id}/merge-in` (the transcript's **Sync from main** button, labelled with the default branch) brings the default branch into the task branch through the agent: it sends `mergeInPrompt` (`internal/agents/mergein.go`) as a `byre` message to the task's session, or starts a session with it when the task has none. The prompt has the agent run `git merge --no-ff --no-commit`, resolve conflicts, regenerate generated files rather than hand-merging them, check the build and tests, and leave the merge uncommitted; byre's end-of-turn commit concludes it as a merge commit. If resolving the conflicts took substantive changes rather than combining both sides as written, the prompt tells the agent to stop there and end its turn describing them; the turn's snapshot then gets its review like any other (see "Reviews"). byre doesn't inspect the merge itself: whether a change is substantive, and whether to stop, is the agent's call. On a Merge turn the prompt adds that byre squashes and pushes when the turn ends, and tells the agent to call the `stop_merge` MCP tool first if it stops (see below). The prompt ends by showing the task's current merge title and asking the agent to set a better one with `set_merge_title` if it no longer describes all the work on the branch. It is refused (409) while a session is `running`.
@@ -254,7 +254,7 @@ Messages byre itself sends an agent (actor `byre`: the restart notice, Sync and
The task switcher (⌘K, or the project ▸ task picker centred in the right pane's top row) lists active tasks with their task state and open inbox item count (the Project tab shows the same state), live while it is open. Tasks with inbox items come first; within each group, the task least recently visited comes first (never-visited tasks before all others, the right pane's own task after all others, ties in the server's order: projects by name, tasks newest first), so ⌘K Enter repeated cycles through the tasks. Visit times are SwiftData `VisitRecord`s synced through iCloud (`docs/macos.md`, "Persistence"), stamped when a task is selected and again when it is left. After the tasks come the projects, one row each; picking one points the right pane at the project with no task, on the Project tab. Typing filters both and adds a last row that creates a task with the typed text as its title, in the right pane's project (the first project when there is none); Enter picks the highlighted row. Text before a first colon that starts at least one project's name (ignoring case, no spaces) picks those projects instead: `by: fix` lists only their tasks whose titles contain "fix", their project rows that match, and one row per project creating the task titled "fix" there. When no project name starts with it, the whole text is the title, so "Fix: login" stays as typed. A create row shows the user's default model (see "Agent sessions"), the model the composer offers first for the new task's first message; the task itself starts with no session. Below each create row, a "Start in …" row creates the same task and starts its agent on the default model with the title as its first message, through `POST /api/projects/{id}/tasks` with `prompt` set to the title. Picking a task points the right pane at it on the Questions tab when it has open inbox items, otherwise on the Transcript tab, scrolled to its latest summary when that is unread (see below and "Lead agent").
**Task state.** The switcher and the Project tab show one label per task in place of its raw session state, the first that applies (`taskState` in `mac/Sources/Byre/Store.swift`), with a dot and a tooltip saying why: **Needs your answer** (an open `question` inbox item), **Merging** (`merging` or `merging_into_parent`), **Merge held** (`merge_queued_at`), **Merge after review** (`merge_after_review`), **Working** (the task's `state` is `running`, `review_pending`, `reviewing` or `discarding`; see "Task state in the store"), **Idle** (anything else, including no session). The Project tab, in its task list and TODO items, draws a spinner in place of the dot for **Working** and **Merging**. The label follows the tasks and inbox state the app already holds, so it needs no extra fetch.
**Task state.** The switcher and the Project tab show one label per task in place of its raw session state, the first that applies (`taskState` in `mac/Sources/Byre/Store.swift`), with a dot and a tooltip saying why: **Needs your answer** (an open `question` inbox item), **Merging** (`merging` or `merging_into_parent`), **Merge held** (`merge_queued_at`), **Merge after review** (`merge_after_review`), **Reviewing** (the task's `state` is `review_pending` or `reviewing`), **Working** (`running` or `discarding`; see "Task state in the store" for both), **Idle** (anything else, including no session). The Project tab, in its task list and TODO items, draws a spinner in place of the dot for **Reviewing**, **Working** and **Merging**. The label follows the tasks and inbox state the app already holds, so it needs no extra fetch.
**Unread summaries.** A Task's `summary_seq` is the seq of its latest `turn.ended` event with `summary: true` (0 for none), read through the partial index `events_summary`, so the app knows of unread summaries without loading transcripts; a `turn.ended` with `summary: true` on the global stream raises it in the app. The switcher's and the Project tab's task rows show a **new summary** state label while `summary_seq` is above the seq the user has read (`unreadSummary` in `mac/Sources/Byre/Store.swift`), except on tasks of a workflow run, which close too soon for their summaries to be read. Picking such a row (in the switcher, only when the task has no open inbox items) points the right pane at the task's Transcript and scrolls it to that turn's final message (`Store.jump`, applied by the transcript once the event is drawn). Read state is per user on the server: `summary_reads` holds one highest-read seq per user and task. `GET /api/me/summary-reads` returns the caller's, for active tasks, as a map from task id to seq, and `PUT /api/tasks/{taskId}/summary-read` with `{seq}` raises it, emitting `summary.read` (`task_id`, `seq`) owned by the user only when it went up, so every Mac the user has byre open on clears the label. A summary counts as read only when the summary turn's end row is on screen in a transcript while that server's window is the key window of the active app (`summaryVisible` in `mac/Sources/Byre/Chat.swift`), so selecting the task on another tab leaves it unread. `sqlite3 byre.db 'SELECT * FROM summary_reads'` shows the read seqs.
@@ -417,7 +417,7 @@ Every new snapshot of a task gets a review: a separate agent session on the vend
- **Forwarding.** When a submitted review has findings, byre sends them to the task's live agent as a `byre` message (`forwardFindings` and `findingsMessage` in `internal/agents/review.go`): each finding's severity, title, location and detail, under a preamble asking the agent to check each one against the code and its brief, act on those that hold up, and say in its final message which it acted on and which it left and why. It goes through `Manager.Message`, so it is queued like any message while the agent is mid-turn. A task with no live agent session gets nothing, and the log says `review findings not forwarded`. Forwarding doesn't set any finding's state; **fix** and **ignore** stay available.
- **Screenshots.** Reviewer sessions also get the Playwright MCP server (`playwrightMCP` in `internal/agents/review.go`: `npx @playwright/mcp` at a pinned version, headless Chrome, isolated profile, absolute file paths), passed to Claude as a `stdio` entry in `--mcp-config` and to Codex as `mcp_servers` in the thread's `config`. Its automatically named files go to `worktrees/<project-id>/review-<review-id>-media`, removed with the checkout. Each `images` entry is `{path, caption}`, the path absolute or relative to the checkout; at `submit_review` byre reads every one, refuses the whole submission (nothing recorded) for a missing file or one that isn't one of `store.ImageTypes` within `store.MaxBlobBytes`, then stores each as a blob and replaces its `path` with `blob`, since the files are deleted with the checkout. They run without the Stop-hook bounce, mid-turn delivery, end-of-turn commit, snapshots or turn summaries, and their `user.message` events carry no `head`. When the reviewer's turn ends byre completes its session, kills its process and removes the worktree; a review still `running` then fails with why (no submission, with the final message; an error; an interrupt). A process that exits mid-turn fails the review the same way. `StopReview` (`POST /api/reviews/{reviewId}/stop`, the Stop button on a running review's card) ends the reviewer the same way and fails the review ("the reviewer was stopped"), and `Watch` calls it for a reviewer whose process the health check finds `stuck` (no stream event for `StuckAfter`), which is how a reviewer whose connection died while the machine slept gets cleared. Merging or discarding the task fails its running reviews ("the task was closed") and ends their reviewers. On startup, a running reviewer resumes like any session, and a running review whose reviewer isn't `running` fails.
- **Events.** `review.started`, `review.finished` (state `done`, `failed` or `skipped`; a skipped review emits only `review.finished`) and `review.finding` (a finding's state set, with the user as actor) carry the whole review, with the task's ids and no `agent_session_id`. A reviewer's own events carry its session id and the task's ids: the transcript skips every event of a session whose `session.created` has `role` `reviewer`, and draws each `review.finished` as a note with its outcome; the lead's `read_transcript` leaves the reviewer's events out and shows each `review.finished` with its summary and findings (`reviewLine`). `GET /api/tasks/{taskId}/sessions` and everything that looks for the task's agent (`ListSessions`) see only implementer sessions.
- **UI.** The **Reviews** tab (`mac/Sources/Byre/Reviews.swift`) shows one review at a time, picked by snapshot (see "Snapshots" for the picker): its snapshot, state or severity counts, error, summary, screenshots and findings, and, while it runs, a **Stop** button (`POST /api/reviews/{reviewId}/stop`). The `change` field is not shown; it goes to the lead and into notes. A finding with `path` and `line` shows, under its title, the lines `line` to `end_line` plus 3 either side from that file's diff for the snapshot against the default branch (`changes?snapshot=<n>`, the diff the reviewer was given), with new-file line numbers and the finding's own lines marked; it shows nothing when those lines aren't in the diff, and "diff not available" with the error once the task is closed. Clicking a screenshot opens it full size. Each open finding has **fix** and **ignore** buttons (also on review cards in the inbox). **fix** sends the finding's severity, title, location and detail to the task's agent as a message from the user: through `POST /api/sessions/{id}/messages` to its latest live session (queued like any message while the agent is mid-turn, see "Agent sessions"), or starting a session when it has none; the view stays where it is, so several findings can be sent in a row. Then it, or **ignore** on its own, records the finding's state (`fixed` or `ignored`) with `PUT /api/reviews/{reviewId}/findings/{index}`, the index being the finding's position in `result.findings`; the review's `finding_states` (the `reviews.finding_states` column, a JSON object from index to state) holds them, and the buttons give way to "fix sent" or "ignored". Only a `done` review's findings take a state, and there is no way to clear one. The tab loads when it opens, when the task changes, and on each `review.*` event in the task's stream. The tab follows the latest `review.*` event of the highest-numbered snapshot: while that review runs, the tab label (and its entry in the More menu) carries no badge, hiding any earlier review's count; once it finishes with open findings (those with no state), the tab (or the More menu holding it) is highlighted with a badge of their count. Each snapshot row's **review** button shows that snapshot's latest `review.*` event: a fill in the accent tint (`Palette.accentTint`, the same in every task) while the review runs, and otherwise a mark over its top-right corner: a grey ● when it was skipped, a green ✓ when it finished with no open findings, and, when it has open findings, the button highlighted with their count in a badge. A failed review leaves the button unmarked. While any of a task's reviews is `running` and its agent is idle, the task's state is `reviewing` (see "Task state in the store"), which the task switcher and the Project tab show as **Working** (see "Task state"). `GET /api/tasks/{taskId}/reviews` lists a task's reviews oldest first. `sqlite3 byre.db "SELECT id, task_id, snapshot_n, vendor, state, error FROM reviews ORDER BY created_at DESC LIMIT 20"` shows recent ones.
- **UI.** The **Reviews** tab (`mac/Sources/Byre/Reviews.swift`) shows one review at a time, picked by snapshot (see "Snapshots" for the picker): its snapshot, state or severity counts, error, summary, screenshots and findings, and, while it runs, a **Stop** button (`POST /api/reviews/{reviewId}/stop`). The `change` field is not shown; it goes to the lead and into notes. A finding with `path` and `line` shows, under its title, the lines `line` to `end_line` plus 3 either side from that file's diff for the snapshot against the default branch (`changes?snapshot=<n>`, the diff the reviewer was given), with new-file line numbers and the finding's own lines marked; it shows nothing when those lines aren't in the diff, and "diff not available" with the error once the task is closed. Clicking a screenshot opens it full size. Each open finding has **fix** and **ignore** buttons (also on review cards in the inbox). **fix** sends the finding's severity, title, location and detail to the task's agent as a message from the user: through `POST /api/sessions/{id}/messages` to its latest live session (queued like any message while the agent is mid-turn, see "Agent sessions"), or starting a session when it has none; the view stays where it is, so several findings can be sent in a row. Then it, or **ignore** on its own, records the finding's state (`fixed` or `ignored`) with `PUT /api/reviews/{reviewId}/findings/{index}`, the index being the finding's position in `result.findings`; the review's `finding_states` (the `reviews.finding_states` column, a JSON object from index to state) holds them, and the buttons give way to "fix sent" or "ignored". Only a `done` review's findings take a state, and there is no way to clear one. The tab loads when it opens, when the task changes, and on each `review.*` event in the task's stream. The tab follows the latest `review.*` event of the highest-numbered snapshot: while that review runs, the tab label (and its entry in the More menu) carries no badge, hiding any earlier review's count; once it finishes with open findings (those with no state), the tab (or the More menu holding it) is highlighted with a badge of their count. Each snapshot row's **review** button shows that snapshot's latest `review.*` event: a fill in the accent tint (`Palette.accentTint`, the same in every task) while the review runs, and otherwise a mark over its top-right corner: a grey ● when it was skipped, a green ✓ when it finished with no open findings, and, when it has open findings, the button highlighted with their count in a badge. A failed review leaves the button unmarked. While any of a task's reviews is `running` and its agent is idle, the task's state is `reviewing` (see "Task state in the store"), which the task switcher and the Project tab show as **Reviewing** (see "Task state"). `GET /api/tasks/{taskId}/reviews` lists a task's reviews oldest first. `sqlite3 byre.db "SELECT id, task_id, snapshot_n, vendor, state, error FROM reviews ORDER BY created_at DESC LIMIT 20"` shows recent ones.
## Workflows
+6 -6
View File
@@ -18,11 +18,11 @@ struct TaskState {
let title: String
}
private let workingTitles: [ByreTask.StatePayload: String] = [
.running: "The agent is mid-turn or its subagents are running",
.reviewPending: "A review of the turn's work is about to start",
.reviewing: "A review is running",
.discarding: "Discarding",
private let workingTitles: [ByreTask.StatePayload: (label: String, title: String)] = [
.running: ("Working", "The agent is mid-turn or its subagents are running"),
.reviewPending: ("Reviewing", "A review of the turn's work is about to start"),
.reviewing: ("Reviewing", "A review is running"),
.discarding: ("Working", "Discarding"),
]
@MainActor
@@ -207,7 +207,7 @@ final class Store {
return TaskState(kind: .held, label: "Merge after review", title: "Merges when the running review finishes with no findings")
}
if let working = workingTitles[task.state] {
return TaskState(kind: .working, label: "Working", title: working)
return TaskState(kind: .working, label: working.label, title: working.title)
}
return TaskState(kind: .idle, label: "Idle", title: "Waiting for a message")
}