Let agents cancel open questions; fix snapshots missing same-size edits
This commit is contained in:
+1
-1
@@ -102,7 +102,7 @@ The task switcher (⌘K, or the navbar crumb) lists active tasks with their sess
|
|||||||
|
|
||||||
## Inbox
|
## Inbox
|
||||||
|
|
||||||
Agents ask questions with the `ask_human` MCP tool. Each question carries a `context` with the background the human needs to answer without reading the transcript; the UI renders it, the question, and the options as Markdown. A malformed call (no `context` or `why_blocked`, fewer than two options, a recommendation that isn't one of the options, an unknown attachment kind, a foreign `thread_id`) is rejected with a message saying what to fix, and nothing is filed. Each turn's final message (as a `report`) and each agent error (a turn that ends in error without a human interrupt, a process that dies mid-turn) also files an item.
|
Agents ask questions with the `ask_human` MCP tool. Each question carries a `context` with the background the human needs to answer without reading the transcript; the UI renders it, the question, and the options as Markdown. A malformed call (no `context` or `why_blocked`, fewer than two options, a recommendation that isn't one of the options, an unknown attachment kind, a foreign `thread_id`) is rejected with a message saying what to fix, and nothing is filed. The call returns the item's id as `question_id`. An agent withdraws a question it no longer needs answered (the human answered it in a chat message, say) with the `cancel_question` MCP tool and that id: an `open` question of its own session becomes `dismissed` with no `resolved_by` (its `inbox.updated` event has actor `agent`), which also stops a blocking one from holding back delivery; anything else is rejected with a message saying why. Each turn's final message (as a `report`) and each agent error (a turn that ends in error without a human interrupt, a process that dies mid-turn) also files an item.
|
||||||
|
|
||||||
`GET /api/inbox` is one query over open, unsnoozed items, oldest first; `project_id` and `task_id` narrow it to a project or task view, and `state` and `include_snoozed` widen it. Blocking questions and errors are `blocked`; reports and non-blocking questions are `fyi`. The inbox view groups items by task, each group headed by project and task: tasks with a blocked item come first, then tasks sort by project name and by their oldest item. Within a task, items are in time order.
|
`GET /api/inbox` is one query over open, unsnoozed items, oldest first; `project_id` and `task_id` narrow it to a project or task view, and `state` and `include_snoozed` widen it. Blocking questions and errors are `blocked`; reports and non-blocking questions are `fyi`. The inbox view groups items by task, each group headed by project and task: tasks with a blocked item come first, then tasks sort by project name and by their oldest item. Within a task, items are in time order.
|
||||||
|
|
||||||
|
|||||||
@@ -124,10 +124,61 @@ func (m *Manager) ask(ctx context.Context, l *live, raw json.RawMessage) (string
|
|||||||
|
|
||||||
m.snapshotSoon(l, api.SnapshotTriggerAskHuman)
|
m.snapshotSoon(l, api.SnapshotTriggerAskHuman)
|
||||||
|
|
||||||
|
cancel := fmt.Sprintf("If the human answers it some other way, such as in a chat message, withdraw it with cancel_question and question_id %q.", id)
|
||||||
if *args.Blocking {
|
if *args.Blocking {
|
||||||
return fmt.Sprintf("Question %s filed in the human's inbox. It is blocking: end your turn now. The answer will arrive as your next message.", id), nil
|
return fmt.Sprintf("Question filed in the human's inbox. question_id: %s. It is blocking: end your turn now. The answer will arrive as your next message. %s", id, cancel), nil
|
||||||
}
|
}
|
||||||
return fmt.Sprintf("Question %s filed in the human's inbox. Continue with work that doesn't depend on it; the answer will arrive as a later message.", id), nil
|
return fmt.Sprintf("Question filed in the human's inbox. question_id: %s. Continue with work that doesn't depend on it; the answer will arrive as a later message. %s", id, cancel), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
type cancelArgs struct {
|
||||||
|
QuestionID string `json:"question_id"`
|
||||||
|
}
|
||||||
|
|
||||||
|
func (m *Manager) cancelQuestionTool(l *live) mcp.Tool {
|
||||||
|
return mcp.Tool{
|
||||||
|
Name: "cancel_question",
|
||||||
|
Description: "Withdraw a question you asked with ask_human that is still waiting in the human's inbox, " +
|
||||||
|
"for example because the human answered it in a chat message instead. It is removed from the inbox and no answer to it will arrive.",
|
||||||
|
InputSchema: json.RawMessage(`{
|
||||||
|
"type": "object",
|
||||||
|
"properties": {
|
||||||
|
"question_id": {"type": "string", "description": "The question_id ask_human returned."}
|
||||||
|
},
|
||||||
|
"required": ["question_id"]
|
||||||
|
}`),
|
||||||
|
Handler: func(ctx context.Context, raw json.RawMessage) (string, error) {
|
||||||
|
return m.cancelQuestion(ctx, l, raw)
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func (m *Manager) cancelQuestion(ctx context.Context, l *live, raw json.RawMessage) (string, error) {
|
||||||
|
args := cancelArgs{}
|
||||||
|
err := json.Unmarshal(raw, &args)
|
||||||
|
if err != nil {
|
||||||
|
return "", fmt.Errorf("malformed cancel_question call: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
item, err := m.store.GetInboxItem(ctx, args.QuestionID)
|
||||||
|
if errors.Is(err, store.ErrNotFound) {
|
||||||
|
return "", fmt.Errorf("question_id %s does not exist", args.QuestionID)
|
||||||
|
}
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
if item.AgentSessionId != l.id || item.Kind != api.Question {
|
||||||
|
return "", fmt.Errorf("question_id %s is not a question you asked", args.QuestionID)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = m.store.WithdrawItem(ctx, item.Id)
|
||||||
|
if errors.Is(err, store.ErrNotOpen) {
|
||||||
|
return "", fmt.Errorf("question %s is already %s; nothing was cancelled", item.Id, item.State)
|
||||||
|
}
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
return fmt.Sprintf("Question %s withdrawn from the human's inbox.", item.Id), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (m *Manager) validateAsk(ctx context.Context, l *live, args askArgs) error {
|
func (m *Manager) validateAsk(ctx context.Context, l *live, args askArgs) error {
|
||||||
|
|||||||
@@ -3,8 +3,10 @@ package agents
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
"g.fc.run/misc/byre/internal/api"
|
"g.fc.run/misc/byre/internal/api"
|
||||||
"g.fc.run/misc/byre/internal/store"
|
"g.fc.run/misc/byre/internal/store"
|
||||||
@@ -52,6 +54,86 @@ func TestValidateAsk(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestCancelQuestion(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
st, err := store.Open(filepath.Join(t.TempDir(), "byre.db"), func(api.Event) {})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("open store: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { st.Close() })
|
||||||
|
|
||||||
|
now := time.Now().UTC()
|
||||||
|
_, err = st.EnsureUser(ctx, "alice", "Alice")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ensure user: %v", err)
|
||||||
|
}
|
||||||
|
err = st.CreateProject(ctx, api.Project{Id: "p1", Name: "demo", RepoPath: "/nonexistent", DefaultBranch: "main", Remote: "none", CreatedBy: "alice", CreatedAt: now})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create project: %v", err)
|
||||||
|
}
|
||||||
|
err = st.CreateTask(ctx, api.Task{Id: "t1", ProjectId: "p1", Title: "Fix the thing", Branch: "b", WorktreePath: "/nonexistent", BaseRef: "x", Status: api.TaskStatusActive, CreatedBy: "alice", CreatedAt: now})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create task: %v", err)
|
||||||
|
}
|
||||||
|
err = st.CreateSession(ctx, api.AgentSession{Id: "s1", TaskId: "t1", Vendor: api.Claude, VendorSessionId: "s1", State: api.AgentSessionStateIdle, Role: api.Implementer, CreatedBy: "alice", CreatedAt: now}, "p1")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create session: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
m := New(st, DefaultLimits, t.TempDir())
|
||||||
|
l := &live{id: "s1", dir: t.TempDir(), vendor: api.Claude, taskID: "t1", projectID: "p1"}
|
||||||
|
other := &live{id: "s2", dir: t.TempDir(), vendor: api.Claude, taskID: "t1", projectID: "p1"}
|
||||||
|
|
||||||
|
yes := true
|
||||||
|
raw, err := json.Marshal(askArgs{Context: "c", Question: "Red or blue?", WhyBlocked: "w", Options: []string{"red", "blue"}, RecommendedOption: "red", Blocking: &yes})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("marshal: %v", err)
|
||||||
|
}
|
||||||
|
out, err := m.ask(ctx, l, raw)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ask: %v", err)
|
||||||
|
}
|
||||||
|
items, err := st.ListInbox(ctx, store.InboxFilter{SessionID: "s1", State: api.Open})
|
||||||
|
if err != nil || len(items) != 1 {
|
||||||
|
t.Fatalf("inbox after ask: %+v %v", items, err)
|
||||||
|
}
|
||||||
|
id := items[0].Id
|
||||||
|
if !strings.Contains(out, "question_id: "+id) || !strings.Contains(out, "cancel_question") {
|
||||||
|
t.Fatalf("ask result doesn't give the id and cancel_question: %s", out)
|
||||||
|
}
|
||||||
|
|
||||||
|
cancel := func(l *live, id string) error {
|
||||||
|
raw, err := json.Marshal(cancelArgs{QuestionID: id})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("marshal: %v", err)
|
||||||
|
}
|
||||||
|
_, err = m.cancelQuestion(ctx, l, raw)
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if cancel(l, "nope") == nil {
|
||||||
|
t.Fatalf("cancelled a question that doesn't exist")
|
||||||
|
}
|
||||||
|
if cancel(other, id) == nil {
|
||||||
|
t.Fatalf("cancelled another session's question")
|
||||||
|
}
|
||||||
|
err = cancel(l, id)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("cancel: %v", err)
|
||||||
|
}
|
||||||
|
item, err := st.GetInboxItem(ctx, id)
|
||||||
|
if err != nil || item.State != api.Dismissed || item.ResolvedBy != nil || item.ResolvedAt == nil {
|
||||||
|
t.Fatalf("after cancel: %+v %v", item, err)
|
||||||
|
}
|
||||||
|
waiting, err := st.AwaitingHuman(ctx, "s1")
|
||||||
|
if err != nil || waiting != 0 {
|
||||||
|
t.Fatalf("still awaiting human after cancel: %d %v", waiting, err)
|
||||||
|
}
|
||||||
|
if cancel(l, id) == nil {
|
||||||
|
t.Fatalf("cancelled a question twice")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestAsksQuestion(t *testing.T) {
|
func TestAsksQuestion(t *testing.T) {
|
||||||
cases := []struct {
|
cases := []struct {
|
||||||
name string
|
name string
|
||||||
|
|||||||
@@ -565,7 +565,7 @@ func (m *Manager) ensureLive(ctx context.Context, sess api.AgentSession, task ap
|
|||||||
GitName: creator.GitName,
|
GitName: creator.GitName,
|
||||||
GitEmail: creator.GitEmail,
|
GitEmail: creator.GitEmail,
|
||||||
Guidance: guidance,
|
Guidance: guidance,
|
||||||
MCPServers: []*mcp.Server{{Name: "byre", Version: "0.0.0", Tools: []mcp.Tool{m.askTool(l), m.createTaskTool(l), m.setMergeTitleTool(l)}}},
|
MCPServers: []*mcp.Server{{Name: "byre", Version: "0.0.0", Tools: []mcp.Tool{m.askTool(l), m.cancelQuestionTool(l), m.createTaskTool(l), m.setMergeTitleTool(l)}}},
|
||||||
Hooks: map[string][]claude.HookMatcher{
|
Hooks: map[string][]claude.HookMatcher{
|
||||||
"UserPromptSubmit": {{Func: remindGuidance}},
|
"UserPromptSubmit": {{Func: remindGuidance}},
|
||||||
"Stop": {{Func: m.bounceProseQuestion(l)}},
|
"Stop": {{Func: m.bounceProseQuestion(l)}},
|
||||||
|
|||||||
@@ -41,7 +41,7 @@ func TestRateLimitedTurn(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
m := New(st, DefaultLimits, t.TempDir())
|
m := New(st, DefaultLimits, t.TempDir())
|
||||||
l := &live{id: "s1", vendor: api.Claude, taskID: "t1", projectID: "p1"}
|
l := &live{id: "s1", dir: t.TempDir(), vendor: api.Claude, taskID: "t1", projectID: "p1"}
|
||||||
|
|
||||||
feed := func(typ, raw string) {
|
feed := func(typ, raw string) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|||||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
+19
-17
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
+15
-16
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -45,6 +45,10 @@ func Snapshot(dir string) (string, error) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return "", err
|
return "", err
|
||||||
}
|
}
|
||||||
|
info, err := os.Stat(index)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
data, err := os.ReadFile(index)
|
data, err := os.ReadFile(index)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", err
|
return "", err
|
||||||
@@ -64,6 +68,11 @@ func Snapshot(dir string) (string, error) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return "", err
|
return "", err
|
||||||
}
|
}
|
||||||
|
// git trusts cached stat data for entries older than the index file; a fresh mtime would hide same-size edits.
|
||||||
|
err = os.Chtimes(tmp.Name(), info.ModTime(), info.ModTime())
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
|
||||||
env := []string{"GIT_INDEX_FILE=" + tmp.Name()}
|
env := []string{"GIT_INDEX_FILE=" + tmp.Name()}
|
||||||
_, err = runEnv(dir, env, "add", "--all")
|
_, err = runEnv(dir, env, "add", "--all")
|
||||||
|
|||||||
@@ -190,6 +190,26 @@ func (s *Store) Respond(ctx context.Context, actor string, resolutions []Resolut
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var ErrNotOpen = errors.New("inbox item is not open")
|
||||||
|
|
||||||
|
func (s *Store) WithdrawItem(ctx context.Context, id string) error {
|
||||||
|
return s.write(ctx, func(tx *sql.Tx, emit emitFunc) error {
|
||||||
|
res, err := tx.ExecContext(ctx, `UPDATE inbox_items SET state = ?, resolved_at = ?, snoozed_until = NULL WHERE id = ? AND state = ?`,
|
||||||
|
string(api.Dismissed), now(), id, string(api.Open))
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
n, err := res.RowsAffected()
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if n == 0 {
|
||||||
|
return ErrNotOpen
|
||||||
|
}
|
||||||
|
return emitItem(ctx, tx, emit, "inbox.updated", id, agentActor)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func dismissTaskInbox(ctx context.Context, tx *sql.Tx, emit emitFunc, taskID, actor string) error {
|
func dismissTaskInbox(ctx context.Context, tx *sql.Tx, emit emitFunc, taskID, actor string) error {
|
||||||
ids := []string{}
|
ids := []string{}
|
||||||
rows, err := tx.QueryContext(ctx, `SELECT id FROM inbox_items WHERE task_id = ? AND state = ?`, taskID, string(api.Open))
|
rows, err := tx.QueryContext(ctx, `SELECT id FROM inbox_items WHERE task_id = ? AND state = ?`, taskID, string(api.Open))
|
||||||
|
|||||||
Reference in New Issue
Block a user