mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-08 09:49:34 -10:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4d9a68b666 | ||
|
|
ad152e0049 | ||
|
|
b4b0418daf |
No files matched your search
@@ -67,39 +67,3 @@ requirements. If a related check fails, fix it. If a required check cannot run
|
||||
because of the environment or fails for an unrelated external reason, report
|
||||
the exact failure and obtain maintainer direction rather than silently ignoring
|
||||
it.
|
||||
|
||||
## 4. Common Review Blockers
|
||||
|
||||
These classes drive multi-round reviews. Fix them before requesting review:
|
||||
|
||||
- **Fresh base:** Rebase onto the current PR base (`main` or stacked target)
|
||||
before review. A stale head that rolls back mainline commits is a hard
|
||||
blocker, not a merge-time detail. Resolve conflicts by keeping upstream
|
||||
behavior unless the PR intentionally changes it.
|
||||
- **Platform truth:** Code and tests must pass on Linux, macOS, and Windows.
|
||||
Compare paths after canonicalization (physical/`EvalSymlinks` form); never
|
||||
assert raw `t.TempDir()` spellings (`/var` vs `/private/var` on macOS, short
|
||||
paths on Windows). Prefer one cross-platform function with small `GOOS`
|
||||
checks over duplicated helpers that drift.
|
||||
- **Security edges:** Keep secrets out of argv, env dumps, and logs. Redact
|
||||
success and error paths (including stderr). Fail closed on ownership, lease,
|
||||
and permission checks. Do not rely on pre-open path resolution
|
||||
(`EvalSymlinks` then open) for containment: that is a check-to-use race.
|
||||
Bind containment at open/use time with rooted or handle-relative,
|
||||
traversal-resistant APIs. If a no-follow API is used, apply it to every
|
||||
traversed component and enforce the platform's reparse-point protections;
|
||||
final-component-only no-follow is insufficient. On multi-step setup, roll back
|
||||
only what this run created; never destroy pre-existing resources you did
|
||||
not create; never report success when cleanup or unlock failed.
|
||||
- **Atomic shared state:** Write a complete temporary file, then atomically
|
||||
replace the destination so concurrent readers never see a partial write.
|
||||
Exclusive create or a write lock alone is not enough for readers. Serialize
|
||||
the full read-modify-write sequence for lockfiles and shared stores.
|
||||
- **Tests match the claim:** Every behavior or security-boundary change needs a
|
||||
regression test, including the failure path. Path-sensitive logic needs at
|
||||
least one non-Linux path case (or a hermetic fake that exercises the same
|
||||
normalization).
|
||||
- **Honest scope:** PR description, help text, and comments must match what
|
||||
shipped. Wire advertised entry points or shrink the claim. Do not bundle
|
||||
unrelated fixes. Do not widen security allowlists by name alone; check
|
||||
classification and side effects too.
|
||||
@@ -127,7 +127,9 @@ func TestExecuteToolCallMissingAdditionalPermissionsErrorIsActionable(t *testing
|
||||
if !strings.Contains(result.Output, `{"network": {"enabled": true}}`) {
|
||||
t.Fatalf("output = %q, want a concrete additional_permissions example", result.Output)
|
||||
}
|
||||
assertNamesOmitRemedy(t, "missing additional_permissions", result.Output)
|
||||
if !strings.Contains(result.Output, "omit sandbox_permissions") {
|
||||
t.Fatalf("output = %q, want guidance that the flag can simply be omitted", result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
// A well-formed additional_permissions payload must still reach the normal
|
||||
@@ -163,123 +165,6 @@ func TestExecuteToolCallStillPromptsForValidAdditionalPermissions(t *testing.T)
|
||||
}
|
||||
}
|
||||
|
||||
// runShellCall drives one shell tool call through the real registry (and so
|
||||
// through Registry.RunWithOptions) exactly as the agent loop would.
|
||||
func runShellCall(t *testing.T, root string, arguments string) ToolResult {
|
||||
t.Helper()
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(tools.NewScopedExecCommandTool(root, nil, nil))
|
||||
result, err := executeToolCall(
|
||||
context.Background(),
|
||||
registry,
|
||||
ToolCall{ID: "c1", Name: "exec_command", Arguments: arguments},
|
||||
PermissionModeUnsafe,
|
||||
Options{Cwd: root},
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("executeToolCall: %v", err)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
// TestAdditionalPermissionsRejectionsConverge is the regression guard for
|
||||
// #845/#847. A model that attaches a permission object to a command needing
|
||||
// none used to be told only "set sandbox_permissions", which pushed it into the
|
||||
// OTHER invalid shape (flag set, payload empty) and its "include a permission"
|
||||
// error pushed it straight back — the reported loop that burned turns on
|
||||
// `git branch --show-current`.
|
||||
//
|
||||
// The contract this pins is convergence, not tolerance: both shapes are still
|
||||
// REJECTED (nothing here grants access), but each rejection must name the one
|
||||
// follow-up call that works, and that follow-up must actually succeed.
|
||||
func TestAdditionalPermissionsRejectionsConverge(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
const readOnly = `"cmd":"git --version"`
|
||||
|
||||
// Step 1 of the reported loop: a permission object with no flag.
|
||||
first := runShellCall(t, root, `{`+readOnly+`,"additional_permissions":{"network":{"enabled":true}}}`)
|
||||
if first.Status != tools.StatusError {
|
||||
t.Fatalf("unflagged additional_permissions status = %q, want error (rejecting is correct)", first.Status)
|
||||
}
|
||||
assertNamesOmitRemedy(t, "unflagged additional_permissions", first.Output)
|
||||
|
||||
// Step 2 of the reported loop: the model keeps the flag but empties the
|
||||
// payload. This must not be the dead end it was.
|
||||
second := runShellCall(t, root, `{`+readOnly+`,"sandbox_permissions":"with_additional_permissions"}`)
|
||||
if second.Status != tools.StatusError {
|
||||
t.Fatalf("flag without payload status = %q, want error", second.Status)
|
||||
}
|
||||
assertNamesOmitRemedy(t, "flag without payload", second.Output)
|
||||
|
||||
// The call both errors point at must actually work.
|
||||
converged := runShellCall(t, root, `{`+readOnly+`}`)
|
||||
if converged.Status != tools.StatusOK {
|
||||
t.Fatalf("the call both errors instruct the model to make failed: status=%q output=%q", converged.Status, converged.Output)
|
||||
}
|
||||
}
|
||||
|
||||
// assertNamesOmitRemedy pins the actionable half of the error text: the message
|
||||
// must spell out that BOTH fields can simply be dropped. "Set the flag" or
|
||||
// "include a permission" alone is what made the two errors chase each other.
|
||||
func assertNamesOmitRemedy(t *testing.T, label string, output string) {
|
||||
t.Helper()
|
||||
if !strings.Contains(output, "omit both sandbox_permissions and additional_permissions") {
|
||||
t.Fatalf("%s error = %q, want it to say to omit both fields entirely", label, output)
|
||||
}
|
||||
if !strings.Contains(output, "retry") {
|
||||
t.Fatalf("%s error = %q, want it to tell the model to retry", label, output)
|
||||
}
|
||||
}
|
||||
|
||||
// The empty-payload rejection carries the same remedy. It is pinned on the
|
||||
// validator directly because normalizeNoopAdditionalPermissions absorbs the
|
||||
// empty payload before executeToolCall reaches this branch;
|
||||
// inlineAdditionalPermissionsProfile is still the shared source of truth that
|
||||
// buildPermissionEvent and the grant path re-run, so its text must converge too.
|
||||
func TestEmptyAdditionalPermissionsErrorNamesOmitRemedy(t *testing.T) {
|
||||
args := map[string]any{
|
||||
"cmd": "git --version",
|
||||
"sandbox_permissions": string(tools.SandboxPermissionsWithAdditionalPermissions),
|
||||
"additional_permissions": map[string]any{
|
||||
"network": map[string]any{"enabled": false},
|
||||
"file_system": map[string]any{"read": []any{}, "write": []any{}},
|
||||
},
|
||||
}
|
||||
_, requested, err := inlineAdditionalPermissionsProfile(args, t.TempDir())
|
||||
if !requested {
|
||||
t.Fatal("requested = false, want true for an explicit with_additional_permissions call")
|
||||
}
|
||||
if err == nil {
|
||||
t.Fatal("an empty additional_permissions payload was accepted; it must still be rejected")
|
||||
}
|
||||
assertNamesOmitRemedy(t, "empty payload", err.Error())
|
||||
}
|
||||
|
||||
// A provider that materializes every optional property may send
|
||||
// "additional_permissions": null. That grants nothing and carries no flag, so it
|
||||
// is the field omitted — rejecting it stalled an ordinary command behind an
|
||||
// error the model could not act on.
|
||||
func TestExecuteToolCallTreatsNullAdditionalPermissionsAsOmitted(t *testing.T) {
|
||||
result := runShellCall(t, t.TempDir(), `{"cmd":"git --version","additional_permissions":null}`)
|
||||
if result.Status != tools.StatusOK {
|
||||
t.Fatalf("null additional_permissions was rejected: status=%q output=%q", result.Status, result.Output)
|
||||
}
|
||||
}
|
||||
|
||||
// A null payload alongside the flag must still be rejected, and must land on
|
||||
// the same error as an absent payload rather than being quietly downgraded.
|
||||
func TestExecuteToolCallRejectsFlaggedNullAdditionalPermissions(t *testing.T) {
|
||||
result := runShellCall(t, t.TempDir(),
|
||||
`{"cmd":"git --version","sandbox_permissions":"with_additional_permissions","additional_permissions":null}`)
|
||||
if result.Status != tools.StatusError {
|
||||
t.Fatalf("flagged null status = %q, want error", result.Status)
|
||||
}
|
||||
if !strings.Contains(result.Output, "no additional_permissions object was provided") {
|
||||
t.Fatalf("output = %q, want the missing-payload error", result.Output)
|
||||
}
|
||||
assertNamesOmitRemedy(t, "flagged null", result.Output)
|
||||
}
|
||||
|
||||
func TestNormalizeProviderExpandedEmptyAdditionalPermissions(t *testing.T) {
|
||||
for _, mode := range []string{
|
||||
string(tools.SandboxPermissionsUseDefault),
|
||||
|
||||
@@ -38,9 +38,8 @@ const compactionTriggerRatio = 0.7
|
||||
// transcript (and so tests can assert on it).
|
||||
const summaryLabel = "[Summary of earlier conversation]"
|
||||
|
||||
// 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. " +
|
||||
// summaryInstructions is the system prompt handed to the summarizer model.
|
||||
const summaryInstructions = "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. " +
|
||||
@@ -76,12 +75,6 @@ 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
|
||||
}
|
||||
@@ -223,11 +216,11 @@ func CompactMessages(messages []zeroruntime.Message, opts CompactionOptions) (Co
|
||||
}, nil
|
||||
}
|
||||
|
||||
summaryResult, err := SummarizeCompactionMessages(middle, opts.Summarize)
|
||||
summary, err := opts.Summarize(middle)
|
||||
if err != nil {
|
||||
return CompactionResult{}, err
|
||||
}
|
||||
summary := summaryResult.SummaryText
|
||||
summary = strings.TrimSpace(summary)
|
||||
|
||||
// Preserve structured state (active plan + loaded skills) from the elided
|
||||
// middle verbatim, so it is not lost or paraphrased away by the prose summary.
|
||||
@@ -245,53 +238,10 @@ 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
|
||||
@@ -626,7 +576,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: CompactionSummaryInstructions},
|
||||
{Role: zeroruntime.MessageRoleSystem, Content: summaryInstructions},
|
||||
{Role: zeroruntime.MessageRoleUser, Content: "Summarize this conversation:\n\n" + renderTranscript(messages)},
|
||||
},
|
||||
// No tools: this is a plain text summarization call.
|
||||
|
||||
@@ -43,14 +43,8 @@ func TestCompactMessagesReturnsMetadataForManualCompaction(t *testing.T) {
|
||||
if result.SummaryText != "manual summary" {
|
||||
t.Fatalf("SummaryText = %q, want trimmed summary", result.SummaryText)
|
||||
}
|
||||
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(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 len(result.Messages) != 4 {
|
||||
t.Fatalf("compacted message count = %d, want 4", len(result.Messages))
|
||||
@@ -102,46 +96,11 @@ 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"},
|
||||
|
||||
@@ -102,7 +102,7 @@ type skillEntry struct {
|
||||
body string
|
||||
}
|
||||
|
||||
// recentEdits returns files mutated by editing calls in messages
|
||||
// recentEdits returns the files mutated by write_file/edit_file 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,7 +113,6 @@ 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 {
|
||||
@@ -130,24 +129,12 @@ 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
|
||||
@@ -438,20 +425,15 @@ 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"`
|
||||
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"`
|
||||
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"`
|
||||
}
|
||||
|
||||
type preservedEdit struct {
|
||||
@@ -486,12 +468,7 @@ func appendPreservedState(summary string, middle []zeroruntime.Message, taskSnap
|
||||
task := priorState.Task
|
||||
if taskSnapshot != nil {
|
||||
task = &preservedTaskState{
|
||||
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),
|
||||
Objective: capTaskObjective(taskSnapshot.Objective),
|
||||
}
|
||||
// Plan parity corroborates only the mutable task projection. The objective
|
||||
// comes directly from the run prompt and is immutable, so it must survive
|
||||
@@ -507,13 +484,6 @@ 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.
|
||||
@@ -547,95 +517,6 @@ 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,7 +6,6 @@ import (
|
||||
"testing"
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/tools"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
@@ -65,9 +64,6 @@ 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,
|
||||
@@ -84,69 +80,6 @@ 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) {
|
||||
|
||||
@@ -1,315 +0,0 @@
|
||||
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,
|
||||
}
|
||||
@@ -1,130 +0,0 @@
|
||||
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,15 +20,11 @@ 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) != 3 {
|
||||
t.Fatalf("expected 3 edited files, got %d: %#v", len(edits), edits)
|
||||
if len(edits) != 2 {
|
||||
t.Fatalf("expected 2 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])
|
||||
@@ -36,9 +32,6 @@ 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
|
||||
|
||||
@@ -58,15 +58,12 @@ func TestCompactKeepsSystemAndPreservedSuffix(t *testing.T) {
|
||||
if last.Content != "most recent question" {
|
||||
t.Fatalf("expected most recent message preserved, got %q", last.Content)
|
||||
}
|
||||
// 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)
|
||||
// The summarized middle excludes system and preserved suffix.
|
||||
if len(captured) != 3 {
|
||||
t.Fatalf("expected 3 summarized messages, got %d: %#v", len(captured), captured)
|
||||
}
|
||||
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)
|
||||
}
|
||||
if captured[0].Content != "first question" {
|
||||
t.Fatalf("expected oldest non-system message first, got %#v", captured[0])
|
||||
}
|
||||
// Compaction must shrink the conversation.
|
||||
if estimateTokens(result) >= estimateTokens(messages) {
|
||||
|
||||
+14
-117
@@ -187,13 +187,6 @@ 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
|
||||
@@ -663,13 +656,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
// between tool_results breaks strict provider replay) — same after-batch
|
||||
// rationale as turnRequestedModel above.
|
||||
var changedFilesThisBatch []string
|
||||
// Images produced by tools this turn, held until every tool_result is
|
||||
// recorded. They travel as user messages, and a user message between two
|
||||
// tool_results breaks strict provider replay — Anthropic coalesces them
|
||||
// into one user block list and requires the tool_result blocks first, so
|
||||
// interleaving yields [tool_result, text, image, tool_result] and a 400.
|
||||
// Same reason the self-correction feedback below is deferred.
|
||||
var toolImageMessages []zeroruntime.Message
|
||||
// Parallel read-ahead state: results for calls[precomputedStart:precomputedEnd]
|
||||
// executed concurrently, consumed strictly in order below.
|
||||
var precomputed []precomputedToolResult
|
||||
@@ -705,7 +691,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, arguments: call.Arguments, toolResult: toolResult})
|
||||
task.observe(taskStateEvent{kind: taskStateEventToolResult, toolResult: toolResult})
|
||||
if options.OnToolResult != nil {
|
||||
options.OnToolResult(toolResult)
|
||||
}
|
||||
@@ -719,22 +705,10 @@ 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,
|
||||
IsError: toolResult.Status == tools.StatusError,
|
||||
ChangedFiles: append([]string(nil), toolResult.ChangedFiles...),
|
||||
Role: zeroruntime.MessageRoleTool,
|
||||
Content: toolResult.Output,
|
||||
ToolCallID: toolResult.ToolCallID,
|
||||
})
|
||||
// Images ride a following USER message rather than the tool result
|
||||
// above. Every provider drops images on a tool-role message —
|
||||
// Anthropic's tool_result content is a string, Gemini's is a
|
||||
// functionResponse, and OpenAI guards its image parts to the user role
|
||||
// — so attaching them there would silently deliver nothing. A separate
|
||||
// message also keeps the one-tool-result-per-tool-call pairing intact,
|
||||
// which the providers validate.
|
||||
if imageMessage, ok := toolResultImageMessage(toolResult); ok {
|
||||
toolImageMessages = append(toolImageMessages, imageMessage)
|
||||
}
|
||||
|
||||
// A tool may demand the run ABORT — a canceled/timed-out ask_user prompt
|
||||
// returns context.Canceled rather than fabricating a headless answer. Stop
|
||||
@@ -745,13 +719,11 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
}
|
||||
if abortErr != nil {
|
||||
messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:])
|
||||
messages = append(messages, toolImageMessages...)
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, abortErr
|
||||
}
|
||||
if stopReason := stopReasonFromToolResult(toolResult); stopReason != "" {
|
||||
messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:])
|
||||
messages = append(messages, toolImageMessages...)
|
||||
result.FinalAnswer = toolResult.Output
|
||||
result.StopReason = stopReason
|
||||
result.Messages = copyMessages(messages)
|
||||
@@ -775,7 +747,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
// messages stay valid for a strict provider replay (Anthropic
|
||||
// rejects a tool_use with no answering tool_result).
|
||||
messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:])
|
||||
messages = append(messages, toolImageMessages...)
|
||||
result.FinalAnswer = toolFailureStopAnswer(call.Name, outcome.Count)
|
||||
result.Messages = copyMessages(messages)
|
||||
return result, nil
|
||||
@@ -794,10 +765,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
|
||||
postEditDiagnostics.enqueue(ctx, toolResult.ChangedFiles)
|
||||
}
|
||||
}
|
||||
// Every tool_result for this turn is now recorded, including aborted
|
||||
// placeholders, so the images can follow without splitting them.
|
||||
messages = append(messages, toolImageMessages...)
|
||||
toolImageMessages = nil
|
||||
|
||||
// Run post-edit self-correction once over the union of files this turn
|
||||
// changed, then append any feedback after every tool_result is recorded so
|
||||
@@ -1469,7 +1436,6 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
|
||||
Output: result.Output,
|
||||
Truncated: result.Truncated,
|
||||
Meta: result.Meta,
|
||||
Images: result.Images,
|
||||
Redacted: result.Redacted,
|
||||
ChangedFiles: result.ChangedFiles,
|
||||
ChangeSummaries: result.ChangeSummaries,
|
||||
@@ -2797,16 +2763,7 @@ func shellCommandAdditionalPermissionsRequested(args map[string]any) bool {
|
||||
// access and malformed payloads are left untouched so validation stays closed.
|
||||
func normalizeNoopAdditionalPermissions(args map[string]any, basePath string, engine *sandbox.Engine) {
|
||||
raw, exists := args["additional_permissions"]
|
||||
if !exists {
|
||||
return
|
||||
}
|
||||
// An explicit null is the field omitted, not a request: drop the key so the
|
||||
// rest of the pipeline sees the same args a model that omitted it would send.
|
||||
// sandbox_permissions is deliberately left alone — a flagged null must reach
|
||||
// the same "no additional_permissions object was provided" error as a flagged
|
||||
// absent field rather than being silently downgraded to a different outcome.
|
||||
if raw == nil {
|
||||
delete(args, "additional_permissions")
|
||||
if !exists || raw == nil {
|
||||
return
|
||||
}
|
||||
data, err := json.Marshal(raw)
|
||||
@@ -2837,41 +2794,21 @@ func normalizeNoopAdditionalPermissions(args map[string]any, basePath string, en
|
||||
}
|
||||
}
|
||||
|
||||
// omitAdditionalPermissionsHint is the one instruction that ends the retry loop
|
||||
// in #845/#847. Every additional_permissions rejection must carry it: the model
|
||||
// reaches these errors by attaching a permission object to a command that needs
|
||||
// none, so "omit it and retry" is the answer in practice, and an error that
|
||||
// only explains how to build a VALID permission object sends the model round
|
||||
// the loop again with a different invalid shape.
|
||||
const omitAdditionalPermissionsHint = "If this command does not need extra sandbox access — ordinary " +
|
||||
"read-only commands such as `git branch --show-current` do not — omit both sandbox_permissions and " +
|
||||
"additional_permissions entirely and retry; that is the fix in almost every case."
|
||||
|
||||
func inlineAdditionalPermissionsProfile(args map[string]any, basePath string) (sandbox.RequestPermissionProfile, bool, error) {
|
||||
if !shellCommandAdditionalPermissionsRequested(args) {
|
||||
// An explicit null grants nothing and carries no flag, so it is
|
||||
// indistinguishable from an omitted field; rejecting it would be a pure
|
||||
// false positive against providers that materialize every optional
|
||||
// property.
|
||||
if raw, exists := args["additional_permissions"]; exists && raw != nil {
|
||||
return sandbox.RequestPermissionProfile{}, false, fmt.Errorf(
|
||||
"additional_permissions was provided without sandbox_permissions set to %q, so it cannot be applied. %s "+
|
||||
"Only if the command genuinely needs file or network access the sandbox denies, resend with "+
|
||||
`sandbox_permissions: %q AND a non-empty additional_permissions, for example {"network": {"enabled": true}}`,
|
||||
tools.SandboxPermissionsWithAdditionalPermissions,
|
||||
omitAdditionalPermissionsHint,
|
||||
tools.SandboxPermissionsWithAdditionalPermissions)
|
||||
if _, exists := args["additional_permissions"]; exists {
|
||||
return sandbox.RequestPermissionProfile{}, false, fmt.Errorf("additional_permissions requires sandbox_permissions set to %q", tools.SandboxPermissionsWithAdditionalPermissions)
|
||||
}
|
||||
return sandbox.RequestPermissionProfile{}, false, nil
|
||||
}
|
||||
raw, ok := args["additional_permissions"]
|
||||
if !ok || raw == nil {
|
||||
return sandbox.RequestPermissionProfile{}, true, fmt.Errorf(
|
||||
"sandbox_permissions was set to %q but no additional_permissions object was provided. %s "+
|
||||
`Otherwise include one, for example additional_permissions: {"network": {"enabled": true}} or `+
|
||||
`{"file_system": {"write": ["/path"]}}`,
|
||||
tools.SandboxPermissionsWithAdditionalPermissions,
|
||||
omitAdditionalPermissionsHint)
|
||||
"sandbox_permissions was set to %q but no additional_permissions object was provided. "+
|
||||
`Include one, for example additional_permissions: {"network": {"enabled": true}} or `+
|
||||
`{"file_system": {"write": ["/path"]}}. If this command does not need elevated permissions, `+
|
||||
"omit sandbox_permissions entirely and retry",
|
||||
tools.SandboxPermissionsWithAdditionalPermissions)
|
||||
}
|
||||
data, err := json.Marshal(raw)
|
||||
if err != nil {
|
||||
@@ -2887,10 +2824,8 @@ func inlineAdditionalPermissionsProfile(args map[string]any, basePath string) (s
|
||||
}
|
||||
if normalized.Empty() {
|
||||
return sandbox.RequestPermissionProfile{}, true, fmt.Errorf(
|
||||
"additional_permissions granted nothing, so it cannot be applied. %s "+
|
||||
`Otherwise include a real permission, for example {"network": {"enabled": true}} or `+
|
||||
`{"file_system": {"write": ["/path"]}}`,
|
||||
omitAdditionalPermissionsHint)
|
||||
"additional_permissions must include at least one of network or file_system, for example " +
|
||||
`{"network": {"enabled": true}} or {"file_system": {"write": ["/path"]}}`)
|
||||
}
|
||||
grantProfile, err := sandbox.RequestPermissionGrantProfile(normalized)
|
||||
if err != nil {
|
||||
@@ -3287,7 +3222,6 @@ func appendAbortedToolResults(messages []Message, remaining []ToolCall) []Messag
|
||||
Role: zeroruntime.MessageRoleTool,
|
||||
Content: abortedToolResultNotice,
|
||||
ToolCallID: call.ID,
|
||||
IsError: true,
|
||||
})
|
||||
}
|
||||
return messages
|
||||
@@ -3318,46 +3252,9 @@ 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
|
||||
}
|
||||
@@ -3592,11 +3592,6 @@ 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 {
|
||||
@@ -3611,33 +3606,6 @@ 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" }
|
||||
|
||||
+10
-217
@@ -1,14 +1,10 @@
|
||||
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"
|
||||
@@ -56,41 +52,17 @@ 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"`
|
||||
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"`
|
||||
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"`
|
||||
}
|
||||
|
||||
type taskStateEventKind int
|
||||
@@ -100,7 +72,6 @@ const (
|
||||
taskStateEventToolResult
|
||||
taskStateEventVerification
|
||||
taskStateEventCompletion
|
||||
taskStateEventPermission
|
||||
)
|
||||
|
||||
type taskStateEvent struct {
|
||||
@@ -109,7 +80,6 @@ type taskStateEvent struct {
|
||||
toolResult ToolResult
|
||||
verification Outcome
|
||||
completion completionEvaluation
|
||||
permission PermissionEvent
|
||||
}
|
||||
|
||||
// taskState is a deterministic projection of facts the loop already observes.
|
||||
@@ -131,7 +101,6 @@ func newTaskState(objective string, recorder *trace.Recorder) *taskState {
|
||||
},
|
||||
changedFiles: map[string]struct{}{},
|
||||
}
|
||||
state.snapshotValue.Constraints = extractExplicitConstraints(objective)
|
||||
state.recorder = recorder
|
||||
return state
|
||||
}
|
||||
@@ -165,7 +134,6 @@ 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 {
|
||||
@@ -192,10 +160,6 @@ 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++
|
||||
@@ -219,180 +183,9 @@ 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
|
||||
|
||||
@@ -115,68 +115,6 @@ 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"}]}`})
|
||||
|
||||
@@ -1,207 +0,0 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/tools"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
// imageTool returns a tool result carrying an image, the way a screenshot tool
|
||||
// should be able to.
|
||||
type imageTool struct {
|
||||
media string
|
||||
data []byte
|
||||
}
|
||||
|
||||
func (imageTool) Name() string { return "capture" }
|
||||
func (imageTool) Description() string { return "Captures an image." }
|
||||
func (imageTool) Parameters() tools.Schema {
|
||||
return tools.Schema{Type: "object", Properties: map[string]tools.PropertySchema{}}
|
||||
}
|
||||
func (imageTool) Safety() tools.Safety {
|
||||
return tools.Safety{Permission: tools.PermissionAllow}
|
||||
}
|
||||
func (t imageTool) Run(context.Context, map[string]any) tools.Result {
|
||||
return tools.Result{
|
||||
Status: tools.StatusOK,
|
||||
Output: "Captured a screenshot.",
|
||||
Images: []zeroruntime.ImageBlock{{MediaType: t.media, Data: t.data}},
|
||||
}
|
||||
}
|
||||
|
||||
// A tool that produces an image must get that image in front of the model.
|
||||
//
|
||||
// It cannot ride the tool-result message: every provider drops images there.
|
||||
// Anthropic maps a tool result to a tool_result block with string content,
|
||||
// Gemini to a functionResponse, and OpenAI guards its image parts to the user
|
||||
// role explicitly. So the image has to arrive as a following user message,
|
||||
// which is also the only shape that keeps one tool result per tool call.
|
||||
func TestRunDeliversToolResultImagesToTheModel(t *testing.T) {
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(imageTool{media: "image/png", data: []byte("\x89PNG\r\n\x1a\nfake")})
|
||||
|
||||
provider := &mockProvider{
|
||||
turns: [][]zeroruntime.StreamEvent{
|
||||
{
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call_1", ToolName: "capture"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call_1", ArgumentsFragment: `{}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call_1"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
{
|
||||
{Type: zeroruntime.StreamEventText, Content: "I can see it."},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
result, err := Run(context.Background(), "screenshot please", provider, Options{
|
||||
Registry: registry,
|
||||
MaxTurns: 2,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Run: %v", err)
|
||||
}
|
||||
|
||||
var carrier *zeroruntime.Message
|
||||
for index := range result.Messages {
|
||||
if len(result.Messages[index].Images) > 0 {
|
||||
carrier = &result.Messages[index]
|
||||
break
|
||||
}
|
||||
}
|
||||
if carrier == nil {
|
||||
t.Fatal("no message carried the tool's image; the model never saw it")
|
||||
}
|
||||
if carrier.Role != zeroruntime.MessageRoleUser {
|
||||
t.Errorf("image rode a %q message; every provider only serializes images on the user role", carrier.Role)
|
||||
}
|
||||
if got := carrier.Images[0].MediaType; got != "image/png" {
|
||||
t.Errorf("media type = %q, want image/png", got)
|
||||
}
|
||||
|
||||
// The tool-result pairing must be untouched: one tool result per tool call.
|
||||
toolResults := 0
|
||||
for _, message := range result.Messages {
|
||||
if message.Role == zeroruntime.MessageRoleTool {
|
||||
toolResults++
|
||||
}
|
||||
}
|
||||
if toolResults != 1 {
|
||||
t.Errorf("got %d tool-result messages for 1 tool call", toolResults)
|
||||
}
|
||||
}
|
||||
|
||||
// textTool is a second, image-free tool so a turn can carry two calls.
|
||||
type textTool struct{}
|
||||
|
||||
func (textTool) Name() string { return "note" }
|
||||
func (textTool) Description() string { return "Notes something." }
|
||||
func (textTool) Parameters() tools.Schema {
|
||||
return tools.Schema{Type: "object", Properties: map[string]tools.PropertySchema{}}
|
||||
}
|
||||
func (textTool) Safety() tools.Safety { return tools.Safety{Permission: tools.PermissionAllow} }
|
||||
func (textTool) Run(context.Context, map[string]any) tools.Result {
|
||||
return tools.Result{Status: tools.StatusOK, Output: "noted"}
|
||||
}
|
||||
|
||||
// Tool results for one assistant turn must stay contiguous. The image messages
|
||||
// are user-role, and a user message between two tool_results breaks strict
|
||||
// provider replay: Anthropic coalesces consecutive user content into one block
|
||||
// list and requires tool_result blocks first, so interleaving produces
|
||||
// [tool_result, text, image, tool_result] and the request is rejected.
|
||||
//
|
||||
// The first version of this feature appended each image immediately after its
|
||||
// own tool result, which is exactly that shape. A single-tool-call test passed
|
||||
// and said nothing about it.
|
||||
func TestRunKeepsToolResultsContiguousWhenAToolReturnsAnImage(t *testing.T) {
|
||||
registry := tools.NewRegistry()
|
||||
registry.Register(imageTool{media: "image/png", data: []byte("\x89PNG\r\n\x1a\nfake")})
|
||||
registry.Register(textTool{})
|
||||
|
||||
provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
|
||||
{
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "c1", ToolName: "capture"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "c1", ArgumentsFragment: `{}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "c1"},
|
||||
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "c2", ToolName: "note"},
|
||||
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "c2", ArgumentsFragment: `{}`},
|
||||
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "c2"},
|
||||
{Type: zeroruntime.StreamEventDone},
|
||||
},
|
||||
{{Type: zeroruntime.StreamEventText, Content: "done"}, {Type: zeroruntime.StreamEventDone}},
|
||||
}}
|
||||
|
||||
result, err := Run(context.Background(), "two calls", provider, Options{Registry: registry, MaxTurns: 2})
|
||||
if err != nil {
|
||||
t.Fatalf("Run: %v", err)
|
||||
}
|
||||
|
||||
// Two separate ifs rather than a switch: a tool-role message that also
|
||||
// carried images would match only the first arm of a switch, so a regression
|
||||
// that put the image back on the tool result would read as "no image at all"
|
||||
// instead of naming what actually happened.
|
||||
type toolResult struct {
|
||||
index int
|
||||
id string
|
||||
}
|
||||
var toolResults []toolResult
|
||||
imageIndex := -1
|
||||
for index, message := range result.Messages {
|
||||
if message.Role == zeroruntime.MessageRoleTool {
|
||||
toolResults = append(toolResults, toolResult{index: index, id: message.ToolCallID})
|
||||
}
|
||||
if len(message.Images) > 0 {
|
||||
if imageIndex >= 0 {
|
||||
t.Fatalf("image delivered twice, at %d and %d; recorded %s", imageIndex, index, messageShape(result.Messages))
|
||||
}
|
||||
imageIndex = index
|
||||
}
|
||||
}
|
||||
if imageIndex < 0 {
|
||||
t.Fatalf("the image never reached the model; recorded %s", messageShape(result.Messages))
|
||||
}
|
||||
if len(toolResults) != 2 {
|
||||
t.Fatalf("got %d tool-result messages for 2 tool calls; recorded %s", len(toolResults), messageShape(result.Messages))
|
||||
}
|
||||
if toolResults[0].id != "c1" || toolResults[1].id != "c2" {
|
||||
t.Errorf("tool results answered %q then %q, want c1 then c2", toolResults[0].id, toolResults[1].id)
|
||||
}
|
||||
// The contract Anthropic enforces: every tool_result block has to come before
|
||||
// any other block in the user message it lands in. Anything wedged between
|
||||
// the two results breaks that, which is the 400 this PR exists to avoid.
|
||||
if toolResults[1].index != toolResults[0].index+1 {
|
||||
t.Errorf("tool results at %d and %d are not adjacent; %q message at %d splits them; recorded %s",
|
||||
toolResults[0].index, toolResults[1].index, result.Messages[toolResults[0].index+1].Role,
|
||||
toolResults[0].index+1, messageShape(result.Messages))
|
||||
}
|
||||
// Follows the last tool result, deliberately NOT pinned to exactly last+1.
|
||||
// appendUserBlocks coalesces consecutive user-role content, so
|
||||
// [tool_result, tool_result, text, image] still maps to tool_result blocks
|
||||
// first and replays fine; pinning would redden that valid shape for nothing.
|
||||
if imageIndex <= toolResults[1].index {
|
||||
t.Errorf("image message at %d does not follow the last tool result at %d; recorded %s",
|
||||
imageIndex, toolResults[1].index, messageShape(result.Messages))
|
||||
}
|
||||
}
|
||||
|
||||
// messageShape renders the conversation as a compact role list so a failure says
|
||||
// what the ordering actually was instead of dumping whole messages.
|
||||
func messageShape(messages []zeroruntime.Message) string {
|
||||
parts := make([]string, 0, len(messages))
|
||||
for index, message := range messages {
|
||||
part := fmt.Sprintf("%d:%s", index, message.Role)
|
||||
if message.ToolCallID != "" {
|
||||
part += "(" + message.ToolCallID + ")"
|
||||
}
|
||||
if len(message.Images) > 0 {
|
||||
part += "+image"
|
||||
}
|
||||
parts = append(parts, part)
|
||||
}
|
||||
return "[" + strings.Join(parts, " ") + "]"
|
||||
}
|
||||
@@ -68,11 +68,8 @@ type ToolResult struct {
|
||||
Output string
|
||||
// Truncated reports that the tool's model-visible output omitted content.
|
||||
// The full result may be recoverable through Meta["spill_path"].
|
||||
Truncated bool
|
||||
Meta map[string]string
|
||||
// Images the tool produced, delivered to the model as a following user
|
||||
// message rather than on this result. See tools.Result.Images.
|
||||
Images []zeroruntime.ImageBlock
|
||||
Truncated bool
|
||||
Meta map[string]string
|
||||
Redacted bool
|
||||
ChangedFiles []string
|
||||
// ChangeSummaries are non-selectable generated-tree summaries emitted by
|
||||
|
||||
@@ -308,34 +308,6 @@ 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 {
|
||||
|
||||
@@ -6,7 +6,6 @@ package cli
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -220,81 +219,3 @@ 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,7 +9,6 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -387,127 +386,6 @@ 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 {
|
||||
|
||||
@@ -88,55 +88,19 @@ 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)})
|
||||
|
||||
// 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})
|
||||
}
|
||||
entries = append(entries,
|
||||
windows.SIDAndAttributes{Sid: sidFromBytes(logonSID)},
|
||||
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
|
||||
|
||||
@@ -7,7 +7,6 @@ import (
|
||||
"unicode/utf8"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/redaction"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
type EventRef struct {
|
||||
@@ -38,7 +37,6 @@ type RewindPlan struct {
|
||||
type CompactionOptions struct {
|
||||
PreserveLast int
|
||||
MaxPromptChars int
|
||||
SkipPrompt bool
|
||||
}
|
||||
|
||||
type CompactionPlan struct {
|
||||
@@ -49,9 +47,8 @@ type CompactionPlan struct {
|
||||
CompactableEvents []EventRef `json:"compactableEvents"`
|
||||
PreservedEvents []EventRef `json:"preservedEvents"`
|
||||
SummaryPrompt string `json:"summaryPrompt"`
|
||||
// PromptChars counts prompt text content and excludes provider protocol framing.
|
||||
PromptChars int `json:"promptChars"`
|
||||
Truncated bool `json:"truncated,omitempty"`
|
||||
PromptChars int `json:"promptChars"`
|
||||
Truncated bool `json:"truncated,omitempty"`
|
||||
}
|
||||
|
||||
type RecordCompactionInput struct {
|
||||
@@ -167,11 +164,7 @@ func (store *Store) PlanCompaction(sessionID string, options CompactionOptions)
|
||||
}
|
||||
compactable := events[:split]
|
||||
preserved := events[split:]
|
||||
prompt := ""
|
||||
truncated := false
|
||||
if !options.SkipPrompt {
|
||||
prompt, truncated = buildCompactionPrompt(compactable, maxPromptChars)
|
||||
}
|
||||
prompt, truncated := buildCompactionPrompt(compactable, maxPromptChars)
|
||||
return CompactionPlan{
|
||||
SessionID: sessionID,
|
||||
PreserveLast: preserveLast,
|
||||
@@ -319,88 +312,6 @@ 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:
|
||||
|
||||
@@ -5,27 +5,8 @@ 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),
|
||||
@@ -149,26 +130,6 @@ 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()})
|
||||
|
||||
@@ -167,13 +167,10 @@ const permissionModeUnsafe = "unsafe"
|
||||
var readOnlySpecialistTools = map[string]bool{
|
||||
"read_file": true,
|
||||
"read_minified_file": true,
|
||||
// Reads one image file through read_file's own path scoping and mutates
|
||||
// nothing, so it carries the same auto-approval as the other pure readers.
|
||||
"view_image": true,
|
||||
"list_directory": true,
|
||||
"grep": true,
|
||||
"glob": true,
|
||||
"update_plan": true,
|
||||
"list_directory": true,
|
||||
"grep": true,
|
||||
"glob": true,
|
||||
"update_plan": true,
|
||||
}
|
||||
|
||||
// IsReadOnlySpecialist reports whether the named specialist resolves to a
|
||||
|
||||
@@ -255,7 +255,7 @@ func TestBuildArgsDefaultsToReadOnlyToolAllowlist(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("BuildArgs returned error: %v", err)
|
||||
}
|
||||
if !containsSequence(result.Args, []string{"--enabled-tools", "glob,grep,list_directory,read_file,read_minified_file,view_image"}) {
|
||||
if !containsSequence(result.Args, []string{"--enabled-tools", "glob,grep,list_directory,read_file,read_minified_file"}) {
|
||||
t.Fatalf("args missing default read-only allowlist: %#v", result.Args)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -82,9 +82,9 @@ var knownMetadataKeys = map[string]bool{
|
||||
}
|
||||
|
||||
var toolCategories = map[string][]string{
|
||||
"read-only": {"read_file", "read_minified_file", "view_image", "list_directory", "grep", "glob"},
|
||||
"edit": {"read_file", "read_minified_file", "view_image", "list_directory", "grep", "glob", "write_file", "edit_file", "apply_patch"},
|
||||
"execute": {"read_file", "read_minified_file", "view_image", "list_directory", "grep", "glob", "exec_command", "write_stdin", "bash"},
|
||||
"read-only": {"read_file", "read_minified_file", "list_directory", "grep", "glob"},
|
||||
"edit": {"read_file", "read_minified_file", "list_directory", "grep", "glob", "write_file", "edit_file", "apply_patch"},
|
||||
"execute": {"read_file", "read_minified_file", "list_directory", "grep", "glob", "exec_command", "write_stdin", "bash"},
|
||||
"plan": {"update_plan"},
|
||||
}
|
||||
|
||||
@@ -102,7 +102,6 @@ var defaultToolSelection = []string{"read-only"}
|
||||
var knownToolNames = map[string]bool{
|
||||
"read_file": true,
|
||||
"read_minified_file": true,
|
||||
"view_image": true,
|
||||
"list_directory": true,
|
||||
"glob": true,
|
||||
"grep": true,
|
||||
|
||||
@@ -1,31 +0,0 @@
|
||||
package specialist
|
||||
|
||||
import (
|
||||
"slices"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// view_image is a pure workspace-scoped read, so every category that already
|
||||
// grants read_file should grant it too. Categories are what specialist authors
|
||||
// actually write ("tools: [execute]"), so an omission here silently denies a
|
||||
// specialist the ability to look at a screenshot it just captured.
|
||||
func TestToolCategoriesIncludeViewImageWhereverReadFileIsGranted(t *testing.T) {
|
||||
for category, names := range toolCategories {
|
||||
if !slices.Contains(names, "read_file") {
|
||||
continue
|
||||
}
|
||||
if !slices.Contains(names, "view_image") {
|
||||
t.Errorf("category %q grants read_file but not view_image", category)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveToolsExpandsExecuteToIncludeViewImage(t *testing.T) {
|
||||
resolved, err := ResolveTools([]string{"execute"})
|
||||
if err != nil {
|
||||
t.Fatalf("ResolveTools: %v", err)
|
||||
}
|
||||
if !slices.Contains(resolved, "view_image") {
|
||||
t.Errorf("execute resolved to %v, missing view_image", resolved)
|
||||
}
|
||||
}
|
||||
@@ -132,34 +132,6 @@ func intPtr(value int) *int {
|
||||
return &value
|
||||
}
|
||||
|
||||
// sandboxPermissionsDescription and additionalPermissionsDescription are shared
|
||||
// by every shell tool (bash, exec_command) so the two fields cannot drift apart.
|
||||
//
|
||||
// Both lead with "omit" on purpose. The pair used to be described only in terms
|
||||
// of what each value does, which left a model that had used
|
||||
// with_additional_permissions once (say, for `gh issue create`) no instruction
|
||||
// to stop attaching a permission object to the next, ordinary command. That
|
||||
// attachment is then rejected — correctly, but the rejection alone does not tell
|
||||
// the model that dropping the field is the fix, so it flips between an
|
||||
// unflagged permission object and a flagged empty one and never converges
|
||||
// (#845, #847). Making "omit it" the first sentence of both descriptions stops
|
||||
// the invalid shape from being emitted in the first place; the validator errors
|
||||
// are the backstop, not the teacher.
|
||||
//
|
||||
// These ride on every turn's exec_command schema, so they are kept close to the
|
||||
// length of the text they replaced; TestEagerToolSchemaTokenBudget is the ratchet.
|
||||
const (
|
||||
sandboxPermissionsDescription = "Per-command sandbox override. Omit it unless the command needs access the " +
|
||||
"sandbox does not already give it; use `with_additional_permissions` with a non-empty " +
|
||||
"`additional_permissions` for sandboxed file/network access, or `require_escalated` only when the command " +
|
||||
"must run outside the sandbox, such as host/global process, socket, service, or desktop state hidden by " +
|
||||
"sandbox namespaces."
|
||||
|
||||
additionalPermissionsDescription = "Extra sandbox access. Omit this field entirely unless `sandbox_permissions` " +
|
||||
"is `with_additional_permissions`; sending it with any other mode is rejected, as is an empty or all-false " +
|
||||
"object. Example: {\"network\": {\"enabled\": true}}."
|
||||
)
|
||||
|
||||
func additionalPermissionsProperties() map[string]PropertySchema {
|
||||
return requestPermissionsProfileSchema().Properties
|
||||
}
|
||||
|
||||
@@ -50,10 +50,10 @@ func NewScopedBashTool(workspaceRoot string, scope PathScope) Tool {
|
||||
"command": {Type: "string", Description: "Shell command to execute using the host shell. " + shellGuidance},
|
||||
"cwd": {Type: "string", Description: "Directory to run the command in. Relative paths stay in the workspace; use an absolute path to run in a granted extra directory. Defaults to workspace root. Prefer cwd over cd when changing directories.", Default: "."},
|
||||
"timeout_ms": {Type: "integer", Description: "Command timeout in milliseconds.", Default: defaultBashTimeoutMS, Minimum: intPtr(1), Maximum: intPtr(maxBashTimeoutMS)},
|
||||
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: sandboxPermissionsDescription, Default: string(SandboxPermissionsUseDefault)},
|
||||
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: "Per-command sandbox override. Defaults to `use_default`; use `with_additional_permissions` with `additional_permissions` for sandboxed file/network access, or `require_escalated` only when the command must run outside the sandbox, such as host/global process, socket, service, or desktop state hidden by sandbox namespaces.", Default: string(SandboxPermissionsUseDefault)},
|
||||
"additional_permissions": {
|
||||
Type: "object",
|
||||
Description: additionalPermissionsDescription,
|
||||
Description: "Sandboxed filesystem or network access for this command; only with `sandbox_permissions: \"with_additional_permissions\"`.",
|
||||
Properties: additionalPermissionsProperties(),
|
||||
},
|
||||
"justification": {Type: "string", Description: "User-facing approval question for `require_escalated`; omit otherwise."},
|
||||
|
||||
@@ -91,10 +91,10 @@ func NewScopedExecCommandTool(workspaceRoot string, scope PathScope, manager *ex
|
||||
"cwd": {Type: "string", Description: "Alias for workdir. Prefer workdir.", Default: "."},
|
||||
"yield_time_ms": {Type: "integer", Description: "Wait before yielding output. Defaults to 10000 ms; effective range is 250-30000 ms.", Default: defaultExecYieldTimeMS, Minimum: intPtr(1), Maximum: intPtr(maxExecYieldTimeMS)},
|
||||
"max_output_tokens": {Type: "integer", Description: "Output token budget. Defaults to 10000 tokens; larger requests may be capped by policy.", Default: defaultMaxOutputTokens, Minimum: intPtr(1), Maximum: intPtr(maxExecOutputTokenRequest)},
|
||||
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: sandboxPermissionsDescription, Default: string(SandboxPermissionsUseDefault)},
|
||||
"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: "Per-command sandbox override. Defaults to `use_default`; use `with_additional_permissions` with `additional_permissions` for sandboxed file/network access, or `require_escalated` only when the command must run outside the sandbox, such as host/global process, socket, service, or desktop state hidden by sandbox namespaces.", Default: string(SandboxPermissionsUseDefault)},
|
||||
"additional_permissions": {
|
||||
Type: "object",
|
||||
Description: additionalPermissionsDescription,
|
||||
Description: "Sandboxed filesystem or network access for this command; only with `sandbox_permissions: \"with_additional_permissions\"`.",
|
||||
Properties: additionalPermissionsProperties(),
|
||||
},
|
||||
"justification": {Type: "string", Description: "User-facing approval question for `require_escalated`; omit otherwise."},
|
||||
|
||||
@@ -4,8 +4,6 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"github.com/Gitlawb/zero/internal/imageinput"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
@@ -347,18 +345,9 @@ func captureOKResult(driver string, request captureArtifactRequest, path string,
|
||||
if helperOutput != "" {
|
||||
output += "\n\n" + helperOutput
|
||||
}
|
||||
// Attach the artifact itself. Without this the model is told a screenshot
|
||||
// exists and cannot look at it, which is the whole point of asking for one.
|
||||
// A PDF or an unreadable file simply yields no image and the text stands on
|
||||
// its own, so a capture is never failed over the attachment.
|
||||
var images []zeroruntime.ImageBlock
|
||||
if image, err := imageinput.LoadFile(path, ""); err == nil {
|
||||
images = []zeroruntime.ImageBlock{image}
|
||||
}
|
||||
return Result{
|
||||
Status: StatusOK,
|
||||
Output: output,
|
||||
Images: images,
|
||||
Meta: map[string]string{
|
||||
"artifact_path": path,
|
||||
"action": request.action,
|
||||
|
||||
@@ -299,9 +299,6 @@ 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),
|
||||
|
||||
@@ -30,8 +30,8 @@ func tempDirOutsideDefaultTemp(t *testing.T) string {
|
||||
|
||||
func TestCoreReadOnlyToolsExposeSafeMetadata(t *testing.T) {
|
||||
toolset := CoreReadOnlyToolsScoped(t.TempDir(), nil)
|
||||
if len(toolset) != 10 {
|
||||
t.Fatalf("expected 10 core read-only tools, got %d", len(toolset))
|
||||
if len(toolset) != 9 {
|
||||
t.Fatalf("expected 9 core read-only tools, got %d", len(toolset))
|
||||
}
|
||||
|
||||
seen := map[string]bool{}
|
||||
@@ -71,11 +71,6 @@ func TestCoreReadOnlyToolsExposeSafeMetadata(t *testing.T) {
|
||||
if !seen[RequestPermissionsToolName] {
|
||||
t.Fatalf("expected %s in core read-only tools", RequestPermissionsToolName)
|
||||
}
|
||||
// By name as well as by count: the count alone still passes if one reader is
|
||||
// swapped for another.
|
||||
if !seen[ViewImageToolName] {
|
||||
t.Fatalf("expected %s in core read-only tools", ViewImageToolName)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegistryAllReturnsToolsSortedByName(t *testing.T) {
|
||||
|
||||
@@ -1,72 +0,0 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// The cheapest fix for #845/#847 is that the invalid shape is never emitted.
|
||||
// A model only learns "omit it" from the schema it is shown, so both shell
|
||||
// tools must say so in the field descriptions themselves — the validator errors
|
||||
// are the backstop, and by the time they fire a turn has already been burned.
|
||||
func TestShellToolsAdvertiseOmittingSandboxPermissionFields(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
for _, shell := range []struct {
|
||||
name string
|
||||
tool Tool
|
||||
}{
|
||||
{"bash", NewScopedBashTool(root, nil)},
|
||||
{"exec_command", NewScopedExecCommandTool(root, nil, nil)},
|
||||
} {
|
||||
properties := shell.tool.Parameters().Properties
|
||||
|
||||
mode, ok := properties["sandbox_permissions"]
|
||||
if !ok {
|
||||
t.Fatalf("%s: sandbox_permissions property missing", shell.name)
|
||||
}
|
||||
if !strings.Contains(mode.Description, "Omit it") {
|
||||
t.Errorf("%s: sandbox_permissions description = %q, want it to lead with omitting the field", shell.name, mode.Description)
|
||||
}
|
||||
// Deliberately NOT "read-only". An earlier revision said every read-only
|
||||
// command could omit the override, and that is false: `curl` and `gh` are
|
||||
// read-only and still need sandboxed network. Conditioning omission on
|
||||
// whether the command needs access it does not already have is the claim
|
||||
// that is actually true, and the one a model can act on without first
|
||||
// having to decide what counts as read-only.
|
||||
if !strings.Contains(mode.Description, "unless the command needs access") {
|
||||
t.Errorf("%s: sandbox_permissions description = %q, want omission conditioned on needing extra access", shell.name, mode.Description)
|
||||
}
|
||||
|
||||
extra, ok := properties["additional_permissions"]
|
||||
if !ok {
|
||||
t.Fatalf("%s: additional_permissions property missing", shell.name)
|
||||
}
|
||||
if !strings.Contains(extra.Description, "Omit this field entirely") {
|
||||
t.Errorf("%s: additional_permissions description = %q, want an explicit instruction to omit the field", shell.name, extra.Description)
|
||||
}
|
||||
// Both invalid shapes from the issue must be named as invalid up front,
|
||||
// so neither is emitted "to be safe".
|
||||
if !strings.Contains(extra.Description, "with any other mode is rejected") {
|
||||
t.Errorf("%s: additional_permissions description = %q, want it to say the unflagged shape is rejected", shell.name, extra.Description)
|
||||
}
|
||||
if !strings.Contains(extra.Description, "empty or all-false object") {
|
||||
t.Errorf("%s: additional_permissions description = %q, want it to say an empty payload is rejected", shell.name, extra.Description)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The two shell tools describe the same two fields; they were copy-pasted and
|
||||
// must not drift apart again, or a model would be taught different rules by
|
||||
// bash and exec_command in the same session.
|
||||
func TestShellToolsShareSandboxPermissionDescriptions(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
bash := NewScopedBashTool(root, nil).Parameters().Properties
|
||||
exec := NewScopedExecCommandTool(root, nil, nil).Parameters().Properties
|
||||
|
||||
for _, field := range []string{"sandbox_permissions", "additional_permissions"} {
|
||||
if bash[field].Description != exec[field].Description {
|
||||
t.Errorf("%s description differs between bash and exec_command:\n bash = %q\n exec = %q",
|
||||
field, bash[field].Description, exec[field].Description)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -5,7 +5,6 @@ import (
|
||||
|
||||
"github.com/Gitlawb/zero/internal/execution"
|
||||
"github.com/Gitlawb/zero/internal/sandbox"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
type SideEffect string
|
||||
@@ -103,16 +102,6 @@ type Result struct {
|
||||
// while callers migrate away from parsing text and string metadata.
|
||||
ExecutionRequest *execution.Request `json:"-"`
|
||||
ExecutionOutcome *execution.Outcome `json:"-"`
|
||||
// Images are pictures the tool is handing the model — a screenshot it just
|
||||
// captured, a file it was asked to look at. Text-only tools leave it nil.
|
||||
//
|
||||
// They are NOT delivered on this result's own tool-result message. Every
|
||||
// provider drops images there: Anthropic maps a tool result to a tool_result
|
||||
// block whose content is a string, Gemini to a functionResponse, and OpenAI
|
||||
// guards its image content-parts to the user role. The agent loop therefore
|
||||
// emits them as a following user message, which is also the only shape that
|
||||
// keeps one tool result per tool call.
|
||||
Images []zeroruntime.ImageBlock `json:"-"`
|
||||
// Redacted is set when secret scrubbing altered Output before it left the
|
||||
// tool-execution boundary.
|
||||
Redacted bool
|
||||
|
||||
@@ -1,80 +0,0 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/imageinput"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
// ViewImageToolName is the canonical registry name of the image viewer.
|
||||
const ViewImageToolName = "view_image"
|
||||
|
||||
// NewViewImageTool builds the workspace-scoped image viewer.
|
||||
func NewViewImageTool(workspaceRoot string) Tool {
|
||||
return NewScopedViewImageTool(workspaceRoot, nil)
|
||||
}
|
||||
|
||||
// NewScopedViewImageTool builds the image viewer bound to a path scope.
|
||||
//
|
||||
// The scope is not optional decoration. imageinput.LoadFile resolves a relative
|
||||
// path by joining it to the workspace root and otherwise takes the path as
|
||||
// given, so on its own it would happily read outside the workspace. Every path
|
||||
// goes through resolveScopedReadPath first, exactly like read_file, so this tool
|
||||
// grants no reach that read_file does not already have.
|
||||
func NewScopedViewImageTool(workspaceRoot string, scope PathScope) Tool {
|
||||
return viewImageTool{
|
||||
baseTool: baseTool{
|
||||
name: ViewImageToolName,
|
||||
description: "Look at an image file. Use this to see a screenshot, diagram, mockup or photo " +
|
||||
"the user mentioned but did not attach, or one a previous tool wrote to disk. " +
|
||||
"Accepts PNG, JPEG, GIF and WebP up to 10 MiB.",
|
||||
parameters: Schema{
|
||||
Type: "object",
|
||||
Properties: map[string]PropertySchema{
|
||||
"path": {Type: "string", Description: "Path of the image to look at."},
|
||||
},
|
||||
Required: []string{"path"},
|
||||
AdditionalProperties: false,
|
||||
},
|
||||
safety: readOnlySafety("Reads an image file without modifying anything."),
|
||||
// Same shape as read_file: reads one file, mutates nothing, and is
|
||||
// safe to run concurrently.
|
||||
capabilities: ToolCapabilities{Effect: EffectReadOnly, ThreadSafe: true, ResourceKeys: fileResourceKeys},
|
||||
},
|
||||
workspaceRoot: workspaceRoot,
|
||||
scope: scope,
|
||||
}
|
||||
}
|
||||
|
||||
type viewImageTool struct {
|
||||
baseTool
|
||||
workspaceRoot string
|
||||
scope PathScope
|
||||
}
|
||||
|
||||
func (tool viewImageTool) Run(_ context.Context, args map[string]any) Result {
|
||||
requestedPath, err := aliasedStringArg(args, []string{"path", "file", "file_path", "filepath", "filename", "image"}, "", true, false)
|
||||
if err != nil {
|
||||
return errorResult("Error: Invalid arguments for " + ViewImageToolName + ": " + err.Error())
|
||||
}
|
||||
absolutePath, relativePath, err := resolveScopedReadPath(tool.workspaceRoot, tool.scope, requestedPath)
|
||||
if err != nil {
|
||||
return errorResult("Error: " + err.Error())
|
||||
}
|
||||
image, err := imageinput.LoadFile(absolutePath, tool.workspaceRoot)
|
||||
if err != nil {
|
||||
// LoadFile's errors already name the problem (missing, too large,
|
||||
// unsupported type) and are safe to show: they carry the path the caller
|
||||
// asked for, which it already knows.
|
||||
return errorResult("Error viewing " + relativePath + ": " + err.Error())
|
||||
}
|
||||
return Result{
|
||||
Status: StatusOK,
|
||||
// The text says what arrived; the image itself reaches the model through
|
||||
// the agent loop, which delivers Images on a following user message.
|
||||
Output: "Viewing " + relativePath + " (" + image.MediaType + ").",
|
||||
Images: []zeroruntime.ImageBlock{image},
|
||||
Meta: map[string]string{"media_type": image.MediaType},
|
||||
}
|
||||
}
|
||||
@@ -1,100 +0,0 @@
|
||||
package tools
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// A 1x1 PNG. Small enough to inline, real enough to pass media-type sniffing.
|
||||
var onePixelPNG = []byte{
|
||||
0x89, 'P', 'N', 'G', 0x0d, 0x0a, 0x1a, 0x0a,
|
||||
0x00, 0x00, 0x00, 0x0d, 'I', 'H', 'D', 'R',
|
||||
0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01,
|
||||
0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4,
|
||||
0x89, 0x00, 0x00, 0x00, 0x0a, 'I', 'D', 'A', 'T',
|
||||
0x78, 0x9c, 0x63, 0x00, 0x01, 0x00, 0x00, 0x05,
|
||||
0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, 0x00, 0x00,
|
||||
0x00, 0x00, 'I', 'E', 'N', 'D', 0xae, 0x42, 0x60, 0x82,
|
||||
}
|
||||
|
||||
func TestViewImageReturnsTheImage(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(root, "shot.png"), onePixelPNG, 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
result := NewViewImageTool(root).Run(context.Background(), map[string]any{"path": "shot.png"})
|
||||
|
||||
if result.Status != StatusOK {
|
||||
t.Fatalf("status = %v: %s", result.Status, result.Output)
|
||||
}
|
||||
if len(result.Images) != 1 {
|
||||
t.Fatalf("got %d images, want 1 — the text alone is useless to the model", len(result.Images))
|
||||
}
|
||||
if result.Images[0].MediaType != "image/png" {
|
||||
t.Errorf("media type = %q, want image/png", result.Images[0].MediaType)
|
||||
}
|
||||
if len(result.Images[0].Data) != len(onePixelPNG) {
|
||||
t.Errorf("image data is %d bytes, want %d", len(result.Images[0].Data), len(onePixelPNG))
|
||||
}
|
||||
}
|
||||
|
||||
// The tool must not reach outside the workspace. imageinput.LoadFile does no
|
||||
// containment check of its own, so the scoping is entirely this tool's job.
|
||||
func TestViewImageRefusesPathsOutsideTheWorkspace(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
outside := t.TempDir()
|
||||
secret := filepath.Join(outside, "outside.png")
|
||||
if err := os.WriteFile(secret, onePixelPNG, 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tool := NewViewImageTool(root)
|
||||
|
||||
for name, path := range map[string]string{
|
||||
"absolute path outside": secret,
|
||||
"parent traversal": filepath.Join("..", filepath.Base(outside), "outside.png"),
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
result := tool.Run(context.Background(), map[string]any{"path": path})
|
||||
if result.Status == StatusOK {
|
||||
t.Fatalf("read an image outside the workspace: %s", result.Output)
|
||||
}
|
||||
if len(result.Images) != 0 {
|
||||
t.Error("a refused read still returned image bytes")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestViewImageRejectsNonImages(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
if err := os.WriteFile(filepath.Join(root, "notes.txt"), []byte("not an image"), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
result := NewViewImageTool(root).Run(context.Background(), map[string]any{"path": "notes.txt"})
|
||||
if result.Status == StatusOK {
|
||||
t.Fatalf("accepted a text file as an image: %s", result.Output)
|
||||
}
|
||||
if len(result.Images) != 0 {
|
||||
t.Error("a rejected file still returned image bytes")
|
||||
}
|
||||
}
|
||||
|
||||
func TestViewImageReportsAMissingFileClearly(t *testing.T) {
|
||||
result := NewViewImageTool(t.TempDir()).Run(context.Background(), map[string]any{"path": "nope.png"})
|
||||
if result.Status == StatusOK {
|
||||
t.Fatal("a missing file reported success")
|
||||
}
|
||||
// The error names the path the caller asked for, matching read_file. Both
|
||||
// also carry the resolved absolute path from the underlying syscall error;
|
||||
// that is pre-existing behaviour shared with read_file, not something this
|
||||
// tool introduces, so it is not asserted either way here.
|
||||
if !strings.Contains(result.Output, "nope.png") {
|
||||
t.Errorf("error should name the file, got %q", result.Output)
|
||||
}
|
||||
if len(result.Images) != 0 {
|
||||
t.Error("a failed read still returned image bytes")
|
||||
}
|
||||
}
|
||||
@@ -1,60 +0,0 @@
|
||||
package tui
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/charmbracelet/x/ansi"
|
||||
)
|
||||
|
||||
// A COPIED NUL KILLED THE WHOLE PROGRAM ON WINDOWS.
|
||||
//
|
||||
// github.com/atotto/clipboard's Windows path calls syscall.StringToUTF16, which
|
||||
// PANICS rather than returning an error when the string contains a NUL — so
|
||||
// selecting transcript text that carried one took the TUI down with "program
|
||||
// experienced a panic" and a stack ending in clipboard.writeAll. A teammate hit
|
||||
// it on a real Windows machine.
|
||||
//
|
||||
// The NUL reaches the clipboard because ansi.Strip removes escape SEQUENCES and
|
||||
// leaves C0 bytes alone, and the transcript carries them from two directions:
|
||||
// tool output (binary reads, git's -z listings) and this package's own card
|
||||
// protocol, whose prefixes are literally "\x00command-card\x00".
|
||||
func TestNulNeverReachesTheClipboard(t *testing.T) {
|
||||
// The card prefix is a real, in-repo source: ansi.Strip leaves it intact.
|
||||
if !strings.ContainsRune(ansi.Strip(commandCardTranscriptPrefix+"Profile"), 0) {
|
||||
t.Fatal("setup: the card prefix no longer carries a NUL; this test guards the wrong thing")
|
||||
}
|
||||
for _, tc := range []struct{ name, in string }{
|
||||
{"card prefix", commandCardTranscriptPrefix + "Profile\nstatus: ok"},
|
||||
{"plan card prefix", planCardTranscriptPrefix + "plan"},
|
||||
{"binary tool output", "before\x00after"},
|
||||
{"git -z listing", "a.go\x00b.go\x00c.go"},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := clipboardSafeText(tc.in)
|
||||
if strings.ContainsRune(got, 0) {
|
||||
t.Fatalf("a NUL survived to the clipboard write: %q — Windows would panic here", got)
|
||||
}
|
||||
// The readable content must survive: this sanitises, it does not truncate.
|
||||
for _, want := range strings.Split(strings.ReplaceAll(tc.in, "\x00", "\n"), "\n") {
|
||||
if want = strings.TrimSpace(want); want != "" && !strings.Contains(got, want) {
|
||||
t.Fatalf("sanitising dropped real content %q from %q", want, got)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Newlines and tabs are legal clipboard content and must survive — a multi-line
|
||||
// selection is the normal case, not the exception.
|
||||
func TestClipboardSanitisingKeepsLayout(t *testing.T) {
|
||||
in := "line one\n\tindented\nline three"
|
||||
if got := clipboardSafeText(in); got != in {
|
||||
t.Fatalf("clean multi-line text was altered:\n got %q\nwant %q", got, in)
|
||||
}
|
||||
// Other C0 bytes go: they would terminate the OSC52 escape early and
|
||||
// corrupt the copy even where they do not panic.
|
||||
if got := clipboardSafeText("a\x07b\x1bc"); got != "abc" {
|
||||
t.Fatalf("control bytes survived: %q", got)
|
||||
}
|
||||
}
|
||||
@@ -21,6 +21,7 @@ import (
|
||||
var responseStyles = []string{"balanced", "concise", "explanatory", "review"}
|
||||
|
||||
const tuiCompactionPreserveLast = 8
|
||||
const tuiCompactionMaxPromptChars = 8000
|
||||
const compactStatusRowID = "compact/status"
|
||||
|
||||
var compactFrames = []string{"⠂", "⠒", "⠲", "⠴"}
|
||||
@@ -841,8 +842,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,
|
||||
SkipPrompt: true,
|
||||
PreserveLast: tuiCompactionPreserveLast,
|
||||
MaxPromptChars: tuiCompactionMaxPromptChars,
|
||||
})
|
||||
if err != nil {
|
||||
return m, CompactResult{}, err
|
||||
@@ -856,26 +857,13 @@ func (m model) compactActiveSession() (model, CompactResult, error) {
|
||||
}, nil
|
||||
}
|
||||
|
||||
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)
|
||||
summary, err := m.summarizeCompactionPlan(plan)
|
||||
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: summaryResult.SummaryText,
|
||||
Summary: summary,
|
||||
}); err != nil {
|
||||
return m, CompactResult{}, err
|
||||
}
|
||||
@@ -897,47 +885,32 @@ func (m model) compactActiveSession() (model, CompactResult, error) {
|
||||
Compacted: len(events) < len(beforeEvents)+1,
|
||||
BeforeTokens: beforeTokens,
|
||||
AfterTokens: estimateTranscriptTokens(m.transcript),
|
||||
Summary: summaryResult.SummaryText,
|
||||
Summary: summary,
|
||||
}, 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
|
||||
func (m model) summarizeCompactionPlan(plan sessions.CompactionPlan) (string, error) {
|
||||
if m.provider == nil {
|
||||
return deterministicCompactionSummary(plan), nil
|
||||
}
|
||||
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
|
||||
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},
|
||||
},
|
||||
})
|
||||
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 {
|
||||
|
||||
@@ -481,15 +481,6 @@ 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",
|
||||
@@ -530,15 +521,6 @@ 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)
|
||||
}
|
||||
@@ -551,56 +533,6 @@ 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")
|
||||
|
||||
@@ -1499,54 +1499,7 @@ func (m model) selectedTranscriptText() string {
|
||||
return strings.Join(parts, "\n")
|
||||
}
|
||||
|
||||
// clipboardSafeText removes the control characters a clipboard write cannot
|
||||
// carry, keeping newlines and tabs.
|
||||
//
|
||||
// A NUL PANICS THE WHOLE PROGRAM ON WINDOWS, and that is not a theoretical
|
||||
// concern: syscall.StringToUTF16 — which github.com/atotto/clipboard calls on
|
||||
// Windows — PANICS rather than erroring when the string contains one, so a
|
||||
// single copied NUL killed the TUI with "program experienced a panic". It
|
||||
// reached the clipboard because ansi.Strip removes escape SEQUENCES and leaves
|
||||
// C0 bytes untouched, and the transcript legitimately carries them from two
|
||||
// directions: tool output (a read of a binary file, git's -z NUL-separated
|
||||
// listings) and this package's own card protocol, whose row prefixes are
|
||||
// literally "\x00command-card\x00".
|
||||
//
|
||||
// Stripped at the WRITE, not at each source, because the sources are unbounded:
|
||||
// any tool result a user can select from is a source, and a fix per source is
|
||||
// one that the next tool re-breaks. The same pass protects the OSC52 fallback,
|
||||
// where a stray control byte would terminate the escape sequence early and
|
||||
// corrupt the copy rather than crash.
|
||||
//
|
||||
// \n and \t survive: a multi-line selection is the normal case, and both are
|
||||
// legal clipboard content.
|
||||
func clipboardSafeText(text string) string {
|
||||
if !strings.ContainsFunc(text, unsafeClipboardRune) {
|
||||
return text
|
||||
}
|
||||
var out strings.Builder
|
||||
out.Grow(len(text))
|
||||
for _, r := range text {
|
||||
if unsafeClipboardRune(r) {
|
||||
continue
|
||||
}
|
||||
out.WriteRune(r)
|
||||
}
|
||||
return out.String()
|
||||
}
|
||||
|
||||
func unsafeClipboardRune(r rune) bool {
|
||||
if r == '\n' || r == '\t' {
|
||||
return false
|
||||
}
|
||||
return r < 0x20 || r == 0x7f
|
||||
}
|
||||
|
||||
func copyTranscriptSelectionCmd(text string) tea.Cmd {
|
||||
// SANITISED ONCE, HERE, before either destination sees it — the native
|
||||
// clipboard would panic on a NUL and the OSC52 fallback would be corrupted
|
||||
// by one.
|
||||
text = clipboardSafeText(text)
|
||||
return func() tea.Msg {
|
||||
// Prefer the native OS clipboard (pbcopy / clip.exe / xclip): it works on
|
||||
// local terminals — including macOS Terminal.app, which has no OSC52 support
|
||||
|
||||
@@ -93,14 +93,12 @@ type ReasoningBlock struct {
|
||||
|
||||
// Message is a normalized conversation turn passed to providers.
|
||||
type Message struct {
|
||||
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
|
||||
Role MessageRole
|
||||
Content string
|
||||
ToolCalls []ToolCall
|
||||
ToolCallID string
|
||||
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.
|
||||
|
||||
Reference in new issue
Block a user