mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 18:00:54 -10:00
* feat(tools): give tool results an image channel and add view_image Closes #824. tools.Result carried only text, so no tool could hand the model a picture. The sharpest edge was our own capture tooling: browser_screenshot, desktop_screenshot and browser_pdf wrote a PNG, set Meta["artifact_path"] and returned "Artifact captured: <path>". Nothing read that path afterwards, so the model asked for a screenshot, was told one existed, and could not look at it. Zero was never image-blind on the way in — zeroruntime.ImageBlock, Message.Images and internal/imageinput are mature. What was missing is the model-initiated leg. Result.Images is delivered as a following USER message, not on the tool result itself. Every provider drops images on a tool-role message: Anthropic maps a tool result to a tool_result block whose content is a string, Gemini to a functionResponse, and OpenAI guards its image content-parts to the user role with an explicit comment saying so. Attaching them to the tool result would have plumbed the whole path and delivered nothing. A separate message also keeps one tool result per tool call, which the providers validate. Blocks with no data, or a media type outside the provider allow-list, are dropped rather than sent — a rejected image would fail the whole turn, and the tool's text still stands on its own. The message names the tool, since the images arrive detached from their result and nothing else says which call produced them when several ran in one turn. view_image resolves through resolveScopedReadPath before imageinput.LoadFile. That scoping is load-bearing: LoadFile joins a relative path to the workspace root and otherwise takes the path as given, so without it the tool would read anywhere on disk. It is registered with the other scoped readers and declared read-only and thread-safe, matching read_file. The capture tools now attach what they wrote. A PDF or an unreadable file yields no image and the text stands alone, so a capture is never failed over its attachment. Co-Authored-By: Claude Opus 5 <[email protected]> * fix(agent): keep tool results contiguous when a tool returns an image The image messages are user-role, and the first version appended each one immediately after its own tool result. With two tool calls in a turn that produces tool_result, user, tool_result — and a user message between two tool results breaks strict provider replay. Anthropic makes it concrete. appendUserBlocks coalesces consecutive user content into one block list, and tool_result blocks must come first in that message, so the interleaved shape maps to: [tool_result, text, image, tool_result] which the API rejects. loop.go already documents this exact rule three lines below the change, for the self-correction feedback it defers for the same reason. Images are now collected during the tool loop and appended once every tool_result is recorded — including the aborted placeholders on the abort, stop-reason and repeated-failure paths, so an early exit cannot strand them mid-sequence either. The original test used a single tool call, which cannot exhibit the bug. The new one runs two calls where the first returns an image and asserts the image message does not fall between the first and last tool result. Also from review: view_image joins the "execute" tool category, which grants read_file but not view_image, so a specialist declared with tools: [execute] could not look at a screenshot it had just captured. Covered by a test that holds for every category granting read_file rather than naming one. And the core read-only set is now asserted by tool name, not only by count — a count still passes when one reader is swapped for another. Co-Authored-By: Claude Opus 5 <[email protected]> * test(agent): pin the tool-result/image ordering contract The assertions only tracked the first and last tool-result index, so four distinct broken orderings passed: a message wedged between the two tool results, a duplicated image flush, a duplicated tool result, and both results answering the same tool_call_id. Mutation-testing each of those against the real loop confirmed the old assertions were green on all four. Now it collects every tool-result index and id and requires exactly two, in c1-then-c2 order, at adjacent indexes, with the image following the last one and delivered exactly once. Adjacency is the assertion that carries the contract: Anthropic needs every tool_result block ahead of any other block in the user message they coalesce into, so anything between the two results is the 400 this change exists to avoid. The image is deliberately NOT pinned to exactly last+1. appendUserBlocks merges consecutive user-role content, so [tool_result, tool_result, text, image] maps to tool_result blocks first and replays fine; pinning would fail that valid shape while killing nothing the adjacency and count assertions do not already catch. messageShape reports the actual role ordering on failure, and the loop uses two ifs rather than a switch so an image left on a tool-role message is reported as the misplacement it is instead of "the image never reached the model". --------- Co-authored-by: Claude Opus 5 <[email protected]>
413 lines
15 KiB
Go
413 lines
15 KiB
Go
package specialist
|
|
|
|
import (
|
|
"context"
|
|
"os"
|
|
"path/filepath"
|
|
"reflect"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
func TestBuildArgsCreatesFreshSpecialistExecInvocation(t *testing.T) {
|
|
executor := Executor{
|
|
NewSessionID: func() (string, error) { return "child_session", nil },
|
|
}
|
|
manifest := Manifest{
|
|
Metadata: Metadata{
|
|
Name: "reviewer",
|
|
Description: "Reviews code",
|
|
Model: "claude-sonnet-4.5",
|
|
ReasoningEffort: "high",
|
|
},
|
|
SystemPrompt: "Review carefully.",
|
|
ResolvedTools: []string{"grep", "read_file"},
|
|
}
|
|
|
|
result, err := executor.BuildArgs(BuildArgsInput{
|
|
Manifest: manifest,
|
|
Prompt: "Review this patch",
|
|
ParentSessionID: "parent_session",
|
|
ParentToolUseID: "toolu_123",
|
|
ParentModel: "gpt-4.1",
|
|
ParentReasoningEffort: "medium",
|
|
CurrentDepth: 1,
|
|
Description: "Auth diff",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs returned error: %v", err)
|
|
}
|
|
if result.SessionID != "child_session" || result.PromptFile != "" {
|
|
t.Fatalf("unexpected result metadata: %#v", result)
|
|
}
|
|
wantArgs := []string{
|
|
"exec", "--init-session-id", "child_session",
|
|
result.Args[3],
|
|
"--model", "claude-sonnet-4.5",
|
|
"--reasoning-effort", "high",
|
|
"--auto", "low", // no PermissionMode set -> fail-safe low (not unsafe high)
|
|
"--output-format", "stream-json",
|
|
"--enabled-tools", "grep,read_file",
|
|
"--depth", "2",
|
|
"--tag", "specialist",
|
|
"--calling-session-id", "parent_session",
|
|
"--calling-tool-use-id", "toolu_123",
|
|
"--session-title", "reviewer: Auth diff",
|
|
}
|
|
if !reflect.DeepEqual(result.Args, wantArgs) {
|
|
t.Fatalf("args mismatch\ngot: %#v\nwant: %#v", result.Args, wantArgs)
|
|
}
|
|
prompt := result.Args[3]
|
|
for _, want := range []string{"Specialist: reviewer", "Task description: Auth diff", "Review carefully.", "Review this patch"} {
|
|
if !strings.Contains(prompt, want) {
|
|
t.Fatalf("wrapped prompt missing %q:\n%s", want, prompt)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestSpecialistAutonomyByPermissionMode(t *testing.T) {
|
|
cases := map[string]string{
|
|
"": "low", // fail-safe: an unset mode does NOT inherit unsafe autonomy
|
|
"unsafe": "high",
|
|
"auto": "low",
|
|
"ask": "low",
|
|
"spec-draft": "low",
|
|
"whatever": "low",
|
|
}
|
|
for mode, want := range cases {
|
|
if got := specialistAutonomy(mode); got != want {
|
|
t.Errorf("specialistAutonomy(%q) = %q, want %q", mode, got, want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestMemberAwareAutonomy(t *testing.T) {
|
|
// A non-unsafe member runs at "member" (write/edit + sandboxed shell), a plain
|
|
// specialist at "low" (read-only), and an unsafe parent at "high" either way.
|
|
cases := []struct {
|
|
mode string
|
|
member bool
|
|
want string
|
|
}{
|
|
{"auto", false, "low"},
|
|
{"ask", false, "low"},
|
|
{"auto", true, "member"},
|
|
{"ask", true, "member"},
|
|
{"", true, "member"}, // member, fail-safe non-unsafe, still write-capable
|
|
{"unsafe", true, "high"}, // unsafe parent keeps full autonomy
|
|
{"unsafe", false, "high"},
|
|
}
|
|
for _, c := range cases {
|
|
if got := memberAwareAutonomy(c.mode, c.member); got != c.want {
|
|
t.Errorf("memberAwareAutonomy(%q, %v) = %q, want %q", c.mode, c.member, got, c.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestBuildArgsMemberAutonomyEmitsMember(t *testing.T) {
|
|
executor := Executor{NewSessionID: func() (string, error) { return "child", nil }}
|
|
manifest := Manifest{Metadata: Metadata{Name: "subagent"}, SystemPrompt: "x", ResolvedTools: []string{"read_file", "write_file"}}
|
|
|
|
// A non-unsafe member → --auto member (write-capable), not the read-only low.
|
|
res, err := executor.BuildArgs(BuildArgsInput{Manifest: manifest, Prompt: "p", PermissionMode: "auto", MemberAutonomy: true})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs: %v", err)
|
|
}
|
|
if !containsSequence(res.Args, []string{"--auto", "member"}) {
|
|
t.Fatalf("non-unsafe member must yield --auto member, got %v", res.Args)
|
|
}
|
|
|
|
// Without the member flag, the same parent stays --auto low (unchanged).
|
|
plain, err := executor.BuildArgs(BuildArgsInput{Manifest: manifest, Prompt: "p", PermissionMode: "auto"})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs(plain): %v", err)
|
|
}
|
|
if !containsSequence(plain.Args, []string{"--auto", "low"}) || containsSequence(plain.Args, []string{"--auto", "member"}) {
|
|
t.Fatalf("a plain specialist must stay --auto low, got %v", plain.Args)
|
|
}
|
|
|
|
// An unsafe member still runs --auto high, never downgraded to member.
|
|
unsafe, err := executor.BuildArgs(BuildArgsInput{Manifest: manifest, Prompt: "p", PermissionMode: "unsafe", MemberAutonomy: true})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs(unsafe member): %v", err)
|
|
}
|
|
if !containsSequence(unsafe.Args, []string{"--auto", "high"}) {
|
|
t.Fatalf("unsafe member must yield --auto high, got %v", unsafe.Args)
|
|
}
|
|
}
|
|
|
|
func TestBuildArgsAutonomyHonorsPermissionMode(t *testing.T) {
|
|
executor := Executor{NewSessionID: func() (string, error) { return "child", nil }}
|
|
manifest := Manifest{Metadata: Metadata{Name: "reviewer"}, SystemPrompt: "x", ResolvedTools: []string{"read_file"}}
|
|
|
|
// A non-unsafe parent mode yields a non-unsafe child (--auto low), so a
|
|
// swarm member never gains more authority than a non-unsafe orchestrator.
|
|
res, err := executor.BuildArgs(BuildArgsInput{Manifest: manifest, Prompt: "p", PermissionMode: "auto"})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs: %v", err)
|
|
}
|
|
if !containsSequence(res.Args, []string{"--auto", "low"}) {
|
|
t.Fatalf("non-unsafe parent must yield --auto low, got %v", res.Args)
|
|
}
|
|
if containsSequence(res.Args, []string{"--auto", "high"}) {
|
|
t.Fatalf("non-unsafe parent must NOT yield --auto high: %v", res.Args)
|
|
}
|
|
|
|
// Only an explicit unsafe parent keeps --auto high.
|
|
out, err := executor.BuildArgs(BuildArgsInput{Manifest: manifest, Prompt: "p", PermissionMode: "unsafe"})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs(unsafe): %v", err)
|
|
}
|
|
if !containsSequence(out.Args, []string{"--auto", "high"}) {
|
|
t.Fatalf("unsafe parent must yield --auto high, got %v", out.Args)
|
|
}
|
|
|
|
// An unset mode is fail-safe low (the foot-gun fix): a caller that forgets to
|
|
// wire PermissionMode does NOT silently get an unsafe child.
|
|
empty, err := executor.BuildArgs(BuildArgsInput{Manifest: manifest, Prompt: "p", PermissionMode: ""})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs(empty): %v", err)
|
|
}
|
|
if !containsSequence(empty.Args, []string{"--auto", "low"}) {
|
|
t.Fatalf("empty mode must yield --auto low (fail-safe), got %v", empty.Args)
|
|
}
|
|
}
|
|
|
|
func TestBuildResumeArgsAutonomyHonorsPermissionMode(t *testing.T) {
|
|
executor := Executor{}
|
|
manifest := Manifest{Metadata: Metadata{Name: "reviewer"}, ResolvedTools: []string{"read_file"}}
|
|
res, err := executor.BuildResumeArgs(BuildResumeArgsInput{
|
|
SessionID: "child_session", Prompt: "p", Manifest: manifest, PermissionMode: "auto",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("BuildResumeArgs: %v", err)
|
|
}
|
|
if !containsSequence(res.Args, []string{"--auto", "low"}) || containsSequence(res.Args, []string{"--auto", "high"}) {
|
|
t.Fatalf("resume non-unsafe parent must yield --auto low, got %v", res.Args)
|
|
}
|
|
}
|
|
|
|
func TestBuildArgsSessionTitleFallsBackToNameWithoutDescription(t *testing.T) {
|
|
executor := Executor{NewSessionID: func() (string, error) { return "child", nil }}
|
|
manifest := Manifest{
|
|
Metadata: Metadata{Name: "reviewer", Description: "Reviews code"},
|
|
SystemPrompt: "Review carefully.",
|
|
ResolvedTools: []string{"read_file"},
|
|
}
|
|
|
|
// Description is optional. When omitted, the session title must still carry
|
|
// the specialist name so AgentName (derived from the title) is non-empty and
|
|
// the session remains resumable.
|
|
result, err := executor.BuildArgs(BuildArgsInput{
|
|
Manifest: manifest,
|
|
Prompt: "Do the thing",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs returned error: %v", err)
|
|
}
|
|
if !containsSequence(result.Args, []string{"--session-title", "reviewer"}) {
|
|
t.Fatalf("args missing name-only session title: %#v", result.Args)
|
|
}
|
|
for index, arg := range result.Args {
|
|
if arg == "--session-title" && index+1 < len(result.Args) {
|
|
if got := result.Args[index+1]; got != "reviewer" {
|
|
t.Fatalf("session title = %q, want %q", got, "reviewer")
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestBuildArgsInheritsParentModelAndReasoning(t *testing.T) {
|
|
executor := Executor{NewSessionID: func() (string, error) { return "child", nil }}
|
|
manifest := Manifest{
|
|
Metadata: Metadata{Name: "worker", Description: "Works"},
|
|
SystemPrompt: "Do work.",
|
|
ResolvedTools: []string{"read_file"},
|
|
}
|
|
|
|
result, err := executor.BuildArgs(BuildArgsInput{
|
|
Manifest: manifest,
|
|
Prompt: "Do the thing",
|
|
ParentModel: "gpt-4.1",
|
|
ParentReasoningEffort: "medium",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs returned error: %v", err)
|
|
}
|
|
if !containsSequence(result.Args, []string{"--model", "gpt-4.1"}) {
|
|
t.Fatalf("args missing inherited model: %#v", result.Args)
|
|
}
|
|
if !containsSequence(result.Args, []string{"--reasoning-effort", "medium"}) {
|
|
t.Fatalf("args missing inherited reasoning effort: %#v", result.Args)
|
|
}
|
|
}
|
|
|
|
func TestBuildArgsDefaultsToReadOnlyToolAllowlist(t *testing.T) {
|
|
executor := Executor{NewSessionID: func() (string, error) { return "child", nil }}
|
|
|
|
result, err := executor.BuildArgs(BuildArgsInput{
|
|
Manifest: Manifest{
|
|
Metadata: Metadata{Name: "worker", Description: "Works"},
|
|
SystemPrompt: "Do work.",
|
|
},
|
|
Prompt: "Do the thing",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs returned error: %v", err)
|
|
}
|
|
if !containsSequence(result.Args, []string{"--enabled-tools", "glob,grep,list_directory,read_file,read_minified_file,view_image"}) {
|
|
t.Fatalf("args missing default read-only allowlist: %#v", result.Args)
|
|
}
|
|
}
|
|
|
|
func TestBuildArgsWritesLargePromptFile(t *testing.T) {
|
|
root := t.TempDir()
|
|
var writtenPrompt string
|
|
executor := Executor{
|
|
NewSessionID: func() (string, error) { return "child", nil },
|
|
PromptFileMaxSize: 16,
|
|
WritePromptFile: func(prompt string) (string, error) {
|
|
writtenPrompt = prompt
|
|
path := filepath.Join(root, "prompt.md")
|
|
return path, os.WriteFile(path, []byte(prompt), 0o600)
|
|
},
|
|
}
|
|
|
|
result, err := executor.BuildArgs(BuildArgsInput{
|
|
Manifest: Manifest{
|
|
Metadata: Metadata{Name: "worker", Description: "Works"},
|
|
SystemPrompt: strings.Repeat("system ", 10),
|
|
},
|
|
Prompt: "Do the large thing",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs returned error: %v", err)
|
|
}
|
|
if result.PromptFile != filepath.Join(root, "prompt.md") {
|
|
t.Fatalf("PromptFile = %q", result.PromptFile)
|
|
}
|
|
if !reflect.DeepEqual(result.Args[:5], []string{"exec", "--init-session-id", "child", "--file", result.PromptFile}) {
|
|
t.Fatalf("prompt file args = %#v", result.Args[:5])
|
|
}
|
|
if !strings.Contains(writtenPrompt, "Do the large thing") {
|
|
t.Fatalf("written prompt missing task: %s", writtenPrompt)
|
|
}
|
|
}
|
|
|
|
func TestBuildResumeArgsUsesExistingSession(t *testing.T) {
|
|
result, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
|
|
SessionID: "child_session",
|
|
Prompt: "Follow up",
|
|
CurrentDepth: 2,
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("BuildResumeArgs returned error: %v", err)
|
|
}
|
|
wantPrefix := []string{"exec", "--resume", "child_session"}
|
|
if !reflect.DeepEqual(result.Args[:3], wantPrefix) {
|
|
t.Fatalf("resume args prefix = %#v", result.Args[:3])
|
|
}
|
|
if !containsSequence(result.Args, []string{"--auto", "low"}) || // no PermissionMode -> fail-safe low
|
|
!containsSequence(result.Args, []string{"--output-format", "stream-json"}) ||
|
|
!containsSequence(result.Args, []string{"--depth", "3"}) ||
|
|
!containsSequence(result.Args, []string{"--tag", "specialist"}) {
|
|
t.Fatalf("resume args missing required flags: %#v", result.Args)
|
|
}
|
|
if !strings.Contains(result.Args[3], "Follow-up Instructions") || !strings.Contains(result.Args[3], "Follow up") {
|
|
t.Fatalf("resume prompt not wrapped correctly: %s", result.Args[3])
|
|
}
|
|
}
|
|
|
|
func TestBuildArgsRejectsInvalidInputs(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
input BuildArgsInput
|
|
want string
|
|
}{
|
|
{name: "negative depth", input: BuildArgsInput{Prompt: "hi", CurrentDepth: -1}, want: "depth"},
|
|
{name: "empty prompt", input: BuildArgsInput{CurrentDepth: 0}, want: "prompt"},
|
|
}
|
|
for _, tc := range tests {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
_, err := (Executor{}).BuildArgs(tc.input)
|
|
if err == nil || !strings.Contains(err.Error(), tc.want) {
|
|
t.Fatalf("BuildArgs error = %v, want %q", err, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestRunRejectsDepthExceedingMax(t *testing.T) {
|
|
_, err := (Executor{}).Run(context.Background(), TaskParameters{
|
|
Prompt: "hi",
|
|
}, TaskRunOptions{CurrentDepth: maxSpecialistDepth + 1})
|
|
if err == nil || !strings.Contains(err.Error(), "depth") {
|
|
t.Fatalf("Run error = %v, want depth error", err)
|
|
}
|
|
}
|
|
|
|
// TestRunRejectsDepthAtMax covers the boundary: a parent already AT the cap
|
|
// must be rejected too, since this Run call would launch a child one level
|
|
// past it (--depth CurrentDepth+1). Only checking ">" here would let that
|
|
// child start before the guard ever caught it. The guard sits before the
|
|
// fresh/resume branch in Run, so both call shapes must be proven to reject
|
|
// here rather than falling through to runFresh/runResume.
|
|
func TestRunRejectsDepthAtMax(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
params TaskParameters
|
|
}{
|
|
{name: "fresh", params: TaskParameters{Prompt: "hi"}},
|
|
{name: "resume", params: TaskParameters{Prompt: "hi", Resume: "child_session"}},
|
|
}
|
|
for _, tc := range tests {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
_, err := (Executor{}).Run(context.Background(), tc.params, TaskRunOptions{CurrentDepth: maxSpecialistDepth})
|
|
if err == nil || !strings.Contains(err.Error(), "depth") {
|
|
t.Fatalf("Run error = %v, want depth error", err)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestBuildArgsRejectsInvalidSessionIDs(t *testing.T) {
|
|
_, err := (Executor{NewSessionID: func() (string, error) { return "../escape", nil }}).BuildArgs(BuildArgsInput{
|
|
Prompt: "hi",
|
|
})
|
|
if err == nil || !strings.Contains(err.Error(), "invalid specialist session id") {
|
|
t.Fatalf("BuildArgs error = %v", err)
|
|
}
|
|
|
|
_, err = (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
|
|
SessionID: "../escape",
|
|
Prompt: "hi",
|
|
})
|
|
if err == nil || !strings.Contains(err.Error(), "invalid resume session id") {
|
|
t.Fatalf("BuildResumeArgs error = %v", err)
|
|
}
|
|
}
|
|
|
|
func TestBuildArgsNormalizesGeneratedSessionID(t *testing.T) {
|
|
result, err := (Executor{
|
|
NewSessionID: func() (string, error) { return " child_session ", nil },
|
|
}).BuildArgs(BuildArgsInput{Prompt: "hi"})
|
|
if err != nil {
|
|
t.Fatalf("BuildArgs returned error: %v", err)
|
|
}
|
|
if result.SessionID != "child_session" {
|
|
t.Fatalf("SessionID = %q, want child_session", result.SessionID)
|
|
}
|
|
if !containsSequence(result.Args, []string{"--init-session-id", "child_session"}) {
|
|
t.Fatalf("args missing normalized session id: %#v", result.Args)
|
|
}
|
|
}
|
|
|
|
func containsSequence(values []string, sequence []string) bool {
|
|
for index := 0; index+len(sequence) <= len(values); index++ {
|
|
if reflect.DeepEqual(values[index:index+len(sequence)], sequence) {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|