Compare commits

..
Author SHA1 Message Date
Vasanthdev2004 8adc625d16 test(tui): pin the inherited-environment limit of the mouse-mode guess
WT_SESSION and TERM_PROGRAM are inherited, so they answer for whichever
process set them rather than for the console host attached now. A shell
started from Windows Terminal or Git Bash that later runs zero.exe
against the legacy console still carries them, and mouseModeFor then
asks for AllMotion on a host that may drop every mouse event: the #870
failure class through a narrower door.

Not fixed, and deliberately so. Telling an inherited variable from a live
one needs the console host rather than the environment, and erring the
other way costs every Windows Terminal user their hover highlighting for
a case that takes an unusual launch path to reach.

So the limit is written down in the code and pinned by a test that also
asserts ZERO_MOUSE_MODE=cell still overrides it, since that override is
the only recourse an affected user has. If real host detection lands
later the test fails by design, and its message says to delete it.

Raised by @jatmn on #872.
2026-08-07 21:06:00 +05:30
Vasanthdev2004 e4bc1f65da fix(tui): fall back to CellMotion where AllMotion is unreliable
AllMotion (1003) buys exactly one thing over CellMotion (1002): hover
highlighting. Wheel, click and drag all arrive under either. But a
terminal that does not implement 1003 does not degrade to 1002, it sends
nothing at all, and because the TUI holds the alternate screen the
terminal's own scrollback and wheel are gone too. The user is left
unable to scroll by any means, and a long answer then reads as a hang
rather than as a missing highlight. That is how #870 arrived: a
completed turn with "59 more" lines below the fold, reported as a
freeze.

The existing code already conceded 1003 is fragile by carving out PRoot.
The legacy Windows console is the same case and was not covered, so a
bare zero.exe outside Windows Terminal asked for a mode its terminal
does not answer.

Move the decision into mouseModeFor, which trusts terminals that
identify themselves (WT_SESSION for Windows Terminal, TERM_PROGRAM for
hosts like VS Code and mintty) and assumes an unidentified Windows
terminal is the legacy console. Guessing wrong that way costs a hover
highlight; guessing wrong the other way costs every mouse event.

ZERO_MOUSE_MODE=all|cell|off overrides it, so a user on a terminal we
guessed wrong about can fix it without waiting for a release. An
unrecognised value is ignored rather than treated as "off", so a typo
cannot silently kill the mouse.

Refs #870.
2026-08-06 21:28:13 +05:30
37 changed files with 331 additions and 1547 deletions

No files matched your search

-36
View File
@@ -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
View File
@@ -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
}
+7 -13
View File
@@ -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.
+1 -2
View File
@@ -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>`
-207
View File
@@ -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, " ") + "]"
}
+2 -5
View File
@@ -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
+2 -2
View File
@@ -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
View File
@@ -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
+7 -48
View File
@@ -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)
}
}
+4 -7
View File
@@ -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
+1 -1
View File
@@ -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)
}
}
+3 -4
View File
@@ -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)
}
}
-28
View File
@@ -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
}
+2 -2
View File
@@ -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."},
+2 -2
View File
@@ -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."},
-35
View File
@@ -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")
+2 -104
View File
@@ -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"] == "" {
+1 -1
View File
@@ -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."
}
+1
View File
@@ -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)},
},
-11
View File
@@ -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
View File
@@ -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{
+8 -71
View File
@@ -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 {
-117
View File
@@ -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 {
+1 -4
View File
@@ -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),
+22 -26
View File
@@ -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)
}
}
}
-11
View File
@@ -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
+1 -1
View File
@@ -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). " +
-80
View File
@@ -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},
}
}
-100
View File
@@ -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")
}
}
-60
View File
@@ -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
View File
@@ -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
}
+64
View File
@@ -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
+151
View File
@@ -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)
}
}
-47
View File
@@ -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