Compare commits

..
Author SHA1 Message Date
anandh8x e45003399d Preserve automatic compaction metadata 2026-08-08 11:12:53 +05:30
anandh8x fe11979971 Report compaction truncation structurally 2026-08-08 10:52:02 +05:30
anandh8x 8b45d995ea Unify manual and automatic compaction 2026-08-08 10:15:06 +05:30
anandh8x b01dba280b Preserve structured tool errors during compaction 2026-08-07 21:49:43 +05:30
Anandan 20c777d91a Merge branch 'main' into feat/conversation-state-compaction 2026-08-07 19:32:22 +05:30
euxaristia 1e73ecae8a docs(agents): Codify recurring review blockers in AGENTS.md (#857)
* docs(agents): codify recurring review blockers in AGENTS.md

Capture the high-frequency classes that drive multi-round maintainer reviews
(stale bases, path canonicalization, secret handling, partial setup, missing
regression tests, scope honesty) so coding agents fix them before first review.

Refs #856

* docs(agents): tighten security and atomic-write review guidance

Require open-time containment binding instead of pre-open EvalSymlinks, and
require temp-file then atomic replace rather than exclusive create alone.

Refs #856

* docs(agents): require traversal-wide containment for no-follow APIs

Final-component-only O_NOFOLLOW is insufficient on Linux and reparse
points need platform protections. Align the Security edges guidance
with the full-path containment requirement from review.
2026-08-07 20:18:11 +08:00
KRATOS cae0269e77 fix(tui): a copied NUL byte no longer panics the TUI on Windows (#876)
syscall.StringToUTF16 PANICS rather than returning an error when the
string contains a NUL — documented standard-library behaviour — and
github.com/atotto/clipboard's Windows path calls it directly. So one NUL
anywhere in a copied transcript selection killed the whole program:
"zero: tui error: program was killed: program experienced a panic",
reported from a real Windows machine.

The NUL reaches the clipboard because ansi.Strip removes escape
SEQUENCES and leaves C0 bytes untouched, and the transcript carries them
from two independent directions: tool output (a binary read, git's -z
NUL-separated listings — which this repo itself parses in
files_git_sweep.go) and Zero's own card protocol, whose row prefixes are
literally "\x00command-card\x00" and "\x00plan-card\x00".

Sanitised at the WRITE rather than at each source, because the sources
are unbounded: any tool result a user can select from is one, and a fix
per source is a fix the next tool re-breaks. The same pass protects the
OSC52 fallback, where a stray control byte terminates the escape
sequence early and silently corrupts the copy instead of crashing —
which is likely why this went unnoticed off Windows, where the identical
NUL produces a bad copy rather than a dead process.

\n and \t survive: a multi-line selection is the normal case and both
are legal clipboard content.

Fixes #875
2026-08-07 20:17:19 +08:00
anandh8x 841bf723b3 Fix durable compaction evidence identity 2026-08-06 21:58:47 +05:30
anandh8x 08a1c31f13 Reduce compaction context while preserving task state 2026-08-06 21:45:54 +05:30
Vasanth TandClaude Opus 5 cd0eb194e4 feat(tools): give tool results an image channel and add view_image (#843)
* feat(tools): give tool results an image channel and add view_image

Closes #824.

tools.Result carried only text, so no tool could hand the model a picture.
The sharpest edge was our own capture tooling: browser_screenshot,
desktop_screenshot and browser_pdf wrote a PNG, set Meta["artifact_path"]
and returned "Artifact captured: <path>". Nothing read that path
afterwards, so the model asked for a screenshot, was told one existed, and
could not look at it.

Zero was never image-blind on the way in — zeroruntime.ImageBlock,
Message.Images and internal/imageinput are mature. What was missing is the
model-initiated leg.

Result.Images is delivered as a following USER message, not on the tool
result itself. Every provider drops images on a tool-role message:
Anthropic maps a tool result to a tool_result block whose content is a
string, Gemini to a functionResponse, and OpenAI guards its image
content-parts to the user role with an explicit comment saying so. Attaching
them to the tool result would have plumbed the whole path and delivered
nothing. A separate message also keeps one tool result per tool call, which
the providers validate.

Blocks with no data, or a media type outside the provider allow-list, are
dropped rather than sent — a rejected image would fail the whole turn, and
the tool's text still stands on its own. The message names the tool, since
the images arrive detached from their result and nothing else says which
call produced them when several ran in one turn.

view_image resolves through resolveScopedReadPath before imageinput.LoadFile.
That scoping is load-bearing: LoadFile joins a relative path to the
workspace root and otherwise takes the path as given, so without it the tool
would read anywhere on disk. It is registered with the other scoped readers
and declared read-only and thread-safe, matching read_file.

The capture tools now attach what they wrote. A PDF or an unreadable file
yields no image and the text stands alone, so a capture is never failed over
its attachment.

Co-Authored-By: Claude Opus 5 <[email protected]>

* fix(agent): keep tool results contiguous when a tool returns an image

The image messages are user-role, and the first version appended each one
immediately after its own tool result. With two tool calls in a turn that
produces tool_result, user, tool_result — and a user message between two
tool results breaks strict provider replay.

Anthropic makes it concrete. appendUserBlocks coalesces consecutive user
content into one block list, and tool_result blocks must come first in that
message, so the interleaved shape maps to:

    [tool_result, text, image, tool_result]

which the API rejects. loop.go already documents this exact rule three lines
below the change, for the self-correction feedback it defers for the same
reason.

Images are now collected during the tool loop and appended once every
tool_result is recorded — including the aborted placeholders on the abort,
stop-reason and repeated-failure paths, so an early exit cannot strand them
mid-sequence either.

The original test used a single tool call, which cannot exhibit the bug. The
new one runs two calls where the first returns an image and asserts the image
message does not fall between the first and last tool result.

Also from review: view_image joins the "execute" tool category, which grants
read_file but not view_image, so a specialist declared with tools: [execute]
could not look at a screenshot it had just captured. Covered by a test that
holds for every category granting read_file rather than naming one. And the
core read-only set is now asserted by tool name, not only by count — a count
still passes when one reader is swapped for another.

Co-Authored-By: Claude Opus 5 <[email protected]>

* test(agent): pin the tool-result/image ordering contract

The assertions only tracked the first and last tool-result index, so four
distinct broken orderings passed: a message wedged between the two tool results,
a duplicated image flush, a duplicated tool result, and both results answering
the same tool_call_id. Mutation-testing each of those against the real loop
confirmed the old assertions were green on all four.

Now it collects every tool-result index and id and requires exactly two, in
c1-then-c2 order, at adjacent indexes, with the image following the last one and
delivered exactly once. Adjacency is the assertion that carries the contract:
Anthropic needs every tool_result block ahead of any other block in the user
message they coalesce into, so anything between the two results is the 400 this
change exists to avoid.

The image is deliberately NOT pinned to exactly last+1. appendUserBlocks merges
consecutive user-role content, so [tool_result, tool_result, text, image] maps to
tool_result blocks first and replays fine; pinning would fail that valid shape
while killing nothing the adjacency and count assertions do not already catch.

messageShape reports the actual role ordering on failure, and the loop uses two
ifs rather than a switch so an image left on a tool-role message is reported as
the misplacement it is instead of "the image never reached the model".

---------

Co-authored-by: Claude Opus 5 <[email protected]>
2026-08-06 21:35:41 +05:30
Vasanth T 4ca4fa75d3 fix(agent): end the additional_permissions retry loop (#864)
* fix(agent): end the additional_permissions retry loop

Asking the agent to run an ordinary read-only command on Windows could leave it
oscillating between two rejections instead of executing:

    additional_permissions requires sandbox_permissions set to "with_additional_permissions"
    additional_permissions must include at least one of network or file_system, ...

Both come from inlineAdditionalPermissionsProfile, and each names only the fix
that ADDS something. The first pushes the model toward setting the flag; the
second pushes it back toward requesting a permission the command does not need.
Neither ever says to omit the field, which is the correct action for a read-only
command, so the two messages form a two-state oscillator. Every rejection now
carries that remedy.

The schema half is why the object is attached at all. Neither field description
said to omit them for ordinary commands, so a model that legitimately used
with_additional_permissions once in a session had nothing telling it to stop
attaching a permission object to the next command. Both descriptions now lead
with "omit", and they are shared constants rather than duplicated verbatim in
bash.go and exec_command.go, where they were free to drift.

Rejection is still rejection. Nothing that was refused is now accepted on the
strength of an empty or mismatched payload, and sandbox_permissions is never
upgraded on the model's behalf; only the guidance changed.

The one behavioural change is that an explicit additional_permissions: null is
now treated as the omitted field, since a null grants nothing and carries no
flag. That flips three shapes from reject to execute: null with use_default,
with require_escalated, and with no mode at all. The escalated one still routes
through the normal escalation prompt and is not a new capability — the identical
request without the null key already behaved that way. A FLAGGED null still
errors, so with_additional_permissions plus null is not silently downgraded.

Fixes #845. Fixes #847.

* fix(tools): condition the omit guidance on needed access, not read-only

The description said to omit sandbox_permissions "for ordinary commands,
including every read-only one". That overstates it: curl and gh are read-only
and still need sandboxed network, so a model following it literally would omit
the override on a command that genuinely needs one and hit the opposite failure.

It now says to omit unless the command needs access the sandbox does not already
give it. That is the claim that is actually true, and it is one a model can act
on without first deciding what counts as read-only.

The schema test asserted the wrong half too — a bare substring check for
"read-only" pinned the overstatement rather than the contract. It now pins the
conditional, with the reasoning recorded so the weaker assertion does not come
back.

Reported by CodeRabbit on #864.
2026-08-06 21:34:54 +05:30
Anandan edf660ab85 perf: compact common test output (#846)
* perf: compact common test output

* test: strengthen command reducer guarantees

* fix: tighten command output summary detection
2026-08-06 20:35:13 +05:30
Vasanth T 91b413c514 fix(sandbox): stop the Windows write jail honouring Everyone-granted paths (#865)
* fix(sandbox): stop the Windows write jail honouring Everyone-granted paths

The restricted token put the World SID (S-1-1-0, Everyone) in its
restricted-SID list unconditionally, from the original sandbox baseline
and with no comment saying why.

A WRITE_RESTRICTED token runs two checks for a write and needs both: the
normal one against its enabled SIDs, and a second against the restricted
list. The write jail rests entirely on that second check only succeeding
where Zero has explicitly ACL'd a capability SID. Everyone is a SID every
principal carries, so on any path whose DACL grants Everyone write, the
restricted half passed for free and confinement fell back to the ordinary
user's own permissions -- the exact boundary this token exists to be
stricter than.

Reachable with no privilege, no symlink and no race: an Everyone-writable
directory is enough, and share roots opened with Everyone:F and loose
third-party installer ACLs supply them. Ruled out as the common case:
C:\Users\Public\Documents grants BATCH, not Everyone.

The runner already stated the rule this broke -- it declines to add the
user SID, Administrators or SYSTEM because "each has write access nearly
everywhere" -- while listing Everyone among the restricted SIDs.

Removed only from the WRITE_RESTRICTED token, because without that flag it
is load-bearing rather than gratuitous. The restricted-SID check then
applies to reads too, and default Windows DACLs grant BUILTIN\Users rather
than anything in this list, so a token with no Everyone cannot open cmd.exe
and dies at launch with STATUS_ACCESS_DENIED (0xC0000022). Dropping it
there was tried first and takes TestWindowsUnelevatedRealSandboxSmoke with
it. That path is taken only when a profile configures DenyRead, which is
already the posture trading capability for read-deny enforcement (#612), so
the bypass survives there deliberately and narrowly; closing it needs a
read-side grant that is not a universal group.

Verified against real tokens rather than reasoned about. The new smoke test
drives the real runner and asserts three outcomes that only agree when the
jail is intact: a granted root is writable, an ordinary outside path is
denied, and an Everyone-writable outside path is denied too. Before the
change its third case wrote the file with exit 0; the first two were
already correct, which is what made the gap invisible. Restoring the SID
unconditionally turns it red again.

* test(sandbox): make a denied write distinguishable from a runner failure

The denial assertions expected exit 1, which the runner also returns for
its own errors -- a failed marker validation, a bad argument, a token it
could not build. Written that way the test passes both when the sandbox
denies the write and when nothing ever ran, and the second case proves
nothing while being indistinguishable from success.

That is a poor property for any test and a bad one for this test, whose
entire job is to notice a confinement regression. The leading
"granted root is writable" assertion covered part of it, but said nothing
about whether the two later commands launched.

The denial commands now report a distinctive exit code the runner cannot
produce for itself, so the code proves cmd.exe actually ran and its
redirect was refused.

Re-checked in both directions rather than assumed: the test passes against
the fix, and restoring the World SID unconditionally fails it with
"exit code = 0, want 77" -- still catching the bug it was written for.

Raised by CodeRabbit on #865.
2026-08-04 18:51:35 +05:30
Vasanth T 021281ebba fix(cli): report MCP servers that failed to start (#822) (#827)
* fix(cli): report MCP servers that failed to start (#822)

`zero mcp check` inspected only the error returned by registration and then
printed "is reachable". RegisterTools is best-effort by design, so a server
that fails to connect is recorded on the runtime and registration returns
nil. That is right for startup, where one bad server must not stop Zero
launching, and wrong here: this is the command a user runs when a server is
misbehaving, and it told them the broken server was fine.

The failure data already existed as SkippedServer{Name, Err}; nothing
consumed it in this path. The check now fails with the recorded error, in
both text and JSON output.

Covered by a test that spawns a genuinely missing command rather than
stubbing the runtime, because the defect was precisely a mismatch between
what registration returns and what it records. Reverting the fix makes it
print "MCP server broken is reachable. 0 tool(s) available."

* test(cli): cover the unreachable --json contract

The text path was covered and the JSON path was not, though the change
adds its own return with a different payload: status "unreachable" plus a
redacted error, rather than status "ok" with a tool count. A caller
parsing that shape was relying on something nothing pinned.

Reverting the fix now fails both cases. The JSON one is the more useful
of the two, because the wrong output is still valid JSON and would decode
cleanly into a caller expecting success.
2026-08-03 22:50:15 +05:30
41 changed files with 2552 additions and 113 deletions

No files matched your search

+36
View File
@@ -67,3 +67,39 @@ 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,9 +127,7 @@ func TestExecuteToolCallMissingAdditionalPermissionsErrorIsActionable(t *testing
if !strings.Contains(result.Output, `{"network": {"enabled": true}}`) {
t.Fatalf("output = %q, want a concrete additional_permissions example", 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)
}
assertNamesOmitRemedy(t, "missing additional_permissions", result.Output)
}
// A well-formed additional_permissions payload must still reach the normal
@@ -165,6 +163,123 @@ 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),
+55 -5
View File
@@ -38,8 +38,9 @@ const compactionTriggerRatio = 0.7
// transcript (and so tests can assert on it).
const summaryLabel = "[Summary of earlier conversation]"
// summaryInstructions is the system prompt handed to the summarizer model.
const summaryInstructions = "You are compacting a coding-assistant conversation to save context. " +
// CompactionSummaryInstructions is the system prompt handed to both automatic
// and manually requested compaction summarizers.
const CompactionSummaryInstructions = "You are compacting a coding-assistant conversation to save context. " +
"Write a dense, factual summary of the conversation so far. Preserve: the user's goals and explicit constraints; " +
"decisions made and why; files created or modified (with paths) and key code changes; commands run and their important " +
"results; and anything still in progress or unresolved. Omit pleasantries. Use terse bullet points. Do not invent details. " +
@@ -75,6 +76,12 @@ type CompactionResult struct {
// summary message also includes summaryLabel and any preserved structured
// state needed by later compactions.
SummaryText string
// ProjectedChars is the text content sent to the summarizer after semantic
// projection, excluding provider protocol framing.
ProjectedChars int
// Truncated reports whether projection dropped content to satisfy its
// bounded brief or previous-summary limits.
Truncated bool
// Compacted reports whether Messages contains an injected summary.
Compacted bool
}
@@ -216,11 +223,11 @@ func CompactMessages(messages []zeroruntime.Message, opts CompactionOptions) (Co
}, nil
}
summary, err := opts.Summarize(middle)
summaryResult, err := SummarizeCompactionMessages(middle, opts.Summarize)
if err != nil {
return CompactionResult{}, err
}
summary = strings.TrimSpace(summary)
summary := summaryResult.SummaryText
// Preserve structured state (active plan + loaded skills) from the elided
// middle verbatim, so it is not lost or paraphrased away by the prose summary.
@@ -238,10 +245,53 @@ func CompactMessages(messages []zeroruntime.Message, opts CompactionOptions) (Co
RemovedCount: len(middle),
PreservedCount: len(messages) - len(middle),
SummaryText: summary,
ProjectedChars: summaryResult.ProjectedChars,
Truncated: summaryResult.Truncated,
Compacted: true,
}, nil
}
// CompactionSummaryResult describes the shared summarization input and output.
type CompactionSummaryResult struct {
SummaryText string
ProjectedChars int
Truncated bool
}
// SummarizeCompactionMessages applies the shared semantic projection before
// invoking a caller-supplied summarizer. Automatic context-pressure compaction
// and manual session compaction both use this path.
func SummarizeCompactionMessages(messages []zeroruntime.Message, summarize func([]zeroruntime.Message) (string, error)) (CompactionSummaryResult, error) {
if summarize == nil {
return CompactionSummaryResult{}, errors.New("compaction requires a Summarize function")
}
projection := projectCompactionInput(messages)
summaryInput := projection.messages
if len(summaryInput) == 0 {
summaryInput = messages
}
summary, err := summarize(summaryInput)
if err != nil {
return CompactionSummaryResult{}, err
}
return CompactionSummaryResult{
SummaryText: strings.TrimSpace(summary),
ProjectedChars: compactionMessageChars(summaryInput),
Truncated: projection.truncated,
}, nil
}
func compactionMessageChars(messages []zeroruntime.Message) int {
total := 0
for _, message := range messages {
total += len(message.Content)
for _, call := range message.ToolCalls {
total += len(call.Name) + len(call.Arguments)
}
}
return total
}
// safeSuffixBoundary walks the preserve boundary backward (toward systemEnd) so
// the preserved suffix begins on a user or assistant message rather than a
// tool/tool_result message. A tool result with no preceding assistant tool call
@@ -576,7 +626,7 @@ func summarizeWithFallback(ctx context.Context, provider Provider, messages []ze
func summarizeMessagesOnce(ctx context.Context, provider Provider, messages []zeroruntime.Message, onUsage func(Usage)) (string, error) {
request := zeroruntime.CompletionRequest{
Messages: []zeroruntime.Message{
{Role: zeroruntime.MessageRoleSystem, Content: summaryInstructions},
{Role: zeroruntime.MessageRoleSystem, Content: CompactionSummaryInstructions},
{Role: zeroruntime.MessageRoleUser, Content: "Summarize this conversation:\n\n" + renderTranscript(messages)},
},
// No tools: this is a plain text summarization call.
+43 -2
View File
@@ -43,8 +43,14 @@ func TestCompactMessagesReturnsMetadataForManualCompaction(t *testing.T) {
if result.SummaryText != "manual summary" {
t.Fatalf("SummaryText = %q, want trimmed summary", result.SummaryText)
}
if len(captured) != 3 || captured[0].Content != "first question" || captured[2].Content != "second question" {
t.Fatalf("summarized middle = %#v, want the three non-preserved non-system messages", captured)
if result.ProjectedChars != compactionMessageChars(captured) || result.ProjectedChars == 0 {
t.Fatalf("ProjectedChars = %d, want %d", result.ProjectedChars, compactionMessageChars(captured))
}
if result.Truncated {
t.Fatal("Truncated = true for a projection within its limits")
}
if len(captured) != 1 || !strings.Contains(captured[0].Content, "first question") || !strings.Contains(captured[0].Content, "second question") {
t.Fatalf("summarized projection = %#v, want intent from the non-preserved middle", captured)
}
if len(result.Messages) != 4 {
t.Fatalf("compacted message count = %d, want 4", len(result.Messages))
@@ -96,11 +102,46 @@ func TestCompactMessagesNoopReturnsUncompactedMetadata(t *testing.T) {
if result.SummaryText != "" {
t.Fatalf("SummaryText = %q, want empty", result.SummaryText)
}
if result.ProjectedChars != 0 || result.Truncated {
t.Fatalf("no-op projection metadata = (%d, %t), want zero values", result.ProjectedChars, result.Truncated)
}
if !reflect.DeepEqual(result.Messages, messages) {
t.Fatalf("Messages changed on no-op: %#v", result.Messages)
}
}
func TestCompactMessagesReturnsTruncatedProjectionMetadata(t *testing.T) {
messages := []zeroruntime.Message{{Role: zeroruntime.MessageRoleSystem, Content: "system"}}
for index := range 20 {
messages = append(messages, zeroruntime.Message{
Role: zeroruntime.MessageRoleUser,
Content: strings.Repeat("contextword ", 256) + string(rune('a'+index)),
})
}
messages = append(messages,
zeroruntime.Message{Role: zeroruntime.MessageRoleAssistant, Content: "recent answer"},
zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: "latest question"},
)
var captured []zeroruntime.Message
result, err := CompactMessages(messages, CompactionOptions{
PreserveLast: 2,
Summarize: func(toSummarize []zeroruntime.Message) (string, error) {
captured = append([]zeroruntime.Message(nil), toSummarize...)
return "bounded summary", nil
},
})
if err != nil {
t.Fatal(err)
}
if !result.Compacted || !result.Truncated {
t.Fatalf("compaction metadata = compacted %t, truncated %t; want both true", result.Compacted, result.Truncated)
}
if result.ProjectedChars != compactionMessageChars(captured) || result.ProjectedChars == 0 {
t.Fatalf("ProjectedChars = %d, want %d", result.ProjectedChars, compactionMessageChars(captured))
}
}
func TestCompactMessagesPropagatesSummarizeError(t *testing.T) {
messages := []zeroruntime.Message{
{Role: zeroruntime.MessageRoleSystem, Content: "system"},
+131 -12
View File
@@ -102,7 +102,7 @@ type skillEntry struct {
body string
}
// recentEdits returns the files mutated by write_file/edit_file calls in messages
// recentEdits returns files mutated by editing calls in messages
// — latest note per path, in last-seen order — as skillEntry{name: path, body:
// note}. After compaction elides the editing turns, this tells the model WHAT it
// changed in each file (from the tool's result) so it need not re-read to
@@ -113,6 +113,7 @@ type skillEntry struct {
// rather than an earlier, now-stale entry.
func recentEdits(messages []zeroruntime.Message) []skillEntry {
pathByID := map[string]string{}
noteByPath := map[string]string{}
sequence := make([]string, 0)
for _, message := range messages {
for _, call := range message.ToolCalls {
@@ -129,12 +130,24 @@ func recentEdits(messages []zeroruntime.Message) []skillEntry {
sequence = append(sequence, path)
}
}
for _, message := range messages {
if message.Role != zeroruntime.MessageRoleTool || len(message.ChangedFiles) == 0 {
continue
}
for _, path := range message.ChangedFiles {
path = strings.TrimSpace(path)
if path == "" {
continue
}
sequence = append(sequence, path)
noteByPath[path] = editNote(message.Content)
}
}
order := lastSeenOrder(sequence)
if len(order) == 0 {
return nil
}
noteByPath := map[string]string{}
for _, message := range messages {
if message.Role != zeroruntime.MessageRoleTool || message.ToolCallID == "" {
continue
@@ -425,15 +438,20 @@ type preservedState struct {
}
type preservedTaskState struct {
Objective string `json:"objective"`
Status taskStatus `json:"status,omitempty"`
Pending int `json:"pending,omitempty"`
InProgress int `json:"in_progress,omitempty"`
Completed int `json:"completed,omitempty"`
Failed int `json:"failed,omitempty"`
VerificationPassed int `json:"verification_passed,omitempty"`
VerificationFailed int `json:"verification_failed,omitempty"`
VerificationOutcome Outcome `json:"verification_outcome,omitempty"`
Objective string `json:"objective"`
Status taskStatus `json:"status,omitempty"`
Pending int `json:"pending,omitempty"`
InProgress int `json:"in_progress,omitempty"`
Completed int `json:"completed,omitempty"`
Failed int `json:"failed,omitempty"`
VerificationPassed int `json:"verification_passed,omitempty"`
VerificationFailed int `json:"verification_failed,omitempty"`
VerificationOutcome Outcome `json:"verification_outcome,omitempty"`
Constraints []string `json:"constraints,omitempty"`
ChangedFiles []string `json:"changed_files,omitempty"`
UnresolvedFailures []taskFailureState `json:"unresolved_failures,omitempty"`
Approvals []taskApprovalState `json:"approvals,omitempty"`
Artifacts []taskArtifactState `json:"artifacts,omitempty"`
}
type preservedEdit struct {
@@ -468,7 +486,12 @@ func appendPreservedState(summary string, middle []zeroruntime.Message, taskSnap
task := priorState.Task
if taskSnapshot != nil {
task = &preservedTaskState{
Objective: capTaskObjective(taskSnapshot.Objective),
Objective: capTaskObjective(taskSnapshot.Objective),
Constraints: mergeBoundedComparable(priorTaskConstraints(priorState.Task), taskSnapshot.Constraints, maxTaskConstraints),
ChangedFiles: mergeBoundedComparable(priorTaskChangedFiles(priorState.Task), taskSnapshot.ChangedFiles, maxTaskEvidenceEntries),
UnresolvedFailures: mergeTaskFailures(priorTaskFailures(priorState.Task), taskSnapshot.UnresolvedFailures, taskSnapshot.ResolvedFailureKeys),
Approvals: mergeBoundedComparable(priorTaskApprovals(priorState.Task), taskSnapshot.Approvals, maxTaskEvidenceEntries),
Artifacts: mergeBoundedComparable(priorTaskArtifacts(priorState.Task), taskSnapshot.Artifacts, maxTaskEvidenceEntries),
}
// Plan parity corroborates only the mutable task projection. The objective
// comes directly from the run prompt and is immutable, so it must survive
@@ -484,6 +507,13 @@ func appendPreservedState(summary string, middle []zeroruntime.Message, taskSnap
task.VerificationOutcome = taskSnapshot.Verification.LastOutcome
}
}
freshConstraints := explicitConstraintsFromMessages(middle)
if len(freshConstraints) > 0 {
if task == nil {
task = &preservedTaskState{}
}
task.Constraints = mergeBoundedComparable(task.Constraints, freshConstraints, maxTaskConstraints)
}
// Plan: a fresh update_plan in middle is authoritative; otherwise carry the
// plan preserved by an earlier compaction.
@@ -517,6 +547,95 @@ func appendPreservedState(summary string, middle []zeroruntime.Message, taskSnap
return summary
}
func explicitConstraintsFromMessages(messages []zeroruntime.Message) []string {
var constraints []string
for _, message := range messages {
if message.Role != zeroruntime.MessageRoleUser || strings.Contains(message.Content, preservedStateLabel) {
continue
}
if _, body := projectInstructionBlock(message.Content); body != "" {
continue
}
constraints = mergeBoundedComparable(constraints, extractExplicitConstraints(message.Content), maxTaskConstraints)
}
return constraints
}
func priorTaskConstraints(task *preservedTaskState) []string {
if task == nil {
return nil
}
return task.Constraints
}
func priorTaskChangedFiles(task *preservedTaskState) []string {
if task == nil {
return nil
}
return task.ChangedFiles
}
func priorTaskFailures(task *preservedTaskState) []taskFailureState {
if task == nil {
return nil
}
return task.UnresolvedFailures
}
func priorTaskApprovals(task *preservedTaskState) []taskApprovalState {
if task == nil {
return nil
}
return task.Approvals
}
func priorTaskArtifacts(task *preservedTaskState) []taskArtifactState {
if task == nil {
return nil
}
return task.Artifacts
}
func mergeTaskFailures(older, newer []taskFailureState, resolved []string) []taskFailureState {
resolvedSet := make(map[string]struct{}, len(resolved))
for _, key := range resolved {
resolvedSet[key] = struct{}{}
}
merged := make([]taskFailureState, 0, len(older)+len(newer))
for _, failure := range append(append([]taskFailureState(nil), older...), newer...) {
if _, ok := resolvedSet[failure.Key]; ok {
continue
}
withoutSameKey := make([]taskFailureState, 0, len(merged))
for _, existing := range merged {
if existing.Key != failure.Key {
withoutSameKey = append(withoutSameKey, existing)
}
}
merged = append(withoutSameKey, failure)
}
if len(merged) > maxTaskEvidenceEntries {
merged = merged[len(merged)-maxTaskEvidenceEntries:]
}
return merged
}
func mergeBoundedComparable[T comparable](older, newer []T, limit int) []T {
merged := make([]T, 0, len(older)+len(newer))
seen := make(map[T]struct{}, len(older)+len(newer))
for _, value := range append(append([]T(nil), older...), newer...) {
if _, ok := seen[value]; ok {
continue
}
seen[value] = struct{}{}
merged = append(merged, value)
}
if len(merged) > limit {
merged = merged[len(merged)-limit:]
}
return merged
}
func capTaskObjective(objective string) string {
objective = strings.TrimSpace(objective)
if len(objective) <= maxTaskObjectiveBytes {
@@ -6,6 +6,7 @@ import (
"testing"
"unicode/utf8"
"github.com/Gitlawb/zero/internal/tools"
"github.com/Gitlawb/zero/internal/zeroruntime"
)
@@ -64,6 +65,9 @@ func TestCompactPreservesBoundedTaskContext(t *testing.T) {
objective := strings.Repeat("世", maxTaskObjectiveBytes)
task := newTaskState(objective, nil)
task.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"write code","status":"in_progress"},{"content":"add tests","status":"pending"}]}`})
task.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: `{"patch":"*** Update File: internal/db.go"}`, toolResult: ToolResult{
Name: "apply_patch", Status: tools.StatusOK, Output: "Done!", ChangedFiles: []string{"internal/db.go"},
}})
messages := stateConversation()
compacted, err := Compact(messages, CompactionOptions{
PreserveLast: 2,
@@ -80,6 +84,69 @@ func TestCompactPreservesBoundedTaskContext(t *testing.T) {
if len(state.Task.Objective) > maxTaskObjectiveBytes || !utf8.ValidString(state.Task.Objective) {
t.Fatalf("objective was not safely bounded: %d bytes %q", len(state.Task.Objective), state.Task.Objective)
}
if len(state.Task.ChangedFiles) != 1 || state.Task.ChangedFiles[0] != "internal/db.go" {
t.Fatalf("changed files were not preserved: %#v", state.Task.ChangedFiles)
}
}
func TestCompactPreservesRuntimeEvidenceAcrossRepeatedCompaction(t *testing.T) {
task := newTaskState("Please keep the change focused.", nil)
task.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: `{"cmd":"go test ./..."}`, toolResult: ToolResult{
Name: "exec_command", Status: tools.StatusError, Output: "Error: tests failed",
Meta: map[string]string{"spill_path": ".zero/artifacts/tests.txt"},
}})
task.observe(taskStateEvent{kind: taskStateEventPermission, permission: PermissionEvent{
ToolName: "exec_command", DecisionAction: PermissionDecisionAllowForSession, Scope: "/tmp",
}})
messages := stateConversation()
first, err := Compact(messages, CompactionOptions{
PreserveLast: 2,
Summarize: func([]zeroruntime.Message) (string, error) { return "FIRST", nil },
taskState: task.snapshotForCompaction(messages),
})
if err != nil {
t.Fatal(err)
}
task.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: `{"cmd":"go test ./..."}`, toolResult: ToolResult{
Name: "exec_command", Status: tools.StatusError, Output: "Error: tests still fail with newer evidence",
Meta: map[string]string{"spill_path": ".zero/artifacts/tests.txt"},
}})
secondInput := append(append([]zeroruntime.Message{}, first...),
zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: "Never add background memory models."},
zeroruntime.Message{Role: zeroruntime.MessageRoleAssistant, Content: "understood"},
zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: "continue"},
zeroruntime.Message{Role: zeroruntime.MessageRoleAssistant, Content: "working"},
)
second, err := Compact(secondInput, CompactionOptions{
PreserveLast: 2,
Summarize: func([]zeroruntime.Message) (string, error) { return "SECOND", nil },
taskState: task.snapshotForCompaction(secondInput),
})
if err != nil {
t.Fatal(err)
}
state := parsePreservedStateBlock(second[1].Content)
if state.Task == nil || len(state.Task.UnresolvedFailures) != 1 || len(state.Task.Approvals) != 1 || len(state.Task.Artifacts) != 1 {
t.Fatalf("runtime evidence did not survive repeated compaction: %#v", state.Task)
}
if got := state.Task.UnresolvedFailures[0].Summary; got != "Error: tests still fail with newer evidence" {
t.Fatalf("newer failure evidence did not replace the older summary: %#v", state.Task.UnresolvedFailures)
}
for _, want := range []string{"Please keep the change focused.", "Never add background memory models."} {
if !containsString(state.Task.Constraints, want) {
t.Fatalf("constraint %q missing after repeated compaction: %#v", want, state.Task.Constraints)
}
}
}
func containsString(values []string, want string) bool {
for _, value := range values {
if value == want {
return true
}
}
return false
}
func TestCompactPreservesObjectiveAfterPlanParityMismatch(t *testing.T) {
+315
View File
@@ -0,0 +1,315 @@
package agent
import (
"encoding/json"
"fmt"
"regexp"
"strings"
"github.com/Gitlawb/zero/internal/zeroruntime"
)
const (
compactionUserWordBudget = 256
compactionAssistantWordBudget = 200
compactionToolCallsPerTurn = 8
compactionToolArgumentBytes = 120
compactionErrorBytes = 150
compactionResultBytes = 1200
compactionPreviousSummaryBytes = 16 * 1024
compactionBriefMaxBytes = 24 * 1024
)
var compactionWordPattern = regexp.MustCompile(`[\p{L}\p{N}]+`)
type compactionProjection struct {
messages []zeroruntime.Message
truncated bool
}
// projectCompactionInput removes reconstructible tool output before the model
// summary call. It retains a bounded transcript of user intent, assistant
// decisions, tool actions, and concise errors. Exact durable state is appended
// separately from the original messages after the summary returns.
func projectCompactionInput(messages []zeroruntime.Message) compactionProjection {
sections := make([]string, 0, len(messages))
previousSummary := ""
truncated := false
for index, message := range messages {
switch message.Role {
case zeroruntime.MessageRoleUser:
content := message.Content
if marker := strings.Index(content, preservedStateLabel); marker >= 0 {
content = content[:marker]
}
trimmed := strings.TrimSpace(content)
if strings.HasPrefix(trimmed, summaryLabel) {
fullSummary := strings.TrimSpace(strings.TrimPrefix(trimmed, summaryLabel))
previousSummary = clipHeadTailBytes(fullSummary, compactionPreviousSummaryBytes)
truncated = truncated || len(fullSummary) > compactionPreviousSummaryBytes
continue
}
if content = clipInformativeWords(content, compactionUserWordBudget); content != "" {
sections = append(sections, fmt.Sprintf("[user #%d]\n%s", index, content))
}
case zeroruntime.MessageRoleAssistant:
lines := make([]string, 0, 1+len(message.ToolCalls))
if content := clipInformativeWords(message.Content, compactionAssistantWordBudget); content != "" {
lines = append(lines, content)
}
calls := message.ToolCalls
omitted := 0
if len(calls) > compactionToolCallsPerTurn {
omitted = len(calls) - compactionToolCallsPerTurn
calls = calls[omitted:]
}
if omitted > 0 {
lines = append(lines, fmt.Sprintf("* (%d earlier tool calls omitted)", omitted))
}
for _, call := range calls {
lines = append(lines, compactionToolCallLine(call))
}
if len(lines) > 0 {
sections = append(sections, fmt.Sprintf("[assistant #%d]\n%s", index, strings.Join(lines, "\n")))
}
case zeroruntime.MessageRoleTool:
// New tool-result messages carry the execution status directly, as the
// source ToolResult does. Keep the text check for older session history
// created before Message exposed IsError.
name := toolNameForResult(messages, index)
mustPreserve := name == "ask_user" || len(message.ChangedFiles) > 0
if !mustPreserve && !message.IsError && !isLikelyToolError(message.Content) {
continue
}
if name == "" {
name = "tool"
}
switch {
case message.IsError || isLikelyToolError(message.Content):
sections = append(sections, fmt.Sprintf("[tool_error #%d] %s\n%s", index, name, clipBytes(firstLine(message.Content), compactionErrorBytes)))
case name == "ask_user":
sections = append(sections, fmt.Sprintf("[user_answer #%d] %s\n%s", index, name, clipBytes(message.Content, compactionResultBytes)))
default:
sections = append(sections, fmt.Sprintf("[tool_result #%d] %s changed %s\n%s", index, name,
clipBytes(strings.Join(message.ChangedFiles, ", "), compactionToolArgumentBytes), clipBytes(firstLine(message.Content), compactionErrorBytes)))
}
}
}
if len(sections) == 0 && previousSummary == "" {
return compactionProjection{}
}
brief, briefTruncated := capCompactionBrief(strings.Join(sections, "\n\n"))
truncated = truncated || briefTruncated
if previousSummary != "" {
brief = "[previous summary]\n" + previousSummary + "\n\n" + brief
}
return compactionProjection{
messages: []zeroruntime.Message{{Role: zeroruntime.MessageRoleUser, Content: strings.TrimSpace(brief)}},
truncated: truncated,
}
}
func capCompactionBrief(brief string) (string, bool) {
if len(brief) <= compactionBriefMaxBytes {
return brief, false
}
marker := "\n\n...[middle transcript omitted to fit compaction budget]...\n\n"
available := compactionBriefMaxBytes - len(marker)
// Keep both chronological edges: the oldest material has not necessarily
// been summarized before, while the newest material is most actionable.
headBytes := available * 2 / 5
tailBytes := available - headBytes
head := clipPrefixAtBoundary(brief, headBytes)
tail := clipSuffixAtBoundary(brief, tailBytes)
return strings.TrimSpace(head) + marker + strings.TrimSpace(tail), true
}
func compactionToolCallLine(call zeroruntime.ToolCall) string {
detail := ""
var arguments map[string]any
if json.Unmarshal([]byte(strings.TrimSpace(call.Arguments)), &arguments) == nil {
for _, key := range []string{"file_path", "path", "url", "query", "pattern", "prompt", "description", "question"} {
if value, ok := arguments[key].(string); ok && strings.TrimSpace(value) != "" {
detail = value
break
}
}
if detail == "" && call.Name == "ask_user" {
detail = askUserQuestionsDetail(arguments)
}
if detail == "" && call.Name == "apply_patch" {
if patch, ok := arguments["patch"].(string); ok {
detail = strings.Join(patchPaths(patch), ", ")
}
}
}
if detail == "" {
detail = commandFromArguments(call.Arguments)
}
if detail == "" {
return "* " + call.Name
}
return fmt.Sprintf("* %s %q", call.Name, clipBytes(detail, compactionToolArgumentBytes))
}
func isLikelyToolError(content string) bool {
prefix := strings.TrimSpace(content)
if len(prefix) > compactionErrorBytes {
prefix = prefix[:compactionErrorBytes]
}
lower := strings.ToLower(prefix)
for _, prefix := range []string{"error:", "tool error:", "failed:", "permission denied", "command failed"} {
if strings.HasPrefix(lower, prefix) {
return true
}
}
return false
}
func clipInformativeWords(text string, limit int) string {
flat := strings.Join(strings.Fields(text), " ")
if flat == "" || limit <= 0 {
return ""
}
count := 0
for _, location := range compactionWordPattern.FindAllStringIndex(flat, -1) {
word := strings.ToLower(flat[location[0]:location[1]])
if compactionStopWords[word] {
continue
}
count++
if count > limit {
end := location[0]
return strings.TrimSpace(flat[:end]) + "...(truncated)"
}
}
return flat
}
func clipBytes(text string, limit int) string {
text = strings.TrimSpace(text)
if limit <= 0 {
return ""
}
if len(text) <= limit {
return text
}
if limit <= 3 {
end := limit
for end > 0 && end < len(text) && text[end]&0xc0 == 0x80 {
end--
}
return text[:end]
}
end := limit - 3
for end > 0 && end < len(text) && text[end]&0xc0 == 0x80 {
end--
}
return strings.TrimSpace(text[:end]) + "..."
}
func clipHeadTailBytes(text string, limit int) string {
text = strings.TrimSpace(text)
if len(text) <= limit {
return text
}
if limit <= 0 {
return ""
}
marker := "\n...[middle omitted]...\n"
if limit <= len(marker) {
return clipBytes(text, limit)
}
available := limit - len(marker)
head := clipPrefixAtBoundary(text, available/2)
tail := clipSuffixAtBoundary(text, available-available/2)
return strings.TrimSpace(head) + marker + strings.TrimSpace(tail)
}
func clipPrefixAtBoundary(text string, limit int) string {
if limit <= 0 {
return ""
}
if len(text) <= limit {
return text
}
end := limit
for end > 0 && text[end]&0xc0 == 0x80 {
end--
}
if boundary := strings.LastIndex(text[:end], "\n\n["); boundary > 0 {
end = boundary
}
return text[:end]
}
func clipSuffixAtBoundary(text string, limit int) string {
if limit <= 0 {
return ""
}
if len(text) <= limit {
return text
}
start := len(text) - limit
for start < len(text) && text[start]&0xc0 == 0x80 {
start++
}
if boundary := strings.Index(text[start:], "\n\n["); boundary >= 0 {
start += boundary + 2
}
return text[start:]
}
func askUserQuestionsDetail(arguments map[string]any) string {
raw, ok := arguments["questions"].([]any)
if !ok {
return ""
}
questions := make([]string, 0, len(raw))
for _, item := range raw {
object, ok := item.(map[string]any)
if !ok {
continue
}
if question, ok := object["question"].(string); ok && strings.TrimSpace(question) != "" {
questions = append(questions, strings.TrimSpace(question))
}
}
return strings.Join(questions, " | ")
}
func patchPaths(patch string) []string {
var paths []string
seen := map[string]bool{}
for _, line := range strings.Split(patch, "\n") {
for _, prefix := range []string{"*** Add File: ", "*** Update File: ", "*** Delete File: ", "*** Move to: "} {
if !strings.HasPrefix(line, prefix) {
continue
}
path := strings.TrimSpace(strings.TrimPrefix(line, prefix))
if path != "" && !seen[path] {
seen[path] = true
paths = append(paths, path)
}
}
}
return paths
}
var compactionStopWords = map[string]bool{
"a": true, "an": true, "the": true, "is": true, "are": true, "was": true, "were": true,
"be": true, "been": true, "being": true, "have": true, "has": true, "had": true,
"do": true, "does": true, "did": true, "will": true, "would": true, "could": true,
"should": true, "may": true, "might": true, "shall": true, "can": true, "need": true, "must": true,
"to": true, "of": true, "in": true, "for": true, "on": true, "with": true, "at": true,
"by": true, "from": true, "as": true, "into": true, "through": true, "during": true,
"before": true, "after": true, "above": true, "below": true, "between": true, "under": true, "over": true,
"and": true, "but": true, "or": true, "nor": true, "not": true, "so": true, "yet": true,
"both": true, "either": true, "neither": true, "each": true, "every": true, "all": true,
"any": true, "few": true, "more": true, "most": true, "other": true, "some": true, "such": true, "no": true,
"that": true, "this": true, "these": true, "those": true, "it": true, "its": true,
"i": true, "me": true, "my": true, "we": true, "our": true, "you": true, "your": true,
"he": true, "him": true, "his": true, "she": true, "her": true, "they": true, "them": true, "their": true,
"who": true, "which": true, "what": true, "if": true, "then": true, "than": true,
"when": true, "where": true, "how": true, "just": true, "also": true,
}
@@ -0,0 +1,130 @@
package agent
import (
"strings"
"testing"
"unicode/utf8"
"github.com/Gitlawb/zero/internal/zeroruntime"
)
func TestClipBytesHandlesTinyLimitsOnRuneBoundaries(t *testing.T) {
for _, limit := range []int{0, 1, 2, 3} {
if clipped := clipBytes("世界", limit); len(clipped) > limit || !utf8.ValidString(clipped) {
t.Fatalf("clipBytes limit %d returned invalid result %q", limit, clipped)
}
}
}
func TestBoundaryClippersHandleNonPositiveLimits(t *testing.T) {
for _, clip := range []func(string, int) string{clipPrefixAtBoundary, clipSuffixAtBoundary} {
for _, limit := range []int{-1, 0} {
if got := clip("世界", limit); got != "" {
t.Fatalf("clipper limit %d = %q, want empty", limit, got)
}
}
}
}
func TestProjectCompactionInputDropsRecoverableToolBodies(t *testing.T) {
largeBody := strings.Repeat("recoverable source line\n", 1000)
messages := []zeroruntime.Message{
{Role: zeroruntime.MessageRoleUser, Content: "Inspect the parser and preserve its current behavior."},
{Role: zeroruntime.MessageRoleAssistant, Content: "I will inspect before editing.", ToolCalls: []zeroruntime.ToolCall{{ID: "read", Name: "read_file", Arguments: `{"path":"internal/parser.go"}`}}},
{Role: zeroruntime.MessageRoleTool, ToolCallID: "read", Content: largeBody},
{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{{ID: "test", Name: "exec_command", Arguments: `{"cmd":"go test ./internal/parser"}`}}},
{Role: zeroruntime.MessageRoleTool, ToolCallID: "test", Content: "Error: parser regression\nlong diagnostic details"},
}
projection := projectCompactionInput(messages)
projected := projection.messages
if len(projected) != 1 {
t.Fatalf("projection = %#v, want one compact message", projected)
}
content := projected[0].Content
for _, want := range []string{"Inspect the parser", `read_file "internal/parser.go"`, `exec_command "go test ./internal/parser"`, "Error: parser regression"} {
if !strings.Contains(content, want) {
t.Fatalf("projection missing %q: %q", want, content)
}
}
if strings.Contains(content, "recoverable source line") || strings.Contains(content, "long diagnostic details") {
t.Fatalf("projection retained reconstructible or overlong tool output: %q", content)
}
if estimateTokens(projected)*20 >= estimateTokens(messages) {
t.Fatalf("projection did not materially reduce tokens: before=%d after=%d", estimateTokens(messages), estimateTokens(projected))
}
}
func TestProjectCompactionInputRetainsStructurallyMarkedToolErrors(t *testing.T) {
messages := []zeroruntime.Message{
{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{{ID: "git", Name: "exec_command", Arguments: `{"cmd":"git status"}`}}},
{Role: zeroruntime.MessageRoleTool, ToolCallID: "git", Content: "fatal: not a git repository", IsError: true},
}
projected := projectCompactionInput(messages).messages
if len(projected) != 1 || !strings.Contains(projected[0].Content, "fatal: not a git repository") {
t.Fatalf("structured tool error was dropped from projection: %#v", projected)
}
}
func TestProjectCompactionInputRetainsAskUserExchangeAndSemanticArguments(t *testing.T) {
messages := []zeroruntime.Message{
{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{{ID: "ask", Name: "ask_user", Arguments: `{"questions":[{"question":"Which database should remain?"}]}`}}},
{Role: zeroruntime.MessageRoleTool, ToolCallID: "ask", Content: "Which database should remain?: Postgres only; do not touch MySQL."},
{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{
{ID: "patch", Name: "apply_patch", Arguments: `{"patch":"*** Begin Patch\n*** Update File: internal/db.go\n*** End Patch"}`},
{ID: "fetch", Name: "web_fetch", Arguments: `{"url":"https://example.com/spec"}`},
{ID: "task", Name: "task", Arguments: `{"prompt":"Inspect the Windows implementation"}`},
}},
{Role: zeroruntime.MessageRoleTool, ToolCallID: "patch", Content: "Done!", ChangedFiles: []string{"internal/db.go"}},
}
content := projectCompactionInput(messages).messages[0].Content
for _, want := range []string{"Which database should remain?", "Postgres only; do not touch MySQL", "internal/db.go", "https://example.com/spec", "Inspect the Windows implementation"} {
if !strings.Contains(content, want) {
t.Fatalf("projection missing %q: %q", want, content)
}
}
}
func TestProjectCompactionInputCapsToolCallsPerTurn(t *testing.T) {
calls := make([]zeroruntime.ToolCall, 12)
for index := range calls {
calls[index] = zeroruntime.ToolCall{Name: "read_file", Arguments: `{"path":"file.go"}`}
}
projected := projectCompactionInput([]zeroruntime.Message{{Role: zeroruntime.MessageRoleAssistant, ToolCalls: calls}}).messages
content := projected[0].Content
if strings.Count(content, "* read_file") != compactionToolCallsPerTurn || !strings.Contains(content, "4 earlier tool calls omitted") {
t.Fatalf("tool-call tail was not bounded: %q", content)
}
}
func TestProjectCompactionInputCarriesPreviousSummaryOutsideBriefCap(t *testing.T) {
previous := "keep the earlier architecture decision\n" + strings.Repeat("prior detail ", 2000) + "\nkeep the newest prior decision"
messages := []zeroruntime.Message{{Role: zeroruntime.MessageRoleUser, Content: summaryLabel + "\n" + previous + "\n\n" + preservedStateLabel + "\n{}"}}
for index := range 500 {
messages = append(messages, zeroruntime.Message{Role: zeroruntime.MessageRoleAssistant, Content: strings.Repeat("later transcript detail ", 20)})
messages = append(messages, zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: "later request " + strings.Repeat("context ", 20) + string(rune('a'+index%26))})
}
projection := projectCompactionInput(messages)
content := projection.messages[0].Content
if !strings.Contains(content, "[previous summary]\nkeep the earlier architecture decision") || !strings.Contains(content, "keep the newest prior decision") {
t.Fatalf("previous summary was lost when the brief tail was capped: %q", content)
}
if !strings.Contains(content, "middle transcript omitted to fit compaction budget") {
t.Fatalf("oversized brief was not head/tail capped: %q", content)
}
if !projection.truncated {
t.Fatal("projection did not structurally report truncation")
}
}
func TestProjectCompactionInputDoesNotInferTruncationFromUserText(t *testing.T) {
projection := projectCompactionInput([]zeroruntime.Message{{
Role: zeroruntime.MessageRoleUser, Content: "Keep this literal marker: [middle omitted]",
}})
if projection.truncated {
t.Fatal("literal user text produced a false truncation signal")
}
}
@@ -20,11 +20,15 @@ func TestRecentEditsExtractsPathsAndNotes(t *testing.T) {
{ID: "e2", Name: "edit_file", Arguments: `{"path":"internal/bar.go","old_string":"a","new_string":"b"}`},
}},
{Role: zeroruntime.MessageRoleTool, ToolCallID: "e2", Content: "Applied edit to internal/bar.go"},
{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{
{ID: "e3", Name: "apply_patch", Arguments: `{"patch":"*** Update File: internal/baz.go"}`},
}},
{Role: zeroruntime.MessageRoleTool, ToolCallID: "e3", Content: "Done!", ChangedFiles: []string{"internal/baz.go"}},
}
edits := recentEdits(messages)
if len(edits) != 2 {
t.Fatalf("expected 2 edited files, got %d: %#v", len(edits), edits)
if len(edits) != 3 {
t.Fatalf("expected 3 edited files, got %d: %#v", len(edits), edits)
}
if edits[0].name != "internal/foo.go" || !strings.Contains(edits[0].body, "12 lines") {
t.Fatalf("first edit = %#v, want foo.go with its note", edits[0])
@@ -32,6 +36,9 @@ func TestRecentEditsExtractsPathsAndNotes(t *testing.T) {
if edits[1].name != "internal/bar.go" || !strings.Contains(edits[1].body, "Applied edit") {
t.Fatalf("second edit = %#v, want bar.go with its note", edits[1])
}
if edits[2].name != "internal/baz.go" || edits[2].body != "Done!" {
t.Fatalf("third edit = %#v, want apply_patch changed file with its note", edits[2])
}
}
// After compaction elides the editing turns, the preserved-state block still
+8 -5
View File
@@ -58,12 +58,15 @@ func TestCompactKeepsSystemAndPreservedSuffix(t *testing.T) {
if last.Content != "most recent question" {
t.Fatalf("expected most recent message preserved, got %q", last.Content)
}
// The summarized middle excludes system and preserved suffix.
if len(captured) != 3 {
t.Fatalf("expected 3 summarized messages, got %d: %#v", len(captured), captured)
// The summarizer receives a compact semantic projection of the middle rather
// than reconstructible raw tool output.
if len(captured) != 1 || captured[0].Role != zeroruntime.MessageRoleUser {
t.Fatalf("expected one projected summary input, got %#v", captured)
}
if captured[0].Content != "first question" {
t.Fatalf("expected oldest non-system message first, got %#v", captured[0])
for _, want := range []string{"[user #0]", "first question", "[assistant #1]", "first answer", "second question"} {
if !strings.Contains(captured[0].Content, want) {
t.Fatalf("projected summary input missing %q: %q", want, captured[0].Content)
}
}
// Compaction must shrink the conversation.
if estimateTokens(result) >= estimateTokens(messages) {
+117 -14
View File
@@ -187,6 +187,13 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
guards := newGuardState()
task := newTaskState(prompt, options.Trace)
onPermission := options.OnPermission
options.OnPermission = func(event PermissionEvent) {
task.observe(taskStateEvent{kind: taskStateEventPermission, permission: event})
if onPermission != nil {
onPermission(event)
}
}
compactor := newCompactionState(options, task)
defer func() {
// A final transcript comparison is observational. It records drift but
@@ -656,6 +663,13 @@ 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
@@ -691,7 +705,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
}
options.Trace.Counter(trace.CounterToolCalls, 1)
recordOutputBudgetTrace(options.Trace, toolResult)
task.observe(taskStateEvent{kind: taskStateEventToolResult, toolResult: toolResult})
task.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: call.Arguments, toolResult: toolResult})
if options.OnToolResult != nil {
options.OnToolResult(toolResult)
}
@@ -705,10 +719,22 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
turnRequestedModel = toolResult.RequestedModel
}
messages = append(messages, zeroruntime.Message{
Role: zeroruntime.MessageRoleTool,
Content: toolResult.Output,
ToolCallID: toolResult.ToolCallID,
Role: zeroruntime.MessageRoleTool,
Content: toolResult.Output,
ToolCallID: toolResult.ToolCallID,
IsError: toolResult.Status == tools.StatusError,
ChangedFiles: append([]string(nil), toolResult.ChangedFiles...),
})
// 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
@@ -719,11 +745,13 @@ 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)
@@ -747,6 +775,7 @@ 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
@@ -765,6 +794,10 @@ 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
@@ -1436,6 +1469,7 @@ 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,
@@ -2763,7 +2797,16 @@ 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 || raw == nil {
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")
return
}
data, err := json.Marshal(raw)
@@ -2794,21 +2837,41 @@ 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) {
if _, exists := args["additional_permissions"]; exists {
return sandbox.RequestPermissionProfile{}, false, fmt.Errorf("additional_permissions requires sandbox_permissions set to %q", tools.SandboxPermissionsWithAdditionalPermissions)
// 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)
}
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. "+
`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)
"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)
}
data, err := json.Marshal(raw)
if err != nil {
@@ -2824,8 +2887,10 @@ func inlineAdditionalPermissionsProfile(args map[string]any, basePath string) (s
}
if normalized.Empty() {
return sandbox.RequestPermissionProfile{}, true, fmt.Errorf(
"additional_permissions must include at least one of network or file_system, for example " +
`{"network": {"enabled": true}} or {"file_system": {"write": ["/path"]}}`)
"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)
}
grantProfile, err := sandbox.RequestPermissionGrantProfile(normalized)
if err != nil {
@@ -3222,6 +3287,7 @@ func appendAbortedToolResults(messages []Message, remaining []ToolCall) []Messag
Role: zeroruntime.MessageRoleTool,
Content: abortedToolResultNotice,
ToolCallID: call.ID,
IsError: true,
})
}
return messages
@@ -3252,9 +3318,46 @@ func copyMessages(messages []Message) []Message {
if message.Reasoning != nil {
copied[index].Reasoning = append([]zeroruntime.ReasoningBlock{}, message.Reasoning...)
}
if message.ChangedFiles != nil {
copied[index].ChangedFiles = append([]string(nil), message.ChangedFiles...)
}
// Deep-copy image attachments (slice AND each Data byte slice) so the
// raw image bytes are never aliased across history/request/result copies.
copied[index].Images = zeroruntime.CloneImageBlocks(message.Images)
}
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
}
+32
View File
@@ -3592,6 +3592,11 @@ func TestRunAppendsAbortedPlaceholderForUnexecutedToolCallsOnGuardStop(t *testin
if !strings.Contains(strings.ToLower(placeholder), "aborted") {
t.Fatalf("expected the placeholder result to mark the call as aborted, got %q", placeholder)
}
for _, message := range result.Messages {
if message.ToolCallID == "flaky-2" && !message.IsError {
t.Fatalf("aborted placeholder must carry error status: %#v", message)
}
}
// Every tool_use in the final assistant message must have a matching result.
for _, message := range result.Messages {
@@ -3606,6 +3611,33 @@ func TestRunAppendsAbortedPlaceholderForUnexecutedToolCallsOnGuardStop(t *testin
}
}
func TestRunCarriesToolErrorStatusIntoMessageHistory(t *testing.T) {
registry := tools.NewRegistry()
registry.Register(alwaysFailingTool{})
provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
{
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "failed-call", ToolName: "flaky"},
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "failed-call"},
{Type: zeroruntime.StreamEventDone},
},
{{Type: zeroruntime.StreamEventText, Content: "done"}, {Type: zeroruntime.StreamEventDone}},
}}
result, err := Run(context.Background(), "go", provider, Options{Registry: registry})
if err != nil {
t.Fatal(err)
}
for _, message := range result.Messages {
if message.ToolCallID == "failed-call" {
if !message.IsError {
t.Fatalf("failed tool result lost its structured status: %#v", message)
}
return
}
}
t.Fatalf("failed tool result missing from message history: %#v", result.Messages)
}
type secretEmittingTool struct{ output string }
func (t secretEmittingTool) Name() string { return "leak" }
+217 -10
View File
@@ -1,10 +1,14 @@
package agent
import (
"crypto/sha256"
"encoding/json"
"fmt"
"reflect"
"regexp"
"sort"
"strings"
"unicode/utf8"
"github.com/Gitlawb/zero/internal/tools"
"github.com/Gitlawb/zero/internal/trace"
@@ -52,17 +56,41 @@ type taskVerificationState struct {
LastOutcome Outcome `json:"last_outcome,omitempty"`
}
type taskFailureState struct {
Key string `json:"key,omitempty"`
Tool string `json:"tool"`
Command string `json:"command,omitempty"`
Summary string `json:"summary"`
}
type taskApprovalState struct {
Tool string `json:"tool"`
Decision PermissionDecisionAction `json:"decision,omitempty"`
Scope string `json:"scope,omitempty"`
Reason string `json:"reason,omitempty"`
}
type taskArtifactState struct {
Tool string `json:"tool"`
Path string `json:"path"`
}
type taskStateSnapshot struct {
Revision int `json:"revision"`
Objective string `json:"objective"`
Status taskStatus `json:"status"`
Plan taskPlanState `json:"plan"`
Tools taskToolState `json:"tools"`
Verification taskVerificationState `json:"verification"`
ChangedFiles []string `json:"changed_files,omitempty"`
CompletionDecision CompletionDecision `json:"completion_decision,omitempty"`
CompletionReason string `json:"completion_reason,omitempty"`
PlanParity taskPlanParity `json:"plan_parity"`
Revision int `json:"revision"`
Objective string `json:"objective"`
Status taskStatus `json:"status"`
Plan taskPlanState `json:"plan"`
Tools taskToolState `json:"tools"`
Verification taskVerificationState `json:"verification"`
ChangedFiles []string `json:"changed_files,omitempty"`
Constraints []string `json:"constraints,omitempty"`
UnresolvedFailures []taskFailureState `json:"unresolved_failures,omitempty"`
Approvals []taskApprovalState `json:"approvals,omitempty"`
Artifacts []taskArtifactState `json:"artifacts,omitempty"`
ResolvedFailureKeys []string `json:"-"`
CompletionDecision CompletionDecision `json:"completion_decision,omitempty"`
CompletionReason string `json:"completion_reason,omitempty"`
PlanParity taskPlanParity `json:"plan_parity"`
}
type taskStateEventKind int
@@ -72,6 +100,7 @@ const (
taskStateEventToolResult
taskStateEventVerification
taskStateEventCompletion
taskStateEventPermission
)
type taskStateEvent struct {
@@ -80,6 +109,7 @@ type taskStateEvent struct {
toolResult ToolResult
verification Outcome
completion completionEvaluation
permission PermissionEvent
}
// taskState is a deterministic projection of facts the loop already observes.
@@ -101,6 +131,7 @@ func newTaskState(objective string, recorder *trace.Recorder) *taskState {
},
changedFiles: map[string]struct{}{},
}
state.snapshotValue.Constraints = extractExplicitConstraints(objective)
state.recorder = recorder
return state
}
@@ -134,6 +165,7 @@ func (state *taskState) observe(event taskStateEvent) bool {
}
}
state.snapshotValue.ChangedFiles = sortedKeys(state.changedFiles)
state.observeToolEvidence(event.arguments, event.toolResult)
changed = true
case taskStateEventVerification:
if event.verification == OutcomeDisabled {
@@ -160,6 +192,10 @@ func (state *taskState) observe(event taskStateEvent) bool {
state.snapshotValue.Status = taskStatusActive
}
changed = true
case taskStateEventPermission:
state.markActive()
state.observePermission(event.permission)
changed = true
}
if changed {
state.snapshotValue.Revision++
@@ -183,9 +219,180 @@ func (state *taskState) snapshot() taskStateSnapshot {
snapshot := state.snapshotValue
snapshot.Plan.Items = append([]taskPlanItem(nil), state.snapshotValue.Plan.Items...)
snapshot.ChangedFiles = append([]string(nil), state.snapshotValue.ChangedFiles...)
snapshot.Constraints = append([]string(nil), state.snapshotValue.Constraints...)
snapshot.UnresolvedFailures = append([]taskFailureState(nil), state.snapshotValue.UnresolvedFailures...)
snapshot.Approvals = append([]taskApprovalState(nil), state.snapshotValue.Approvals...)
snapshot.Artifacts = append([]taskArtifactState(nil), state.snapshotValue.Artifacts...)
snapshot.ResolvedFailureKeys = append([]string(nil), state.snapshotValue.ResolvedFailureKeys...)
return snapshot
}
const (
maxTaskEvidenceEntries = 8
maxTaskEvidenceBytes = 320
maxTaskConstraints = 10
)
func (state *taskState) observeToolEvidence(arguments string, result ToolResult) {
command := capTaskEvidence(commandFromArguments(arguments))
key := taskFailureKey(result.Name, command, arguments)
if result.Status == tools.StatusOK {
if hasTaskFailure(state.snapshotValue.UnresolvedFailures, key) {
state.snapshotValue.UnresolvedFailures = removeTaskFailure(state.snapshotValue.UnresolvedFailures, key)
state.snapshotValue.ResolvedFailureKeys = appendBoundedUnique(state.snapshotValue.ResolvedFailureKeys, key, maxTaskEvidenceEntries)
}
} else {
state.snapshotValue.ResolvedFailureKeys = removeString(state.snapshotValue.ResolvedFailureKeys, key)
state.snapshotValue.UnresolvedFailures = removeTaskFailure(state.snapshotValue.UnresolvedFailures, key)
state.snapshotValue.UnresolvedFailures = appendBounded(state.snapshotValue.UnresolvedFailures, taskFailureState{
Key: key, Tool: result.Name, Command: command, Summary: capTaskEvidence(firstLine(result.Output)),
}, maxTaskEvidenceEntries)
}
for _, metaKey := range []string{"spill_path", "artifact_path"} {
if path := strings.TrimSpace(result.Meta[metaKey]); path != "" {
state.snapshotValue.Artifacts = appendBoundedUnique(state.snapshotValue.Artifacts,
taskArtifactState{Tool: result.Name, Path: capTaskEvidence(path)}, maxTaskEvidenceEntries)
}
}
}
func taskFailureKey(toolName, command, arguments string) string {
identity := command
if identity == "" {
canonical := []byte(strings.TrimSpace(arguments))
var value any
if json.Unmarshal(canonical, &value) == nil {
if encoded, err := json.Marshal(value); err == nil {
canonical = encoded
}
}
digest := sha256.Sum256(canonical)
identity = fmt.Sprintf("args:%x", digest[:8])
}
return strings.TrimSpace(toolName) + "\x00" + identity
}
func (state *taskState) observePermission(event PermissionEvent) {
decision := event.DecisionAction
if decision == "" {
decision = PermissionDecisionAction(event.Action)
}
state.snapshotValue.Approvals = appendBoundedUnique(state.snapshotValue.Approvals, taskApprovalState{
Tool: event.ToolName, Decision: decision, Scope: capTaskEvidence(event.Scope),
Reason: capTaskEvidence(firstNonEmptyString(event.DecisionReason, event.Reason)),
}, maxTaskEvidenceEntries)
}
func commandFromArguments(arguments string) string {
var object map[string]json.RawMessage
if json.Unmarshal([]byte(strings.TrimSpace(arguments)), &object) != nil {
return ""
}
for _, key := range []string{"cmd", "command", "script", "shell"} {
var command string
if raw, ok := object[key]; ok && json.Unmarshal(raw, &command) == nil {
return strings.TrimSpace(command)
}
}
return ""
}
func firstLine(value string) string {
value = strings.TrimSpace(value)
if index := strings.IndexByte(value, '\n'); index >= 0 {
value = value[:index]
}
return value
}
func capTaskEvidence(value string) string {
value = strings.TrimSpace(value)
if len(value) <= maxTaskEvidenceBytes {
return value
}
limit := maxTaskEvidenceBytes - 3
for limit > 0 && !utf8.RuneStart(value[limit]) {
limit--
}
return strings.TrimSpace(value[:limit]) + "..."
}
var explicitConstraintPatterns = []*regexp.Regexp{
regexp.MustCompile(`(?i)\bprefer(?:s|red|ring)?\s+\w`),
regexp.MustCompile(`(?i)\bdon'?t want\b`),
regexp.MustCompile(`(?i)\balways (?:use|do|run|prefer|keep|make|format|write|add|set|put|prefix|start|include|append)\b`),
regexp.MustCompile(`(?i)\bnever (?:use|do|run|push|commit|write|ignore|add|set|put|remove|delete|include|deploy)\b`),
regexp.MustCompile(`(?i)\bplease (?:use|avoid|keep|make|don'?t|do not|format|write)\b`),
regexp.MustCompile(`(?i)\b(?:style|format|language|naming)\s*[:=]\s*\S`),
}
func extractExplicitConstraints(text string) []string {
var constraints []string
for _, line := range strings.Split(text, "\n") {
line = strings.TrimSpace(line)
if len(line) < 5 || len(line) > 200 || strings.HasSuffix(line, "?") || strings.Contains(line, "?...") {
continue
}
for _, pattern := range explicitConstraintPatterns {
if pattern.MatchString(line) {
constraints = appendBoundedUnique(constraints, line, maxTaskConstraints)
break
}
}
if len(constraints) == maxTaskConstraints {
break
}
}
return constraints
}
func appendBounded[T any](values []T, value T, limit int) []T {
values = append(values, value)
if len(values) > limit {
values = values[len(values)-limit:]
}
return values
}
func appendBoundedUnique[T comparable](values []T, value T, limit int) []T {
filtered := values[:0]
for _, existing := range values {
if existing != value {
filtered = append(filtered, existing)
}
}
return appendBounded(filtered, value, limit)
}
func hasTaskFailure(values []taskFailureState, key string) bool {
for _, value := range values {
if value.Key == key {
return true
}
}
return false
}
func removeTaskFailure(values []taskFailureState, key string) []taskFailureState {
out := values[:0]
for _, value := range values {
if value.Key != key {
out = append(out, value)
}
}
return out
}
func removeString(values []string, target string) []string {
out := make([]string, 0, len(values))
for _, value := range values {
if value != target {
out = append(out, value)
}
}
return out
}
// observePlanParity compares only the latest plan projection with the plan tool
// calls still present in messages. It mutates the snapshot and emits when the
// parity value changes; objective, tool, and verification fields are not part of
+62
View File
@@ -115,6 +115,68 @@ func TestTaskStateSnapshotIsImmutable(t *testing.T) {
}
}
func TestTaskStateRecordsDurableRuntimeEvidence(t *testing.T) {
state := newTaskState("Please keep the change focused.\nNever commit generated reports.", nil)
arguments := `{"cmd":"go test ./internal/agent"}`
state.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: arguments, toolResult: ToolResult{
ToolCallID: "test-1", Name: "exec_command", Status: tools.StatusError,
Output: "Error: TestResumeRetainsState failed\nfull diagnostic",
Meta: map[string]string{"spill_path": ".zero/artifacts/test-1.txt"},
}})
state.observe(taskStateEvent{kind: taskStateEventPermission, permission: PermissionEvent{
ToolName: "exec_command", DecisionAction: PermissionDecisionAllowForSession,
Scope: "/tmp", DecisionReason: "write a temporary fixture",
}})
snapshot := state.snapshot()
if !reflect.DeepEqual(snapshot.Constraints, []string{"Please keep the change focused.", "Never commit generated reports."}) {
t.Fatalf("unexpected explicit constraints: %#v", snapshot.Constraints)
}
if len(snapshot.UnresolvedFailures) != 1 || snapshot.UnresolvedFailures[0].Command != "go test ./internal/agent" || snapshot.UnresolvedFailures[0].Summary != "Error: TestResumeRetainsState failed" {
t.Fatalf("unexpected failure evidence: %#v", snapshot.UnresolvedFailures)
}
if len(snapshot.Artifacts) != 1 || snapshot.Artifacts[0].Path != ".zero/artifacts/test-1.txt" {
t.Fatalf("unexpected artifact evidence: %#v", snapshot.Artifacts)
}
if len(snapshot.Approvals) != 1 || snapshot.Approvals[0].Decision != PermissionDecisionAllowForSession || snapshot.Approvals[0].Scope != "/tmp" {
t.Fatalf("unexpected approval evidence: %#v", snapshot.Approvals)
}
state.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: `{"path":"a.go"}`, toolResult: ToolResult{
ToolCallID: "read-a", Name: "read_file", Status: tools.StatusError, Output: "Error: a.go is unavailable",
}})
state.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: `{"path":"b.go"}`, toolResult: ToolResult{
ToolCallID: "read-b", Name: "read_file", Status: tools.StatusOK, Output: "package b",
}})
if failures := state.snapshot().UnresolvedFailures; len(failures) != 2 || failures[1].Summary != "Error: a.go is unavailable" {
t.Fatalf("success for a different target resolved the wrong failure: %#v", failures)
}
state.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: arguments, toolResult: ToolResult{
ToolCallID: "test-2", Name: "exec_command", Status: tools.StatusOK, Output: "ok",
}})
if failures := state.snapshot().UnresolvedFailures; len(failures) != 1 || failures[0].Summary != "Error: a.go is unavailable" {
t.Fatalf("successful retry did not resolve only its matching failure: %#v", failures)
}
state.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: `{"path":"a.go"}`, toolResult: ToolResult{
ToolCallID: "read-a-2", Name: "read_file", Status: tools.StatusOK, Output: "package a",
}})
if failures := state.snapshot().UnresolvedFailures; len(failures) != 0 {
t.Fatalf("successful non-command retry did not resolve its matching failure: %#v", failures)
}
state.observe(taskStateEvent{kind: taskStateEventToolResult, arguments: `{"path":"a.go"}`, toolResult: ToolResult{
ToolCallID: "read-a-3", Name: "read_file", Status: tools.StatusError, Output: "Error: a.go failed again",
}})
snapshot = state.snapshot()
if len(snapshot.UnresolvedFailures) != 1 || snapshot.UnresolvedFailures[0].Summary != "Error: a.go failed again" {
t.Fatalf("a failure recurring after success was hidden: %#v", snapshot.UnresolvedFailures)
}
for _, key := range snapshot.ResolvedFailureKeys {
if key == snapshot.UnresolvedFailures[0].Key {
t.Fatalf("recurring failure retained a stale resolved key: %#v", snapshot.ResolvedFailureKeys)
}
}
}
func TestTaskStatePlanParityUsesLatestPlan(t *testing.T) {
state := newTaskState("objective", nil)
state.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"old","status":"completed"}]}`})
+207
View File
@@ -0,0 +1,207 @@
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, " ") + "]"
}
+5 -2
View File
@@ -68,8 +68,11 @@ 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
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
Redacted bool
ChangedFiles []string
// ChangeSummaries are non-selectable generated-tree summaries emitted by
+28
View File
@@ -308,6 +308,34 @@ func runMCPCheck(ctx context.Context, args []string, stdout io.Writer, stderr io
}
defer closeMCPRuntime(stderr, mcpRuntime)
// A connect failure does not come back as err. RegisterTools is deliberately
// best-effort so one unreachable server cannot stop Zero launching: it records
// the failure on the runtime and returns nil. That is right for startup and
// wrong to ignore here, because this is the command a user runs precisely when
// a server is misbehaving, and reporting "is reachable" for a server that never
// started sends them looking somewhere else.
for _, skipped := range mcpRuntime.Skipped() {
if skipped.Name != serverName {
continue
}
message := fmt.Sprintf("MCP server %s is not reachable", serverName)
if skipped.Err != nil {
message = fmt.Sprintf("%s: %s", message, redaction.ErrorMessage(skipped.Err, redaction.Options{}))
}
if options.json {
payload := struct {
ServerName string `json:"serverName"`
Status string `json:"status"`
Error string `json:"error,omitempty"`
}{ServerName: serverName, Status: "unreachable", Error: message}
if err := writePrettyJSON(stdout, redaction.RedactValue(payload, redaction.Options{})); err != nil {
return exitCrash
}
return exitCrash
}
return writeAppError(stderr, message, exitCrash)
}
items := mcpToolList(registry)
if options.json {
payload := struct {
+79
View File
@@ -6,6 +6,7 @@ package cli
import (
"bytes"
"encoding/json"
"errors"
"os"
"path/filepath"
@@ -219,3 +220,81 @@ func TestRunMCPCheckNormalizeError(t *testing.T) {
t.Fatalf("want normalize error, got %q", got)
}
}
// TestRunMCPCheckReportsUnreachableServer covers the case the command exists
// for: a server that fails to start.
//
// RegisterTools is best-effort by design, so a connect failure is recorded on
// the runtime and registration returns nil. runMCPCheck used to inspect only
// that nil error and then print "is reachable", which told the user their
// broken server was fine. The command is spawned for real here rather than
// stubbed, because the whole failure mode was a mismatch between what
// registration returns and what it records.
func TestRunMCPCheckReportsUnreachableServer(t *testing.T) {
setTrustConfigRoot(t)
deps := appDeps{
getwd: func() (string, error) { return t.TempDir(), nil },
resolveMCPConfig: func(_ string, _ bool) (config.MCPConfig, error) {
return config.MCPConfig{Servers: map[string]config.MCPServerConfig{
"broken": {Type: "stdio", Command: "zero-definitely-not-a-real-binary-xyz"},
}}, nil
},
}
var out, errBuf bytes.Buffer
if code := runWithDeps([]string{"mcp", "check", "broken"}, &out, &errBuf, deps); code == exitSuccess {
t.Fatalf("mcp check must fail for a server that did not start; stdout=%q stderr=%q", out.String(), errBuf.String())
}
if got := out.String(); strings.Contains(got, "is reachable") {
t.Fatalf("mcp check reported a failed server as reachable: %q", got)
}
if got := errBuf.String(); !strings.Contains(got, "not reachable") {
t.Fatalf("want an unreachable error naming the server, got %q", got)
}
}
// The --json contract for an unreachable server, which the text case above does
// not touch.
//
// The JSON branch is a separate return path with its own payload, so a caller
// parsing it would otherwise be relying on a shape nothing pins: reverting the
// check leaves it emitting status "ok" with a tool count, which is valid JSON
// and completely wrong.
func TestRunMCPCheckReportsUnreachableServerJSON(t *testing.T) {
setTrustConfigRoot(t)
deps := appDeps{
getwd: func() (string, error) { return t.TempDir(), nil },
resolveMCPConfig: func(_ string, _ bool) (config.MCPConfig, error) {
return config.MCPConfig{Servers: map[string]config.MCPServerConfig{
"broken": {Type: "stdio", Command: "zero-definitely-not-a-real-binary-xyz"},
}}, nil
},
}
var out, errBuf bytes.Buffer
if code := runWithDeps([]string{"mcp", "check", "broken", "--json"}, &out, &errBuf, deps); code == exitSuccess {
t.Fatalf("mcp check --json must fail for a server that did not start; stdout=%q stderr=%q", out.String(), errBuf.String())
}
var payload struct {
ServerName string `json:"serverName"`
Status string `json:"status"`
Error string `json:"error"`
ToolCount int `json:"toolCount"`
}
if err := json.Unmarshal(out.Bytes(), &payload); err != nil {
t.Fatalf("decode mcp check --json output %q: %v", out.String(), err)
}
if payload.ServerName != "broken" {
t.Fatalf("serverName = %q, want the server that was checked", payload.ServerName)
}
if payload.Status != "unreachable" {
t.Fatalf("status = %q, want %q", payload.Status, "unreachable")
}
if !strings.Contains(payload.Error, "not reachable") {
t.Fatalf("error = %q, want it to say the server is not reachable", payload.Error)
}
// A reachable payload carries a tool count; an unreachable one must not
// imply zero tools were found, which reads as a server that simply exposes
// none.
if payload.ToolCount != 0 {
t.Fatalf("toolCount = %d, want it absent from an unreachable payload", payload.ToolCount)
}
}
@@ -9,6 +9,7 @@ import (
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"testing"
"time"
@@ -386,6 +387,127 @@ func runWindowsRealSmokeCommand(t *testing.T, runnerExe string, base WindowsSand
}
}
// The write jail must hold on a path whose DACL grants Everyone write access.
//
// A WRITE_RESTRICTED token runs TWO checks for a write and needs both to pass:
// the normal one against its enabled SIDs, and a second against its RESTRICTED
// SID list. The jail is built on the second check only succeeding where Zero has
// explicitly ACL'd a capability SID. Putting the World SID (S-1-1-0) in the
// restricted list breaks that globally — every principal is a member of
// Everyone, so on a DACL that grants Everyone write, the restricted half passes
// for free and confinement collapses back to the ordinary user's own
// permissions, which is precisely the boundary the sandbox exists to be
// stricter than.
//
// The runner already states this rule for the SIDs it refuses to add ("None of
// the granted SIDs can be added to the restricted list without collapsing the
// write jail"); Everyone was in the list anyway, since the original sandbox
// baseline.
//
// Realistic rather than theoretical: administrators open share roots with
// Everyone:F, and third-party installers ship loose ACLs. It needs no privilege,
// no symlink and no race. Checked and ruled out: C:\Users\Public\Documents
// grants BATCH, not Everyone, so the common "public folder" case is not this.
func TestWindowsRestrictedTokenDeniesWritesToEveryoneWritablePaths(t *testing.T) {
if os.Getenv("ZERO_SANDBOX_REAL_SMOKE") != "1" {
t.Skip("set ZERO_SANDBOX_REAL_SMOKE=1 to run real Windows sandbox smoke tests")
}
runnerExe := realSmokeExecutable(t, "ZERO_WINDOWS_COMMAND_RUNNER_EXE", WindowsSandboxCommandRunnerName)
root := t.TempDir()
outside := t.TempDir()
everyoneDir := filepath.Join(outside, "everyone-writable")
if err := os.MkdirAll(everyoneDir, 0o700); err != nil {
t.Fatalf("MkdirAll everyone-writable: %v", err)
}
// Granted through the production ACL applier, so the hostile DACL is built
// the same way a real one is rather than by a test-only shortcut.
snapshot, applied, err := applyWindowsACLPathGroup(windowsACLPathGroup{
Path: everyoneDir,
Entries: []WindowsACLEntry{{
Action: WindowsACLAllowWrite,
Path: everyoneDir,
Capability: "S-1-1-0",
}},
})
if err != nil {
t.Fatalf("grant Everyone write: %v", err)
}
if !applied {
t.Fatal("precondition: the Everyone grant did not apply, so there is no bypass to test for")
}
t.Cleanup(func() { _ = rollbackWindowsACLSnapshots([]windowsACLSnapshot{snapshot}) })
sandboxHome := filepath.Join(root, ".zero-sandbox")
// No DenyRead on purpose: that is what makes the runner choose the
// WRITE_RESTRICTED token, which is the default posture and the one whose
// restricted-SID list this test is about.
profile := PermissionProfile{
FileSystem: FileSystemPolicy{
Kind: FileSystemRestricted,
ReadRoots: []string{root},
WriteRoots: []WritableRoot{{Root: root, ProtectedMetadataNames: []string{".git", ".zero", ".agents"}}},
IncludePlatformRoots: true,
AllowTemp: true,
},
Network: NetworkPolicy{Mode: NetworkDeny},
}
config := WindowsSandboxCommandArgsOptions{
SandboxHome: sandboxHome,
CommandCWD: root,
WorkspaceRoots: []string{root},
PermissionProfile: profile,
SandboxLevel: WindowsSandboxLevelUnelevated,
}
// Granted root first. A jail that denies everything would satisfy the two
// assertions below while being useless, and this separates the two outcomes.
insideMarker := filepath.Join(root, "write-ok.txt")
runWindowsRealSmokeCommand(t, runnerExe, config, []string{
"cmd.exe", "/d", "/s", "/c", "echo ok>" + insideMarker,
}, 0)
if contents, err := os.ReadFile(insideMarker); err != nil || strings.TrimSpace(string(contents)) != "ok" {
t.Fatalf("sandboxed write to a granted root = %q, %v; want ok", contents, err)
}
// Control: an ordinary directory outside every granted root. This is the
// behaviour the Everyone case must match.
controlMarker := filepath.Join(outside, "control-denied.txt")
runWindowsRealSmokeCommand(t, runnerExe, config, deniedWriteCommand(controlMarker), deniedWriteExitCode)
if _, err := os.Stat(controlMarker); err == nil {
t.Fatal("precondition: the sandbox allowed a write to an ordinary path outside every granted root, so this test cannot measure the Everyone case")
}
// The assertion. Same as the control in every respect except the DACL.
everyoneMarker := filepath.Join(everyoneDir, "everyone-denied.txt")
runWindowsRealSmokeCommand(t, runnerExe, config, deniedWriteCommand(everyoneMarker), deniedWriteExitCode)
if _, err := os.Stat(everyoneMarker); err == nil {
t.Error("the sandbox wrote outside every granted root because the path grants Everyone write; the restricted-SID list is satisfied by a SID every principal carries")
} else if !os.IsNotExist(err) {
t.Errorf("stat the Everyone-writable marker: %v", err)
}
}
// deniedWriteExitCode is chosen so a denial cannot be confused with the runner
// failing to start the command at all.
//
// The obvious spelling of these assertions is `echo leaked>path` expecting exit
// 1, but the runner itself exits 1 for its own errors — a failed marker
// validation, a bad argument, a token it could not build. A test written that
// way passes both when the sandbox denies the write and when nothing ever ran,
// and the second case proves nothing while looking identical to success. On a
// test whose whole job is catching a confinement regression, that is a failure
// mode to design out rather than hope about.
//
// 77 is arbitrary beyond being outside the range the runner produces for itself.
const deniedWriteExitCode = 77
// deniedWriteCommand attempts a write and reports deniedWriteExitCode when the
// redirect is refused, so the exit code also proves cmd.exe actually ran.
func deniedWriteCommand(marker string) []string {
return []string{"cmd.exe", "/d", "/s", "/c", "echo leaked>" + marker + " || exit " + strconv.Itoa(deniedWriteExitCode)}
}
func powershellSingleQuote(value string) string {
out := "'"
for _, r := range value {
+45 -9
View File
@@ -88,19 +88,55 @@ func createWindowsRestrictedTokenFromBase(base windows.Token, capabilitySIDs []w
if err != nil {
return 0, err
}
worldSID, err := windows.CreateWellKnownSid(windows.WinWorldSid)
if err != nil {
return 0, fmt.Errorf("create world SID: %w", err)
}
entries := make([]windows.SIDAndAttributes, 0, len(capabilitySIDs)+2)
for _, sid := range capabilitySIDs {
entries = append(entries, windows.SIDAndAttributes{Sid: sid.sid})
}
entries = append(entries,
windows.SIDAndAttributes{Sid: sidFromBytes(logonSID)},
windows.SIDAndAttributes{Sid: worldSID},
)
entries = append(entries, windows.SIDAndAttributes{Sid: sidFromBytes(logonSID)})
// The World SID (S-1-1-0, Everyone) is added ONLY to the token that does not
// carry WRITE_RESTRICTED, and putting it back unconditionally would reopen a
// write-jail bypass.
//
// A restricted SID is a key to every object whose DACL names it. That is why
// the runner refuses to add the user SID, Administrators or SYSTEM — "each
// has write access nearly everywhere". Everyone is the broadest of the lot:
// every principal carries it, so under WRITE_RESTRICTED the second
// (restricted-SID) check passes for free on any path whose DACL grants
// Everyone write, and confinement falls back to the ordinary user's own
// permissions — the exact boundary this token exists to be stricter than. It
// needs no privilege, no symlink and no race: an Everyone-writable directory
// is enough, and share roots opened with Everyone:F and loose installer ACLs
// supply them. It was present from the original sandbox baseline,
// uncommented, and under WRITE_RESTRICTED nothing depends on it, because that
// flag already exempts reads from the restricted-SID check.
//
// Without the flag it IS load-bearing and cannot simply be dropped. The
// restricted-SID check then applies to READS too, and default Windows DACLs
// grant BUILTIN\Users rather than anything in this list, so a token without
// Everyone cannot open cmd.exe — the process dies at launch with
// STATUS_ACCESS_DENIED (0xC0000022) before it runs anything. That path is
// only taken when the profile configures DenyRead, which is already the
// posture that trades capability for read-deny enforcement (#612).
//
// So the bypass survives for DenyRead profiles, deliberately and narrowly,
// rather than being traded for a sandbox that cannot start a command. Closing
// it there needs a different mechanism (a read-side grant that is not a
// universal group), tracked separately.
//
// The logon SID above stays in both modes: it is this token's own rather than
// a broad group, and broadenWindowsRestrictedTokenDefaultDacl depends on it so
// the process can use pipes and events it creates for itself.
//
// Anything added here needs the same scrutiny — Authenticated Users, Users,
// INTERACTIVE and BATCH would each produce this bypass on a DACL naming them.
if !writeRestricted {
worldSID, err := windows.CreateWellKnownSid(windows.WinWorldSid)
if err != nil {
return 0, fmt.Errorf("create world SID: %w", err)
}
entries = append(entries, windows.SIDAndAttributes{Sid: worldSID})
}
// WRITE_RESTRICTED scopes the restricted-SID check to write-type accesses:
// reads use only the normal token identity, so the sandboxed process can
+92 -3
View File
@@ -7,6 +7,7 @@ import (
"unicode/utf8"
"github.com/Gitlawb/zero/internal/redaction"
"github.com/Gitlawb/zero/internal/zeroruntime"
)
type EventRef struct {
@@ -37,6 +38,7 @@ type RewindPlan struct {
type CompactionOptions struct {
PreserveLast int
MaxPromptChars int
SkipPrompt bool
}
type CompactionPlan struct {
@@ -47,8 +49,9 @@ type CompactionPlan struct {
CompactableEvents []EventRef `json:"compactableEvents"`
PreservedEvents []EventRef `json:"preservedEvents"`
SummaryPrompt string `json:"summaryPrompt"`
PromptChars int `json:"promptChars"`
Truncated bool `json:"truncated,omitempty"`
// PromptChars counts prompt text content and excludes provider protocol framing.
PromptChars int `json:"promptChars"`
Truncated bool `json:"truncated,omitempty"`
}
type RecordCompactionInput struct {
@@ -164,7 +167,11 @@ func (store *Store) PlanCompaction(sessionID string, options CompactionOptions)
}
compactable := events[:split]
preserved := events[split:]
prompt, truncated := buildCompactionPrompt(compactable, maxPromptChars)
prompt := ""
truncated := false
if !options.SkipPrompt {
prompt, truncated = buildCompactionPrompt(compactable, maxPromptChars)
}
return CompactionPlan{
SessionID: sessionID,
PreserveLast: preserveLast,
@@ -312,6 +319,88 @@ func buildCompactionPrompt(events []Event, maxChars int) (string, bool) {
return prompt, false
}
// CompactionMessages converts durable session events into the same normalized
// message shape used by the running agent's compaction pipeline. Provider usage
// and checkpoint blobs are omitted. Message/tool fields rely on the session
// writer's invariant that tool output is scrubbed before persistence;
// ask_user answers and generic event previews are redacted here.
func CompactionMessages(events []Event) []zeroruntime.Message {
messages := make([]zeroruntime.Message, 0, len(events))
for _, event := range events {
var payload map[string]any
_ = json.Unmarshal(event.Payload, &payload)
stringField := func(key string) string {
value, _ := payload[key].(string)
return strings.TrimSpace(value)
}
switch event.Type {
case EventMessage:
role := strings.ToLower(stringField("role"))
content := stringField("content")
switch role {
case "user":
messages = append(messages, zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: content})
case "assistant":
messages = append(messages, zeroruntime.Message{Role: zeroruntime.MessageRoleAssistant, Content: content})
case "ask_user_answers":
if answers, ok := payload["answers"]; ok {
encoded, _ := json.Marshal(answers)
messages = append(messages, zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: "User answers: " + redaction.RedactString(string(encoded), redaction.Options{})})
}
default:
if content != "" {
messages = append(messages, zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: content})
}
}
case EventToolCall:
messages = append(messages, zeroruntime.Message{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{{
ID: firstSessionString(stringField("toolCallId"), stringField("id")), Name: firstSessionString(stringField("name"), stringField("toolName")), Arguments: stringField("arguments"),
}}})
case EventToolResult:
status := strings.ToLower(stringField("status"))
messages = append(messages, zeroruntime.Message{
Role: zeroruntime.MessageRoleTool, ToolCallID: firstSessionString(stringField("toolCallId"), stringField("id")),
Content: stringField("output"), IsError: status != "" && status != "ok", ChangedFiles: sessionStringSlice(payload["changedFiles"]),
})
case EventCompaction:
if summary := stringField("summary"); summary != "" {
messages = append(messages, zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: "[Summary of earlier conversation]\n" + summary})
}
case EventProviderUsage, EventSessionCheckpoint:
continue
default:
preview := shapedPayloadPreview(event)
if strings.TrimSpace(preview) != "" && preview != "{}" {
messages = append(messages, zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: fmt.Sprintf("[%s] %s", event.Type, preview)})
}
}
}
return messages
}
func firstSessionString(values ...string) string {
for _, value := range values {
if strings.TrimSpace(value) != "" {
return strings.TrimSpace(value)
}
}
return ""
}
func sessionStringSlice(value any) []string {
raw, ok := value.([]any)
if !ok {
return nil
}
values := make([]string, 0, len(raw))
for _, item := range raw {
if text, ok := item.(string); ok && strings.TrimSpace(text) != "" {
values = append(values, strings.TrimSpace(text))
}
}
return values
}
func shapedPayloadPreview(event Event) string {
switch event.Type {
case EventPermission, EventPermissionRequest, EventPermissionDecision:
+39
View File
@@ -5,8 +5,27 @@ import (
"strings"
"testing"
"time"
"github.com/Gitlawb/zero/internal/zeroruntime"
)
func TestCompactionMessagesPreservesInteractiveAndMutationEvidence(t *testing.T) {
events := []Event{
{Type: EventToolCall, Payload: json.RawMessage(`{"id":"ask","name":"ask_user","arguments":"{\"questions\":[{\"question\":\"Which database?\"}]}"}`)},
{Type: EventToolResult, Payload: json.RawMessage(`{"toolCallId":"ask","name":"ask_user","status":"ok","output":"Postgres only"}`)},
{Type: EventToolCall, Payload: json.RawMessage(`{"id":"patch","name":"apply_patch","arguments":"{\"patch\":\"*** Update File: db.go\"}"}`)},
{Type: EventToolResult, Payload: json.RawMessage(`{"toolCallId":"patch","name":"apply_patch","status":"ok","output":"Done!","changedFiles":["db.go"]}`)},
}
messages := CompactionMessages(events)
if len(messages) != 4 || messages[0].Role != zeroruntime.MessageRoleAssistant || messages[1].Role != zeroruntime.MessageRoleTool {
t.Fatalf("unexpected normalized compaction messages: %#v", messages)
}
if messages[1].Content != "Postgres only" || len(messages[3].ChangedFiles) != 1 || messages[3].ChangedFiles[0] != "db.go" {
t.Fatalf("interactive or mutation evidence was lost: %#v", messages)
}
}
func TestStorePlansRewindBySequence(t *testing.T) {
store := NewStore(StoreOptions{RootDir: t.TempDir(), Now: sequenceClock([]time.Time{
time.Date(2026, 6, 6, 10, 0, 0, 0, time.UTC),
@@ -130,6 +149,26 @@ func TestStorePlansCompactionWindow(t *testing.T) {
}
}
func TestStoreCanPlanCompactionWithoutBuildingLegacyPrompt(t *testing.T) {
store := NewStore(StoreOptions{RootDir: t.TempDir()})
session, err := store.Create(CreateInput{SessionID: "compactnoprompt"})
if err != nil {
t.Fatal(err)
}
for _, content := range []string{"one", "two", "three"} {
if _, err := store.AppendEvent(session.SessionID, AppendEventInput{Type: EventMessage, Payload: map[string]string{"content": content}}); err != nil {
t.Fatal(err)
}
}
plan, err := store.PlanCompaction(session.SessionID, CompactionOptions{PreserveLast: 1, SkipPrompt: true})
if err != nil {
t.Fatal(err)
}
if plan.CompactableCount != 2 || plan.SummaryPrompt != "" || plan.PromptChars != 0 || plan.Truncated {
t.Fatalf("unexpected prompt-free plan: %#v", plan)
}
}
func TestStoreCompactionShapesSensitivePermissionEvents(t *testing.T) {
secret := "sk-proj-abcdefghijklmnopqrstuvwxyz"
store := NewStore(StoreOptions{RootDir: t.TempDir()})
+7 -4
View File
@@ -167,10 +167,13 @@ const permissionModeUnsafe = "unsafe"
var readOnlySpecialistTools = map[string]bool{
"read_file": true,
"read_minified_file": true,
"list_directory": true,
"grep": true,
"glob": true,
"update_plan": 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,
}
// 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"}) {
if !containsSequence(result.Args, []string{"--enabled-tools", "glob,grep,list_directory,read_file,read_minified_file,view_image"}) {
t.Fatalf("args missing default read-only allowlist: %#v", result.Args)
}
}
+4 -3
View File
@@ -82,9 +82,9 @@ var knownMetadataKeys = map[string]bool{
}
var toolCategories = map[string][]string{
"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"},
"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"},
"plan": {"update_plan"},
}
@@ -102,6 +102,7 @@ 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,
@@ -0,0 +1,31 @@
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,6 +132,34 @@ 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: "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)},
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: sandboxPermissionsDescription, Default: string(SandboxPermissionsUseDefault)},
"additional_permissions": {
Type: "object",
Description: "Sandboxed filesystem or network access for this command; only with `sandbox_permissions: \"with_additional_permissions\"`.",
Description: additionalPermissionsDescription,
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: "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)},
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: sandboxPermissionsDescription, Default: string(SandboxPermissionsUseDefault)},
"additional_permissions": {
Type: "object",
Description: "Sandboxed filesystem or network access for this command; only with `sandbox_permissions: \"with_additional_permissions\"`.",
Description: additionalPermissionsDescription,
Properties: additionalPermissionsProperties(),
},
"justification": {Type: "string", Description: "User-facing approval question for `require_escalated`; omit otherwise."},
+11
View File
@@ -4,6 +4,8 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/Gitlawb/zero/internal/imageinput"
"github.com/Gitlawb/zero/internal/zeroruntime"
"os"
"path/filepath"
"regexp"
@@ -345,9 +347,18 @@ 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,
+3
View File
@@ -299,6 +299,9 @@ func CoreReadOnlyToolsScoped(workspaceRoot string, scope PathScope) []Tool {
return []Tool{
NewScopedReadFileTool(workspaceRoot, scope),
NewScopedReadMinifiedFileTool(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),
NewScopedListDirectoryTool(workspaceRoot, scope),
NewScopedGlobTool(workspaceRoot, scope),
NewScopedGrepTool(workspaceRoot, scope),
+7 -2
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) != 9 {
t.Fatalf("expected 9 core read-only tools, got %d", len(toolset))
if len(toolset) != 10 {
t.Fatalf("expected 10 core read-only tools, got %d", len(toolset))
}
seen := map[string]bool{}
@@ -71,6 +71,11 @@ 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) {
@@ -0,0 +1,72 @@
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,6 +5,7 @@ import (
"github.com/Gitlawb/zero/internal/execution"
"github.com/Gitlawb/zero/internal/sandbox"
"github.com/Gitlawb/zero/internal/zeroruntime"
)
type SideEffect string
@@ -102,6 +103,16 @@ 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
+80
View File
@@ -0,0 +1,80 @@
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
@@ -0,0 +1,100 @@
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
@@ -0,0 +1,60 @@
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)
}
}
+53 -26
View File
@@ -21,7 +21,6 @@ import (
var responseStyles = []string{"balanced", "concise", "explanatory", "review"}
const tuiCompactionPreserveLast = 8
const tuiCompactionMaxPromptChars = 8000
const compactStatusRowID = "compact/status"
var compactFrames = []string{"⠂", "⠒", "⠲", "⠴"}
@@ -842,8 +841,8 @@ func (m model) compactActiveSession() (model, CompactResult, error) {
beforeEvents := append([]sessions.Event{}, m.sessionEvents...)
beforeTokens := estimateTranscriptTokens(m.transcript)
plan, err := m.sessionStore.PlanCompaction(m.activeSession.SessionID, sessions.CompactionOptions{
PreserveLast: tuiCompactionPreserveLast,
MaxPromptChars: tuiCompactionMaxPromptChars,
PreserveLast: tuiCompactionPreserveLast,
SkipPrompt: true,
})
if err != nil {
return m, CompactResult{}, err
@@ -857,13 +856,26 @@ func (m model) compactActiveSession() (model, CompactResult, error) {
}, nil
}
summary, err := m.summarizeCompactionPlan(plan)
rawEvents, err := m.sessionStore.ReadEvents(m.activeSession.SessionID)
if err != nil {
return m, CompactResult{}, fmt.Errorf("read session events for compaction: %w", err)
}
compactableEvents, err := sessionEventsForRefs(rawEvents, plan.CompactableEvents)
if err != nil {
return m, CompactResult{}, err
}
summaryResult, err := m.summarizeCompactionPlan(plan, sessions.CompactionMessages(compactableEvents))
if err != nil {
return m, CompactResult{}, err
}
// PromptChars counts the text content sent to the provider: the shared
// system instruction plus projected message/tool payloads. Provider protocol
// framing is intentionally excluded.
plan.PromptChars = len(agent.CompactionSummaryInstructions) + summaryResult.ProjectedChars
plan.Truncated = summaryResult.Truncated
if _, err := m.sessionStore.RecordCompaction(m.activeSession.SessionID, sessions.RecordCompactionInput{
Plan: plan,
Summary: summary,
Summary: summaryResult.SummaryText,
}); err != nil {
return m, CompactResult{}, err
}
@@ -885,32 +897,47 @@ func (m model) compactActiveSession() (model, CompactResult, error) {
Compacted: len(events) < len(beforeEvents)+1,
BeforeTokens: beforeTokens,
AfterTokens: estimateTranscriptTokens(m.transcript),
Summary: summary,
Summary: summaryResult.SummaryText,
}, nil
}
func (m model) summarizeCompactionPlan(plan sessions.CompactionPlan) (string, error) {
if m.provider == nil {
return deterministicCompactionSummary(plan), nil
func sessionEventsForRefs(events []sessions.Event, refs []sessions.EventRef) ([]sessions.Event, error) {
byID := make(map[string]sessions.Event, len(events))
for _, event := range events {
byID[event.ID] = event
}
stream, err := m.provider.StreamCompletion(m.ctx, zeroruntime.CompletionRequest{
Messages: []zeroruntime.Message{
{Role: zeroruntime.MessageRoleSystem, Content: "Summarize compacted Zero session events for future coding context. Preserve user goals, decisions, files, tool outcomes, blockers, and exact next steps. Omit secrets and do not invent details."},
{Role: zeroruntime.MessageRoleUser, Content: plan.SummaryPrompt},
},
selected := make([]sessions.Event, 0, len(refs))
for _, ref := range refs {
event, ok := byID[ref.ID]
if !ok || event.Sequence != ref.Sequence || event.Type != ref.Type {
return nil, fmt.Errorf("session changed while compaction was being planned; retry /compact")
}
selected = append(selected, event)
}
return selected, nil
}
func (m model) summarizeCompactionPlan(plan sessions.CompactionPlan, messages []zeroruntime.Message) (agent.CompactionSummaryResult, error) {
return agent.SummarizeCompactionMessages(messages, func(projected []zeroruntime.Message) (string, error) {
if m.provider == nil {
return deterministicCompactionSummary(plan), nil
}
stream, streamErr := m.provider.StreamCompletion(m.ctx, zeroruntime.CompletionRequest{
Messages: append([]zeroruntime.Message{{Role: zeroruntime.MessageRoleSystem, Content: agent.CompactionSummaryInstructions}}, projected...),
})
if streamErr != nil {
return "", fmt.Errorf("summarize compacted session: %w", streamErr)
}
collected := zeroruntime.CollectStream(m.ctx, stream)
if collected.Error != "" {
return "", fmt.Errorf("summarize compacted session: %s", collected.Error)
}
result := strings.TrimSpace(collected.Text)
if result == "" {
return "", fmt.Errorf("summarize compacted session: empty summary")
}
return result, nil
})
if err != nil {
return "", fmt.Errorf("summarize compacted session: %w", err)
}
collected := zeroruntime.CollectStream(m.ctx, stream)
if collected.Error != "" {
return "", fmt.Errorf("summarize compacted session: %s", collected.Error)
}
summary := strings.TrimSpace(collected.Text)
if summary == "" {
return "", fmt.Errorf("summarize compacted session: empty summary")
}
return summary, nil
}
func deterministicCompactionSummary(plan sessions.CompactionPlan) string {
+68
View File
@@ -481,6 +481,15 @@ func TestCompactCommandUsesProviderSummaryWhenAvailable(t *testing.T) {
if err != nil {
t.Fatal(err)
}
for _, input := range []sessions.AppendEventInput{
{Type: sessions.EventToolCall, Payload: map[string]any{"id": "ask", "name": "ask_user", "arguments": `{"questions":[{"question":"Which database should remain?"}]}`}},
{Type: sessions.EventToolResult, Payload: map[string]any{"toolCallId": "ask", "name": "ask_user", "status": "ok", "output": "Postgres only"}},
} {
m, err = m.appendSessionEvent(input.Type, input.Payload)
if err != nil {
t.Fatal(err)
}
}
for _, content := range []string{
"old decision A",
"old decision B",
@@ -521,6 +530,15 @@ func TestCompactCommandUsesProviderSummaryWhenAvailable(t *testing.T) {
if len(provider.requests) != 1 {
t.Fatalf("expected one provider summarization request, got %d", len(provider.requests))
}
request := provider.requests[0]
if len(request.Messages) != 2 || request.Messages[0].Content != agent.CompactionSummaryInstructions {
t.Fatalf("manual compaction did not use the shared agent compaction prompt: %#v", request.Messages)
}
for _, want := range []string{"Which database should remain?", "Postgres only"} {
if !strings.Contains(request.Messages[1].Content, want) {
t.Fatalf("manual compaction projection lost %q: %q", want, request.Messages[1].Content)
}
}
if next.lastCompactResult == nil || next.lastCompactResult.Summary != "Provider summary keeps the actual old decisions." {
t.Fatalf("expected provider summary result, got %#v", next.lastCompactResult)
}
@@ -533,6 +551,56 @@ func TestCompactCommandUsesProviderSummaryWhenAvailable(t *testing.T) {
}
}
func TestManualCompactionPersistsStructuralTruncationMetadata(t *testing.T) {
store := sessions.NewStore(sessions.StoreOptions{RootDir: t.TempDir()})
provider := &fakeProvider{events: []zeroruntime.StreamEvent{
{Type: zeroruntime.StreamEventText, Content: "Bounded summary."},
{Type: zeroruntime.StreamEventDone},
}}
m := newModel(context.Background(), Options{ModelName: "gpt-4.1", Provider: provider, SessionStore: store})
var err error
m, err = m.ensureActiveSession("compact a large session")
if err != nil {
t.Fatal(err)
}
for index := range 20 {
content := strings.Repeat("contextword ", 256) + string(rune('a'+index))
m, err = m.appendSessionEvent(sessions.EventMessage, map[string]any{"role": "user", "content": content})
if err != nil {
t.Fatal(err)
}
m.transcript = appendTranscriptRow(m.transcript, transcriptRow{kind: rowUser, text: content})
}
next, result, err := m.compactActiveSession()
if err != nil || !result.Compacted {
t.Fatalf("compactActiveSession() = (%#v, %v)", result, err)
}
raw, err := store.ReadEvents(next.activeSession.SessionID)
if err != nil {
t.Fatal(err)
}
latest := raw[len(raw)-1]
if latest.Type != sessions.EventCompaction {
t.Fatalf("latest event = %s, want compaction", latest.Type)
}
payload := sessionPayload(latest)
if !payloadBool(payload, "truncated") {
t.Fatalf("compaction did not persist structural truncation: %#v", payload)
}
request := provider.requests[0]
wantPromptChars := len(agent.CompactionSummaryInstructions)
for _, message := range request.Messages[1:] {
wantPromptChars += len(message.Content)
for _, call := range message.ToolCalls {
wantPromptChars += len(call.Name) + len(call.Arguments)
}
}
if promptChars, ok := payload["promptChars"].(float64); !ok || int(promptChars) != wantPromptChars {
t.Fatalf("promptChars = %#v, want %d", payload["promptChars"], wantPromptChars)
}
}
func TestCompactCommandRecordsRequestWhenNoCompactorIsAvailable(t *testing.T) {
m := newModel(context.Background(), Options{})
m.input.SetValue("/compact")
+47
View File
@@ -1499,7 +1499,54 @@ 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
+8 -6
View File
@@ -93,12 +93,14 @@ type ReasoningBlock struct {
// Message is a normalized conversation turn passed to providers.
type Message struct {
Role MessageRole
Content string
ToolCalls []ToolCall
ToolCallID string
Images []ImageBlock // optional; nil for text-only messages
Reasoning []ReasoningBlock // optional; preserved thinking blocks to replay
Role MessageRole
Content string
ToolCalls []ToolCall
ToolCallID string
IsError bool // tool-result status; ignored for non-tool messages
ChangedFiles []string // durable tool-result mutation targets; ignored by providers
Images []ImageBlock // optional; nil for text-only messages
Reasoning []ReasoningBlock // optional; preserved thinking blocks to replay
}
// ToolDefinition describes a model-visible tool and its JSON-schema parameters.