Render Markdown, add ask_human context, simplify inbox responses, group inbox by task
This commit is contained in:
+25
-19
@@ -22,6 +22,7 @@ var attachmentKinds = []api.AttachmentKind{
|
||||
}
|
||||
|
||||
type askArgs struct {
|
||||
Context string `json:"context"`
|
||||
Question string `json:"question"`
|
||||
WhyBlocked string `json:"why_blocked"`
|
||||
Options []string `json:"options"`
|
||||
@@ -35,6 +36,8 @@ func (m *Manager) askTool(l *live) mcp.Tool {
|
||||
return mcp.Tool{
|
||||
Name: "ask_human",
|
||||
Description: "Ask the human a question through their inbox. Use this for every question; never ask in plain prose. " +
|
||||
"The human answers from their inbox without having read your transcript, so every question must stand on its own: " +
|
||||
"put the background they need in context. " +
|
||||
"Offer concrete options and recommend one. If blocking is true, call report and end your turn after asking; " +
|
||||
"the answer arrives as your next message. If blocking is false, continue with work that doesn't depend on the answer. " +
|
||||
"Ask several questions in one turn by calling this once per question; they are answered together. " +
|
||||
@@ -42,7 +45,8 @@ func (m *Manager) askTool(l *live) mcp.Tool {
|
||||
InputSchema: json.RawMessage(`{
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"question": {"type": "string"},
|
||||
"context": {"type": "string", "description": "Everything the human needs to answer well without reading your transcript: what you are working on, what you found, the relevant facts and data, and what each option would concretely do. Several paragraphs is normal. Markdown is rendered."},
|
||||
"question": {"type": "string", "description": "The question itself, in one or two sentences. Markdown is rendered."},
|
||||
"why_blocked": {"type": "string", "description": "What you can't do until this is answered, and why you can't decide it yourself."},
|
||||
"options": {"type": "array", "items": {"type": "string"}, "minItems": 2},
|
||||
"recommended_option": {"type": "string", "description": "One of options."},
|
||||
@@ -65,7 +69,7 @@ func (m *Manager) askTool(l *live) mcp.Tool {
|
||||
"blocking": {"type": "boolean"},
|
||||
"thread_id": {"type": "string"}
|
||||
},
|
||||
"required": ["question", "why_blocked", "options", "recommended_option", "blocking"]
|
||||
"required": ["context", "question", "why_blocked", "options", "recommended_option", "blocking"]
|
||||
}`),
|
||||
Handler: func(ctx context.Context, raw json.RawMessage) (string, error) {
|
||||
return m.ask(ctx, l, raw)
|
||||
@@ -102,6 +106,7 @@ func (m *Manager) ask(ctx context.Context, l *live, raw json.RawMessage) (string
|
||||
}
|
||||
|
||||
payload := map[string]any{
|
||||
"context": args.Context,
|
||||
"question": args.Question,
|
||||
"why_blocked": args.WhyBlocked,
|
||||
"options": args.Options,
|
||||
@@ -109,7 +114,7 @@ func (m *Manager) ask(ctx context.Context, l *live, raw json.RawMessage) (string
|
||||
"attachments": attachments,
|
||||
}
|
||||
|
||||
err = m.store.CreateInboxItem(ctx, m.newItem(l, id, thread, api.InboxItemKindQuestion, priority, *args.Blocking, store.ToPayload(payload)))
|
||||
err = m.store.CreateInboxItem(ctx, m.newItem(l, id, thread, api.Question, priority, *args.Blocking, store.ToPayload(payload)))
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
@@ -123,6 +128,9 @@ func (m *Manager) ask(ctx context.Context, l *live, raw json.RawMessage) (string
|
||||
func (m *Manager) validateAsk(ctx context.Context, l *live, args askArgs) error {
|
||||
problems := []string{}
|
||||
|
||||
if strings.TrimSpace(args.Context) == "" {
|
||||
problems = append(problems, "context is empty: give the background the human needs to answer without reading your transcript")
|
||||
}
|
||||
if strings.TrimSpace(args.Question) == "" {
|
||||
problems = append(problems, "question is empty")
|
||||
}
|
||||
@@ -164,7 +172,7 @@ func (m *Manager) validateAsk(ctx context.Context, l *live, args askArgs) error
|
||||
|
||||
func (m *Manager) fileReport(l *live, r report.Report) {
|
||||
id := store.NewID()
|
||||
err := m.store.CreateInboxItem(context.Background(), m.newItem(l, id, id, api.InboxItemKindReport, api.Fyi, false, store.ToPayload(r)))
|
||||
err := m.store.CreateInboxItem(context.Background(), m.newItem(l, id, id, api.Report, api.Fyi, false, store.ToPayload(r)))
|
||||
if err != nil {
|
||||
slog.Error("file report", "session", l.id, "err", err)
|
||||
}
|
||||
@@ -172,7 +180,7 @@ func (m *Manager) fileReport(l *live, r report.Report) {
|
||||
|
||||
func (m *Manager) fileError(l *live, msg string) {
|
||||
id := store.NewID()
|
||||
err := m.store.CreateInboxItem(context.Background(), m.newItem(l, id, id, api.InboxItemKindSessionError, api.Blocked, true, map[string]any{"error": msg}))
|
||||
err := m.store.CreateInboxItem(context.Background(), m.newItem(l, id, id, api.SessionError, api.Blocked, true, map[string]any{"error": msg}))
|
||||
if err != nil {
|
||||
slog.Error("file session error", "session", l.id, "err", err)
|
||||
}
|
||||
@@ -181,7 +189,7 @@ func (m *Manager) fileError(l *live, msg string) {
|
||||
func (m *Manager) fileRateLimited(l *live, info map[string]any, failure string) {
|
||||
id := store.NewID()
|
||||
payload := map[string]any{"vendor": string(l.vendor), "rate_limit": info, "error": failure}
|
||||
err := m.store.CreateInboxItem(context.Background(), m.newItem(l, id, id, api.InboxItemKindRateLimited, api.Blocked, true, payload))
|
||||
err := m.store.CreateInboxItem(context.Background(), m.newItem(l, id, id, api.RateLimited, api.Blocked, true, payload))
|
||||
if err != nil {
|
||||
slog.Error("file rate limit", "session", l.id, "err", err)
|
||||
}
|
||||
@@ -261,28 +269,26 @@ func deliveryMessage(pending []store.Pending) string {
|
||||
b.WriteString("\n")
|
||||
|
||||
switch p.Item.Kind {
|
||||
case api.InboxItemKindQuestion:
|
||||
case api.Question:
|
||||
fmt.Fprintf(b, "Your question %s: %s\n", p.Item.Id, p.Item.Payload["question"])
|
||||
case api.InboxItemKindReport:
|
||||
case api.Report:
|
||||
fmt.Fprintf(b, "Your report %s\n", p.Item.Id)
|
||||
case api.InboxItemKindSessionError:
|
||||
case api.SessionError:
|
||||
fmt.Fprintf(b, "Your error %s: %s\n", p.Item.Id, p.Item.Payload["error"])
|
||||
case api.InboxItemKindRateLimited:
|
||||
case api.RateLimited:
|
||||
fmt.Fprintf(b, "Your rate-limited turn %s: %s\n", p.Item.Id, p.Item.Payload["error"])
|
||||
}
|
||||
|
||||
switch r.Type {
|
||||
case api.InboxResponseTypeOption:
|
||||
case api.Option:
|
||||
fmt.Fprintf(b, "%s chose: %s\n", r.User, *r.Option)
|
||||
if r.Text != nil && *r.Text != "" {
|
||||
fmt.Fprintf(b, "Note: %s\n", *r.Text)
|
||||
case api.Other:
|
||||
if p.Item.Kind == api.Question {
|
||||
fmt.Fprintf(b, "%s answered (none of your options): %s\n", r.User, *r.Text)
|
||||
fmt.Fprintf(b, "If this asks you something back, answer by calling ask_human with thread_id %q.\n", p.Item.ThreadId)
|
||||
} else {
|
||||
fmt.Fprintf(b, "%s replied: %s\n", r.User, *r.Text)
|
||||
}
|
||||
case api.InboxResponseTypeFreeForm:
|
||||
fmt.Fprintf(b, "%s answered (none of your options): %s\n", r.User, *r.Text)
|
||||
case api.InboxResponseTypeRedirect:
|
||||
fmt.Fprintf(b, "%s redirected you. These instructions replace your current direction: %s\n", r.User, *r.Text)
|
||||
case api.InboxResponseTypeQuestion:
|
||||
fmt.Fprintf(b, "%s asked a follow-up question: %s\nAnswer it by calling ask_human with thread_id %q.\n", r.User, *r.Text, p.Item.ThreadId)
|
||||
}
|
||||
|
||||
for _, a := range r.Attachments {
|
||||
|
||||
@@ -19,11 +19,11 @@ func TestValidateAsk(t *testing.T) {
|
||||
args askArgs
|
||||
want []string
|
||||
}{
|
||||
{"valid", askArgs{Question: "q", WhyBlocked: "w", Options: []string{"a", "b"}, RecommendedOption: "a", Blocking: &yes}, nil},
|
||||
{"empty", askArgs{}, []string{"question is empty", "why_blocked is empty", "at least two", "recommended_option", "blocking is required"}},
|
||||
{"one option", askArgs{Question: "q", WhyBlocked: "w", Options: []string{"a"}, RecommendedOption: "a", Blocking: &yes}, []string{"at least two"}},
|
||||
{"bad recommendation", askArgs{Question: "q", WhyBlocked: "w", Options: []string{"a", "b"}, RecommendedOption: "c", Blocking: &yes}, []string{"recommended_option"}},
|
||||
{"bad attachment", askArgs{Question: "q", WhyBlocked: "w", Options: []string{"a", "b"}, RecommendedOption: "a", Blocking: &yes,
|
||||
{"valid", askArgs{Context: "c", Question: "q", WhyBlocked: "w", Options: []string{"a", "b"}, RecommendedOption: "a", Blocking: &yes}, nil},
|
||||
{"empty", askArgs{}, []string{"context is empty", "question is empty", "why_blocked is empty", "at least two", "recommended_option", "blocking is required"}},
|
||||
{"one option", askArgs{Context: "c", Question: "q", WhyBlocked: "w", Options: []string{"a"}, RecommendedOption: "a", Blocking: &yes}, []string{"at least two"}},
|
||||
{"bad recommendation", askArgs{Context: "c", Question: "q", WhyBlocked: "w", Options: []string{"a", "b"}, RecommendedOption: "c", Blocking: &yes}, []string{"recommended_option"}},
|
||||
{"bad attachment", askArgs{Context: "c", Question: "q", WhyBlocked: "w", Options: []string{"a", "b"}, RecommendedOption: "a", Blocking: &yes,
|
||||
Attachments: []api.Attachment{{Kind: "video"}}}, []string{`attachments[0].kind "video"`}},
|
||||
}
|
||||
|
||||
@@ -53,25 +53,24 @@ func TestValidateAsk(t *testing.T) {
|
||||
|
||||
func TestDeliveryMessage(t *testing.T) {
|
||||
str := func(s string) *string { return &s }
|
||||
question := api.InboxItem{Id: "q1", ThreadId: "q1", Kind: api.InboxItemKindQuestion, Payload: map[string]any{"question": "Which color?"}}
|
||||
report := api.InboxItem{Id: "r1", ThreadId: "r1", Kind: api.InboxItemKindReport, Payload: map[string]any{}}
|
||||
question := api.InboxItem{Id: "q1", ThreadId: "q1", Kind: api.Question, Payload: map[string]any{"question": "Which color?"}}
|
||||
failure := api.InboxItem{Id: "e1", ThreadId: "e1", Kind: api.SessionError, Payload: map[string]any{"error": "process died"}}
|
||||
|
||||
msg := deliveryMessage([]store.Pending{
|
||||
{Item: question, Response: api.InboxResponse{User: "alice", Type: api.InboxResponseTypeOption, Option: str("blue"), Text: str("navy if possible")}},
|
||||
{Item: question, Response: api.InboxResponse{User: "bob", Type: api.InboxResponseTypeQuestion, Text: str("what is it for?"),
|
||||
{Item: question, Response: api.InboxResponse{User: "alice", Type: api.Option, Option: str("blue")}},
|
||||
{Item: question, Response: api.InboxResponse{User: "bob", Type: api.Other, Text: str("what is it for?"),
|
||||
Attachments: []api.Attachment{{Kind: api.AttachmentKindUrl, Url: str("https://example.com")}}}},
|
||||
{Item: report, Response: api.InboxResponse{User: "alice", Type: api.InboxResponseTypeRedirect, Text: str("stop and write tests")}},
|
||||
{Item: failure, Response: api.InboxResponse{User: "alice", Type: api.Other, Text: str("stop and write tests")}},
|
||||
})
|
||||
|
||||
for _, want := range []string{
|
||||
"Your question q1: Which color?",
|
||||
"alice chose: blue",
|
||||
"Note: navy if possible",
|
||||
"bob asked a follow-up question: what is it for?",
|
||||
"bob answered (none of your options): what is it for?",
|
||||
`calling ask_human with thread_id "q1"`,
|
||||
`"url":"https://example.com"`,
|
||||
"Your report r1",
|
||||
"alice redirected you. These instructions replace your current direction: stop and write tests",
|
||||
"Your error e1: process died",
|
||||
"alice replied: stop and write tests",
|
||||
} {
|
||||
if !strings.Contains(msg, want) {
|
||||
t.Fatalf("message missing %q:\n%s", want, msg)
|
||||
|
||||
@@ -317,7 +317,7 @@ func (e *env) openQuestions(sessionID string) []api.InboxItem {
|
||||
}
|
||||
questions := []api.InboxItem{}
|
||||
for _, item := range items {
|
||||
if item.Kind == api.InboxItemKindQuestion {
|
||||
if item.Kind == api.Question {
|
||||
questions = append(questions, item)
|
||||
}
|
||||
}
|
||||
@@ -379,7 +379,7 @@ func TestAskHumanRoundTrip(t *testing.T) {
|
||||
t.Fatalf("awaiting human: %d %v", waiting, err)
|
||||
}
|
||||
|
||||
e.respond(m, q, api.InboxResponseTypeOption, "blue", "", api.Answered)
|
||||
e.respond(m, q, api.Option, "blue", "", api.Answered)
|
||||
events := e.waitIdle()
|
||||
|
||||
delivered := false
|
||||
@@ -423,7 +423,7 @@ func TestFollowUpQuestionThread(t *testing.T) {
|
||||
}
|
||||
first := questions[0]
|
||||
|
||||
e.respond(m, first, api.InboxResponseTypeQuestion, "", "Before I pick: which of the two did you recommend, and why?", api.Open)
|
||||
e.respond(m, first, api.Other, "", "Before I pick: which of the two did you recommend, and why?", api.Answered)
|
||||
e.waitIdle()
|
||||
|
||||
questions = e.openQuestions(sess.Id)
|
||||
@@ -435,15 +435,7 @@ func TestFollowUpQuestionThread(t *testing.T) {
|
||||
t.Fatalf("follow-up didn't continue the thread: first %s/%s, second %s/%s", first.Id, first.ThreadId, second.Id, second.ThreadId)
|
||||
}
|
||||
|
||||
old, err := e.store.GetInboxItem(e.ctx, first.Id)
|
||||
if err != nil {
|
||||
t.Fatalf("get first question: %v", err)
|
||||
}
|
||||
if old.State != api.Superseded {
|
||||
t.Fatalf("first question is %s, want superseded", old.State)
|
||||
}
|
||||
|
||||
e.respond(m, second, api.InboxResponseTypeFreeForm, "", "Use green instead of either.", api.Answered)
|
||||
e.respond(m, second, api.Other, "", "Use green instead of either.", api.Answered)
|
||||
e.waitIdle()
|
||||
|
||||
out, err := os.ReadFile(filepath.Join(e.task.WorktreePath, "color.txt"))
|
||||
|
||||
@@ -68,7 +68,7 @@ func TestRateLimitedTurn(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("list inbox: %v", err)
|
||||
}
|
||||
if len(items) != 1 || items[0].Kind != api.InboxItemKindRateLimited || items[0].Priority != api.Blocked {
|
||||
if len(items) != 1 || items[0].Kind != api.RateLimited || items[0].Priority != api.Blocked {
|
||||
t.Fatalf("want one blocked rate_limited item, got %+v", items)
|
||||
}
|
||||
limit, ok := items[0].Payload["rate_limit"].(map[string]any)
|
||||
|
||||
Reference in New Issue
Block a user