Skip to content

Commit 3c81fea

Browse files
authored
feat: agent quality, caching, retry, and tooling upgrades (#506)
* feat: agent quality, caching, retry, and tooling upgrades Ten self-contained improvements to the core agent engine and tools: - edit_file: fuzzy fallback matching (7-strategy cascade with uniqueness protection and a disproportion guard) when old_string does not match byte-for-byte; exact match remains the fast path - edit_file/write_file: inline error-severity LSP diagnostics in tool output so the model sees a break it introduced in the same turn - lsp: expand server registry from 4 to 28 languages; doctor grades only tier-1 servers and lists the long tail informationally - agent: consecutive read-only tool calls execute concurrently (bounded at 8), consumed in original order, never spanning a mutating call - anthropic: cache_control breakpoints on the last two conversation messages so the transcript prefix stays a prompt-cache hit - providerio: retry schedule raised to 6 attempts with exponential 2s-30s backoff when the server sends no Retry-After - tools: truncated exec/bash output spills to a redacted temp file referenced in the truncation notice; bash output (previously unbounded) now shares exec_command's token budget - web_fetch: dependency-free HTML-to-markdown conversion (format: auto|raw|markdown) and larger limits (256KiB default, 2MiB max, 30s) - modelregistry: live models.dev overlay refreshes context limits and base pricing from a TTL-cached fetch; opt-in at the CLI entrypoint - tools: opt-in format-on-write (ZERO_FORMAT_ON_WRITE=1) runs standard formatters across 27 extensions, ordered before the file-tracker re-baseline so conflict detection is preserved * fix: address PR review feedback on fuzzy edits, diagnostics, spill - edit_file: adapt the replacement to the fuzzy-resolved span — when the span is a uniformly deeper-indented copy of old_string (line-trimmed / indentation-flexible matches), the same indentation delta is applied to new_string, and a span ending in a bare CR keeps its CR so CRLF files never gain a mixed line ending; block-anchor matches with drifted interiors are deliberately left untouched - diagnostics: render workspace-relative paths in inline post-edit diagnostics (base name outside the workspace) so absolute paths never leak the local username or directory layout into the model prompt or session transcript - spill: harden the spill directory for shared temp dirs — per-uid name, reject symlinked or non-directory paths (MkdirAll follows symlinks), and on Unix require current-user ownership - bash: the branch's output cap and truncation-flag threading are superseded by main's capture-time bounding (boundedBuffer + budgetBashCapture), which already sets Result.Truncated; this rebase adopts that design and keeps spill-to-disk on exec_command, where the full output is still buffered and a spill is genuinely complete * fix: address review round 2 — permission-event race, fuzzy ambiguity, spill reachability - agent: serialize OnPermission behind the same mutex as OnPermissionRequest in the parallel read batch. buildPermissionEvent fires for batched reads with a non-trivial sandbox decision (e.g. a grant-matched extra root), and front-end handlers append to shared session-recording state without their own locking — two batched reads emitting concurrently was a data race the pre-batch serial loop never allowed. - edit_file: reject two or more DISTINCT candidate spans from one fuzzy strategy as ambiguous instead of silently editing the first. Duplicate blocks at different indentation each resolve to a unique literal, so the previous occurrence-uniqueness check passed and wrote the wrong span. A single candidate occurring at multiple positions still cascades to stricter strategies as before. - tools: make spill files actually reachable by the recovery path the truncation notice advertises — resolveScopedReadPath now recognizes files inside the per-uid spill dir (containment verified after symlink resolution, so a planted link cannot smuggle an out-of-scope file through the gate); read_file/grep can follow the notice instead of re-running the command. * fix: surface competing block-anchor spans; make write-barrier test observe cross-probe order - edit_file: blockAnchorReplacer now returns every span clearing the similarity threshold (best first) instead of silently picking the single most-similar block. Hiding competitors from the cascade's distinct-candidate check could edit the wrong block when two blocks share anchors and both interiors are plausible; one clear winner still resolves, two plausible blocks now report ambiguity (covered by a two-block test that asserts the file is untouched). - agent tests: the parallel write-barrier test now records both probes into one mutex-guarded shared log and asserts the cross-probe ordering it always claimed to (write starts after the first read batch ends; second read batch starts after the write ends). The previous version only compared indices within the read probe's own log, which a regression letting the write overlap a batch could still pass; the stray no-op mutex assignment is gone.
1 parent f6c0606 commit 3c81fea

37 files changed

Lines changed: 3155 additions & 59 deletions

‎internal/agent/file_diagnostics.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package agent
2+
3+
import (
4+
"context"
5+
"os"
6+
"path/filepath"
7+
"strings"
8+
"time"
9+
10+
"github.com/Gitlawb/zero/internal/lsp"
11+
)
12+
13+
// fileDiagnosticsTimeout bounds one inline post-edit diagnostics check so a
14+
// slow or wedged language server can never hang a tool call; on timeout the
15+
// edit simply reports without a diagnostics block.
16+
const fileDiagnosticsTimeout = 10 * time.Second
17+
18+
// NewFileDiagnostics adapts an *lsp.Manager to the per-edit inline diagnostics
19+
// callback (tools.RunOptions.Diagnostics): it reads the just-written file,
20+
// checks it against the file's language server, and formats error-severity
21+
// diagnostics for the model. Warnings and hints are excluded — nagging about
22+
// style on every edit is noise, while a type error the edit just introduced is
23+
// exactly what the model should see before its next step. Diagnostics are
24+
// rendered with workspace-relative paths: the absolute path would put the
25+
// local username/home directory into the model prompt and session transcript
26+
// on every edit. Returns nil when manager is nil, disabling inline diagnostics
27+
// entirely.
28+
func NewFileDiagnostics(manager *lsp.Manager, workspaceRoot string) func(context.Context, string) string {
29+
if manager == nil {
30+
return nil
31+
}
32+
return func(ctx context.Context, absPath string) string {
33+
text, err := os.ReadFile(absPath)
34+
if err != nil {
35+
return ""
36+
}
37+
checkCtx, cancel := context.WithTimeout(ctx, fileDiagnosticsTimeout)
38+
defer cancel()
39+
diagnostics, err := manager.Check(checkCtx, absPath, string(text))
40+
if err != nil {
41+
return ""
42+
}
43+
errors := lsp.FilterBySeverity(diagnostics, lsp.SeverityError)
44+
if len(errors) == 0 {
45+
return ""
46+
}
47+
return lsp.FormatDiagnostics(diagnosticsDisplayPath(workspaceRoot, absPath), errors)
48+
}
49+
}
50+
51+
// diagnosticsDisplayPath renders absPath relative to the workspace root for
52+
// model-facing output, falling back to the file's base name when the path is
53+
// outside the workspace (a bare name still identifies the file without
54+
// exposing the directory layout).
55+
func diagnosticsDisplayPath(workspaceRoot, absPath string) string {
56+
if workspaceRoot != "" {
57+
if rel, err := filepath.Rel(workspaceRoot, absPath); err == nil && !strings.HasPrefix(rel, "..") {
58+
return rel
59+
}
60+
}
61+
return filepath.Base(absPath)
62+
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package agent
2+
3+
import (
4+
"path/filepath"
5+
"testing"
6+
)
7+
8+
// Diagnostics are model-facing: absolute paths would leak the local username
9+
// and directory layout into the prompt and session transcript on every edit.
10+
func TestDiagnosticsDisplayPath(t *testing.T) {
11+
root := filepath.Join("/Users", "someone", "project")
12+
cases := []struct {
13+
root, abs, want string
14+
}{
15+
{root, filepath.Join(root, "internal", "a.go"), filepath.Join("internal", "a.go")},
16+
{root, filepath.Join("/etc", "other.go"), "other.go"}, // outside root -> base name only
17+
{"", filepath.Join("/home", "user", "x.go"), "x.go"}, // no root -> base name only
18+
}
19+
for _, c := range cases {
20+
if got := diagnosticsDisplayPath(c.root, c.abs); got != c.want {
21+
t.Errorf("diagnosticsDisplayPath(%q, %q) = %q, want %q", c.root, c.abs, got, c.want)
22+
}
23+
}
24+
}

‎internal/agent/loop.go‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,11 +495,35 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
495495
// between tool_results breaks strict provider replay) — same after-batch
496496
// rationale as turnRequestedModel above.
497497
var changedFilesThisBatch []string
498+
// Parallel read-ahead state: results for calls[precomputedStart:precomputedEnd]
499+
// executed concurrently, consumed strictly in order below.
500+
var precomputed []precomputedToolResult
501+
precomputedStart, precomputedEnd := 0, 0
498502
for index, call := range collected.ToolCalls {
503+
// When this call starts a consecutive run of >= 2 auto-allowed read-only
504+
// calls, execute the whole run concurrently now (see parallel_tools.go).
505+
// The scan happens lazily at the run's first index — never ahead of a
506+
// pending mutating call — so read-after-write ordering is preserved.
507+
if index >= precomputedEnd {
508+
runEnd := index
509+
for runEnd < len(collected.ToolCalls) && parallelSafeToolCall(registry, collected.ToolCalls[runEnd], options) {
510+
runEnd++
511+
}
512+
if runEnd-index >= 2 {
513+
precomputed = executeParallelReadBatch(ctx, registry, collected.ToolCalls, index, runEnd, permissionMode, options)
514+
precomputedStart, precomputedEnd = index, runEnd
515+
}
516+
}
499517
if options.OnToolCall != nil {
500518
options.OnToolCall(call)
501519
}
502-
toolResult, abortErr := executeToolCall(ctx, registry, call, permissionMode, options)
520+
var toolResult ToolResult
521+
var abortErr error
522+
if index >= precomputedStart && index < precomputedEnd {
523+
toolResult, abortErr = precomputed[index-precomputedStart].result, precomputed[index-precomputedStart].abortErr
524+
} else {
525+
toolResult, abortErr = executeToolCall(ctx, registry, call, permissionMode, options)
526+
}
503527
if options.OnToolResult != nil {
504528
options.OnToolResult(toolResult)
505529
}
@@ -1071,6 +1095,8 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
10711095
// Per-session file version tracker so write_file/edit_file refuse to clobber
10721096
// a file that changed on disk outside Zero since it was last read.
10731097
FileTracker: options.FileTracker,
1098+
// Inline post-edit diagnostics for mutating tools (nil = disabled).
1099+
Diagnostics: options.FileDiagnostics,
10741100
// Forward the run's operator tool filters so a filter-aware tool
10751101
// (tool_search) never discloses or loads an operator-hidden deferred tool.
10761102
EnabledTools: options.EnabledTools,

‎internal/agent/parallel_tools.go‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package agent
2+
3+
import (
4+
"context"
5+
"sync"
6+
7+
"github.com/Gitlawb/zero/internal/tools"
8+
)
9+
10+
// Parallel read-ahead for tool batches. When a turn requests several
11+
// independent lookups (read_file + grep + glob is the common shape), executing
12+
// them one after another serializes pure I/O waits. A consecutive run of
13+
// auto-allowed read-only calls is executed concurrently instead; results are
14+
// then consumed in the original call order, so guard counters, message
15+
// ordering, abort semantics, and the surface's call/result event pairing are
16+
// byte-identical to sequential execution. Runs never span a mutating call: a
17+
// read that follows a write must observe the write, so eligibility is decided
18+
// per consecutive run, not per batch.
19+
20+
// maxParallelReadTools bounds concurrent read-only tool executions in a turn.
21+
const maxParallelReadTools = 8
22+
23+
// precomputedToolResult is one parallel read-ahead execution, keyed back to
24+
// its batch index by the caller.
25+
type precomputedToolResult struct {
26+
result ToolResult
27+
abortErr error
28+
}
29+
30+
// parallelSafeToolCall reports whether call may run concurrently with its
31+
// neighbors: the tool must exist, be side-effect-free (SideEffectRead), and be
32+
// auto-allowed for these args, so no interactive prompt or workspace mutation
33+
// is on the hot path. Loop-intercepted tools (ask_user, request_permissions)
34+
// and tool_search (mutates the deferred-tool set) stay sequential.
35+
func parallelSafeToolCall(registry *tools.Registry, call ToolCall, options Options) bool {
36+
switch call.Name {
37+
case "ask_user", tools.RequestPermissionsToolName, tools.ToolSearchToolName:
38+
return false
39+
}
40+
tool, found := registry.Get(call.Name)
41+
if !found || tool.Safety().SideEffect != tools.SideEffectRead {
42+
return false
43+
}
44+
args := map[string]any{}
45+
if call.Arguments != "" {
46+
if err := decodeToolArguments(call.Arguments, &args); err != nil {
47+
return false
48+
}
49+
}
50+
return effectivePermission(tool, args) == tools.PermissionAllow
51+
}
52+
53+
// executeParallelReadBatch runs calls[start:end] concurrently (bounded by
54+
// maxParallelReadTools) and returns results indexed relative to start. All
55+
// execution-side callbacks that can fire inside executeToolCall are serialized
56+
// behind one mutex: a permission prompt (a sandbox preflight can demand one
57+
// even for an auto-allowed read) must never appear twice at once on an
58+
// interactive front-end, and OnPermission event handlers append to shared
59+
// session-recording state without their own locking — two batched reads under
60+
// a granted extra root would otherwise race (the pre-batch serial loop never
61+
// had two callbacks in flight at once).
62+
func executeParallelReadBatch(ctx context.Context, registry *tools.Registry, calls []ToolCall, start, end int, permissionMode PermissionMode, options Options) []precomputedToolResult {
63+
batchOptions := options
64+
var callbackMutex sync.Mutex
65+
if options.OnPermissionRequest != nil {
66+
inner := options.OnPermissionRequest
67+
batchOptions.OnPermissionRequest = func(ctx context.Context, request PermissionRequest) (PermissionDecision, error) {
68+
callbackMutex.Lock()
69+
defer callbackMutex.Unlock()
70+
return inner(ctx, request)
71+
}
72+
}
73+
if options.OnPermission != nil {
74+
inner := options.OnPermission
75+
batchOptions.OnPermission = func(event PermissionEvent) {
76+
callbackMutex.Lock()
77+
defer callbackMutex.Unlock()
78+
inner(event)
79+
}
80+
}
81+
82+
results := make([]precomputedToolResult, end-start)
83+
semaphore := make(chan struct{}, maxParallelReadTools)
84+
var waitGroup sync.WaitGroup
85+
for index := start; index < end; index++ {
86+
waitGroup.Add(1)
87+
go func(index int) {
88+
defer waitGroup.Done()
89+
semaphore <- struct{}{}
90+
defer func() { <-semaphore }()
91+
result, abortErr := executeToolCall(ctx, registry, calls[index], permissionMode, batchOptions)
92+
results[index-start] = precomputedToolResult{result: result, abortErr: abortErr}
93+
}(index)
94+
}
95+
waitGroup.Wait()
96+
return results
97+
}

0 commit comments

Comments
 (0)