mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 18:00:54 -10:00
perf/tool-effect-metadata
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 KRATOS
|
a3a19b26b6 |
perf(tools): add explicit tool effect metadata
Introduce fail-closed EffectClass and ToolCapabilities on every built-in tool so a later concurrent read-only batch PR can decide safety from explicit metadata rather than tool names. Plugin and MCP tools stay EffectUnknown. No concurrent execution is enabled. |
||
|
euxaristiaandClaude Fable 5
|
81aad58d97 |
fix(tools): Block MSYS coreutils under Windows sandbox (#476)
* fix(tools): block MSYS coreutils under Windows sandbox Git for Windows usr\bin coreutils (head, grep, cat, ...) are MSYS PE binaries that fail with ACCESS_DENIED and cygheap/signal-pipe errors when Zero runs them in the write-restricted Windows sandbox. Detect and block these commands before execution, surface MSYS runtime failures from stderr, steer agents toward native Zero tools or require_escalated, and exclude MSYS-prone names from known-safe command tails. Refs #458 * fix(tools): anchor the last MSYS failure heuristic to MSYS markers CodeRabbit review feedback: the win32-error-5 plus terminating check had no MSYS-specific anchor, so unrelated access-denied failures could be mislabeled as windows_msys_sandbox. Require an MSYS marker like the neighboring checks do. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(tools): let escalation bypass the MSYS guard and make detection quote-aware Addresses jatmn's review on PR #476: - bash and exec_command called detectShellCommandIssue before resolving sandbox_permissions into a command engine, so a require_escalated retry of a blocked command was still hard-blocked by the same preflight check. Resolve the command engine first and only bypass the windows_msys_sandbox issue kind when that resolves to true unsandboxed execution; other issue kinds (real cmd.exe syntax problems) still block regardless of escalation. - Unify all MSYS-prone command name checks (MsysProneCommandName, the preflight scan, and transitively command_prefix.go's known-safe guard) behind one windowsMsysProneNames set. This fixes expr (in the name set but missing from the old regex alternations, so never proactively flagged) and ls (hit the older windows_shell_syntax branch first, so it got different metadata than every other MSYS-prone name). - Make the preflight scan quote-aware: split the command into cmd.exe- operator-separated segments respecting double-quote grouping, and only check the first word of each segment instead of scanning raw text anywhere. Fixes false positives on quoted text like commit messages or PR comment bodies that merely mention a coreutil name. * test(tools): decouple MSYS-guard bypass tests from real command output TestBashToolRequireEscalatedMsysGuard and TestExecCommandRequireEscalatedBypassesMsysGuardAfterApproval asserted shell_issue == "" to prove the require_escalated bypass worked. Once bypassed, "cat somefile.txt" actually runs unsandboxed, and its real, PATH-dependent output could trip the unrelated post-execution detectShellOutputIssue heuristic, failing the assertion for reasons unrelated to the preflight guard under test. Assert on the exit_code "-1" sentinel (set only by shellIssueBlockResult) instead, which directly reflects whether the preflight guard blocked the command. * fix(tools): anchor MSYS path detection to command position, fix prefix approval gap Address jatmn's latest review on PR #476: - shell_runtime.go: the explicit MSYS binary-path check (usr\bin\, mingw64\bin\, msys-2.0.dll, cygwin1.dll) ran against the raw command string, so a quoted mention (e.g. a commit message describing a usr\bin\head.exe failure) was still hard-blocked. It now runs against the first word of each operator-separated segment, same as the coreutil-name check. windowsCommandSegments also stopped treating ; as a command separator (cmd.exe does not split on ;, unlike bash) and now respects cmd.exe's ^ escape character, so escaped operators like echo ^| head are no longer misread as a real head invocation. - command_prefix.go: proposedCommandPrefix could offer to approve a single segment (e.g. "ps aux" in "ps aux | head -5") without checking that every other segment was independently safe. Since approving a prefix escalates the whole command to bypass the sandbox, an MSYS-prone or otherwise unsafe segment could ride along uncovered. It now only proposes a segment as the prefix when every other segment in the command is known-safe on its own. Added regression tests for quoted MSYS path mentions, caret-escaped operators, semicolon text, and the prefix-approval gap. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(tools): treat redirection as a command boundary, stop using command text as output evidence Addresses jatmn's latest review on PR #476: - firstCommandWord only split on whitespace, so cmd.exe redirection attached directly to a command name (head>out.txt, cat<in.txt, grep>matches.txt) was read as one word and never matched against the MSYS-prone name set. It now also stops at < and >. - detectShellOutputIssue scanned command+output together, so a command whose own argument text merely quoted an MSYS failure string (e.g. a gh pr comment --body describing the bug) could be mislabeled after running even though the real stdout/stderr was unrelated. Removed the command parameter entirely from detectShellOutputIssue and both call sites (bash.go, exec_command.go), so the command line can never be used as post-execution evidence again. * fix(tools): remove unused truncateHeadTail wrapper CodeRabbit nitpick: truncateHeadTailWithTotal is called directly everywhere, so this wrapper is dead code that trips golangci-lint's unused check and fails CI. --------- Co-authored-by: Claude Fable 5 <[email protected]> |
||
|
Kevin Codex
|
3c81fea228 |
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. |
||
|
KRATOS
|
c0254e3266 |
fix(tui): context-fill gauge for custom/local Ollama models + related fixes (#356)
* fix(tui): show the context-fill gauge for custom/local Ollama models
The colored context gauge (used/window · N%) only renders when Zero knows
a model's context window. It resolves that two ways: a curated remote
catalog (how Grok/hosted models are known), or live /v1/models discovery —
but parseModelsResponse only ever extracts id/description from that
response; OpenAI-compatible model listings don't carry context-window
metadata at all. A custom/local Ollama model tag (e.g. a locally-pulled
":cloud"-tagged model) has no curated-catalog entry to borrow one from
either, so it always resolved to an unknown window and the gauge silently
fell back to a bare token count with no color grading.
Ollama does expose this, just via a different, Ollama-native endpoint
(/api/show, not /v1/models) whose model_info map reports it under an
architecture-prefixed key like "qwen2.context_length". Added
DiscoverOllamaContextWindow (providermodeldiscovery) to probe it, wired
through an async command scoped to the local "ollama" catalog ID only
(not "ollama-cloud", a different hosted service this endpoint isn't
assumed to exist on) — fired from Init() for the provider active at
launch, and from switchProviderModel for a provider switched to
mid-session (which previously warmed no discovery at all for the new
provider, a separate gap this also fixes).
New tests: DiscoverOllamaContextWindow's endpoint construction and
model_info parsing (including a mixed-type model_info and a missing
*.context_length key), the discovery command's scoping to local Ollama
only, the Update() handler populating modelContextWindow's fallback, and
switchProviderModel warming discovery for the newly active provider.
Local only — not pushed. I can't run a live Ollama daemon in this
sandbox, so the actual /api/show request/response shape (verified against
Ollama's documented API, not a live instance) needs confirming against a
real daemon before this goes out.
* fix(tui): stop showing the token count twice next to the context gauge
contextWindowSegment (the gauge) and usageStatusSegment (the plain "N tok"
figure) both read the exact same value (latestUsageTokens) — the code
already suppressed the plain figure when the sidebar shows it ("the token
figure shows on both sides"), but never extended that same dedup to the
gauge. So as soon as a model's context window becomes known (e.g. after
the previous commit's Ollama fix), the footer showed the identical number
twice: once inside the gauge, once again right next to it.
Fixed by falling back to cost-only whenever the gauge is shown, same as
the sidebar-active branch already does.
* fix: check collect()'s deadline even while output keeps draining
The wait deadline in execSession.collect() was only checked in the branch
reached when drainString() returns empty. A background process producing
output continuously enough to keep the buffer perpetually non-empty could
theoretically starve that check forever, turning the caller's requested
timeout into an unbounded block regardless of yield_time_ms.
Investigated as part of a cross-provider "stuck" report (multiple sessions
across different models/providers all hanging on the same prompt, each
with a background terminal running and no forward progress). Could not
empirically reproduce a hang via a synthetic continuous-writer test under
Go's real scheduler — the reader won the race often enough in practice
that this doesn't appear to be the actual cause of those reports. Keeping
the fix regardless since it closes a real, if hard-to-trigger, gap: the
deadline is now checked in both branches, so continuous output can never
prevent collect() from returning within its requested window.
|
||
|
KRATOS
|
2a8802e39c |
fix: cap unbounded exec_command output buffer to prevent OOM kill (#353)
* fix: cap unbounded exec_command output buffer to prevent OOM kill
execOutputBuffer.data accumulated every byte a backgrounded exec_command
session wrote, with no size cap, unlike buffer.recent which is correctly
bounded to recentExecOutputBytes for the /ps preview. drainString() only
clears data when the agent actually polls that session again — so a
long-lived background session nobody polls (e.g. a dev server left running
after the run that started it was cancelled) grows this buffer forever as
long as the process keeps writing, with no ceiling.
Traced this directly to a real crash: a session that had been running for
~5.5 hours (an earlier turn invoking swarm agents got cancelled mid-flight,
per that session's own event log) accumulated to ~42.7GB of compressed
memory, at which point macOS's memorystatus killed the process outright
("killing zero due to low-swap").
Fixed by capping data at maxExecOutputBufferBytes (2MiB), trimming from the
front like recent already does, and surfacing a one-time truncation notice
on the next drain so data loss is visible rather than silent.
* fix: pre-push review findings on the exec_command buffer-cap fix
Ran an adversarial review before opening the PR. It found the truncation
notice added alongside the buffer cap didn't actually work: prefixing
execOutputTruncatedMessage into drainString()'s text meant the notice
always sat ~2MiB before the end of the combined output, but the tool's
own downstream truncateExecOutput() keeps at most a 400KB tail (even at
the maximum allowed max_output_tokens) — so the notice was reliably
swallowed, or chopped into an unreadable fragment at smaller budgets.
Verified with direct reproductions against the real functions.
Fixed by moving the signal out of the text stream entirely:
- execOutputBuffer gains consumeTruncated() (read+reset, called once per
collect()) and peekTruncated() (read-only, for status views) instead of
injecting a marker into the drained bytes.
- collect() now returns (string, bool); both call sites thread the bool
through to execToolResult, which sets meta["output_buffer_truncated"]
and appends the notice to the response body *after* truncateExecOutput
runs, so it can never be re-truncated or land in a discarded region.
- Also flagged: a session torn down without ever being polled again after
truncation loses the signal entirely, since nothing drains/logs it on
teardown. Full audit-trail logging felt like overkill for a soft
transparency signal on top of a hard memory-safety fix, but exposed
peekTruncated() on ExecSessionSnapshot and surfaced it in the /ps row
(background_terminals.go) so it's visible any time a still-running
session is checked — covers the common case (the review's own scenario:
a long-lived dev server nobody's polling but that's still alive) even
though a session reaped before ever being looked at is still a gap.
New tests: reworked TestExecOutputBufferCapsUndrainedData for the
consume/peek API, and TestExecToolResultSurfacesBufferTruncationOutsideByteBudget
reproducing the review's exact failure case (max_output_tokens=1) and
proving the notice now survives it.
|
||
|
Anandan
|
d29ab98a79 |
Improve sandbox permission handling (#323)
Retry shell commands outside the sandbox when sandboxed output only reflects the sandbox namespace, and keep network retries sandboxed when a network grant is enough. Show user-facing permission reasons for escalated shell commands instead of exposing internal sandbox policy text. Tested: env GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/tui -run 'TestRunUsesEscalatedJustificationForPermissionPrompt|TestRunUsesUserFacingEscalatedFallbackForPermissionPrompt|TestPermissionPromptMapsEscalatedSandboxReason|TestRunRetriesShellUnsandboxedAfterSandboxNamespaceLimitedOutput' Tested: env GOCACHE=/tmp/zero-go-cache go test ./internal/tools Tested: env HOME=/tmp/zero-test-home-clean XDG_CONFIG_HOME=/tmp/zero-test-config-clean GOCACHE=/tmp/zero-go-cache go test ./... |
||
|
Anandan
|
80ab238361 |
Align sandbox command approval behavior (#322)
* Align shell approval sandbox execution Make approved shell command-prefix executions use the escalated sandbox path when denied-read policy allows it, so remembered approvals do not still hit the restrictive sandbox. Allow auto sandbox mode to degrade to a direct command plan when native command wrapping is unavailable, while strict sandbox require continues to fail closed. Update regression tests for degraded execution, prefix-approved shell calls, and unavailable native sandbox metadata. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/sandbox Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tools -run 'TestRegistryRunsWithDegradedUnavailableNativeSandbox|TestBashToolRunsWithDegradedUnavailableNativeSandbox|TestBashRequireEscalatedBypassesNativeSandboxAfterApproval|TestExecCommandRequireEscalatedBypassesNativeSandboxAfterApproval' Tested: GOCACHE=/tmp/zero-go-cache go test ./... -run '^$' Tested: git diff --check * Align sandbox command approval behavior Evaluate reusable shell approvals across plain command segments so prefix grants can cover commands with safe read-only tails without relying on command-specific sandbox exceptions. Keep process inspection commands sandboxed by default; unsandboxed host access continues to require explicit escalation or a narrow command-prefix approval. Improve shell tool guidance around sandboxed additional permissions, full escalation, justifications, and narrow reusable prefixes. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/tools ./internal/agent Tested: env HOME=/tmp/zero-test-home-clean XDG_CONFIG_HOME=/tmp/zero-test-config-clean GOCACHE=/tmp/zero-go-cache go test ./... * Normalize explicit sandbox backend platforms Infer the platform from a caller-provided sandbox backend name before choosing the manager runtime platform. This keeps Linux, macOS, and Windows backend plans deterministic across host OSes when tests or callers provide a backend without an explicit Platform field. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/tools Tested: env HOME=/tmp/zero-test-home-clean XDG_CONFIG_HOME=/tmp/zero-test-config-clean GOCACHE=/tmp/zero-go-cache go test ./... |
||
|
Anandan
|
6be50cd3a7 |
fix(sandbox): retry approved network shell commands (#301)
* fix(sandbox): retry approved network shell commands * test(sandbox): fix portable smoke expectations * fix(sandbox): restore Windows offline egress block |
||
|
KRATOSandgnanam1990
|
f7ee189ba5 |
fix: resolve 15 findings from the 2026-06-20 deep audit (#281)
* docs(audit): 2026-06-20 deep adversarial audit report
Audit-only deliverable (no source changed). Fresh origin/main (
|
||
|
Anandan
|
f5038151c8 |
Add managed background terminals (#270)
* Add managed background terminal sessions * Stop background terminals on TUI exit * Cover approved local network sandbox mode * Polish background terminal rendering * Tighten exec session cleanup guidance * Address background terminal review feedback * Kill exec session trees on Windows * Trigger CI for Windows cleanup |
||
|
Anandan
|
cf54e2cb0e |
Add persistent exec command sessions (#267)
* Add persistent exec command sessions Introduce exec_command and write_stdin tools backed by a shared session manager so long-running shell commands can remain alive and be polled or interrupted across tool calls. Wire exec_command through the existing shell permission, network prompt, command-prefix approval, sandbox grant scope, specialist manifest, and prompt/documentation paths. Keep web_fetch limited to public remote HTTP(S) URLs so localhost/private URLs go through shell commands where sandbox network permission applies. Tested: git diff --cached --check * Fix exec session lifecycle review issues |