mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-08 01:09:46 -10:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8adc625d16 | ||
|
|
e4bc1f65da |
No files matched your search
@@ -67,39 +67,3 @@ requirements. If a related check fails, fix it. If a required check cannot run
|
||||
because of the environment or fails for an unrelated external reason, report
|
||||
the exact failure and obtain maintainer direction rather than silently ignoring
|
||||
it.
|
||||
|
||||
## 4. Common Review Blockers
|
||||
|
||||
These classes drive multi-round reviews. Fix them before requesting review:
|
||||
|
||||
- **Fresh base:** Rebase onto the current PR base (`main` or stacked target)
|
||||
before review. A stale head that rolls back mainline commits is a hard
|
||||
blocker, not a merge-time detail. Resolve conflicts by keeping upstream
|
||||
behavior unless the PR intentionally changes it.
|
||||
- **Platform truth:** Code and tests must pass on Linux, macOS, and Windows.
|
||||
Compare paths after canonicalization (physical/`EvalSymlinks` form); never
|
||||
assert raw `t.TempDir()` spellings (`/var` vs `/private/var` on macOS, short
|
||||
paths on Windows). Prefer one cross-platform function with small `GOOS`
|
||||
checks over duplicated helpers that drift.
|
||||
- **Security edges:** Keep secrets out of argv, env dumps, and logs. Redact
|
||||
success and error paths (including stderr). Fail closed on ownership, lease,
|
||||
and permission checks. Do not rely on pre-open path resolution
|
||||
(`EvalSymlinks` then open) for containment: that is a check-to-use race.
|
||||
Bind containment at open/use time with rooted or handle-relative,
|
||||
traversal-resistant APIs. If a no-follow API is used, apply it to every
|
||||
traversed component and enforce the platform's reparse-point protections;
|
||||
final-component-only no-follow is insufficient. On multi-step setup, roll back
|
||||
only what this run created; never destroy pre-existing resources you did
|
||||
not create; never report success when cleanup or unlock failed.
|
||||
- **Atomic shared state:** Write a complete temporary file, then atomically
|
||||
replace the destination so concurrent readers never see a partial write.
|
||||
Exclusive create or a write lock alone is not enough for readers. Serialize
|
||||
the full read-modify-write sequence for lockfiles and shared stores.
|
||||
- **Tests match the claim:** Every behavior or security-boundary change needs a
|
||||
regression test, including the failure path. Path-sensitive logic needs at
|
||||
least one non-Linux path case (or a hermetic fake that exercises the same
|
||||
normalization).
|
||||
- **Honest scope:** PR description, help text, and comments must match what
|
||||
shipped. Wire advertised entry points or shrink the claim. Do not bundle
|
||||
unrelated fixes. Do not widen security allowlists by name alone; check
|
||||
classification and side effects too.
|
||||
@@ -127,7 +127,9 @@ func TestExecuteToolCallMissingAdditionalPermissionsErrorIsActionable(t *testing
|
||||
if !strings.Contains(result.Output, `{"network": {"enabled": true}}`) {
|
||||
t.Fatalf("output = %q, want a concrete additional_permissions example", result.Output)
|
||||
}
|
||||
assertNamesOmitRemedy(t, "missing additional_permissions", result.Output)
|
||||
if !strings.Contains(result.Output, "omit sandbox_permissions") {
|
||||
t.Fatalf("output = %q, want guidance that the flag can simply be omitted", result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
// A well-formed additional_permissions payload must still reach the normal
|
||||
@@ -163,123 +165,6 @@ func TestExecuteToolCallStillPromptsForValidAdditionalPermissions(t *testing.T)
|
||||
}
|
||||
}
|
||||
|
||||
// runShellCall drives one shell tool call through the real registry (and so
|
||||
// through Registry.RunWithOptions) exactly as the agent loop would.
|
||||
func runShellCall(t *testing.T, root string, arguments string) ToolResult {
|
||||
t.Helper()
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(tools.NewScopedExecCommandTool(root, nil, nil))
|
||||
result, err := executeToolCall(
|
||||
context.Background(),
|
||||
registry,
|
||||
ToolCall{ID: "c1", Name: "exec_command", Arguments: arguments},
|
||||
PermissionModeUnsafe,
|
||||
Options{Cwd: root},
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("executeToolCall: %v", err)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// TestAdditionalPermissionsRejectionsConverge is the regression guard for
|
||||
// #845/#847. A model that attaches a permission object to a command needing
|
||||
// none used to be told only "set sandbox_permissions", which pushed it into the
|
||||
// OTHER invalid shape (flag set, payload empty) and its "include a permission"
|
||||
// error pushed it straight back — the reported loop that burned turns on
|
||||
// `git branch --show-current`.
|
||||
//
|
||||
// The contract this pins is convergence, not tolerance: both shapes are still
|
||||
// REJECTED (nothing here grants access), but each rejection must name the one
|
||||
// follow-up call that works, and that follow-up must actually succeed.
|
||||
func TestAdditionalPermissionsRejectionsConverge(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
const readOnly = `"cmd":"git --version"`
|
||||
|
||||
// Step 1 of the reported loop: a permission object with no flag.
|
||||
first := runShellCall(t, root, `{`+readOnly+`,"additional_permissions":{"network":{"enabled":true}}}`)
|
||||
if first.Status != tools.StatusError {
|
||||
t.Fatalf("unflagged additional_permissions status = %q, want error (rejecting is correct)", first.Status)
|
||||
}
|
||||
assertNamesOmitRemedy(t, "unflagged additional_permissions", first.Output)
|
||||
|
||||
// Step 2 of the reported loop: the model keeps the flag but empties the
|
||||
// payload. This must not be the dead end it was.
|
||||
second := runShellCall(t, root, `{`+readOnly+`,"sandbox_permissions":"with_additional_permissions"}`)
|
||||
if second.Status != tools.StatusError {
|
||||
t.Fatalf("flag without payload status = %q, want error", second.Status)
|
||||
}
|
||||
assertNamesOmitRemedy(t, "flag without payload", second.Output)
|
||||
|
||||
// The call both errors point at must actually work.
|
||||
converged := runShellCall(t, root, `{`+readOnly+`}`)
|
||||
if converged.Status != tools.StatusOK {
|
||||
t.Fatalf("the call both errors instruct the model to make failed: status=%q output=%q", converged.Status, converged.Output)
|
||||
}
|
||||
}
|
||||
|
||||
// assertNamesOmitRemedy pins the actionable half of the error text: the message
|
||||
// must spell out that BOTH fields can simply be dropped. "Set the flag" or
|
||||
// "include a permission" alone is what made the two errors chase each other.
|
||||
func assertNamesOmitRemedy(t *testing.T, label string, output string) {
|
||||
t.Helper()
|
||||
if !strings.Contains(output, "omit both sandbox_permissions and additional_permissions") {
|
||||
t.Fatalf("%s error = %q, want it to say to omit both fields entirely", label, output)
|
||||
}
|
||||
if !strings.Contains(output, "retry") {
|
||||
t.Fatalf("%s error = %q, want it to tell the model to retry", label, output)
|
||||
}
|
||||
}
|
||||
|
||||
// The empty-payload rejection carries the same remedy. It is pinned on the
|
||||
// validator directly because normalizeNoopAdditionalPermissions absorbs the
|
||||
// empty payload before executeToolCall reaches this branch;
|
||||
// inlineAdditionalPermissionsProfile is still the shared source of truth that
|
||||
// buildPermissionEvent and the grant path re-run, so its text must converge too.
|
||||
func TestEmptyAdditionalPermissionsErrorNamesOmitRemedy(t *testing.T) {
|
||||
args := map[string]any{
|
||||
"cmd": "git --version",
|
||||
"sandbox_permissions": string(tools.SandboxPermissionsWithAdditionalPermissions),
|
||||
"additional_permissions": map[string]any{
|
||||
"network": map[string]any{"enabled": false},
|
||||
"file_system": map[string]any{"read": []any{}, "write": []any{}},
|
||||
},
|
||||
}
|
||||
_, requested, err := inlineAdditionalPermissionsProfile(args, t.TempDir())
|
||||
if !requested {
|
||||
t.Fatal("requested = false, want true for an explicit with_additional_permissions call")
|
||||
}
|
||||
if err == nil {
|
||||
t.Fatal("an empty additional_permissions payload was accepted; it must still be rejected")
|
||||
}
|
||||
assertNamesOmitRemedy(t, "empty payload", err.Error())
|
||||
}
|
||||
|
||||
// A provider that materializes every optional property may send
|
||||
// "additional_permissions": null. That grants nothing and carries no flag, so it
|
||||
// is the field omitted — rejecting it stalled an ordinary command behind an
|
||||
// error the model could not act on.
|
||||
func TestExecuteToolCallTreatsNullAdditionalPermissionsAsOmitted(t *testing.T) {
|
||||
result := runShellCall(t, t.TempDir(), `{"cmd":"git --version","additional_permissions":null}`)
|
||||
if result.Status != tools.StatusOK {
|
||||
t.Fatalf("null additional_permissions was rejected: status=%q output=%q", result.Status, result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
// A null payload alongside the flag must still be rejected, and must land on
|
||||
// the same error as an absent payload rather than being quietly downgraded.
|
||||
func TestExecuteToolCallRejectsFlaggedNullAdditionalPermissions(t *testing.T) {
|
||||
result := runShellCall(t, t.TempDir(),
|
||||
`{"cmd":"git --version","sandbox_permissions":"with_additional_permissions","additional_permissions":null}`)
|
||||
if result.Status != tools.StatusError {
|
||||
t.Fatalf("flagged null status = %q, want error", result.Status)
|
||||
}
|
||||
if !strings.Contains(result.Output, "no additional_permissions object was provided") {
|
||||
t.Fatalf("output = %q, want the missing-payload error", result.Output)
|
||||
}
|
||||
assertNamesOmitRemedy(t, "flagged null", result.Output)
|
||||
}
|
||||
|
||||
func TestNormalizeProviderExpandedEmptyAdditionalPermissions(t *testing.T) {
|
||||
for _, mode := range []string{
|
||||
string(tools.SandboxPermissionsUseDefault),
|
||||
|
||||
+10
-100
@@ -656,13 +656,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
// between tool_results breaks strict provider replay) — same after-batch
|
||||
// rationale as turnRequestedModel above.
|
||||
var changedFilesThisBatch []string
|
||||
// Images produced by tools this turn, held until every tool_result is
|
||||
// recorded. They travel as user messages, and a user message between two
|
||||
// tool_results breaks strict provider replay — Anthropic coalesces them
|
||||
// into one user block list and requires the tool_result blocks first, so
|
||||
// interleaving yields [tool_result, text, image, tool_result] and a 400.
|
||||
// Same reason the self-correction feedback below is deferred.
|
||||
var toolImageMessages []zeroruntime.Message
|
||||
// Parallel read-ahead state: results for calls[precomputedStart:precomputedEnd]
|
||||
// executed concurrently, consumed strictly in order below.
|
||||
var precomputed []precomputedToolResult
|
||||
@@ -716,16 +709,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
Content: toolResult.Output,
|
||||
ToolCallID: toolResult.ToolCallID,
|
||||
})
|
||||
// Images ride a following USER message rather than the tool result
|
||||
// above. Every provider drops images on a tool-role message —
|
||||
// Anthropic's tool_result content is a string, Gemini's is a
|
||||
// functionResponse, and OpenAI guards its image parts to the user role
|
||||
// — so attaching them there would silently deliver nothing. A separate
|
||||
// message also keeps the one-tool-result-per-tool-call pairing intact,
|
||||
// which the providers validate.
|
||||
if imageMessage, ok := toolResultImageMessage(toolResult); ok {
|
||||
toolImageMessages = append(toolImageMessages, imageMessage)
|
||||
}
|
||||
|
||||
// A tool may demand the run ABORT — a canceled/timed-out ask_user prompt
|
||||
// returns context.Canceled rather than fabricating a headless answer. Stop
|
||||
@@ -736,13 +719,11 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
}
|
||||
if abortErr != nil {
|
||||
messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:])
|
||||
messages = append(messages, toolImageMessages...)
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, abortErr
|
||||
}
|
||||
if stopReason := stopReasonFromToolResult(toolResult); stopReason != "" {
|
||||
messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:])
|
||||
messages = append(messages, toolImageMessages...)
|
||||
result.FinalAnswer = toolResult.Output
|
||||
result.StopReason = stopReason
|
||||
result.Messages = copyMessages(messages)
|
||||
@@ -766,7 +747,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
// messages stay valid for a strict provider replay (Anthropic
|
||||
// rejects a tool_use with no answering tool_result).
|
||||
messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:])
|
||||
messages = append(messages, toolImageMessages...)
|
||||
result.FinalAnswer = toolFailureStopAnswer(call.Name, outcome.Count)
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, nil
|
||||
@@ -785,10 +765,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
postEditDiagnostics.enqueue(ctx, toolResult.ChangedFiles)
|
||||
}
|
||||
}
|
||||
// Every tool_result for this turn is now recorded, including aborted
|
||||
// placeholders, so the images can follow without splitting them.
|
||||
messages = append(messages, toolImageMessages...)
|
||||
toolImageMessages = nil
|
||||
|
||||
// Run post-edit self-correction once over the union of files this turn
|
||||
// changed, then append any feedback after every tool_result is recorded so
|
||||
@@ -1460,7 +1436,6 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
|
||||
Output: result.Output,
|
||||
Truncated: result.Truncated,
|
||||
Meta: result.Meta,
|
||||
Images: result.Images,
|
||||
Redacted: result.Redacted,
|
||||
ChangedFiles: result.ChangedFiles,
|
||||
ChangeSummaries: result.ChangeSummaries,
|
||||
@@ -2788,16 +2763,7 @@ func shellCommandAdditionalPermissionsRequested(args map[string]any) bool {
|
||||
// access and malformed payloads are left untouched so validation stays closed.
|
||||
func normalizeNoopAdditionalPermissions(args map[string]any, basePath string, engine *sandbox.Engine) {
|
||||
raw, exists := args["additional_permissions"]
|
||||
if !exists {
|
||||
return
|
||||
}
|
||||
// An explicit null is the field omitted, not a request: drop the key so the
|
||||
// rest of the pipeline sees the same args a model that omitted it would send.
|
||||
// sandbox_permissions is deliberately left alone — a flagged null must reach
|
||||
// the same "no additional_permissions object was provided" error as a flagged
|
||||
// absent field rather than being silently downgraded to a different outcome.
|
||||
if raw == nil {
|
||||
delete(args, "additional_permissions")
|
||||
if !exists || raw == nil {
|
||||
return
|
||||
}
|
||||
data, err := json.Marshal(raw)
|
||||
@@ -2828,41 +2794,21 @@ func normalizeNoopAdditionalPermissions(args map[string]any, basePath string, en
|
||||
}
|
||||
}
|
||||
|
||||
// omitAdditionalPermissionsHint is the one instruction that ends the retry loop
|
||||
// in #845/#847. Every additional_permissions rejection must carry it: the model
|
||||
// reaches these errors by attaching a permission object to a command that needs
|
||||
// none, so "omit it and retry" is the answer in practice, and an error that
|
||||
// only explains how to build a VALID permission object sends the model round
|
||||
// the loop again with a different invalid shape.
|
||||
const omitAdditionalPermissionsHint = "If this command does not need extra sandbox access — ordinary " +
|
||||
"read-only commands such as `git branch --show-current` do not — omit both sandbox_permissions and " +
|
||||
"additional_permissions entirely and retry; that is the fix in almost every case."
|
||||
|
||||
func inlineAdditionalPermissionsProfile(args map[string]any, basePath string) (sandbox.RequestPermissionProfile, bool, error) {
|
||||
if !shellCommandAdditionalPermissionsRequested(args) {
|
||||
// An explicit null grants nothing and carries no flag, so it is
|
||||
// indistinguishable from an omitted field; rejecting it would be a pure
|
||||
// false positive against providers that materialize every optional
|
||||
// property.
|
||||
if raw, exists := args["additional_permissions"]; exists && raw != nil {
|
||||
return sandbox.RequestPermissionProfile{}, false, fmt.Errorf(
|
||||
"additional_permissions was provided without sandbox_permissions set to %q, so it cannot be applied. %s "+
|
||||
"Only if the command genuinely needs file or network access the sandbox denies, resend with "+
|
||||
`sandbox_permissions: %q AND a non-empty additional_permissions, for example {"network": {"enabled": true}}`,
|
||||
tools.SandboxPermissionsWithAdditionalPermissions,
|
||||
omitAdditionalPermissionsHint,
|
||||
tools.SandboxPermissionsWithAdditionalPermissions)
|
||||
if _, exists := args["additional_permissions"]; exists {
|
||||
return sandbox.RequestPermissionProfile{}, false, fmt.Errorf("additional_permissions requires sandbox_permissions set to %q", tools.SandboxPermissionsWithAdditionalPermissions)
|
||||
}
|
||||
return sandbox.RequestPermissionProfile{}, false, nil
|
||||
}
|
||||
raw, ok := args["additional_permissions"]
|
||||
if !ok || raw == nil {
|
||||
return sandbox.RequestPermissionProfile{}, true, fmt.Errorf(
|
||||
"sandbox_permissions was set to %q but no additional_permissions object was provided. %s "+
|
||||
`Otherwise include one, for example additional_permissions: {"network": {"enabled": true}} or `+
|
||||
`{"file_system": {"write": ["/path"]}}`,
|
||||
tools.SandboxPermissionsWithAdditionalPermissions,
|
||||
omitAdditionalPermissionsHint)
|
||||
"sandbox_permissions was set to %q but no additional_permissions object was provided. "+
|
||||
`Include one, for example additional_permissions: {"network": {"enabled": true}} or `+
|
||||
`{"file_system": {"write": ["/path"]}}. If this command does not need elevated permissions, `+
|
||||
"omit sandbox_permissions entirely and retry",
|
||||
tools.SandboxPermissionsWithAdditionalPermissions)
|
||||
}
|
||||
data, err := json.Marshal(raw)
|
||||
if err != nil {
|
||||
@@ -2878,10 +2824,8 @@ func inlineAdditionalPermissionsProfile(args map[string]any, basePath string) (s
|
||||
}
|
||||
if normalized.Empty() {
|
||||
return sandbox.RequestPermissionProfile{}, true, fmt.Errorf(
|
||||
"additional_permissions granted nothing, so it cannot be applied. %s "+
|
||||
`Otherwise include a real permission, for example {"network": {"enabled": true}} or `+
|
||||
`{"file_system": {"write": ["/path"]}}`,
|
||||
omitAdditionalPermissionsHint)
|
||||
"additional_permissions must include at least one of network or file_system, for example " +
|
||||
`{"network": {"enabled": true}} or {"file_system": {"write": ["/path"]}}`)
|
||||
}
|
||||
grantProfile, err := sandbox.RequestPermissionGrantProfile(normalized)
|
||||
if err != nil {
|
||||
@@ -3314,37 +3258,3 @@ func copyMessages(messages []Message) []Message {
|
||||
}
|
||||
return copied
|
||||
}
|
||||
|
||||
// toolResultImageMessage builds the user message that carries a tool's images
|
||||
// to the model, or reports false when the tool produced none.
|
||||
//
|
||||
// The text names the tool so the model can tell which call an image came from
|
||||
// when several ran in one turn — the images arrive detached from their tool
|
||||
// result, so nothing else associates them.
|
||||
func toolResultImageMessage(result ToolResult) (zeroruntime.Message, bool) {
|
||||
images := make([]zeroruntime.ImageBlock, 0, len(result.Images))
|
||||
for _, image := range result.Images {
|
||||
if len(image.Data) == 0 {
|
||||
continue
|
||||
}
|
||||
mediaType := zeroruntime.NormalizeImageMediaType(image.MediaType)
|
||||
if mediaType == "" {
|
||||
// Outside the provider allow-list. Dropping it beats sending bytes a
|
||||
// provider will reject and failing the whole turn.
|
||||
continue
|
||||
}
|
||||
images = append(images, zeroruntime.ImageBlock{MediaType: mediaType, Data: image.Data})
|
||||
}
|
||||
if len(images) == 0 {
|
||||
return zeroruntime.Message{}, false
|
||||
}
|
||||
label := result.Name
|
||||
if label == "" {
|
||||
label = "tool"
|
||||
}
|
||||
return zeroruntime.Message{
|
||||
Role: zeroruntime.MessageRoleUser,
|
||||
Content: "Image output from " + label + ":",
|
||||
Images: images,
|
||||
}, true
|
||||
}
|
||||
@@ -30,13 +30,11 @@ work.
|
||||
read-before-edit discipline: inspect the target file and nearby callers,
|
||||
tests, or config before you modify behavior. Never edit a file you have not
|
||||
read.
|
||||
2. **Plan.** Use update_plan for implementation or investigation that has at
|
||||
least three meaningful dependent steps, then keep it current at milestones.
|
||||
Mark completed work and the next in-progress step together when practical;
|
||||
do not spend separate turns updating it after every file or command. Keep at
|
||||
most one item in_progress. Skip plans for simple lookups, explanations, code
|
||||
navigation, and other short tasks. Never create a plan after the work is
|
||||
already complete merely to describe what you did.
|
||||
2. **Plan.** For multi-step work, call update_plan with an ordered checklist and
|
||||
keep it current at meaningful milestones. Mark completed work and the next
|
||||
in-progress step together when practical; do not spend separate model turns
|
||||
updating the plan after every file or command. Keep at most one item
|
||||
in_progress. Skip the plan for trivial one-step tasks.
|
||||
3. **Implement.** Make focused changes that match the surrounding code's style,
|
||||
naming, and conventions. Prefer the smallest change that fully solves the
|
||||
problem. Avoid broad refactors, unrelated rewrites, dependency churn, and
|
||||
@@ -51,13 +49,9 @@ work.
|
||||
## Editing discipline
|
||||
|
||||
- Choose the narrowest tool that safely accomplishes the step. Prefer native
|
||||
file tools - read_file, read_minified_file, list_directory, glob, grep,
|
||||
write_file, edit_file, apply_patch - over shelling out to
|
||||
cat/sed/awk/python for file operations.
|
||||
file tools - read_file, list_directory, glob, grep, write_file, edit_file,
|
||||
apply_patch - over shelling out to cat/sed/awk/python for file operations.
|
||||
They are safer, reviewable, and produce clean diffs.
|
||||
- Prefer read_minified_file when initially exploring source code; it preserves
|
||||
code while removing comments and redundant whitespace. Use read_file when
|
||||
exact text, comments, or line numbers are needed.
|
||||
- Keep edits focused and reviewable. A single patch may update several related
|
||||
files when they form one coherent change; do not hide unrelated edits in a
|
||||
bulk shell or script rewrite.
|
||||
|
||||
@@ -71,6 +71,5 @@ const geminiPromptAddendum = `<model_guidance>
|
||||
equivalent shell commands; they are safer and produce cleaner diffs.
|
||||
- Be concise and concrete. When you run a shell command with side effects, state
|
||||
in one short clause why it is needed.
|
||||
- Use update_plan only for work with at least three meaningful dependent steps;
|
||||
skip it for simple lookups and never create one after the work is complete.
|
||||
- Use update_plan for any multi-step task and keep it current.
|
||||
</model_guidance>`
|
||||
@@ -1,207 +0,0 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/tools"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
// imageTool returns a tool result carrying an image, the way a screenshot tool
|
||||
// should be able to.
|
||||
type imageTool struct {
|
||||
media string
|
||||
data []byte
|
||||
}
|
||||
|
||||
func (imageTool) Name() string { return "capture" }
|
||||
func (imageTool) Description() string { return "Captures an image." }
|
||||
func (imageTool) Parameters() tools.Schema {
|
||||
return tools.Schema{Type: "object", Properties: map[string]tools.PropertySchema{}}
|
||||
}
|
||||
func (imageTool) Safety() tools.Safety {
|
||||
return tools.Safety{Permission: tools.PermissionAllow}
|
||||
}
|
||||
func (t imageTool) Run(context.Context, map[string]any) tools.Result {
|
||||
return tools.Result{
|
||||
Status: tools.StatusOK,
|
||||
Output: "Captured a screenshot.",
|
||||
Images: []zeroruntime.ImageBlock{{MediaType: t.media, Data: t.data}},
|
||||
}
|
||||
}
|
||||
|
||||
// A tool that produces an image must get that image in front of the model.
|
||||
//
|
||||
// It cannot ride the tool-result message: every provider drops images there.
|
||||
// Anthropic maps a tool result to a tool_result block with string content,
|
||||
// Gemini to a functionResponse, and OpenAI guards its image parts to the user
|
||||
// role explicitly. So the image has to arrive as a following user message,
|
||||
// which is also the only shape that keeps one tool result per tool call.
|
||||
func TestRunDeliversToolResultImagesToTheModel(t *testing.T) {
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(imageTool{media: "image/png", data: []byte("\x89PNG\r\n\x1a\nfake")})
|
||||
|
||||
provider := &mockProvider{
|
||||
turns: [][]zeroruntime.StreamEvent{
|
||||
{
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call_1", ToolName: "capture"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call_1", ArgumentsFragment: `{}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call_1"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
{
|
||||
{Type: zeroruntime.StreamEventText, Content: "I can see it."},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
result, err := Run(context.Background(), "screenshot please", provider, Options{
|
||||
Registry: registry,
|
||||
MaxTurns: 2,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Run: %v", err)
|
||||
}
|
||||
|
||||
var carrier *zeroruntime.Message
|
||||
for index := range result.Messages {
|
||||
if len(result.Messages[index].Images) > 0 {
|
||||
carrier = &result.Messages[index]
|
||||
break
|
||||
}
|
||||
}
|
||||
if carrier == nil {
|
||||
t.Fatal("no message carried the tool's image; the model never saw it")
|
||||
}
|
||||
if carrier.Role != zeroruntime.MessageRoleUser {
|
||||
t.Errorf("image rode a %q message; every provider only serializes images on the user role", carrier.Role)
|
||||
}
|
||||
if got := carrier.Images[0].MediaType; got != "image/png" {
|
||||
t.Errorf("media type = %q, want image/png", got)
|
||||
}
|
||||
|
||||
// The tool-result pairing must be untouched: one tool result per tool call.
|
||||
toolResults := 0
|
||||
for _, message := range result.Messages {
|
||||
if message.Role == zeroruntime.MessageRoleTool {
|
||||
toolResults++
|
||||
}
|
||||
}
|
||||
if toolResults != 1 {
|
||||
t.Errorf("got %d tool-result messages for 1 tool call", toolResults)
|
||||
}
|
||||
}
|
||||
|
||||
// textTool is a second, image-free tool so a turn can carry two calls.
|
||||
type textTool struct{}
|
||||
|
||||
func (textTool) Name() string { return "note" }
|
||||
func (textTool) Description() string { return "Notes something." }
|
||||
func (textTool) Parameters() tools.Schema {
|
||||
return tools.Schema{Type: "object", Properties: map[string]tools.PropertySchema{}}
|
||||
}
|
||||
func (textTool) Safety() tools.Safety { return tools.Safety{Permission: tools.PermissionAllow} }
|
||||
func (textTool) Run(context.Context, map[string]any) tools.Result {
|
||||
return tools.Result{Status: tools.StatusOK, Output: "noted"}
|
||||
}
|
||||
|
||||
// Tool results for one assistant turn must stay contiguous. The image messages
|
||||
// are user-role, and a user message between two tool_results breaks strict
|
||||
// provider replay: Anthropic coalesces consecutive user content into one block
|
||||
// list and requires tool_result blocks first, so interleaving produces
|
||||
// [tool_result, text, image, tool_result] and the request is rejected.
|
||||
//
|
||||
// The first version of this feature appended each image immediately after its
|
||||
// own tool result, which is exactly that shape. A single-tool-call test passed
|
||||
// and said nothing about it.
|
||||
func TestRunKeepsToolResultsContiguousWhenAToolReturnsAnImage(t *testing.T) {
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(imageTool{media: "image/png", data: []byte("\x89PNG\r\n\x1a\nfake")})
|
||||
registry.Register(textTool{})
|
||||
|
||||
provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
|
||||
{
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "c1", ToolName: "capture"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "c1", ArgumentsFragment: `{}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "c1"},
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "c2", ToolName: "note"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "c2", ArgumentsFragment: `{}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "c2"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
{{Type: zeroruntime.StreamEventText, Content: "done"}, {Type: zeroruntime.StreamEventDone}},
|
||||
}}
|
||||
|
||||
result, err := Run(context.Background(), "two calls", provider, Options{Registry: registry, MaxTurns: 2})
|
||||
if err != nil {
|
||||
t.Fatalf("Run: %v", err)
|
||||
}
|
||||
|
||||
// Two separate ifs rather than a switch: a tool-role message that also
|
||||
// carried images would match only the first arm of a switch, so a regression
|
||||
// that put the image back on the tool result would read as "no image at all"
|
||||
// instead of naming what actually happened.
|
||||
type toolResult struct {
|
||||
index int
|
||||
id string
|
||||
}
|
||||
var toolResults []toolResult
|
||||
imageIndex := -1
|
||||
for index, message := range result.Messages {
|
||||
if message.Role == zeroruntime.MessageRoleTool {
|
||||
toolResults = append(toolResults, toolResult{index: index, id: message.ToolCallID})
|
||||
}
|
||||
if len(message.Images) > 0 {
|
||||
if imageIndex >= 0 {
|
||||
t.Fatalf("image delivered twice, at %d and %d; recorded %s", imageIndex, index, messageShape(result.Messages))
|
||||
}
|
||||
imageIndex = index
|
||||
}
|
||||
}
|
||||
if imageIndex < 0 {
|
||||
t.Fatalf("the image never reached the model; recorded %s", messageShape(result.Messages))
|
||||
}
|
||||
if len(toolResults) != 2 {
|
||||
t.Fatalf("got %d tool-result messages for 2 tool calls; recorded %s", len(toolResults), messageShape(result.Messages))
|
||||
}
|
||||
if toolResults[0].id != "c1" || toolResults[1].id != "c2" {
|
||||
t.Errorf("tool results answered %q then %q, want c1 then c2", toolResults[0].id, toolResults[1].id)
|
||||
}
|
||||
// The contract Anthropic enforces: every tool_result block has to come before
|
||||
// any other block in the user message it lands in. Anything wedged between
|
||||
// the two results breaks that, which is the 400 this PR exists to avoid.
|
||||
if toolResults[1].index != toolResults[0].index+1 {
|
||||
t.Errorf("tool results at %d and %d are not adjacent; %q message at %d splits them; recorded %s",
|
||||
toolResults[0].index, toolResults[1].index, result.Messages[toolResults[0].index+1].Role,
|
||||
toolResults[0].index+1, messageShape(result.Messages))
|
||||
}
|
||||
// Follows the last tool result, deliberately NOT pinned to exactly last+1.
|
||||
// appendUserBlocks coalesces consecutive user-role content, so
|
||||
// [tool_result, tool_result, text, image] still maps to tool_result blocks
|
||||
// first and replays fine; pinning would redden that valid shape for nothing.
|
||||
if imageIndex <= toolResults[1].index {
|
||||
t.Errorf("image message at %d does not follow the last tool result at %d; recorded %s",
|
||||
imageIndex, toolResults[1].index, messageShape(result.Messages))
|
||||
}
|
||||
}
|
||||
|
||||
// messageShape renders the conversation as a compact role list so a failure says
|
||||
// what the ordering actually was instead of dumping whole messages.
|
||||
func messageShape(messages []zeroruntime.Message) string {
|
||||
parts := make([]string, 0, len(messages))
|
||||
for index, message := range messages {
|
||||
part := fmt.Sprintf("%d:%s", index, message.Role)
|
||||
if message.ToolCallID != "" {
|
||||
part += "(" + message.ToolCallID + ")"
|
||||
}
|
||||
if len(message.Images) > 0 {
|
||||
part += "+image"
|
||||
}
|
||||
parts = append(parts, part)
|
||||
}
|
||||
return "[" + strings.Join(parts, " ") + "]"
|
||||
}
|
||||
@@ -68,11 +68,8 @@ type ToolResult struct {
|
||||
Output string
|
||||
// Truncated reports that the tool's model-visible output omitted content.
|
||||
// The full result may be recoverable through Meta["spill_path"].
|
||||
Truncated bool
|
||||
Meta map[string]string
|
||||
// Images the tool produced, delivered to the model as a following user
|
||||
// message rather than on this result. See tools.Result.Images.
|
||||
Images []zeroruntime.ImageBlock
|
||||
Truncated bool
|
||||
Meta map[string]string
|
||||
Redacted bool
|
||||
ChangedFiles []string
|
||||
// ChangeSummaries are non-selectable generated-tree summaries emitted by
|
||||
|
||||
@@ -79,8 +79,8 @@ func TestRegisterToolSearchIfEligibleSkipsWhenThresholdZero(t *testing.T) {
|
||||
|
||||
func TestDeferredEligibleCountIncludesOptionalBuiltins(t *testing.T) {
|
||||
registry := newCoreRegistry(t.TempDir())
|
||||
if got := deferredEligibleCount(registry, agent.PermissionModeAuto, nil, nil); got < 2 {
|
||||
t.Fatalf("deferredEligibleCount(core) = %d, want at least 2 optional built-ins", got)
|
||||
if got := deferredEligibleCount(registry, agent.PermissionModeAuto, nil, nil); got < 3 {
|
||||
t.Fatalf("deferredEligibleCount(core) = %d, want at least 3 optional built-ins", got)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+2
-134
@@ -13,10 +13,8 @@ package minify
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"go/ast"
|
||||
"go/parser"
|
||||
"go/printer"
|
||||
"go/scanner"
|
||||
"go/token"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
@@ -36,11 +34,8 @@ func File(path string, content []byte) Result {
|
||||
if out, ok := minifyGo(content); ok {
|
||||
return Result{Content: out, Language: "go", Applied: true}
|
||||
}
|
||||
if out, ok := minifyGoFragment(content); ok {
|
||||
return Result{Content: out, Language: "go-fragment", Applied: true}
|
||||
}
|
||||
// An incomplete fragment that cannot be wrapped safely falls through to
|
||||
// whitespace-only normalization; no source text is guessed or discarded.
|
||||
// Unparsable Go (a snippet, generics edge, or syntax error): fall through
|
||||
// to the safe generic path rather than risk a partial AST reprint.
|
||||
} else if style, ok := commentStyles[ext]; ok {
|
||||
// Strip comments with a string-aware lexer, then collapse the whitespace the
|
||||
// removed comments left behind. The stripper only handles languages whose
|
||||
@@ -51,133 +46,6 @@ func File(path string, content []byte) Result {
|
||||
return Result{Content: minifyGeneric(content), Language: "text", Applied: false}
|
||||
}
|
||||
|
||||
// Fragment minifies a bounded source range without assuming lexical state from
|
||||
// text outside the range. Go remains parser-validated; other languages use the
|
||||
// conservative whitespace-only transform because a range may begin inside a
|
||||
// multiline string or block comment.
|
||||
func Fragment(path string, content []byte) Result {
|
||||
if strings.EqualFold(filepath.Ext(path), ".go") {
|
||||
if out, ok := minifyGo(content); ok {
|
||||
return Result{Content: out, Language: "go", Applied: true}
|
||||
}
|
||||
if out, ok := minifyGoFragment(content); ok {
|
||||
return Result{Content: out, Language: "go-fragment", Applied: true}
|
||||
}
|
||||
}
|
||||
return Result{Content: minifyGeneric(content), Language: "text", Applied: false}
|
||||
}
|
||||
|
||||
// ContextualFragment minifies a bounded fragment after checking its starting
|
||||
// position against the complete source. A Go range that begins inside a string
|
||||
// or comment must remain verbatim apart from whitespace normalization: parsing
|
||||
// that fragment by itself would mistake literal text for Go syntax.
|
||||
func ContextualFragment(path string, source, content []byte, startOffset int) Result {
|
||||
if strings.EqualFold(filepath.Ext(path), ".go") && goFragmentStartsInsideToken(source, startOffset) {
|
||||
return Result{Content: minifyGeneric(content), Language: "text", Applied: false}
|
||||
}
|
||||
return Fragment(path, content)
|
||||
}
|
||||
|
||||
func goFragmentStartsInsideToken(source []byte, startOffset int) bool {
|
||||
if startOffset <= 0 || startOffset >= len(source) {
|
||||
return false
|
||||
}
|
||||
fset := token.NewFileSet()
|
||||
file := fset.AddFile("", fset.Base(), len(source))
|
||||
var lexer scanner.Scanner
|
||||
lexer.Init(file, source, nil, scanner.ScanComments)
|
||||
for {
|
||||
position, kind, literal := lexer.Scan()
|
||||
if kind == token.EOF {
|
||||
return false
|
||||
}
|
||||
if kind != token.STRING && kind != token.COMMENT {
|
||||
continue
|
||||
}
|
||||
start := file.Offset(position)
|
||||
if startOffset > start && startOffset < start+len(literal) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// minifyGoFragment handles bounded reads that do not contain a package clause.
|
||||
// It finds the longest complete declaration or statement prefix that the real
|
||||
// Go parser accepts, prints that prefix without comments, and preserves any
|
||||
// incomplete tail conservatively. Ranges are intentionally capped: repeatedly
|
||||
// parsing a large broken file would otherwise turn a cheap read into O(n²) work.
|
||||
func minifyGoFragment(content []byte) (string, bool) {
|
||||
lines := strings.Split(strings.ReplaceAll(string(content), "\r\n", "\n"), "\n")
|
||||
if len(lines) > 1 && lines[len(lines)-1] == "" {
|
||||
lines = lines[:len(lines)-1]
|
||||
}
|
||||
if len(lines) > 512 {
|
||||
return "", false
|
||||
}
|
||||
for end := len(lines); end > 0; end-- {
|
||||
prefix := strings.Join(lines[:end], "\n")
|
||||
if compact, ok := parseGoDeclarations(prefix); ok {
|
||||
return joinCompactPrefix(compact, lines[end:]), true
|
||||
}
|
||||
if compact, ok := parseGoStatements(prefix); ok {
|
||||
return joinCompactPrefix(compact, lines[end:]), true
|
||||
}
|
||||
}
|
||||
return "", false
|
||||
}
|
||||
|
||||
func parseGoDeclarations(fragment string) (string, bool) {
|
||||
fset := token.NewFileSet()
|
||||
file, err := parser.ParseFile(fset, "", "package compact\n"+fragment, parser.SkipObjectResolution)
|
||||
if err != nil || len(file.Decls) == 0 {
|
||||
return "", false
|
||||
}
|
||||
nodes := make([]ast.Node, len(file.Decls))
|
||||
for i, declaration := range file.Decls {
|
||||
nodes[i] = declaration
|
||||
}
|
||||
return printGoNodes(fset, nodes)
|
||||
}
|
||||
|
||||
func parseGoStatements(fragment string) (string, bool) {
|
||||
fset := token.NewFileSet()
|
||||
file, err := parser.ParseFile(fset, "", "package compact\nfunc _(){\n"+fragment+"\n}", parser.SkipObjectResolution)
|
||||
if err != nil || len(file.Decls) != 1 {
|
||||
return "", false
|
||||
}
|
||||
function, ok := file.Decls[0].(*ast.FuncDecl)
|
||||
if !ok || function.Body == nil || len(function.Body.List) == 0 {
|
||||
return "", false
|
||||
}
|
||||
nodes := make([]ast.Node, len(function.Body.List))
|
||||
for i, statement := range function.Body.List {
|
||||
nodes[i] = statement
|
||||
}
|
||||
return printGoNodes(fset, nodes)
|
||||
}
|
||||
|
||||
func printGoNodes(fset *token.FileSet, nodes []ast.Node) (string, bool) {
|
||||
var buf bytes.Buffer
|
||||
cfg := printer.Config{Mode: printer.TabIndent, Tabwidth: 1}
|
||||
for i, node := range nodes {
|
||||
if i > 0 {
|
||||
buf.WriteByte('\n')
|
||||
}
|
||||
if err := cfg.Fprint(&buf, fset, node); err != nil {
|
||||
return "", false
|
||||
}
|
||||
}
|
||||
return strings.TrimSpace(buf.String()), true
|
||||
}
|
||||
|
||||
func joinCompactPrefix(compact string, remainder []string) string {
|
||||
tail := minifyGeneric([]byte(strings.Join(remainder, "\n")))
|
||||
if tail == "" {
|
||||
return compact
|
||||
}
|
||||
return compact + "\n" + tail
|
||||
}
|
||||
|
||||
// minifyGo parses Go WITHOUT comments (omitting parser.ParseComments leaves them
|
||||
// unattached) and reprints the AST, yielding valid, comment-free, gofmt-shaped
|
||||
// source with tab indentation (1 char per level). It returns ok=false on any
|
||||
|
||||
@@ -39,58 +39,17 @@ func Greet(name string) string {
|
||||
}
|
||||
}
|
||||
|
||||
func TestMinifyGoStatementFragment(t *testing.T) {
|
||||
func TestMinifyGoFallsBackOnUnparsableGo(t *testing.T) {
|
||||
// A snippet (no package clause) cannot parse as a file -> safe generic path.
|
||||
r := File("snippet.go", []byte("x := 1 \n\n\n// keep me\ny := 2\n"))
|
||||
if !r.Applied || r.Language != "go-fragment" {
|
||||
t.Fatalf("expected parsed Go fragment, got %+v", r)
|
||||
if r.Applied {
|
||||
t.Fatalf("expected fallback for unparsable Go, got %+v", r)
|
||||
}
|
||||
if strings.Contains(r.Content, "\n\n\n") {
|
||||
t.Errorf("fragment should collapse blank runs:\n%q", r.Content)
|
||||
t.Errorf("generic should collapse blank runs:\n%q", r.Content)
|
||||
}
|
||||
if strings.Contains(r.Content, "keep me") || !strings.Contains(r.Content, "y := 2") {
|
||||
t.Errorf("fragment was not compacted safely: %q", r.Content)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMinifyGoFragmentAccepts512LinesWithFinalNewline(t *testing.T) {
|
||||
src := strings.Repeat("x++\n", 512)
|
||||
r := File("snippet.go", []byte(src))
|
||||
if !r.Applied || r.Language != "go-fragment" {
|
||||
t.Fatalf("expected 512-line fragment to remain eligible, got %+v", r)
|
||||
}
|
||||
if got := strings.Count(r.Content, "x++"); got != 512 {
|
||||
t.Fatalf("compacted statement count = %d, want 512", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMinifyGoDeclarationPrefixPreservesIncompleteTail(t *testing.T) {
|
||||
src := `func Handle737(n int) (int, error) {
|
||||
if n%2 == 1 {
|
||||
return 0, errBad
|
||||
}
|
||||
return n + 737, nil
|
||||
}
|
||||
|
||||
// Handle738 begins outside the requested range.
|
||||
func Handle738(n int) (int, error) {
|
||||
if n%2 == 1 {`
|
||||
r := File("range.go", []byte(src))
|
||||
if !r.Applied || r.Language != "go-fragment" {
|
||||
t.Fatalf("expected declaration-prefix compaction, got %+v", r)
|
||||
}
|
||||
if strings.Contains(r.Content, "Handle737 returns") || !strings.Contains(r.Content, "return n + 737") {
|
||||
t.Fatalf("complete declaration was not compacted correctly:\n%s", r.Content)
|
||||
}
|
||||
if !strings.Contains(r.Content, "func Handle738") || !strings.Contains(r.Content, "if n%2 == 1 {") {
|
||||
t.Fatalf("incomplete tail was not preserved:\n%s", r.Content)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMinifyGoLargeInvalidInputFallsBackConservatively(t *testing.T) {
|
||||
src := strings.Repeat("x := 1 // keep\n", 513)
|
||||
r := File("broken.go", []byte(src))
|
||||
if r.Applied || !strings.Contains(r.Content, "// keep") {
|
||||
t.Fatalf("large invalid input must retain conservative fallback: %+v", r)
|
||||
if !strings.Contains(r.Content, "// keep me") {
|
||||
t.Errorf("generic must NOT strip comments (unsafe without a parser): %q", r.Content)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -167,13 +167,10 @@ const permissionModeUnsafe = "unsafe"
|
||||
var readOnlySpecialistTools = map[string]bool{
|
||||
"read_file": true,
|
||||
"read_minified_file": true,
|
||||
// Reads one image file through read_file's own path scoping and mutates
|
||||
// nothing, so it carries the same auto-approval as the other pure readers.
|
||||
"view_image": true,
|
||||
"list_directory": true,
|
||||
"grep": true,
|
||||
"glob": true,
|
||||
"update_plan": true,
|
||||
"list_directory": true,
|
||||
"grep": true,
|
||||
"glob": true,
|
||||
"update_plan": true,
|
||||
}
|
||||
|
||||
// IsReadOnlySpecialist reports whether the named specialist resolves to a
|
||||
|
||||
@@ -255,7 +255,7 @@ func TestBuildArgsDefaultsToReadOnlyToolAllowlist(t *testing.T) {
|
||||
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"}) {
|
||||
if !containsSequence(result.Args, []string{"--enabled-tools", "glob,grep,list_directory,read_file,read_minified_file"}) {
|
||||
t.Fatalf("args missing default read-only allowlist: %#v", result.Args)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -82,9 +82,9 @@ var knownMetadataKeys = map[string]bool{
|
||||
}
|
||||
|
||||
var toolCategories = map[string][]string{
|
||||
"read-only": {"read_file", "read_minified_file", "view_image", "list_directory", "grep", "glob"},
|
||||
"edit": {"read_file", "read_minified_file", "view_image", "list_directory", "grep", "glob", "write_file", "edit_file", "apply_patch"},
|
||||
"execute": {"read_file", "read_minified_file", "view_image", "list_directory", "grep", "glob", "exec_command", "write_stdin", "bash"},
|
||||
"read-only": {"read_file", "read_minified_file", "list_directory", "grep", "glob"},
|
||||
"edit": {"read_file", "read_minified_file", "list_directory", "grep", "glob", "write_file", "edit_file", "apply_patch"},
|
||||
"execute": {"read_file", "read_minified_file", "list_directory", "grep", "glob", "exec_command", "write_stdin", "bash"},
|
||||
"plan": {"update_plan"},
|
||||
}
|
||||
|
||||
@@ -102,7 +102,6 @@ var defaultToolSelection = []string{"read-only"}
|
||||
var knownToolNames = map[string]bool{
|
||||
"read_file": true,
|
||||
"read_minified_file": true,
|
||||
"view_image": true,
|
||||
"list_directory": true,
|
||||
"glob": true,
|
||||
"grep": true,
|
||||
|
||||
@@ -1,31 +0,0 @@
|
||||
package specialist
|
||||
|
||||
import (
|
||||
"slices"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// view_image is a pure workspace-scoped read, so every category that already
|
||||
// grants read_file should grant it too. Categories are what specialist authors
|
||||
// actually write ("tools: [execute]"), so an omission here silently denies a
|
||||
// specialist the ability to look at a screenshot it just captured.
|
||||
func TestToolCategoriesIncludeViewImageWhereverReadFileIsGranted(t *testing.T) {
|
||||
for category, names := range toolCategories {
|
||||
if !slices.Contains(names, "read_file") {
|
||||
continue
|
||||
}
|
||||
if !slices.Contains(names, "view_image") {
|
||||
t.Errorf("category %q grants read_file but not view_image", category)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveToolsExpandsExecuteToIncludeViewImage(t *testing.T) {
|
||||
resolved, err := ResolveTools([]string{"execute"})
|
||||
if err != nil {
|
||||
t.Fatalf("ResolveTools: %v", err)
|
||||
}
|
||||
if !slices.Contains(resolved, "view_image") {
|
||||
t.Errorf("execute resolved to %v, missing view_image", resolved)
|
||||
}
|
||||
}
|
||||
@@ -132,34 +132,6 @@ func intPtr(value int) *int {
|
||||
return &value
|
||||
}
|
||||
|
||||
// sandboxPermissionsDescription and additionalPermissionsDescription are shared
|
||||
// by every shell tool (bash, exec_command) so the two fields cannot drift apart.
|
||||
//
|
||||
// Both lead with "omit" on purpose. The pair used to be described only in terms
|
||||
// of what each value does, which left a model that had used
|
||||
// with_additional_permissions once (say, for `gh issue create`) no instruction
|
||||
// to stop attaching a permission object to the next, ordinary command. That
|
||||
// attachment is then rejected — correctly, but the rejection alone does not tell
|
||||
// the model that dropping the field is the fix, so it flips between an
|
||||
// unflagged permission object and a flagged empty one and never converges
|
||||
// (#845, #847). Making "omit it" the first sentence of both descriptions stops
|
||||
// the invalid shape from being emitted in the first place; the validator errors
|
||||
// are the backstop, not the teacher.
|
||||
//
|
||||
// These ride on every turn's exec_command schema, so they are kept close to the
|
||||
// length of the text they replaced; TestEagerToolSchemaTokenBudget is the ratchet.
|
||||
const (
|
||||
sandboxPermissionsDescription = "Per-command sandbox override. Omit it unless the command needs access the " +
|
||||
"sandbox does not already give it; use `with_additional_permissions` with a non-empty " +
|
||||
"`additional_permissions` for sandboxed file/network access, or `require_escalated` only when the command " +
|
||||
"must run outside the sandbox, such as host/global process, socket, service, or desktop state hidden by " +
|
||||
"sandbox namespaces."
|
||||
|
||||
additionalPermissionsDescription = "Extra sandbox access. Omit this field entirely unless `sandbox_permissions` " +
|
||||
"is `with_additional_permissions`; sending it with any other mode is rejected, as is an empty or all-false " +
|
||||
"object. Example: {\"network\": {\"enabled\": true}}."
|
||||
)
|
||||
|
||||
func additionalPermissionsProperties() map[string]PropertySchema {
|
||||
return requestPermissionsProfileSchema().Properties
|
||||
}
|
||||
|
||||
@@ -50,10 +50,10 @@ func NewScopedBashTool(workspaceRoot string, scope PathScope) Tool {
|
||||
"command": {Type: "string", Description: "Shell command to execute using the host shell. " + shellGuidance},
|
||||
"cwd": {Type: "string", Description: "Directory to run the command in. Relative paths stay in the workspace; use an absolute path to run in a granted extra directory. Defaults to workspace root. Prefer cwd over cd when changing directories.", Default: "."},
|
||||
"timeout_ms": {Type: "integer", Description: "Command timeout in milliseconds.", Default: defaultBashTimeoutMS, Minimum: intPtr(1), Maximum: intPtr(maxBashTimeoutMS)},
|
||||
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: sandboxPermissionsDescription, Default: string(SandboxPermissionsUseDefault)},
|
||||
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: "Per-command sandbox override. Defaults to `use_default`; use `with_additional_permissions` with `additional_permissions` for sandboxed file/network access, or `require_escalated` only when the command must run outside the sandbox, such as host/global process, socket, service, or desktop state hidden by sandbox namespaces.", Default: string(SandboxPermissionsUseDefault)},
|
||||
"additional_permissions": {
|
||||
Type: "object",
|
||||
Description: additionalPermissionsDescription,
|
||||
Description: "Sandboxed filesystem or network access for this command; only with `sandbox_permissions: \"with_additional_permissions\"`.",
|
||||
Properties: additionalPermissionsProperties(),
|
||||
},
|
||||
"justification": {Type: "string", Description: "User-facing approval question for `require_escalated`; omit otherwise."},
|
||||
|
||||
@@ -91,10 +91,10 @@ func NewScopedExecCommandTool(workspaceRoot string, scope PathScope, manager *ex
|
||||
"cwd": {Type: "string", Description: "Alias for workdir. Prefer workdir.", Default: "."},
|
||||
"yield_time_ms": {Type: "integer", Description: "Wait before yielding output. Defaults to 10000 ms; effective range is 250-30000 ms.", Default: defaultExecYieldTimeMS, Minimum: intPtr(1), Maximum: intPtr(maxExecYieldTimeMS)},
|
||||
"max_output_tokens": {Type: "integer", Description: "Output token budget. Defaults to 10000 tokens; larger requests may be capped by policy.", Default: defaultMaxOutputTokens, Minimum: intPtr(1), Maximum: intPtr(maxExecOutputTokenRequest)},
|
||||
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: sandboxPermissionsDescription, Default: string(SandboxPermissionsUseDefault)},
|
||||
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: "Per-command sandbox override. Defaults to `use_default`; use `with_additional_permissions` with `additional_permissions` for sandboxed file/network access, or `require_escalated` only when the command must run outside the sandbox, such as host/global process, socket, service, or desktop state hidden by sandbox namespaces.", Default: string(SandboxPermissionsUseDefault)},
|
||||
"additional_permissions": {
|
||||
Type: "object",
|
||||
Description: additionalPermissionsDescription,
|
||||
Description: "Sandboxed filesystem or network access for this command; only with `sandbox_permissions: \"with_additional_permissions\"`.",
|
||||
Properties: additionalPermissionsProperties(),
|
||||
},
|
||||
"justification": {Type: "string", Description: "User-facing approval question for `require_escalated`; omit otherwise."},
|
||||
|
||||
@@ -4,7 +4,6 @@ import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
@@ -296,40 +295,6 @@ func TestEditFileAllowsSeenRangeAndRejectsUnseenRange(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanonicalLimitedReadAuthorizesEdit(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
var source strings.Builder
|
||||
for line := 1; line <= 1000; line++ {
|
||||
source.WriteString("line ")
|
||||
source.WriteString(strconv.Itoa(line))
|
||||
source.WriteByte('\n')
|
||||
}
|
||||
path := filepath.Join(dir, "f.txt")
|
||||
if err := os.WriteFile(path, []byte(source.String()), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
registry := safetyRegistry(t, dir)
|
||||
tracker := NewFileTracker()
|
||||
opts := grantedOpts(tracker)
|
||||
|
||||
read := registry.RunWithOptions(context.Background(), "read_file", map[string]any{
|
||||
"path": "f.txt", "offset": 500, "limit": 10,
|
||||
}, opts)
|
||||
if read.Status != StatusOK || read.Truncated {
|
||||
t.Fatalf("bounded canonical read must be exact: status=%s truncated=%v meta=%#v", read.Status, read.Truncated, read.Meta)
|
||||
}
|
||||
if read.Meta["seen_lines"] != "500-509" {
|
||||
t.Fatalf("bounded canonical read did not receive exact-read credit: %#v", read.Meta)
|
||||
}
|
||||
|
||||
edit := registry.RunWithOptions(context.Background(), "edit_file", map[string]any{
|
||||
"path": "f.txt", "old_string": "line 505", "new_string": "updated 505",
|
||||
}, opts)
|
||||
if edit.Status != StatusOK {
|
||||
t.Fatalf("edit inside canonical read range was refused: %s", edit.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEditFileReplaceAllRecordsEveryShiftedReplacementSpan(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "f.txt")
|
||||
|
||||
@@ -38,108 +38,6 @@ func TestReadFileToolReadsLineRanges(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadFileToolReadsCanonicalLineRange(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta\ngamma\ndelta")
|
||||
|
||||
result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "notes.txt",
|
||||
"offset": 2,
|
||||
"limit": 2,
|
||||
})
|
||||
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("expected ok status, got %s: %s", result.Status, result.Output)
|
||||
}
|
||||
if result.Truncated {
|
||||
t.Fatalf("canonical limit defines the requested range and must not mark it truncated: %#v", result.Meta)
|
||||
}
|
||||
if !strings.Contains(result.Output, "2 | beta") || !strings.Contains(result.Output, "3 | gamma") {
|
||||
t.Fatalf("canonical range returned the wrong lines: %q", result.Output)
|
||||
}
|
||||
if strings.Contains(result.Output, "alpha") || strings.Contains(result.Output, "delta") {
|
||||
t.Fatalf("canonical range leaked outside requested slice: %q", result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadFileToolCanonicalOffsetPastEnd(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta")
|
||||
|
||||
result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "notes.txt", "offset": 10,
|
||||
})
|
||||
|
||||
if result.Status != StatusOK || !strings.Contains(result.Output, "offset 10 is past the end") {
|
||||
t.Fatalf("expected canonical out-of-range message, got status=%s output=%q", result.Status, result.Output)
|
||||
}
|
||||
if strings.Contains(result.Output, "start_line") {
|
||||
t.Fatalf("out-of-range message exposed legacy argument: %q", result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadFileToolMixedLegacyRangesPreferLines(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta\ngamma\ndelta")
|
||||
|
||||
result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "notes.txt",
|
||||
"start_line": 2,
|
||||
"end_line": 3,
|
||||
"max_lines": 2,
|
||||
"byte_offset": 0,
|
||||
"byte_limit": readFileByteChunkMax,
|
||||
})
|
||||
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("mixed legacy range should recover, got %s: %s", result.Status, result.Output)
|
||||
}
|
||||
if !strings.Contains(result.Output, "2 | beta") || !strings.Contains(result.Output, "3 | gamma") {
|
||||
t.Fatalf("mixed legacy range did not prefer lines: %q", result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadFileToolCombinesLegacyStartWithCanonicalLimit(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta\ngamma\ndelta")
|
||||
|
||||
result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
|
||||
"path": "notes.txt", "start_line": 2, "limit": 2,
|
||||
})
|
||||
|
||||
if result.Status != StatusOK || result.Truncated {
|
||||
t.Fatalf("mixed compatible range should be exact, got status=%s truncated=%v: %s", result.Status, result.Truncated, result.Output)
|
||||
}
|
||||
if !strings.Contains(result.Output, "2 | beta") || !strings.Contains(result.Output, "3 | gamma") {
|
||||
t.Fatalf("mixed compatible range returned the wrong lines: %q", result.Output)
|
||||
}
|
||||
if strings.Contains(result.Output, "alpha") || strings.Contains(result.Output, "delta") {
|
||||
t.Fatalf("mixed compatible range leaked outside requested slice: %q", result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileToolSchemasExposeOnlyCanonicalArguments(t *testing.T) {
|
||||
readProperties := NewScopedReadFileTool(t.TempDir(), nil).Parameters().Properties
|
||||
for _, want := range []string{"path", "offset", "limit", "byte_offset", "byte_limit"} {
|
||||
if _, ok := readProperties[want]; !ok {
|
||||
t.Fatalf("read_file schema missing %q", want)
|
||||
}
|
||||
}
|
||||
for _, legacy := range []string{"start_line", "end_line", "max_lines"} {
|
||||
if _, ok := readProperties[legacy]; ok {
|
||||
t.Fatalf("read_file schema must not expose legacy argument %q", legacy)
|
||||
}
|
||||
}
|
||||
|
||||
grepProperties := NewScopedGrepTool(t.TempDir(), nil).Parameters().Properties
|
||||
if _, ok := grepProperties["case_insensitive"]; !ok {
|
||||
t.Fatal("grep schema missing case_insensitive")
|
||||
}
|
||||
if _, ok := grepProperties["-i"]; ok {
|
||||
t.Fatal("grep schema must not expose the -i alias")
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadFileToolMarksTruncation(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
writeTestFile(t, filepath.Join(root, "notes.txt"), "a\nb\nc\nd\ne")
|
||||
@@ -154,7 +52,7 @@ func TestReadFileToolMarksTruncation(t *testing.T) {
|
||||
}
|
||||
// The cut must be visible in the rendered output, not just the Truncated flag,
|
||||
// and must tell the model where to continue.
|
||||
if !strings.Contains(result.Output, "[truncated:") || !strings.Contains(result.Output, "offset=3") {
|
||||
if !strings.Contains(result.Output, "[truncated:") || !strings.Contains(result.Output, "start_line=3") {
|
||||
t.Fatalf("expected truncation marker pointing to the next line, got %q", result.Output)
|
||||
}
|
||||
}
|
||||
@@ -170,7 +68,7 @@ func TestReadFileToolAppliesByteBudget(t *testing.T) {
|
||||
if result.Status != StatusOK || !result.Truncated {
|
||||
t.Fatalf("expected ok+truncated, got status=%s truncated=%v", result.Status, result.Truncated)
|
||||
}
|
||||
if !strings.Contains(result.Output, "output exceeded") || !strings.Contains(result.Output, "offset/limit") {
|
||||
if !strings.Contains(result.Output, "output exceeded") || !strings.Contains(result.Output, "start_line") {
|
||||
t.Fatalf("expected byte-budget continuation hint, got %q", result.Output[len(result.Output)-200:])
|
||||
}
|
||||
if result.Meta["raw_bytes"] == "" || result.Meta["emitted_bytes"] == "" {
|
||||
|
||||
@@ -377,5 +377,5 @@ func fileConflictMessage(relativePath string) string {
|
||||
}
|
||||
|
||||
func fileUnseenMessage(relativePath string) string {
|
||||
return "Error writing " + relativePath + ": the intended change depends on content that has not been read exactly in this session. Read the affected lines with read_file using offset/limit (or byte_offset/byte_limit for a very long line), then retry the edit."
|
||||
return "Error writing " + relativePath + ": the intended change depends on content that has not been read exactly in this session. Read the affected lines with read_file, or use byte_offset/byte_limit for an oversized single line, then retry the edit."
|
||||
}
|
||||
@@ -48,6 +48,7 @@ func NewScopedGrepTool(workspaceRoot string, scope PathScope) Tool {
|
||||
"path": {Type: "string", Description: "Directory or file to search. Relative paths stay in the workspace; use an absolute path to search a granted extra root. Defaults to workspace root.", Default: "."},
|
||||
"glob": {Type: "string", Description: `Optional glob filter, for example "**/*.go".`},
|
||||
"output_mode": {Type: "string", Description: "Output mode.", Enum: []string{"content", "files_with_matches", "count"}, Default: "content"},
|
||||
"-i": {Type: "boolean", Description: "Case insensitive search.", Default: false},
|
||||
"case_insensitive": {Type: "boolean", Description: "Case insensitive search.", Default: false},
|
||||
"head_limit": {Type: "integer", Description: "Maximum content results to return.", Default: 50, Minimum: intPtr(1)},
|
||||
},
|
||||
|
||||
@@ -4,8 +4,6 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"github.com/Gitlawb/zero/internal/imageinput"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
@@ -347,18 +345,9 @@ func captureOKResult(driver string, request captureArtifactRequest, path string,
|
||||
if helperOutput != "" {
|
||||
output += "\n\n" + helperOutput
|
||||
}
|
||||
// Attach the artifact itself. Without this the model is told a screenshot
|
||||
// exists and cannot look at it, which is the whole point of asking for one.
|
||||
// A PDF or an unreadable file simply yields no image and the text stands on
|
||||
// its own, so a capture is never failed over the attachment.
|
||||
var images []zeroruntime.ImageBlock
|
||||
if image, err := imageinput.LoadFile(path, ""); err == nil {
|
||||
images = []zeroruntime.ImageBlock{image}
|
||||
}
|
||||
return Result{
|
||||
Status: StatusOK,
|
||||
Output: output,
|
||||
Images: images,
|
||||
Meta: map[string]string{
|
||||
"artifact_path": path,
|
||||
"action": request.action,
|
||||
|
||||
+25
-50
@@ -31,15 +31,16 @@ func NewScopedReadFileTool(workspaceRoot string, scope PathScope) Tool {
|
||||
return readFileTool{
|
||||
baseTool: baseTool{
|
||||
name: "read_file",
|
||||
description: "Read exact file text with line numbers. Use for comments, formatting, or edits; prefer read_minified_file for initial code understanding. Use offset and limit for a line range.",
|
||||
description: "Read a file by 1-based inclusive line range, or by exact byte chunks for oversized single-line files.",
|
||||
parameters: Schema{
|
||||
Type: "object",
|
||||
Properties: map[string]PropertySchema{
|
||||
"path": {Type: "string", Description: "File path."},
|
||||
"offset": {Type: "integer", Description: "Optional 1-based source line to start from.", Minimum: intPtr(1)},
|
||||
"limit": {Type: "integer", Description: "Optional number of source lines to return.", Minimum: intPtr(1)},
|
||||
"byte_offset": {Type: "integer", Description: "Optional zero-based offset for exact byte reads.", Minimum: intPtr(0)},
|
||||
"byte_limit": {Type: "integer", Description: "Optional byte count for an exact byte read; use with byte_offset.", Minimum: intPtr(1), Maximum: intPtr(readFileByteChunkMax)},
|
||||
"path": {Type: "string", Description: "Path of the file to read."},
|
||||
"start_line": {Type: "integer", Description: "1-based inclusive line number to start reading from.", Minimum: intPtr(1)},
|
||||
"end_line": {Type: "integer", Description: "1-based inclusive line number to stop reading at.", Minimum: intPtr(1)},
|
||||
"max_lines": {Type: "integer", Description: "Maximum number of lines to return.", Minimum: intPtr(1)},
|
||||
"byte_offset": {Type: "integer", Description: "Zero-based byte offset for an exact chunk read. Use with byte_limit for oversized single-line files; cannot be combined with line ranges.", Minimum: intPtr(0)},
|
||||
"byte_limit": {Type: "integer", Description: "Maximum source bytes returned in exact byte mode. Defaults to 65536.", Default: readFileByteChunkMax, Minimum: intPtr(1), Maximum: intPtr(readFileByteChunkMax)},
|
||||
},
|
||||
Required: []string{"path"},
|
||||
AdditionalProperties: false,
|
||||
@@ -68,27 +69,7 @@ func (tool readFileTool) run(args map[string]any, options RunOptions, directBudg
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_file: " + err.Error())
|
||||
}
|
||||
_, hasOffset := args["offset"]
|
||||
_, hasLimit := args["limit"]
|
||||
_, hasStartLine := args["start_line"]
|
||||
_, hasEndLine := args["end_line"]
|
||||
_, hasMaxLines := args["max_lines"]
|
||||
if hasOffset && hasStartLine {
|
||||
return errorResult("Error: Invalid arguments for read_file: use either offset or start_line, not both")
|
||||
}
|
||||
if hasLimit && (hasEndLine || hasMaxLines) {
|
||||
return errorResult("Error: Invalid arguments for read_file: use limit instead of end_line or max_lines, not both")
|
||||
}
|
||||
|
||||
startKey := "offset"
|
||||
if !hasOffset && hasStartLine {
|
||||
startKey = "start_line"
|
||||
}
|
||||
startLine, err := intArg(args, startKey, 1, 1, 0)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_file: " + err.Error())
|
||||
}
|
||||
limit, err := intArg(args, "limit", 0, 1, 0)
|
||||
startLine, err := intArg(args, "start_line", 1, 1, 0)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_file: " + err.Error())
|
||||
}
|
||||
@@ -100,25 +81,19 @@ func (tool readFileTool) run(args map[string]any, options RunOptions, directBudg
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_file: " + err.Error())
|
||||
}
|
||||
if hasLimit {
|
||||
const maxInt = int(^uint(0) >> 1)
|
||||
endLine = startLine + min(limit-1, maxInt-startLine)
|
||||
}
|
||||
hasLineRange := hasOffset || hasLimit || hasStartLine || hasEndLine || hasMaxLines
|
||||
_, hasByteOffset := args["byte_offset"]
|
||||
_, hasByteLimit := args["byte_limit"]
|
||||
byteMode := (hasByteOffset || hasByteLimit) && !hasLineRange
|
||||
byteOffset := 0
|
||||
byteLimit := readFileByteChunkMax
|
||||
if byteMode {
|
||||
byteOffset, err = intArg(args, "byte_offset", 0, 0, 0)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_file: " + err.Error())
|
||||
}
|
||||
byteLimit, err = intArg(args, "byte_limit", readFileByteChunkMax, 1, readFileByteChunkMax)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_file: " + err.Error())
|
||||
}
|
||||
byteMode := hasByteOffset || hasByteLimit
|
||||
byteOffset, err := intArg(args, "byte_offset", 0, 0, 0)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_file: " + err.Error())
|
||||
}
|
||||
byteLimit, err := intArg(args, "byte_limit", readFileByteChunkMax, 1, readFileByteChunkMax)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_file: " + err.Error())
|
||||
}
|
||||
if byteMode && (args["start_line"] != nil || args["end_line"] != nil || args["max_lines"] != nil) {
|
||||
return errorResult("Error: Invalid arguments for read_file: byte_offset/byte_limit cannot be combined with line ranges")
|
||||
}
|
||||
|
||||
absolutePath, relativePath, err := resolveScopedReadPath(tool.workspaceRoot, tool.scope, requestedPath)
|
||||
@@ -194,7 +169,7 @@ func renderedReadRange(total, start, end, maxLines int) (int, int) {
|
||||
|
||||
func renderReadFileRange(absolutePath string, relativePath string, total int, startLine int, endLine int, maxLines int, maxBytes int) Result {
|
||||
if startLine > total {
|
||||
return okResult(fmt.Sprintf("File: %s\n(offset %d is past the end of the file, which has %d lines)", relativePath, startLine, total))
|
||||
return okResult(fmt.Sprintf("File: %s\n(start_line %d is past the end of the file, which has %d lines)", relativePath, startLine, total))
|
||||
}
|
||||
if endLine == 0 || endLine > total {
|
||||
endLine = total
|
||||
@@ -233,9 +208,9 @@ func renderReadFileRange(absolutePath string, relativePath string, total int, st
|
||||
// Tool.Run calls preserve the legacy prefix-only byte budget.
|
||||
var budgetedOutput *outputBudgetBuilder
|
||||
if maxBytes > 0 {
|
||||
budgetedOutput = newOutputBudgetBuilder(maxBytes, "use offset/limit to request a smaller line range")
|
||||
budgetedOutput = newOutputBudgetBuilder(maxBytes, "use start_line/end_line or max_lines for normal files; use byte_offset/byte_limit for an oversized single line")
|
||||
} else {
|
||||
budgetedOutput = newHeadTailOutputBudgetBuilder(readOutputBudgetBytes, "use offset/limit to request a smaller line range")
|
||||
budgetedOutput = newHeadTailOutputBudgetBuilder(readOutputBudgetBytes, "use start_line/end_line or max_lines for normal files; use byte_offset/byte_limit for an oversized single line")
|
||||
}
|
||||
budgetedOutput.WriteString(header)
|
||||
budgetedOutput.WriteString("\n")
|
||||
@@ -249,9 +224,9 @@ func renderReadFileRange(absolutePath string, relativePath string, total int, st
|
||||
}
|
||||
if truncated {
|
||||
// The Truncated flag alone is invisible to the model in the rendered
|
||||
// output, so it cannot tell a limit cut from a complete read. Make the
|
||||
// output, so it cannot tell a max_lines cut from a complete read. Make the
|
||||
// cut explicit and tell it how to continue.
|
||||
budgetedOutput.WriteString(fmt.Sprintf("\n\n[truncated: %d more line(s) in the requested range not shown; set offset=%d to continue]", endLine-lastLine, lastLine+1))
|
||||
budgetedOutput.WriteString(fmt.Sprintf("\n\n[truncated: %d more line(s) in the requested range not shown; set start_line=%d to continue]", endLine-lastLine, lastLine+1))
|
||||
}
|
||||
|
||||
budgeted := budgetedOutput.Result()
|
||||
@@ -266,7 +241,7 @@ func renderReadFileRange(absolutePath string, relativePath string, total int, st
|
||||
if budgeted.Truncated {
|
||||
meta["truncation_reason"] = "byte_budget"
|
||||
} else {
|
||||
meta["truncation_reason"] = "limit"
|
||||
meta["truncation_reason"] = "max_lines"
|
||||
}
|
||||
}
|
||||
return Result{
|
||||
|
||||
@@ -28,13 +28,12 @@ func NewScopedReadMinifiedFileTool(workspaceRoot string, scope PathScope) Tool {
|
||||
return readMinifiedFileTool{
|
||||
baseTool: baseTool{
|
||||
name: "read_minified_file",
|
||||
description: "Read source code in a token-efficient, language-aware form. Prefer this for initial understanding; use read_file for exact text or line numbers.",
|
||||
description: "Read a source file in a dense, token-cheap form: comments and redundant whitespace removed, no line numbers. Use it to scan or understand code for far fewer tokens than read_file. For exact text, comments, line numbers, or before editing, use read_file instead.",
|
||||
deferred: true,
|
||||
parameters: Schema{
|
||||
Type: "object",
|
||||
Properties: map[string]PropertySchema{
|
||||
"path": {Type: "string", Description: "Source file path."},
|
||||
"offset": {Type: "integer", Description: "Optional 1-based source line to start from.", Minimum: intPtr(1)},
|
||||
"limit": {Type: "integer", Description: "Optional maximum number of source lines to minify.", Minimum: intPtr(1)},
|
||||
"path": {Type: "string", Description: "Path of the file to read in minified form."},
|
||||
},
|
||||
Required: []string{"path"},
|
||||
AdditionalProperties: false,
|
||||
@@ -60,14 +59,6 @@ func (tool readMinifiedFileTool) run(args map[string]any, options RunOptions, di
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_minified_file: " + err.Error())
|
||||
}
|
||||
offset, err := intArg(args, "offset", 1, 1, 0)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_minified_file: " + err.Error())
|
||||
}
|
||||
limit, err := intArg(args, "limit", 0, 1, 0)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for read_minified_file: " + err.Error())
|
||||
}
|
||||
|
||||
absolutePath, relativePath, err := resolveScopedReadPath(tool.workspaceRoot, tool.scope, requestedPath)
|
||||
if err != nil {
|
||||
@@ -84,19 +75,11 @@ func (tool readMinifiedFileTool) run(args map[string]any, options RunOptions, di
|
||||
info, _ := os.Stat(absolutePath)
|
||||
options.FileTracker.Record(absolutePath, content, info)
|
||||
|
||||
selected := selectSourceLines(content, offset, limit)
|
||||
if selected.pastEnd {
|
||||
return okResult(fmt.Sprintf("File: %s\n(offset %d is past the end of the file, which has %d lines)", relativePath, offset, selected.totalLines))
|
||||
}
|
||||
ranged := offset > 1 || limit > 0
|
||||
result := minify.File(relativePath, selected.content)
|
||||
if ranged {
|
||||
result = minify.ContextualFragment(relativePath, content, selected.content, selected.startByte)
|
||||
}
|
||||
rawLines := lineCount(string(selected.content))
|
||||
result := minify.File(relativePath, content)
|
||||
rawLines := lineCount(string(content))
|
||||
minLines := lineCount(result.Content)
|
||||
pct := 0
|
||||
if rawBytes := len(selected.content); rawBytes > 0 {
|
||||
if rawBytes := len(content); rawBytes > 0 {
|
||||
if saved := rawBytes - len(result.Content); saved > 0 {
|
||||
pct = saved * 100 / rawBytes
|
||||
}
|
||||
@@ -106,15 +89,12 @@ func (tool readMinifiedFileTool) run(args map[string]any, options RunOptions, di
|
||||
if result.Applied {
|
||||
header = fmt.Sprintf("File: %s — minified %s view (comments stripped, no line numbers; %d→%d lines, ~%d%% fewer bytes). For exact text/comments or before editing, use read_file.",
|
||||
relativePath, result.Language, rawLines, minLines, pct)
|
||||
} else if ranged {
|
||||
header = fmt.Sprintf("File: %s — safe ranged view (whitespace normalized, no line numbers; %d→%d lines; context-sensitive stripping disabled because the range may begin inside a multiline construct). For exact text, use read_file.",
|
||||
relativePath, rawLines, minLines)
|
||||
} else {
|
||||
header = fmt.Sprintf("File: %s — whitespace-normalized view (no line numbers; %d→%d lines; full minification not available for this file type). For exact text, use read_file.",
|
||||
relativePath, rawLines, minLines)
|
||||
}
|
||||
|
||||
rawBytes := len(selected.content)
|
||||
rawBytes := len(content)
|
||||
compactBytes := len(result.Content)
|
||||
savedTokens := 0
|
||||
if savedBytes := rawBytes - compactBytes; savedBytes > 0 {
|
||||
@@ -133,54 +113,11 @@ func (tool readMinifiedFileTool) run(args map[string]any, options RunOptions, di
|
||||
meta["estimated_tokens_saved"] = strconv.Itoa(savedTokens)
|
||||
toolResult := Result{Status: StatusOK, Output: output, Meta: meta}
|
||||
if directBudget {
|
||||
return applyLegacyByteBudgetToResult(toolResult, readOutputBudgetBytes, "use offset/limit to request a smaller source range, or read_file when exact text is required")
|
||||
return applyLegacyByteBudgetToResult(toolResult, readOutputBudgetBytes, "use read_file with start_line/end_line or max_lines for normal files, or byte_offset/byte_limit for an oversized single line")
|
||||
}
|
||||
return toolResult
|
||||
}
|
||||
|
||||
type sourceSelection struct {
|
||||
content []byte
|
||||
startByte int
|
||||
totalLines int
|
||||
pastEnd bool
|
||||
}
|
||||
|
||||
func selectSourceLines(content []byte, offset, limit int) sourceSelection {
|
||||
if offset <= 1 && limit == 0 {
|
||||
return sourceSelection{content: content, totalLines: sourceLineCount(content)}
|
||||
}
|
||||
lines := strings.Split(string(content), "\n")
|
||||
totalLines := sourceLineCount(content)
|
||||
start := offset - 1
|
||||
if start >= totalLines {
|
||||
return sourceSelection{startByte: len(content), totalLines: totalLines, pastEnd: true}
|
||||
}
|
||||
startByte := 0
|
||||
for index := 0; index < start; index++ {
|
||||
startByte += len(lines[index]) + 1
|
||||
}
|
||||
end := totalLines
|
||||
if limit > 0 && limit < end-start {
|
||||
end = start + limit
|
||||
}
|
||||
return sourceSelection{
|
||||
content: []byte(strings.Join(lines[start:end], "\n")),
|
||||
startByte: startByte,
|
||||
totalLines: totalLines,
|
||||
}
|
||||
}
|
||||
|
||||
func sourceLineCount(content []byte) int {
|
||||
if len(content) == 0 {
|
||||
return 1
|
||||
}
|
||||
lines := strings.Count(string(content), "\n")
|
||||
if content[len(content)-1] != '\n' {
|
||||
lines++
|
||||
}
|
||||
return lines
|
||||
}
|
||||
|
||||
// lineCount reports the number of newline-separated lines in s (an empty string
|
||||
// counts as 0 lines, matching how a reader perceives an empty file).
|
||||
func lineCount(s string) int {
|
||||
|
||||
@@ -45,123 +45,6 @@ func TestReadMinifiedFileRejectsTraversal(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadMinifiedFileSelectsSourceLineRangeBeforeMinifying(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
src := "package demo\n\nfunc One() int { return 1 }\n\nfunc Two() int { return 2 }\n"
|
||||
if err := os.WriteFile(filepath.Join(dir, "f.go"), []byte(src), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
|
||||
"path": "f.go", "offset": 5, "limit": 1,
|
||||
})
|
||||
if res.Status != StatusOK || !strings.Contains(res.Output, "func Two") || strings.Contains(res.Output, "func One") {
|
||||
t.Fatalf("unexpected ranged compact read: status=%s\n%s", res.Status, res.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadMinifiedFileRangesPreserveUnknownLexicalContext(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
path string
|
||||
src string
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
name: "multiline string",
|
||||
path: "snippet.py",
|
||||
src: "value = \"\"\"\n# literal text\nstill literal\n\"\"\"\nprint(value)\n",
|
||||
want: []string{"# literal text", "still literal"},
|
||||
},
|
||||
{
|
||||
name: "template literal",
|
||||
path: "snippet.js",
|
||||
src: "const value = `\n// literal text\nstill literal\n`;\n",
|
||||
want: []string{"// literal text", "still literal"},
|
||||
},
|
||||
{
|
||||
name: "block comment",
|
||||
path: "snippet.c",
|
||||
src: "/* open\ncomment body\n*/\nint live;\n",
|
||||
want: []string{"comment body", "*/"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(dir, tc.path), []byte(tc.src), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
|
||||
"path": tc.path, "offset": 2, "limit": 2,
|
||||
})
|
||||
if res.Status != StatusOK || res.Meta["compacted"] != "false" {
|
||||
t.Fatalf("ranged read must use conservative normalization: status=%s meta=%#v\n%s", res.Status, res.Meta, res.Output)
|
||||
}
|
||||
for _, want := range tc.want {
|
||||
if !strings.Contains(res.Output, want) {
|
||||
t.Fatalf("ranged read lost lexical content %q:\n%s", want, res.Output)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadMinifiedFileRangeInsideGoRawStringIsConservative(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
src := "package demo\nvar value = `first\nSECRET-MARKER-ONE\n// literal line\nlast`\n"
|
||||
if err := os.WriteFile(filepath.Join(dir, "f.go"), []byte(src), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
|
||||
"path": "f.go", "offset": 3, "limit": 2,
|
||||
})
|
||||
if res.Status != StatusOK || res.Meta["compacted"] != "false" {
|
||||
t.Fatalf("raw-string range must use conservative normalization: status=%s meta=%#v\n%s", res.Status, res.Meta, res.Output)
|
||||
}
|
||||
for _, want := range []string{"SECRET-MARKER-ONE", "// literal line"} {
|
||||
if !strings.Contains(res.Output, want) {
|
||||
t.Fatalf("raw-string range lost or rewrote %q:\n%s", want, res.Output)
|
||||
}
|
||||
}
|
||||
if strings.Contains(res.Output, "SECRET - MARKER - ONE") {
|
||||
t.Fatalf("raw-string contents were parsed as Go code:\n%s", res.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadMinifiedFileCanonicalOffsetPastEnd(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(dir, "f.go"), []byte("package demo\nvar value = 1\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
|
||||
"path": "f.go", "offset": 10,
|
||||
})
|
||||
if res.Status != StatusOK || !strings.Contains(res.Output, "offset 10 is past the end") {
|
||||
t.Fatalf("expected canonical out-of-range response, got status=%s output=%q", res.Status, res.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadMinifiedFileMaximumLimitDoesNotOverflow(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("alpha\nbeta\ngamma\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
const maxInt = int(^uint(0) >> 1)
|
||||
|
||||
res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
|
||||
"path": "notes.txt", "offset": 2, "limit": maxInt,
|
||||
})
|
||||
if res.Status != StatusOK {
|
||||
t.Fatalf("maximum limit should return the remaining range without panicking: status=%s output=%q", res.Status, res.Output)
|
||||
}
|
||||
if !strings.Contains(res.Output, "beta") || !strings.Contains(res.Output, "gamma") || strings.Contains(res.Output, "alpha") {
|
||||
t.Fatalf("maximum limit returned the wrong range: %q", res.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReadMinifiedFileAppliesByteBudget(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(dir, "large.txt"), []byte(strings.Repeat("0123456789abcdef\n", 9000)), 0o644); err != nil {
|
||||
|
||||
@@ -297,11 +297,8 @@ func scrubResultSecrets(res Result) Result {
|
||||
|
||||
func CoreReadOnlyToolsScoped(workspaceRoot string, scope PathScope) []Tool {
|
||||
return []Tool{
|
||||
NewScopedReadMinifiedFileTool(workspaceRoot, scope),
|
||||
NewScopedReadFileTool(workspaceRoot, scope),
|
||||
// view_image is read-only and shares read_file's path scoping, so it
|
||||
// belongs with the other readers rather than behind a gate.
|
||||
NewScopedViewImageTool(workspaceRoot, scope),
|
||||
NewScopedReadMinifiedFileTool(workspaceRoot, scope),
|
||||
NewScopedListDirectoryTool(workspaceRoot, scope),
|
||||
NewScopedGlobTool(workspaceRoot, scope),
|
||||
NewScopedGrepTool(workspaceRoot, scope),
|
||||
|
||||
@@ -30,8 +30,8 @@ func tempDirOutsideDefaultTemp(t *testing.T) string {
|
||||
|
||||
func TestCoreReadOnlyToolsExposeSafeMetadata(t *testing.T) {
|
||||
toolset := CoreReadOnlyToolsScoped(t.TempDir(), nil)
|
||||
if len(toolset) != 10 {
|
||||
t.Fatalf("expected 10 core read-only tools, got %d", len(toolset))
|
||||
if len(toolset) != 9 {
|
||||
t.Fatalf("expected 9 core read-only tools, got %d", len(toolset))
|
||||
}
|
||||
|
||||
seen := map[string]bool{}
|
||||
@@ -71,11 +71,6 @@ func TestCoreReadOnlyToolsExposeSafeMetadata(t *testing.T) {
|
||||
if !seen[RequestPermissionsToolName] {
|
||||
t.Fatalf("expected %s in core read-only tools", RequestPermissionsToolName)
|
||||
}
|
||||
// By name as well as by count: the count alone still passes if one reader is
|
||||
// swapped for another.
|
||||
if !seen[ViewImageToolName] {
|
||||
t.Fatalf("expected %s in core read-only tools", ViewImageToolName)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegistryAllReturnsToolsSortedByName(t *testing.T) {
|
||||
@@ -523,24 +518,25 @@ func TestIsDeferralEligibleDecouplesFromDeferred(t *testing.T) {
|
||||
|
||||
func TestOptionalBuiltinsUseDeferredDiscovery(t *testing.T) {
|
||||
wantDeferred := map[string]bool{
|
||||
"bash": true,
|
||||
"browser_action": true,
|
||||
"browser_click": true,
|
||||
"browser_connect": true,
|
||||
"browser_install": true,
|
||||
"browser_launch": true,
|
||||
"browser_open": true,
|
||||
"browser_press": true,
|
||||
"browser_snapshot": true,
|
||||
"browser_type": true,
|
||||
"capture_artifact": true,
|
||||
"desktop_action": true,
|
||||
"desktop_snapshot": true,
|
||||
"desktop_windows": true,
|
||||
"lsp_navigate": true,
|
||||
"terminal_session": true,
|
||||
"web_fetch": true,
|
||||
"web_search": true,
|
||||
"bash": true,
|
||||
"browser_action": true,
|
||||
"browser_click": true,
|
||||
"browser_connect": true,
|
||||
"browser_install": true,
|
||||
"browser_launch": true,
|
||||
"browser_open": true,
|
||||
"browser_press": true,
|
||||
"browser_snapshot": true,
|
||||
"browser_type": true,
|
||||
"capture_artifact": true,
|
||||
"desktop_action": true,
|
||||
"desktop_snapshot": true,
|
||||
"desktop_windows": true,
|
||||
"lsp_navigate": true,
|
||||
"read_minified_file": true,
|
||||
"terminal_session": true,
|
||||
"web_fetch": true,
|
||||
"web_search": true,
|
||||
}
|
||||
for _, tool := range BuiltinCatalog(t.TempDir()) {
|
||||
if _, listed := wantDeferred[tool.Name()]; listed {
|
||||
@@ -548,7 +544,7 @@ func TestOptionalBuiltinsUseDeferredDiscovery(t *testing.T) {
|
||||
t.Errorf("%s should be deferred", tool.Name())
|
||||
}
|
||||
delete(wantDeferred, tool.Name())
|
||||
} else if tool.Name() == "read_file" || tool.Name() == "read_minified_file" || tool.Name() == "grep" || tool.Name() == ExecCommandToolName || tool.Name() == "edit_file" {
|
||||
} else if tool.Name() == "read_file" || tool.Name() == "grep" || tool.Name() == ExecCommandToolName || tool.Name() == "edit_file" {
|
||||
if IsDeferred(tool) {
|
||||
t.Errorf("essential tool %s must stay eager", tool.Name())
|
||||
}
|
||||
|
||||
@@ -1,72 +0,0 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// The cheapest fix for #845/#847 is that the invalid shape is never emitted.
|
||||
// A model only learns "omit it" from the schema it is shown, so both shell
|
||||
// tools must say so in the field descriptions themselves — the validator errors
|
||||
// are the backstop, and by the time they fire a turn has already been burned.
|
||||
func TestShellToolsAdvertiseOmittingSandboxPermissionFields(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
for _, shell := range []struct {
|
||||
name string
|
||||
tool Tool
|
||||
}{
|
||||
{"bash", NewScopedBashTool(root, nil)},
|
||||
{"exec_command", NewScopedExecCommandTool(root, nil, nil)},
|
||||
} {
|
||||
properties := shell.tool.Parameters().Properties
|
||||
|
||||
mode, ok := properties["sandbox_permissions"]
|
||||
if !ok {
|
||||
t.Fatalf("%s: sandbox_permissions property missing", shell.name)
|
||||
}
|
||||
if !strings.Contains(mode.Description, "Omit it") {
|
||||
t.Errorf("%s: sandbox_permissions description = %q, want it to lead with omitting the field", shell.name, mode.Description)
|
||||
}
|
||||
// Deliberately NOT "read-only". An earlier revision said every read-only
|
||||
// command could omit the override, and that is false: `curl` and `gh` are
|
||||
// read-only and still need sandboxed network. Conditioning omission on
|
||||
// whether the command needs access it does not already have is the claim
|
||||
// that is actually true, and the one a model can act on without first
|
||||
// having to decide what counts as read-only.
|
||||
if !strings.Contains(mode.Description, "unless the command needs access") {
|
||||
t.Errorf("%s: sandbox_permissions description = %q, want omission conditioned on needing extra access", shell.name, mode.Description)
|
||||
}
|
||||
|
||||
extra, ok := properties["additional_permissions"]
|
||||
if !ok {
|
||||
t.Fatalf("%s: additional_permissions property missing", shell.name)
|
||||
}
|
||||
if !strings.Contains(extra.Description, "Omit this field entirely") {
|
||||
t.Errorf("%s: additional_permissions description = %q, want an explicit instruction to omit the field", shell.name, extra.Description)
|
||||
}
|
||||
// Both invalid shapes from the issue must be named as invalid up front,
|
||||
// so neither is emitted "to be safe".
|
||||
if !strings.Contains(extra.Description, "with any other mode is rejected") {
|
||||
t.Errorf("%s: additional_permissions description = %q, want it to say the unflagged shape is rejected", shell.name, extra.Description)
|
||||
}
|
||||
if !strings.Contains(extra.Description, "empty or all-false object") {
|
||||
t.Errorf("%s: additional_permissions description = %q, want it to say an empty payload is rejected", shell.name, extra.Description)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The two shell tools describe the same two fields; they were copy-pasted and
|
||||
// must not drift apart again, or a model would be taught different rules by
|
||||
// bash and exec_command in the same session.
|
||||
func TestShellToolsShareSandboxPermissionDescriptions(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
bash := NewScopedBashTool(root, nil).Parameters().Properties
|
||||
exec := NewScopedExecCommandTool(root, nil, nil).Parameters().Properties
|
||||
|
||||
for _, field := range []string{"sandbox_permissions", "additional_permissions"} {
|
||||
if bash[field].Description != exec[field].Description {
|
||||
t.Errorf("%s description differs between bash and exec_command:\n bash = %q\n exec = %q",
|
||||
field, bash[field].Description, exec[field].Description)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -5,7 +5,6 @@ import (
|
||||
|
||||
"github.com/Gitlawb/zero/internal/execution"
|
||||
"github.com/Gitlawb/zero/internal/sandbox"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
type SideEffect string
|
||||
@@ -103,16 +102,6 @@ type Result struct {
|
||||
// while callers migrate away from parsing text and string metadata.
|
||||
ExecutionRequest *execution.Request `json:"-"`
|
||||
ExecutionOutcome *execution.Outcome `json:"-"`
|
||||
// Images are pictures the tool is handing the model — a screenshot it just
|
||||
// captured, a file it was asked to look at. Text-only tools leave it nil.
|
||||
//
|
||||
// They are NOT delivered on this result's own tool-result message. Every
|
||||
// provider drops images there: 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. The agent loop therefore
|
||||
// emits them as a following user message, which is also the only shape that
|
||||
// keeps one tool result per tool call.
|
||||
Images []zeroruntime.ImageBlock `json:"-"`
|
||||
// Redacted is set when secret scrubbing altered Output before it left the
|
||||
// tool-execution boundary.
|
||||
Redacted bool
|
||||
|
||||
@@ -26,7 +26,7 @@ func NewUpdatePlanTool() *updatePlanTool {
|
||||
return &updatePlanTool{
|
||||
baseTool: baseTool{
|
||||
name: "update_plan",
|
||||
description: "Create or update the in-memory plan for implementation or investigation with at least three meaningful dependent steps. Do not use it for simple lookups, explanations, or code navigation. " +
|
||||
description: "Create or update the in-memory plan for the current task. " +
|
||||
"Pass the full ordered list of steps each call; it replaces the previous plan. " +
|
||||
"Each item needs a `content` string; `status` defaults to \"pending\" and `id` is " +
|
||||
"auto-numbered, so you only need to supply `content` (and `status` as the task progresses). " +
|
||||
|
||||
@@ -1,80 +0,0 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/imageinput"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
// ViewImageToolName is the canonical registry name of the image viewer.
|
||||
const ViewImageToolName = "view_image"
|
||||
|
||||
// NewViewImageTool builds the workspace-scoped image viewer.
|
||||
func NewViewImageTool(workspaceRoot string) Tool {
|
||||
return NewScopedViewImageTool(workspaceRoot, nil)
|
||||
}
|
||||
|
||||
// NewScopedViewImageTool builds the image viewer bound to a path scope.
|
||||
//
|
||||
// The scope is not optional decoration. imageinput.LoadFile resolves a relative
|
||||
// path by joining it to the workspace root and otherwise takes the path as
|
||||
// given, so on its own it would happily read outside the workspace. Every path
|
||||
// goes through resolveScopedReadPath first, exactly like read_file, so this tool
|
||||
// grants no reach that read_file does not already have.
|
||||
func NewScopedViewImageTool(workspaceRoot string, scope PathScope) Tool {
|
||||
return viewImageTool{
|
||||
baseTool: baseTool{
|
||||
name: ViewImageToolName,
|
||||
description: "Look at an image file. Use this to see a screenshot, diagram, mockup or photo " +
|
||||
"the user mentioned but did not attach, or one a previous tool wrote to disk. " +
|
||||
"Accepts PNG, JPEG, GIF and WebP up to 10 MiB.",
|
||||
parameters: Schema{
|
||||
Type: "object",
|
||||
Properties: map[string]PropertySchema{
|
||||
"path": {Type: "string", Description: "Path of the image to look at."},
|
||||
},
|
||||
Required: []string{"path"},
|
||||
AdditionalProperties: false,
|
||||
},
|
||||
safety: readOnlySafety("Reads an image file without modifying anything."),
|
||||
// Same shape as read_file: reads one file, mutates nothing, and is
|
||||
// safe to run concurrently.
|
||||
capabilities: ToolCapabilities{Effect: EffectReadOnly, ThreadSafe: true, ResourceKeys: fileResourceKeys},
|
||||
},
|
||||
workspaceRoot: workspaceRoot,
|
||||
scope: scope,
|
||||
}
|
||||
}
|
||||
|
||||
type viewImageTool struct {
|
||||
baseTool
|
||||
workspaceRoot string
|
||||
scope PathScope
|
||||
}
|
||||
|
||||
func (tool viewImageTool) Run(_ context.Context, args map[string]any) Result {
|
||||
requestedPath, err := aliasedStringArg(args, []string{"path", "file", "file_path", "filepath", "filename", "image"}, "", true, false)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for " + ViewImageToolName + ": " + err.Error())
|
||||
}
|
||||
absolutePath, relativePath, err := resolveScopedReadPath(tool.workspaceRoot, tool.scope, requestedPath)
|
||||
if err != nil {
|
||||
return errorResult("Error: " + err.Error())
|
||||
}
|
||||
image, err := imageinput.LoadFile(absolutePath, tool.workspaceRoot)
|
||||
if err != nil {
|
||||
// LoadFile's errors already name the problem (missing, too large,
|
||||
// unsupported type) and are safe to show: they carry the path the caller
|
||||
// asked for, which it already knows.
|
||||
return errorResult("Error viewing " + relativePath + ": " + err.Error())
|
||||
}
|
||||
return Result{
|
||||
Status: StatusOK,
|
||||
// The text says what arrived; the image itself reaches the model through
|
||||
// the agent loop, which delivers Images on a following user message.
|
||||
Output: "Viewing " + relativePath + " (" + image.MediaType + ").",
|
||||
Images: []zeroruntime.ImageBlock{image},
|
||||
Meta: map[string]string{"media_type": image.MediaType},
|
||||
}
|
||||
}
|
||||
@@ -1,100 +0,0 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// A 1x1 PNG. Small enough to inline, real enough to pass media-type sniffing.
|
||||
var onePixelPNG = []byte{
|
||||
0x89, 'P', 'N', 'G', 0x0d, 0x0a, 0x1a, 0x0a,
|
||||
0x00, 0x00, 0x00, 0x0d, 'I', 'H', 'D', 'R',
|
||||
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01,
|
||||
0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4,
|
||||
0x89, 0x00, 0x00, 0x00, 0x0a, 'I', 'D', 'A', 'T',
|
||||
0x78, 0x9c, 0x63, 0x00, 0x01, 0x00, 0x00, 0x05,
|
||||
0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, 0x00, 0x00,
|
||||
0x00, 0x00, 'I', 'E', 'N', 'D', 0xae, 0x42, 0x60, 0x82,
|
||||
}
|
||||
|
||||
func TestViewImageReturnsTheImage(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(root, "shot.png"), onePixelPNG, 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
result := NewViewImageTool(root).Run(context.Background(), map[string]any{"path": "shot.png"})
|
||||
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("status = %v: %s", result.Status, result.Output)
|
||||
}
|
||||
if len(result.Images) != 1 {
|
||||
t.Fatalf("got %d images, want 1 — the text alone is useless to the model", len(result.Images))
|
||||
}
|
||||
if result.Images[0].MediaType != "image/png" {
|
||||
t.Errorf("media type = %q, want image/png", result.Images[0].MediaType)
|
||||
}
|
||||
if len(result.Images[0].Data) != len(onePixelPNG) {
|
||||
t.Errorf("image data is %d bytes, want %d", len(result.Images[0].Data), len(onePixelPNG))
|
||||
}
|
||||
}
|
||||
|
||||
// The tool must not reach outside the workspace. imageinput.LoadFile does no
|
||||
// containment check of its own, so the scoping is entirely this tool's job.
|
||||
func TestViewImageRefusesPathsOutsideTheWorkspace(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
outside := t.TempDir()
|
||||
secret := filepath.Join(outside, "outside.png")
|
||||
if err := os.WriteFile(secret, onePixelPNG, 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tool := NewViewImageTool(root)
|
||||
|
||||
for name, path := range map[string]string{
|
||||
"absolute path outside": secret,
|
||||
"parent traversal": filepath.Join("..", filepath.Base(outside), "outside.png"),
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
result := tool.Run(context.Background(), map[string]any{"path": path})
|
||||
if result.Status == StatusOK {
|
||||
t.Fatalf("read an image outside the workspace: %s", result.Output)
|
||||
}
|
||||
if len(result.Images) != 0 {
|
||||
t.Error("a refused read still returned image bytes")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestViewImageRejectsNonImages(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(root, "notes.txt"), []byte("not an image"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
result := NewViewImageTool(root).Run(context.Background(), map[string]any{"path": "notes.txt"})
|
||||
if result.Status == StatusOK {
|
||||
t.Fatalf("accepted a text file as an image: %s", result.Output)
|
||||
}
|
||||
if len(result.Images) != 0 {
|
||||
t.Error("a rejected file still returned image bytes")
|
||||
}
|
||||
}
|
||||
|
||||
func TestViewImageReportsAMissingFileClearly(t *testing.T) {
|
||||
result := NewViewImageTool(t.TempDir()).Run(context.Background(), map[string]any{"path": "nope.png"})
|
||||
if result.Status == StatusOK {
|
||||
t.Fatal("a missing file reported success")
|
||||
}
|
||||
// The error names the path the caller asked for, matching read_file. Both
|
||||
// also carry the resolved absolute path from the underlying syscall error;
|
||||
// that is pre-existing behaviour shared with read_file, not something this
|
||||
// tool introduces, so it is not asserted either way here.
|
||||
if !strings.Contains(result.Output, "nope.png") {
|
||||
t.Errorf("error should name the file, got %q", result.Output)
|
||||
}
|
||||
if len(result.Images) != 0 {
|
||||
t.Error("a failed read still returned image bytes")
|
||||
}
|
||||
}
|
||||
@@ -1,60 +0,0 @@
|
||||
package tui
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/charmbracelet/x/ansi"
|
||||
)
|
||||
|
||||
// A COPIED NUL KILLED THE WHOLE PROGRAM ON WINDOWS.
|
||||
//
|
||||
// github.com/atotto/clipboard's Windows path calls syscall.StringToUTF16, which
|
||||
// PANICS rather than returning an error when the string contains a NUL — so
|
||||
// selecting transcript text that carried one took the TUI down with "program
|
||||
// experienced a panic" and a stack ending in clipboard.writeAll. A teammate hit
|
||||
// it on a real Windows machine.
|
||||
//
|
||||
// The NUL reaches the clipboard because ansi.Strip removes escape SEQUENCES and
|
||||
// leaves C0 bytes alone, and the transcript carries them from two directions:
|
||||
// tool output (binary reads, git's -z listings) and this package's own card
|
||||
// protocol, whose prefixes are literally "\x00command-card\x00".
|
||||
func TestNulNeverReachesTheClipboard(t *testing.T) {
|
||||
// The card prefix is a real, in-repo source: ansi.Strip leaves it intact.
|
||||
if !strings.ContainsRune(ansi.Strip(commandCardTranscriptPrefix+"Profile"), 0) {
|
||||
t.Fatal("setup: the card prefix no longer carries a NUL; this test guards the wrong thing")
|
||||
}
|
||||
for _, tc := range []struct{ name, in string }{
|
||||
{"card prefix", commandCardTranscriptPrefix + "Profile\nstatus: ok"},
|
||||
{"plan card prefix", planCardTranscriptPrefix + "plan"},
|
||||
{"binary tool output", "before\x00after"},
|
||||
{"git -z listing", "a.go\x00b.go\x00c.go"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := clipboardSafeText(tc.in)
|
||||
if strings.ContainsRune(got, 0) {
|
||||
t.Fatalf("a NUL survived to the clipboard write: %q — Windows would panic here", got)
|
||||
}
|
||||
// The readable content must survive: this sanitises, it does not truncate.
|
||||
for _, want := range strings.Split(strings.ReplaceAll(tc.in, "\x00", "\n"), "\n") {
|
||||
if want = strings.TrimSpace(want); want != "" && !strings.Contains(got, want) {
|
||||
t.Fatalf("sanitising dropped real content %q from %q", want, got)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Newlines and tabs are legal clipboard content and must survive — a multi-line
|
||||
// selection is the normal case, not the exception.
|
||||
func TestClipboardSanitisingKeepsLayout(t *testing.T) {
|
||||
in := "line one\n\tindented\nline three"
|
||||
if got := clipboardSafeText(in); got != in {
|
||||
t.Fatalf("clean multi-line text was altered:\n got %q\nwant %q", got, in)
|
||||
}
|
||||
// Other C0 bytes go: they would terminate the OSC52 escape early and
|
||||
// corrupt the copy even where they do not panic.
|
||||
if got := clipboardSafeText("a\x07b\x1bc"); got != "abc" {
|
||||
t.Fatalf("control bytes survived: %q", got)
|
||||
}
|
||||
}
|
||||
+9
-17
@@ -5,6 +5,7 @@ import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
"runtime"
|
||||
"strings"
|
||||
"time"
|
||||
"unicode"
|
||||
@@ -2788,23 +2789,14 @@ func (m model) View() tea.View {
|
||||
// only when the value changes, so gating this costs nothing (§10).
|
||||
view.KeyboardEnhancements.ReportEventTypes = m.dictation.voiceModeEnabled
|
||||
if m.wantsMouseCapture() {
|
||||
if isRunningUnderPRoot() {
|
||||
// Under PRoot the AllMotion (1003) sequence doesn't work
|
||||
// reliably, breaking touch-gesture scrolling. Fall back to
|
||||
// CellMotion which still delivers wheel events, clicks, and
|
||||
// drag — the only thing lost is hover-highlighting.
|
||||
view.MouseMode = tea.MouseModeCellMotion
|
||||
} else {
|
||||
// AllMotion (not CellMotion) is required for hover highlighting:
|
||||
// it reports cursor movement even with no button pressed.
|
||||
// CellMotion only reports motion while a button is held (drag) —
|
||||
// see bubbletea's MouseMode docs. AllMotion has marginally worse
|
||||
// terminal compatibility but is well supported by the terminals
|
||||
// this app targets; the existing 15ms mouse-event throttle
|
||||
// (mouseEventThrottleInterval) already bounds the redraw rate
|
||||
// from the extra motion events.
|
||||
view.MouseMode = tea.MouseModeAllMotion
|
||||
}
|
||||
// AllMotion (1003) is what hover highlighting needs: it reports cursor
|
||||
// movement with no button pressed, where CellMotion (1002) reports motion
|
||||
// only while a button is held. mouseModeFor decides which is safe here,
|
||||
// and documents why one is not simply better than the other: a terminal
|
||||
// that does not implement 1003 sends nothing at all rather than degrading.
|
||||
// The 15ms throttle (mouseEventThrottleInterval) already bounds the redraw
|
||||
// rate from AllMotion's extra motion events.
|
||||
view.MouseMode = mouseModeFor(runtime.GOOS, os.Getenv, isRunningUnderPRoot())
|
||||
}
|
||||
return view
|
||||
}
|
||||
|
||||
@@ -8,6 +8,70 @@ import (
|
||||
tea "charm.land/bubbletea/v2"
|
||||
)
|
||||
|
||||
// mouseModeEnv forces a mouse mode: "all", "cell", or "off"/"none". It exists
|
||||
// because the failure it guards against is invisible from here and total for the
|
||||
// user: a terminal we guessed wrong about delivers no mouse events at all, and
|
||||
// there is no in-app way to recover. Anything unrecognised is ignored rather
|
||||
// than treated as "off", so a typo cannot silently kill the mouse.
|
||||
const mouseModeEnv = "ZERO_MOUSE_MODE"
|
||||
|
||||
// mouseModeFor picks the mouse reporting mode, preferring the widest support
|
||||
// over the nicest behaviour.
|
||||
//
|
||||
// The trade is asymmetric. AllMotion (1003) buys exactly one thing, hover
|
||||
// highlighting, because wheel, click and drag all arrive under CellMotion
|
||||
// (1002). But when a terminal does not implement 1003, it does not degrade to
|
||||
// 1002: no mouse event arrives at all. The app holds the alternate screen, so
|
||||
// the terminal's own scrollback and wheel are gone too, and the user is left
|
||||
// with no way to scroll by mouse. A long answer then looks like a hang rather
|
||||
// than like a missing highlight, which is how #870 was reported.
|
||||
//
|
||||
// So AllMotion is asked for only where it is known to work.
|
||||
func mouseModeFor(goos string, env func(string) string, underPRoot bool) tea.MouseMode {
|
||||
switch strings.ToLower(strings.TrimSpace(env(mouseModeEnv))) {
|
||||
case "all":
|
||||
return tea.MouseModeAllMotion
|
||||
case "cell":
|
||||
return tea.MouseModeCellMotion
|
||||
case "off", "none":
|
||||
return tea.MouseModeNone
|
||||
}
|
||||
if underPRoot {
|
||||
return tea.MouseModeCellMotion
|
||||
}
|
||||
if goos == "windows" && !windowsTerminalReportsAllMotion(env) {
|
||||
return tea.MouseModeCellMotion
|
||||
}
|
||||
return tea.MouseModeAllMotion
|
||||
}
|
||||
|
||||
// windowsTerminalReportsAllMotion reports whether the Windows terminal in use is
|
||||
// one that handles 1003.
|
||||
//
|
||||
// Identified terminals are trusted: Windows Terminal sets WT_SESSION, and hosts
|
||||
// like VS Code and mintty set TERM_PROGRAM. The legacy console host sets
|
||||
// neither, and it is what a user gets by double-clicking zero.exe or running it
|
||||
// from an old cmd window, so an unidentified Windows terminal is assumed to be
|
||||
// that one. Guessing wrong in this direction costs a hover highlight; guessing
|
||||
// wrong in the other direction costs every mouse event.
|
||||
// KNOWN LIMIT: these variables are INHERITED, so they answer for the process
|
||||
// that set them rather than for the console host attached right now. A shell
|
||||
// launched from Windows Terminal, or from Git Bash, that later runs zero.exe
|
||||
// against the legacy console still carries them, and this returns true for a
|
||||
// host that may drop every mouse event.
|
||||
//
|
||||
// Left as is on purpose. Deciding it properly means asking the console host
|
||||
// rather than the environment, and erring the other way costs every Windows
|
||||
// Terminal user their hover highlighting for a case that needs an unusual
|
||||
// launch path to reach. ZERO_MOUSE_MODE=cell is the recourse in the meantime,
|
||||
// which is the main reason that override exists.
|
||||
// TestInheritedWindowsTerminalEnvStillAsksForAllMotion pins this, so it is a
|
||||
// documented limit rather than a surprise.
|
||||
func windowsTerminalReportsAllMotion(env func(string) string) bool {
|
||||
return strings.TrimSpace(env("WT_SESSION")) != "" ||
|
||||
strings.TrimSpace(env("TERM_PROGRAM")) != ""
|
||||
}
|
||||
|
||||
// parseTracerPid returns true when /proc/self/status contains a non-zero
|
||||
// TracerPid. This detects any ptrace tracer (PRoot, gdb, strace, dlv, etc.),
|
||||
// not PRoot specifically. That's intentional: under any ptrace tracer the
|
||||
|
||||
@@ -0,0 +1,151 @@
|
||||
package tui
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
tea "charm.land/bubbletea/v2"
|
||||
)
|
||||
|
||||
func envFrom(pairs map[string]string) func(string) string {
|
||||
return func(key string) string { return pairs[key] }
|
||||
}
|
||||
|
||||
// AllMotion (1003) buys hover highlighting and nothing else: wheel, click and
|
||||
// drag all arrive under CellMotion (1002). When a terminal does not support
|
||||
// 1003 the cost is not a missing highlight, it is that NO mouse event arrives
|
||||
// at all, and because the app holds the alternate screen the terminal's own
|
||||
// scrollback is gone too. The user cannot scroll by any means, and a screen of
|
||||
// cut-off output reads as a hang (#870).
|
||||
//
|
||||
// So the rule is: ask for AllMotion only where it is known to work.
|
||||
func TestMouseModeFallsBackWhereAllMotionIsUnreliable(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
goos string
|
||||
env map[string]string
|
||||
underPRoot bool
|
||||
want tea.MouseMode
|
||||
}{
|
||||
{
|
||||
name: "linux terminal",
|
||||
goos: "linux",
|
||||
want: tea.MouseModeAllMotion,
|
||||
},
|
||||
{
|
||||
name: "macOS terminal",
|
||||
goos: "darwin",
|
||||
want: tea.MouseModeAllMotion,
|
||||
},
|
||||
{
|
||||
// Pre-existing carve-out: under PRoot the 1003 sequence is unreliable
|
||||
// and breaks touch-gesture scrolling.
|
||||
name: "under PRoot",
|
||||
goos: "linux",
|
||||
underPRoot: true,
|
||||
want: tea.MouseModeCellMotion,
|
||||
},
|
||||
{
|
||||
// #870. The legacy Windows console does not handle 1003 reliably, and
|
||||
// it is what a user gets by running zero.exe from anywhere that is not
|
||||
// Windows Terminal.
|
||||
name: "legacy Windows console",
|
||||
goos: "windows",
|
||||
want: tea.MouseModeCellMotion,
|
||||
},
|
||||
{
|
||||
name: "Windows Terminal",
|
||||
goos: "windows",
|
||||
env: map[string]string{"WT_SESSION": "d1c2-session"},
|
||||
want: tea.MouseModeAllMotion,
|
||||
},
|
||||
{
|
||||
name: "VS Code terminal on Windows",
|
||||
goos: "windows",
|
||||
env: map[string]string{"TERM_PROGRAM": "vscode"},
|
||||
want: tea.MouseModeAllMotion,
|
||||
},
|
||||
}
|
||||
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
got := mouseModeFor(test.goos, envFrom(test.env), test.underPRoot)
|
||||
if got != test.want {
|
||||
t.Fatalf("mouseModeFor(%q) = %v, want %v", test.goos, got, test.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A KNOWN LIMIT, pinned deliberately rather than left to be rediscovered.
|
||||
//
|
||||
// Windows children inherit the parent environment, so WT_SESSION or
|
||||
// TERM_PROGRAM can outlive the terminal that set them: a shell started from
|
||||
// Windows Terminal, or from Git Bash, that later runs zero.exe against a
|
||||
// different console host still carries them, and mouseModeFor then asks for
|
||||
// AllMotion on a host that may drop every mouse event. That is the #870 failure
|
||||
// class arriving by a narrower door.
|
||||
//
|
||||
// It is not fixed here, and this test says so out loud. Telling an inherited
|
||||
// variable from a live one needs the actual console host rather than the
|
||||
// environment, and guessing wrong in the other direction costs every Windows
|
||||
// Terminal user their hover highlighting. ZERO_MOUSE_MODE=cell is the recourse
|
||||
// until that host check exists, which is why the override is not a nicety.
|
||||
//
|
||||
// If someone later adds real host detection, this test SHOULD fail. That is the
|
||||
// point of it.
|
||||
func TestInheritedWindowsTerminalEnvStillAsksForAllMotion(t *testing.T) {
|
||||
for _, env := range []map[string]string{
|
||||
{"WT_SESSION": "inherited-from-an-ancestor"},
|
||||
{"TERM_PROGRAM": "vscode"},
|
||||
} {
|
||||
if got := mouseModeFor("windows", envFrom(env), false); got != tea.MouseModeAllMotion {
|
||||
t.Fatalf("mouseModeFor(windows, %v) = %v; if this now returns CellMotion the "+
|
||||
"inherited-environment limit has been fixed, so delete this test and its note", env, got)
|
||||
}
|
||||
// The escape hatch has to keep working for exactly this case, since it is
|
||||
// the only recourse a user has when the guess is wrong.
|
||||
withOverride := map[string]string{mouseModeEnv: "cell"}
|
||||
for key, value := range env {
|
||||
withOverride[key] = value
|
||||
}
|
||||
if got := mouseModeFor("windows", envFrom(withOverride), false); got != tea.MouseModeCellMotion {
|
||||
t.Fatalf("ZERO_MOUSE_MODE=cell did not override inherited %v, so an affected user has no recourse", env)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The override exists so a user who hits an unsupported terminal can fix it
|
||||
// themselves without waiting for a release, and so a maintainer can ask a bug
|
||||
// reporter to test one specific mode.
|
||||
func TestMouseModeOverrideWins(t *testing.T) {
|
||||
tests := []struct {
|
||||
value string
|
||||
want tea.MouseMode
|
||||
}{
|
||||
{"cell", tea.MouseModeCellMotion},
|
||||
{"CELL", tea.MouseModeCellMotion},
|
||||
{" cell ", tea.MouseModeCellMotion},
|
||||
{"all", tea.MouseModeAllMotion},
|
||||
{"off", tea.MouseModeNone},
|
||||
{"none", tea.MouseModeNone},
|
||||
}
|
||||
for _, test := range tests {
|
||||
t.Run(test.value, func(t *testing.T) {
|
||||
// windows + PRoot would otherwise force CellMotion, so "all" proves the
|
||||
// override beats the fallbacks rather than agreeing with them.
|
||||
got := mouseModeFor("windows", envFrom(map[string]string{mouseModeEnv: test.value}), true)
|
||||
if got != test.want {
|
||||
t.Fatalf("%s=%q gave %v, want %v", mouseModeEnv, test.value, got, test.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// An unreadable override must not silently disable the mouse: fall through to
|
||||
// the normal decision rather than guessing.
|
||||
func TestMouseModeIgnoresAnUnknownOverride(t *testing.T) {
|
||||
got := mouseModeFor("linux", envFrom(map[string]string{mouseModeEnv: "sideways"}), false)
|
||||
if got != tea.MouseModeAllMotion {
|
||||
t.Fatalf("an unknown override changed the mode to %v, want the normal AllMotion", got)
|
||||
}
|
||||
}
|
||||
@@ -1499,54 +1499,7 @@ func (m model) selectedTranscriptText() string {
|
||||
return strings.Join(parts, "\n")
|
||||
}
|
||||
|
||||
// clipboardSafeText removes the control characters a clipboard write cannot
|
||||
// carry, keeping newlines and tabs.
|
||||
//
|
||||
// A NUL PANICS THE WHOLE PROGRAM ON WINDOWS, and that is not a theoretical
|
||||
// concern: syscall.StringToUTF16 — which github.com/atotto/clipboard calls on
|
||||
// Windows — PANICS rather than erroring when the string contains one, so a
|
||||
// single copied NUL killed the TUI with "program experienced a panic". It
|
||||
// reached the clipboard because ansi.Strip removes escape SEQUENCES and leaves
|
||||
// C0 bytes untouched, and the transcript legitimately carries them from two
|
||||
// directions: tool output (a read of a binary file, git's -z NUL-separated
|
||||
// listings) and this package's own card protocol, whose row prefixes are
|
||||
// literally "\x00command-card\x00".
|
||||
//
|
||||
// Stripped at the WRITE, not at each source, because the sources are unbounded:
|
||||
// any tool result a user can select from is a source, and a fix per source is
|
||||
// one that the next tool re-breaks. The same pass protects the OSC52 fallback,
|
||||
// where a stray control byte would terminate the escape sequence early and
|
||||
// corrupt the copy rather than crash.
|
||||
//
|
||||
// \n and \t survive: a multi-line selection is the normal case, and both are
|
||||
// legal clipboard content.
|
||||
func clipboardSafeText(text string) string {
|
||||
if !strings.ContainsFunc(text, unsafeClipboardRune) {
|
||||
return text
|
||||
}
|
||||
var out strings.Builder
|
||||
out.Grow(len(text))
|
||||
for _, r := range text {
|
||||
if unsafeClipboardRune(r) {
|
||||
continue
|
||||
}
|
||||
out.WriteRune(r)
|
||||
}
|
||||
return out.String()
|
||||
}
|
||||
|
||||
func unsafeClipboardRune(r rune) bool {
|
||||
if r == '\n' || r == '\t' {
|
||||
return false
|
||||
}
|
||||
return r < 0x20 || r == 0x7f
|
||||
}
|
||||
|
||||
func copyTranscriptSelectionCmd(text string) tea.Cmd {
|
||||
// SANITISED ONCE, HERE, before either destination sees it — the native
|
||||
// clipboard would panic on a NUL and the OSC52 fallback would be corrupted
|
||||
// by one.
|
||||
text = clipboardSafeText(text)
|
||||
return func() tea.Msg {
|
||||
// Prefer the native OS clipboard (pbcopy / clip.exe / xclip): it works on
|
||||
// local terminals — including macOS Terminal.app, which has no OSC52 support
|
||||
|
||||
Reference in new issue
Block a user