From e7e6fae7586afb544bf4e6aa6eaf6bb2a13061a4 Mon Sep 17 00:00:00 2001 From: Ian Gulliver Date: Mon, 19 Oct 2026 20:05:55 -0700 Subject: [PATCH] suggest_discard again --- docs/operations.md | 2 +- internal/agents/ready.go | 14 ++++++++++--- .../064-ready-prompt-by-event-seq.sql | 9 +++++++++ internal/store/store.go | 20 ++++++++++--------- 4 files changed, 32 insertions(+), 13 deletions(-) create mode 100644 internal/store/migrations/064-ready-prompt-by-event-seq.sql diff --git a/docs/operations.md b/docs/operations.md index b252e2a..cd5f383 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -37,7 +37,7 @@ A task ends `merged` or `discarded`; either way its sessions are stopped, its sn A task's summary is what its agent writes, before the task closes, about the whole task: what it set out to do, what was built or tried, the key decisions and why, and what was left out. The agent sets it with the `set_task_summary` MCP tool (`internal/agents/summary.go`), any time while the task is active; each call replaces `tasks.summary` and records a `task.summary` event carrying it, which the transcript shows as a note. The Merge turn's prompt and the discard turn's prompt (`summaryRequest`) both ask for it. Turn summaries don't stand in for it: each covers only its own turn, and a merged task's last one is always the Merge turn's. Tasks closed before the column existed have none. `sqlite3 byre.db "SELECT id, status, summary FROM tasks WHERE status != 'active' ORDER BY closed_at DESC LIMIT 5"` shows recent ones. -An implementer says its task is ready to merge with the `ready_for_merge` MCP tool (`internal/agents/ready.go`), whose `overview` covers the whole task for a human switching between tasks: a summary, tradeoffs and compromises, what was not done, and future TODOs. Its description tells the agent to call it only once the review of its most recent changes has come back, never in the same turn as a change or while that review is pending. Each call records a `task.ready` event carrying it, which the lead gets as a signal and the transcript draws as a bubble (`ReadyBubble` in `mac/Sources/Byre/Chat.swift`) headed "ready for merge". The latest one, while the task is active and neither a question nor a snapshot other than an answer snapshot has come since it (a question, or newer work the overview doesn't cover), is drawn like an open question (the warn tint and border) and carries small Merge and Discard buttons that act like the header's, confirmation dialogs included; earlier ones are drawn like an answered question (the faint warn tint), without buttons. Recording it changes nothing else. byre asks for it (`readyPrompt`, sent as a byre message) when a task's state goes to `idle` or its workflow finishes (`Manager.Observe` calls `promptReady`) and the latest non-answer snapshot's review is `done` or `skipped`, once per snapshot. It checks and sends under the session's delivery lock (the one human messages and answers take), and only to an agent that isn't running, so the prompt never queues behind a turn that could add unreviewed work; `tasks.ready_prompt_n` holds the last snapshot asked about, set only once the prompt's turn started, so turns that change no files don't ask again and a new snapshot does. It doesn't ask for a workflow stage task until its run has a `task.workflow_finished` event (by then the run's other stage tasks are being discarded, leaving the final one for the human), a task with a merge after review set, or a review that failed, and it skips the `reviewing` → `idle` change of a review with findings, since those go to the agent; the turn that answers them asks once it ends without new work. The prompt tells the agent not to call the tool, and to say what's left, when the task isn't ready. +An implementer says its task is ready to merge with the `ready_for_merge` MCP tool (`internal/agents/ready.go`), whose `overview` covers the whole task for a human switching between tasks: a summary, tradeoffs and compromises, what was not done, and future TODOs. Its description tells the agent to call it only once the review of its most recent changes has come back, never in the same turn as a change or while that review is pending. Each call records a `task.ready` event carrying it, which the lead gets as a signal and the transcript draws as a bubble (`ReadyBubble` in `mac/Sources/Byre/Chat.swift`) headed "ready for merge". The latest one, while the task is active and neither a question nor a snapshot other than an answer snapshot has come since it (a question, or newer work the overview doesn't cover), is drawn like an open question (the warn tint and border) and carries small Merge and Discard buttons that act like the header's, confirmation dialogs included; earlier ones are drawn like an answered question (the faint warn tint), without buttons. Recording it changes nothing else. byre asks for it (`readyPrompt`, sent as a byre message) when a task's state goes to `idle` or its workflow finishes (`Manager.Observe` calls `promptReady`) and the latest non-answer snapshot's review is `done` or `skipped`. It checks and sends under the session's delivery lock (the one human messages and answers take), and only to an agent that isn't running, so the prompt never queues behind a turn that could add unreviewed work. `tasks.ready_prompt_seq` holds the task's last event seq when the prompt's turn started; byre asks again only once a non-answer snapshot or a message from the human (a chat message or a question's answer, not one from byre, the agent or the lead) has come after it, so the agent's own turns that change no files don't ask again, and a human unblocking an agent that said it was waiting does. It doesn't ask while the task has an open question. It doesn't ask for a workflow stage task until its run has a `task.workflow_finished` event (by then the run's other stage tasks are being discarded, leaving the final one for the human), a task with a merge after review set, or a review that failed, and it skips the `reviewing` → `idle` change of a review with findings, since those go to the agent; the turn that answers them asks once it ends without new work. The prompt tells the agent not to call the tool, and to say what's left, when the task isn't ready. An implementer that thinks its task shouldn't land at all (the approach failed, the task turned out unnecessary or already done) says so with the `suggest_discard` MCP tool (also `internal/agents/ready.go`), whose `reason` explains why; `readyPrompt` offers it as the alternative to `ready_for_merge`. Each call records a `task.discard_suggested` event, which the lead gets as a signal and the transcript draws with the same `ReadyBubble`, headed "suggests discarding". It takes part in "latest" with the ready-for-merge overviews, so whichever of the two came last is the current one, drawn and cleared under the same rules, and it carries only the Discard button. It only suggests: the task stays open until the human discards it. diff --git a/internal/agents/ready.go b/internal/agents/ready.go index 42a79f2..e838eac 100644 --- a/internal/agents/ready.go +++ b/internal/agents/ready.go @@ -156,15 +156,23 @@ func (m *Manager) tryPromptReady(ctx context.Context, taskID string, fromReview return nil } } - prompted, err := m.store.ReadyPromptN(ctx, taskID) - if err != nil || prompted >= latest.N { + due, err := m.store.ReadyPromptDue(ctx, taskID) + if err != nil || !due { + return err + } + open, err := m.store.ListInbox(ctx, store.InboxFilter{TaskID: taskID, State: api.Open}) + if err != nil || len(open) > 0 { return err } _, err = m.Send(ctx, sess.Id, "byre", readyPrompt, "") if err != nil { return err } - err = m.store.SetReadyPromptN(ctx, taskID, latest.N) + seq, err := m.store.LastTaskSeq(ctx, taskID) + if err != nil { + return err + } + err = m.store.SetReadyPromptSeq(ctx, taskID, seq) if err != nil { return err } diff --git a/internal/store/migrations/064-ready-prompt-by-event-seq.sql b/internal/store/migrations/064-ready-prompt-by-event-seq.sql new file mode 100644 index 0000000..168a274 --- /dev/null +++ b/internal/store/migrations/064-ready-prompt-by-event-seq.sql @@ -0,0 +1,9 @@ +ALTER TABLE tasks ADD COLUMN ready_prompt_seq INTEGER NOT NULL DEFAULT 0; + +UPDATE tasks SET ready_prompt_seq = COALESCE(( + SELECT MAX(e.seq) FROM events e + WHERE e.task_id = tasks.id AND e.kind = 'user.message' AND e.actor = 'byre' + AND json_extract(e.payload, '$.text') LIKE 'Your turn has ended and the review of your latest work has finished.%' +), 0); + +ALTER TABLE tasks DROP COLUMN ready_prompt_n; diff --git a/internal/store/store.go b/internal/store/store.go index a52c52a..eaee689 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -746,17 +746,19 @@ func (s *Store) SetTaskSummary(ctx context.Context, id, summary, actor string) e }) } -func (s *Store) ReadyPromptN(ctx context.Context, id string) (int, error) { - n := 0 - err := s.db.QueryRowContext(ctx, `SELECT ready_prompt_n FROM tasks WHERE id = ?`, id).Scan(&n) - if errors.Is(err, sql.ErrNoRows) { - return 0, ErrNotFound - } - return n, err +func (s *Store) ReadyPromptDue(ctx context.Context, id string) (bool, error) { + due := false + err := s.db.QueryRowContext(ctx, `SELECT EXISTS ( + SELECT 1 FROM events e, tasks t + WHERE t.id = ? AND e.task_id = t.id AND e.seq > t.ready_prompt_seq + AND ((e.kind = 'snapshot.created' AND json_extract(e.payload, '$.trigger') != ?) + OR (e.kind = 'user.message' AND e.actor NOT IN ('byre', ?) AND e.actor NOT LIKE '% (lead)')) + )`, id, string(api.SnapshotTriggerAnswer), agentActor).Scan(&due) + return due, err } -func (s *Store) SetReadyPromptN(ctx context.Context, id string, snapshotN int) error { - _, err := s.db.ExecContext(ctx, `UPDATE tasks SET ready_prompt_n = ? WHERE id = ?`, snapshotN, id) +func (s *Store) SetReadyPromptSeq(ctx context.Context, id string, seq int64) error { + _, err := s.db.ExecContext(ctx, `UPDATE tasks SET ready_prompt_seq = ? WHERE id = ?`, seq, id) return err }