mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 23:40:28 -10:00
feat/skill-invocation
70
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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. |
||
|
Anandan
|
afb5010a46 |
polish v2: smoothness & delight (#484)
* feat(tui): actionable hints on provider/model errors Classify a failed turn's provider error into auth / rate-limit / connectivity / model-not-found / context-overflow and render a one-line next step below the red error row, instead of dumping an identical raw blob for every failure mode. The classifier lives in a new internal/errhint package so both the TUI error row and the CLI exec path can share it (TUIHint references slash commands, CLIHint references zero subcommands). Classification is a conservative string heuristic keyed off providerio.ClassifiedError's prefixes plus lower-level DNS/TLS/timeout/context-length signatures, since the numeric HTTP status is gone by the time the error reaches a UI surface. * feat(tools): budget bash output with head+tail truncation bash was the one tool that returned command output with no byte cap — 'cat large.log' or a verbose test run could dump megabytes into context and force compaction, while every other read/search tool already applies a budget. Cap stdout and stderr at 96KB each, keeping the head and tail of an oversized stream (build/test failures usually land at the tail) and dropping the middle behind a marker that points at redirect-to-file + read_file. Record raw_bytes/emitted_bytes/estimated_tokens/truncated in Meta like the other tools. Sandbox-denial and shell-issue detection still run on the full raw output before budgeting. * fix(gemini): retry 401 with refreshed OAuth token The Gemini provider used providerio.SendWithRetry (429/503/529 only), so an expired OAuth token surfaced as a terminal 'API key not valid' auth error while OpenAI and Anthropic silently recovered via SendWithAuthRetry's force-refresh-and-replay on 401. Plumb OAuthResolver through gemini.Options/Provider and the factory (exactly as the other two providers already do) and switch the stream send to SendWithAuthRetry. With a nil resolver (API-key users) behavior is unchanged. * fix(errhint): gate classification on provider-origin marker Address CodeRabbit review on #1: agentResponseMsg.err can also carry local failures (a tool's 'permission denied', a 'file does not exist', a config error), and broad substrings would have attached a bogus /provider or /model hint to those. Classify now returns Unknown unless the message carries a provider marker the provider layer always attaches (auth error: / rate limit error: / provider error: / provider request error: / provider stream error:). Local errors never draw a provider hint; provider errors sub-classify as before. Added local-failure test cases proving they stay Unknown. * feat(cli): actionable hint on text-mode exec provider errors zero exec printed provider failures as a bare '[zero] <raw error>' with no next step. Append a one-line hint (reusing the shared errhint classifier) for recognized provider failures — 'run `zero auth`', 'run `zero doctor`', etc. The provider-origin gate means non-provider codes (sandbox_error, mcp_error) and JSON/stream-json output are untouched. * fix(tools): set Result.Truncated for budgeted bash output Address CodeRabbit review on #3: budgetBashOutput recorded truncation in meta["truncated"] but never set the Result.Truncated struct field, so consumers reading the struct (like glob/web_fetch/read_minified do) would miss truncated bash results. Return the bool from budgetBashOutput and set Result.Truncated on every return path. * perf(tui): reuse one LSP manager across prompts runAgentWithOptions built a fresh lsp.NewManager per run and shut it down when the run returned, so gopls (and every other language server) cold-started on the first edit of every turn — 200ms-2s of latency the manager's own design (long-lived, reused servers) exists to avoid. Build one manager per session in newModel (cheap: servers start lazily on first Check) and reuse it across runs; the SelfCorrector still gets a fresh checker wrapping the shared manager each run. Torn down with a short deadline in quit(). Runs fall back to a per-run manager when the session manager is absent (cwd unknown, or a directly-built test model). * perf(tui): accumulate streamingText as []byte (O(1) append) m.streamingText += msg.delta was O(len) per delta -> O(n²) across a long generation (a 10k-token code gen allocates hundreds of MB of intermediate strings and the UI gets progressively laggier). Accumulate into a []byte with append (O(1) amortized) and read via streamingTextString(). A []byte rather than strings.Builder because the TUI model is copied by value on every Update, which would trip strings.Builder's copy check; []byte is nil-safe and copies cleanly like the other slice fields. * perf(tui): coalesce streamed text deltas to one frame Every OnText delta was its own tea.Msg, so a fast provider (100+ tok/s) drove 100+ full Update->View cycles per second, each re-parsing the growing markdown — visible stutter over SSH and wasted CPU. Batch agentTextMsg deltas at the runtime sink over a ~16ms frame and forward them as a single message, decoupling render rate from token rate. Any non-text message flushes pending text first so ordering with tool-call / reasoning / row messages is preserved; the final agentResponseMsg (a tea.Cmd return, not a sink message) is safe because the model already drops deltas for an inactive runID. * fix(agent): raise tool-failure stop threshold 4->6 guardrails.go halted a run after 4 consecutive same-error tool failures. A corrective hint fires at 2, and a model iterating on a genuinely tricky edit can legitimately fail a couple more times after the hint while converging — stopping at 4 cut those runs short. Raise to 6; the streak still resets the instant the tool succeeds or hits a different error, so only true same-error loops are affected. * fix(agent): more reconnect retries with jitter; narrow to transport errors #19: bump maxStreamReconnects 2->4 and add up-to-50% jitter on the exponential backoff (capped at 8s), so a multi-second network blip is ridden out instead of killing the run on a 2s hiccup, and concurrent runs (swarms, cron fleets) don't reconnect in lockstep. #34: drop the '502'/'503' substring matches from shouldReconnect. 503 already exhausted providerio.SendWithRetry (retrying here is a redundant double-retry) and 502 is non-idempotent by providerio's rule (the POST may have been processed). Only genuine transport failures — where no response was received — reconnect now. * fix(agent): reduce stall retries 2->1 to bound stuck-session time A no-output stream stall is detected only after the full stream idle timeout (~5min) elapses, and each retry can idle again — so 2 retries left an interactive session frozen for ~15min. Drop to 1 retry: keeps the common single-hiccup recovery while bounding the worst case to ~2x the idle timeout. * fix(tui): serialize coalescer forwarding to preserve message order Address CodeRabbit review on #7: flush() and the run-switch path drained the text buffer under c.mu but called forward() after releasing it, so a timer-fired text flush could race a concurrent non-text send() and land after the tool-call/reasoning message it should precede. Hold c.mu across drain AND forward (drainAndForwardLocked), so whoever holds the lock delivers atomically and the other caller blocks until it is done — text can never overtake a following non-text message. Added a concurrency stress test (timer racing an inline send) that passes under -race. * feat(tui): accept /compact now Bare /compact already triggers compaction, but /compact now — what users reach for when the context gauge climbs — hit the usage-error branch. Accept the 'now' keyword and advertise it in the command usage. * feat(tui): add /retry, /edit, /copy, /export commands /retry resends the last prompt; /edit recalls it into the composer to tweak and resend — both read a new lastPrompt field captured verbatim (pre-expansion) in launchPrompt. /copy puts the last answer on the clipboard via the existing copy machinery; /export writes a plain role-prefixed transcript to a file (timestamped default, or a given path). Real gaps for SSH users with no mouse select. * feat(tools): separator-insensitive tool_search matching tool_search ranked deferred tools by exact substring, so a model that typed 'webfetch' (dropping the underscore) matched nothing and looped. Add a separator-squashing fallback ('web_fetch' -> 'webfetch') scored below an exact substring match, so precise queries still rank first. * feat(tools): signal ask_user dismissal distinctly from a blank field Empty answers all rendered as '(no answer provided)', so the model couldn't tell a wholesale dismissal (user closed the prompt without answering anything) from one field left blank amid real answers — and might invent a default. FormatAskUserAnswers now flags a full dismissal up front as a skip and marks individual empties '(left blank)' vs '(skipped)'. * feat(tui): fuzzy ranking in model/session pickers The pickers filtered by flat strings.Contains with no ranking, so finding 'sonnet 4.5' among 50+ models meant an exact prefix or scrolling. Rank matches (exact < prefix < contains < subsequence, reusing fuzzySubsequenceGap) so the closest match lands on top, and 'snt45' now matches 'Sonnet 4.5'. Groups stay contiguous — ordered by their best match, never split — so the grouped model picker still renders one header per provider. Covers both /model and /resume (shared commandPicker). * perf(agent): cache per-tool schema render across turns partitionTools re-ran the recursive schema->map conversion (schemaToRuntimeMap) for every tool on every turn — redundant, since a tool's advertised name/description/schema is stable for the run. Add a per-run definition cache keyed by tool name (nil-safe; the plain partitionTools entrypoint and tests pass nil for fresh renders). The partitioning itself (visibility, deferral, ordering) still recomputes every turn — it must, because a tool's deferred state can flip mid-run (swarm tools un-defer once a swarm is active). Only the expensive schema render is memoized. tool_search is excluded by its callers (dynamic description) so it never poisons the cache. * perf(agent): calibrate compaction estimate against real prompt tokens The byte/4 heuristic (ApproxTextTokens) over-counts code-heavy content ~15-20%, so with triggerRatio=0.7 compaction fired at ~60% of true capacity — premature summarizer calls that degrade quality. Each turn now folds (rawEstimate, provider InputTokens) into an EMA calibration ratio, clamped per-sample to a sane band; maybeCompact and the reactive path scale their estimates by it. Turn 1 uses the raw estimate (no data yet); later turns compact near real capacity. * style: gofmt the new agent test files The CI gofmt check flagged the #22/#25 test files (struct-field and switch alignment). No behavior change. * feat(agent): turn-based plan staleness reminder The stale-plan nudge only fired after 10 tool calls since the last update_plan, so a plan that drifts stale across many low-tool-call turns (one call per turn) took many turns to catch — the 'plan set turn 1 stays pending forever' case. Add a turn-count complement: after stalePlanTurnThreshold (8) turns without an update AND with items still pending, fire the same one-shot reminder. Gated on pending items so a fully-completed plan is never nagged; the existing tool-call trigger is unchanged. (The roadmap's fuzzy 'tool calls unrelated to plan items' relevance match is deliberately omitted — too false-positive-prone.) * feat(agent): preserve recent-edits summary across compaction Compaction preserved the plan, loaded skills, tool schemas, and project instructions, but not file edits — so after the editing turns were elided the model knew it had changed a file only vaguely, and would re-read to rediscover its own footprint. Extract write_file/edit_file targets from the elided middle with a one-line note from each tool result, merge them across repeated compactions (newer note per path wins), and carry them in the preserved-state JSON (capped at 20 paths / 160 bytes per note). * feat(tui): workspace-scoped /resume + fewer event reads /resume listed sessions from every project globally, and read the full event file for each of them to test for resumable content. Filter the picker (and /resume latest) by the current workspace Cwd, checked BEFORE the per-session read so a large global history no longer pays N full file reads to build one workspace's list; also skip zero-event sessions via metadata without any read. Sessions with no recorded Cwd (older runs) and an unknown current workspace stay visible so nothing is hidden that can't be confidently placed. Explicit /resume <id> still resolves any session. * feat(tui): warn on model switch that drops staged images Staged images were silently dropped at submit when the active model had no vision support — the user only learned after sending. Chips already render above the composer (renderAttachmentChips); add an immediate amber warning line to the /model switch status (both same-provider and cross-provider paths) when images are staged and the newly selected model can't accept them, so the drop is surfaced at switch time. * feat(tui): inline recovery affordance on setup errors A first-run setup error rendered as a bare red 'error: <text>' with no pointer to the fix. Append a faint, stage-tailored recovery line (edit the endpoint/name/key above then Enter, or pick another model) that names only keys the stage's footer confirms, so the guidance is always accurate. Empty for stages whose footer already makes recovery obvious. * fix: address CodeRabbit review (5 findings) - loop.go: recompute the compaction calibration estimate at calibration time, not request-build time, so a reactive-compaction reissue that shrinks the request feeds the EMA the sample that actually completed. - errhint: match bare HTTP status codes (401/403/429/529) only at digit boundaries, so an incidental number ('4290ms', 'id 14015') isn't mis-bucketed as auth/rate-limit. - bash: drop the stray unmatched ']' in the output-truncation marker. - tui /retry: apply the same exiting/compactInFlight guards commandPrompt has, so a retry can't race compactResultMsg's rewrite of session state. - tui /export: write the transcript 0o600 (it may contain echoed secrets), not world/group-readable 0o644. * test(tui): skip export file-mode assertion on Windows os.WriteFile ignores Unix permission bits on Windows and Stat reports 0666, so the 0o600 assertion failed there. The production 0o600 is honored on Linux/macOS (where it matters); guard the check with runtime.GOOS so Windows smoke passes. * fix(tui): align /resume latest filters + case-insensitive workspace match on Windows Address CodeRabbit review on #21: - latestResumableInWorkspace now applies the SAME filters as the picker (skip zero-event metadata and empty/failed runs), so /resume latest can't land on a session the picker intentionally hides. - sessionMatchesWorkspace compares paths case-insensitively on Windows (its filesystem is), so the same workspace spelled with different casing no longer hides resumable sessions; other platforms stay case-sensitive. * fix(tui): /retry resends the last prompt's image/PDF attachments /retry relaunched only lastPrompt's text, but launchPrompt clears the pending image/document queues once a turn is sent. A vision- or PDF-backed prompt that failed would silently retry as text-only and answer a different task. Snapshot the consumed attachments at launch and re-stage them on /retry so launchPrompt rebuilds an identical request (document preamble, images, and the submit-time vision re-check). startNewSession drops the snapshot so a post-/new /retry cannot leak the previous session's attachments. Also correct the OAuthResolver factory comment, which omitted the Gemini provider it is now passed to. * fix: address jatmn review (reconnect 5xx, /edit attachments, edit-cap, coalescer test) - reconnect: exclude HTTP 5xx (500/502/503/504) before the transport substring match so "504 Gateway Timeout" no longer slips through on the generic "timeout" needle. A gateway timeout is non-idempotent — the completion POST may already have reached the model — so replaying the connect risked duplicate billable work. Reuses a digit-boundary status matcher exported from errhint (HasStatusCode). Test asserts 504/500 stay non-reconnectable. - tui /edit: re-stage the remembered image/PDF snapshot alongside the recalled prompt text, mirroring the /retry fix — editing a vision- or document-backed prompt no longer silently resends a text-only version. - compaction recent-edits: order edits by LAST edit, not first, and move a re-touched path to the newest position on merge, so the maxRecentEdits tail cap keeps the file the model most recently edited instead of dropping it. Adds mergeRecentEdits (edit-specific) and a capped-list regression. - coalescer test: inject the frame timer via afterFunc so TestCoalescerBatchesDeltas proves buffering deterministically instead of racing the real 16ms wall-clock timer. * fix(tools): bound bash output capture in memory, not just model-visible text Previously stdout/stderr streamed into unbounded bytes.Buffers and were only truncated to the 96 KiB budget AFTER the command finished, so a runaway command (cat huge.log, yes) could grow Zero's memory until it stalled or OOMed before budgetBashOutput ever ran. Replace the capture with boundedBuffer, an io.Writer that keeps only the head+tail each stream will surface and discards the middle as it arrives, while counting the true total for the truncation marker. Memory is now bounded to ~head+2×tail per stream regardless of output size. truncateHeadTail is refactored onto a total-aware core so the string path (and its tests) are unchanged, and budgetBashCapture reports the true raw_bytes even though only a bounded slice was held. Adds boundedBuffer/capture unit tests and an end-to-end test that streams ~5× the budget and asserts bounded emission with an accurate raw_bytes. |
||
|
euxaristiaandClaude Fable 5
|
190241bd59 |
fix(tools): fix cmd.exe /S/C corrupting commands with embedded quotes (#465)
* fix(tools): fix cmd.exe /S/C corrupting commands with embedded quotes Any shell command whose own text contains embedded double quotes - python -c "print(15 / 3)", git commit -m "message", node -e "..." - was silently corrupted on Windows before reaching the target program. Reported live: a model ran python -c "print(15 / 3)" and got back a SyntaxError showing the code Python received as literally "print(15, truncated right before the /. Root cause: bash.go passes the full command text as ONE argv element to cmd.exe /d /s /c, and Go's os/exec escapes it correctly for a standard CommandLineToArgvW-style parser (wrapping it in quotes, backslash-escaping the embedded ones). But cmd.exe's own /C remainder parsing does not follow those rules once /S is present: per its documented behavior, it strips the FIRST and LAST literal quote character in the whole remainder - not a matched pair - before handing the (now corrupted) text along to be re-tokenized. The escaping was correct for one layer of parsing; the command line actually goes through two. Fix: on Windows, write commandText to a temp .cmd script file and point /c at the file's path instead of the raw command text. This sidesteps cmd.exe's /C-remainder heuristic entirely, since the remainder is then just a plain path with no quotes to mis-strip. @echo off is prepended since cmd.exe treats a /c target ending in .cmd as a batch file and echoes each line it runs by default, which would otherwise pollute captured stdout (caught by the existing bash tool test suite before this commit, not left in). buildBashCommand now returns a cleanup func (in addition to plan.Cleanup) for the temp file. Both callers updated: bash.go's one-shot run defers it immediately; exec_command.go's persistent session composes it with startExecProcess's own cleanup and runs it after the session's process actually exits, not before. This fix lives upstream of the sandbox wrapping, in how the Name/Args spec is constructed before either the direct or sandboxed code path consumes it, so both benefit without separate sandbox-specific changes - cmd.exe's own command-line parsing does not depend on what token launched it. New TestBashToolPreservesEmbeddedQuotesOnWindows pins the exact failure shape via a self-dispatch test helper (a quoted argument containing a space and a slash, matching the original bug report's truncation point), with no dependency on python being installed. Full internal/tools suite green, excluding 3 failures pre-existing and identical on unmodified main in my environment (unrelated path-assumption issues under a deeply nested working directory). Co-Authored-By: Claude Fable 5 <[email protected]> * fix(tools): pass the shell command line to cmd.exe raw instead of via a temp script Review on the original fix found real problems with running commandText from a temporary .cmd file: it silently changed cmd.exe command-line syntax into batch-file syntax (for example, for %i loops need %%i only inside an actual .bat/.cmd file), /s was still corrupting the script path itself if it contained spaces, and the script leaked on sandbox planning errors. The actual root cause was narrower than the temp-file approach assumed. cmd.exe's own /C remainder parsing is not CommandLineToArgvW-compatible: it strips the first and last literal quote from the remainder whenever it starts with a quote, without undoing backslash-escaping. Passing commandText as a normal exec.Cmd argument makes Go wrap it in exactly that shape (quoted because it has spaces, inner quotes escaped as \") since it contains embedded quotes, which is what the stripping then corrupts. Fixes it at the source instead: commandText now reaches cmd.exe completely unescaped (via SysProcAttr.CmdLine for the unwrapped path, and the same treatment inside the sandboxed runner's own process creation for the wrapped path), exactly as if it had been typed at an interactive prompt. No outer quotes means the stripping heuristic never triggers for a command like `python -c "print(15 / 3)"`, and a command that legitimately starts with a quoted executable path is handled by cmd.exe's own careful quote-preserving special case for that shape. Also drops /s, which forced the more aggressive of cmd.exe's two stripping behaviors. No temp file, so nothing to leak on error, and no batch-file reinterpretation of the command text. internal/sandbox/windows_runner.go now owns the shared shell-argv shape (WindowsShellArgs/WindowsShellCommandLine) so internal/tools/bash.go and the sandboxed runner's windowsCommandLine build the exact same raw command line instead of duplicating the flag literals in two places. * chore: retrigger CI The Windows smoke job failed on TestLoadOrCreateSecretConcurrentConverges in internal/oauth (a lock-file access-denied flake), unrelated to this change. internal/tools and internal/sandbox, the packages this PR touches, both passed. Retriggering to confirm. --------- Co-authored-by: Claude Fable 5 <[email protected]> |
||
|
euxaristiaandClaude Fable 5
|
ba6c026469 |
fix(tools): make grep and glob respect run cancellation (#464)
* fix(tools): make grep and glob respect run cancellation grep and glob's Run/RunWithSandbox discarded their context parameter entirely (bound to _) and ran a synchronous filepath.WalkDir with no way to know it should stop. An unscoped search over a large tree - a broad parent directory, not just the workspace, which the model can and does reach for - could run long enough to be effectively unkillable: the TUI's Ctrl+C/exit path calls the stored CancelFunc and immediately marks the run "cancelled" in the UI, but the underlying walk kept running regardless, because nothing in it ever checked ctx.Err(). Worse, the TUI's quit path explicitly waits for the cancelled run's own response (to drain flushRunIDs, so /rewind checkpoints stay consistent) before it will actually exit - so with the walk unable to hear the cancellation, neither a second Ctrl+C nor /exit could complete. The only way out was killing the process. Thread ctx through Run/RunWithSandbox into grepFiles/scanGlob and check ctx.Err() at the top of each WalkDir callback, ahead of walkErr, so a cancelled run aborts the walk on its very next entry instead of visiting every remaining file. Checked the rest of the tools package for the same _ context.Context pattern: list_directory (single-level os.ReadDir, no walk), skill, ask_user, and update_plan don't touch the filesystem, so grep and glob were the only two tools with this gap. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(tools): thread cancellation through grep's match-collection phase The walk-level fix in grepFiles stopped a cancelled search from visiting new filesystem entries, but collectGrepMatches still read and regex-scanned every file the walk had already found, so a search that had discovered its full file list before cancellation kept running regardless. Adds the same per-item ctx.Err() check to that phase, plus a deterministic test that cancels after a fixed number of files instead of relying on timing. * test(tools): add mid-walk cancellation coverage and shared helper Add deterministic mid-walk tests for grepFiles and scanGlob using countingCancelContext, and extract searchCancelledResult so grep/glob share the same cancellation Result mapping. * style(tools): gofmt search_cancel.go (add missing trailing newline) Co-Authored-By: Claude Fable 5 <[email protected]> --------- Co-authored-by: Claude Fable 5 <[email protected]> |
||
|
PierrunoYTandClaude Sonnet 5
|
5658a36627 |
fix(tools): flag piped POSIX utilities before running on Windows (#412)
* fix(tools): flag piped POSIX utilities (head/tail/grep/...) before running on Windows Zero always executes the bash tool through native cmd.exe on Windows (shellExecutable() in bash.go), but the pre-flight Windows-syntax detector only caught bash-style `cd /...` and bare `ls`. Anything piped into head/tail/grep/wc/awk/sed/cut/xargs/tr — very common in model-generated commands like `git log ... | head` — ran straight through to cmd.exe, which has no such builtins, wasting a turn on a guaranteed failure. Reported in the wild via `git log ... | head` and a `git blame ... | grep` pipeline. detectShellCommandIssue now also flags these before the command runs, with the same "translate to cmd.exe / use Zero's native tools" guidance already used for the cd/ls cases. Note: the existing detectShellOutputIssue path (matching cmd.exe's error text post-execution) only recognizes English strings and will miss this on non-English Windows locales — the pre-flight check above is locale-independent since it inspects the model-generated command text rather than cmd.exe's translated error output. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(tools): require whitespace/end after POSIX utility name, not just \b CodeRabbit review on PR #412: windowsPosixUtilityPattern used \b after the utility alternation, but \b only requires a non-word character next — so tail-log.sh, grep-cli, and sed.exe would all false-positive (the hyphen/dot satisfies \b without being a real command boundary). Match windowsLSCommandPattern's existing convention: require whitespace or end-of-string after the utility name instead. Co-Authored-By: Claude Sonnet 5 <[email protected]> --------- Co-authored-by: Claude Sonnet 5 <[email protected]> |
||
|
Gautam ManchandaniandGautam Manchandani
|
960db9660e |
fix(tools): require permission before web_search requests (#382)
* fix(tools): gate web_search behind network permission * chore(tools): clarify web_search permission reason --------- Co-authored-by: Gautam Manchandani <[email protected]> |
||
|
Kunj SHAH
|
33dc7ae2cc |
fix(tools): CRLF line ending mismatch in edit_file tool on Windows (#378)
* fix(tools): CRLF line ending mismatch in edit_file tool on Windows * fix: resolve gofmt error, add robust CRLF guard, and fix Windows sandbox and OAuth test flakiness |
||
|
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.
|
||
|
KRATOS
|
fb3277f46e |
feat(tui): tabbed ask_user questionnaire in the composer with suggested answers (#327)
* feat(tui): tabbed ask_user questionnaire rendered in the composer
Redesign the interactive ask_user prompt to match the requested mockup: when a
question is raised the composer text box becomes the questionnaire.
- Multi-question prompts render as a row of TABS (one per question + a trailing
Confirm tab). Tab / Shift+Tab switch questions, answers are given in any order,
and Enter on Confirm submits them all. A single-question prompt submits on
answer (no Confirm step) and shows no tab row.
- Each option can carry a one-line DESCRIPTION (contract: per-option
optionDescriptions, or option objects {label, description}); options render
numbered with the description dimmer underneath and the recommended one tagged.
- Per-question optional `header` gives each tab a short title (falls back to a
trimmed question).
- Rendered in the composer/footer region (footerView), replacing the text box
while active, instead of a scrolling transcript card.
- Keys: Tab move · ↑↓ select · Enter confirm · Esc dismiss; a printable key in
the picker drops into free-text; Esc from "type your own" returns to the picker;
multi-select questions are free-text with the options shown as suggestions.
- Headless unaffected (zero exec never wires OnAskUser).
Removed the old single-question sequential state (cursor/typing/index,
renderFocusedAskUserPrompt, resolveAskUser). Tests rewritten for the tabbed flow
(picker default, type-your-own, multi-question tabs + Confirm, Esc dismiss,
descriptions render). make build / go vet / go test ./... -race / make lint green.
* fix(tui): ask_user questionnaire renders on the black terminal canvas
It was painting a gray panel background (styledBlockFill with zeroTheme.panel +
onPanel-wrapped foregrounds). Since the questionnaire replaces the composer, it
should match it: bare foregrounds on the terminal canvas (black) with the
composer's lineStrong border and no fill. The lime cursor/badge highlights and
the recommended tag stay.
* fix(tui): address PR review — schema/parser alignment, single-question Tab
- ask_user schema: options.items is declared as string, so advertise only the
string form (descriptions go via the parallel optionDescriptions array). The
parser stays object-tolerant defensively, but the schema no longer tells
strict-schema providers to send objects it would reject.
- moveAskUserTab no-ops for single-question prompts (no tab strip / Confirm tab),
so Tab/Shift+Tab can't move the prompt into the hidden Confirm state. Test added.
|
||
|
KRATOS
|
1d1fa7c514 |
feat(tui): ask_user suggested answers with a recommended default (#326)
* feat(tui): suggested answers with a recommended default for ask_user When the agent asks a clarifying question mid-task, it can now offer 2-4 suggested answers and mark one recommended; the interactive TUI renders a selector (arrow keys, lime cursor, "(recommended)" tag) with the cursor resting on the recommended option, plus a trailing "✎ Type my own answer…" entry that drops into the existing free-text composer. Whichever the user picks — a suggested option's text or their typed answer — flows through the same answer channel the agent loop already waits on. Optional and backward-compatible: a question with no options renders the plain free-text prompt exactly as before. Headless `zero exec` is unaffected (it never wires OnAskUser; the non-interactive best-assumption fallback stands). - contract: agent.AskUserQuestion / tools.AskUserQuestion gain Recommended; the tool schema advertises optional options + recommended; parser keeps recommended only when it resolves to one of the options (canonical text). - system prompt: tells the model it MAY supply options + a recommended one. - TUI: pendingAskUserPrompt gains cursor/typing; ask_user_prompt.go holds the selector logic mirroring permission_prompt.go; renderFocusedAskUserPrompt renders the picker or the free-text prompt. Tests: selector defaults to recommended and returns its text; "type my own" falls back to free-text and returns the typed text; no-options stays plain free-text. make build / go vet / go test ./... -race / make lint green. * fix(tui): ask_user — single input, Esc steps back from "type my own" Two UX fixes from interactive testing: - No double display: the focused ask card no longer echoes the in-progress free-text answer (it was shown both inside the card AND in the composer text box). The composer is now the single input; the card just shows the question and a hint. renderFocusedAskUserPrompt no longer takes the input string. - Esc from "type my own" returns to the selector for that question instead of cancelling the whole questionnaire (escapeAskUser); a question with no options, or the selector itself, still cancels on Esc as before. Tests: type-my-own asserts the card does not echo the typed text and the hint flips to "type your own answer"; new test covers Esc returning to the selector (no cancel, no answer delivered) then selecting a suggested option. * fix(tui): ask_user review fixes — multi-select, selector typing, dead code Address the three confirmed findings from the code review: - multi-select no longer silently narrowed to one option: a single-pick selector cannot represent MultiSelect, so a multi-select question now renders as free-text with its options surfaced as suggestions; the user types one or more and the answer is returned verbatim (syncQuestionState; escapeAskUser and the renderer updated to match). - typing in selector mode no longer discarded: a printable keystroke while the selector is showing flips into free-text and captures the key (model.go key capture), instead of accumulating in a hidden input that Enter then overrode with the highlighted option. - removed dead submitAskUserAnswer (the Enter handler now routes through confirmAskUser; the orphan had no callers). Tests: multi-select falls back to free-text and returns the typed answer; a printable key in the selector switches to free-text and is captured. Full gate green (build / vet / go test ./... -race / make lint). * fix(tui): address PR review — element-wise option compare, clearer Esc copy - ask_user_test: compare option slices with slices.Equal instead of strings.Join, so the parser test can't be fooled by ["A,B"] vs ["A","B"] or nil vs [""]. - ask_user prompt: the Esc hint said "skip", which reads like "skip this question" on a multi-question prompt even though Esc ends the rest of the questionnaire (remaining answered by best-assumption). Reworded to "skip the rest" in the selector and free-text hints; "type my own" keeps "go back". |
||
|
Anandan
|
b8b0bb7f65 |
Move deferred tool discovery into tool search (#324)
* Move deferred tool discovery into tool search Keep deferred tool discovery in the tool_search definition instead of appending it as a per-turn user message, so models do not answer or acknowledge internal discovery metadata. Preserve deferred loading behavior: hidden tools remain hidden until loaded, tool_search stays reachable for allowlisted deferred tools, and retry paths keep loaded/deferred state. Tested: env GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/tools -run 'Test.*Deferred|Test.*ToolSearch|TestRunLoadsDeferredToolThenAdvertisesNextTurn|TestRunReactiveRetryKeepsLoadedDeferredToolAndDiscovery|TestRunConnectTimeReactiveRetryKeepsLoadedDeferredToolAndDiscovery|TestAllowlistedDeferredToolsKeepToolSearchReachable|TestDisabledToolSearchFallsBackToEager' Tested: env GOCACHE=/tmp/zero-go-cache go test ./internal/tools Tested: env GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/tools ./internal/cli Tested: git diff --check * Clarify deferred tool search selection |
||
|
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
|
f1502f0bfe |
Add local control browser and terminal tools (#319)
* Add local control browser and terminal tools Add configurable local browser, desktop, and terminal control helpers with packaged helper discovery for install and release flows. Improve sandbox scope classification for local control commands and shared temporary paths. Add structured browser actions, terminal session line input/key normalization/read support, and artifact capture for local-control outputs. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/localcontrol ./internal/tools ./internal/cli ./internal/config -run 'Test.*LocalControl|TestTerminal|TestBrowser|TestDesktop|TestCapture' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/localcontrol ./internal/tools ./internal/cli ./internal/config ./internal/agent Tested: tuistory smoke launched bash, sent interactive input, waited for expected output, and captured a no-cursor snapshot Tested: git diff --cached --check * Fix npm wrapper helper manifest test on macOS Canonicalize expected helper paths in the npm wrapper manifest test so macOS /var and /private/var temp path aliases compare correctly. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/npmwrapper Tested: git diff --check * Harden local control review fixes Respect resolved local-control config before tool listing and filter validation while preserving stream-json usage error ordering. Harden browser, desktop, and artifact tool validation; guard Linux-only app launch behavior; clear stale helper manifests from the npm wrapper. Make helper packaging lockfile-based with npm ci and add installer/package tests for helper staging. Tested: focused local-control/tool/CLI/release/install/sandbox suite Tested: isolated full go test ./... Tested: macOS and Windows compile-only checks for touched packages |
||
|
KRATOS
|
0e7318ff72 |
feat(tui): clickable plan-step detail + plan/sidebar UX & rendering polish (#315)
* fix(tui): plan panel reflects live progress + prompt cadence
The plan panel could read as stuck mid-task even while work continued. Three
fixes:
- Header showed steps[0] forever, so the title never advanced as the plan
progressed; show the current step (in_progress, else first incomplete).
- updateFromItems matched steps by content only, so an in-place reword reset a
step's elapsed timer to zero; add positional carry-over when the step count is
unchanged.
- Prompt: tell the model to mark each step completed + the next in_progress
before starting it, so the plan tracks real progress instead of jumping at the
end.
Regression tests included.
* feat(tui): click a plan step to see what was built for it
Each plan step now records the file mutations (write_file/edit_file/apply_patch)
made while it was in_progress, captured from tool-result rows. Clicking a step
row in the context sidebar drops a transcript card listing those changes — the
implementation for that step. Step->line mapping mirrors sidebarAgentSelectables'
offset accounting (one line per step); capture is keyed to the active step and
cleared per run. Read-only and additive. Tests cover the offset math + capture.
* feat(tui): plan-step detail shows diffs + commands
Extend the clickable plan-step detail: capture bash/exec_command runs in addition
to file mutations, store each tool's full output (the diff for edit/apply_patch,
stdout/stderr for commands), and render the card grouped into Changes and
Commands with a short truncated excerpt of each item's diff/output.
* fix(tui): plan-step detail toggles instead of stacking duplicates
Re-clicking a plan step now hides its card (a stable transcript id + drop-by-id),
and clicking a different step replaces it, so at most one detail card is shown.
Previously each click appended a fresh card and they piled up.
* feat(tui): elaborate plan-step detail by status (what we did / will do)
* feat(tui): plan-step card leads with a prose explanation (agent's own narration)
* feat(tui): write plan-step explanations with the model on click
Clicking a PLAN step now shows a fresh, plain-English write-up of that
step, generated on demand by the active model:
- past tense ('what we did') for completed/failed steps, future tense
('what we'll do') for pending/in_progress steps
- immediate feedback: the card drops in showing 'Writing explanation…'
and updates in place (by stable row id) when the text returns
- cached per step (content+status) so re-clicking is instant with no
second model call; re-clicking still toggles the card closed
- the prompt is built from already-captured local data (step text,
status, notes, plus a compact digest of file edits, commands, and the
agent's narration) and asks for short, non-technical prose
- reuses the TUI's existing provider via a one-shot StreamCompletion;
no new client, no hardcoded provider. Falls back to the local summary
when no provider is configured or the request fails.
* fix(tui): minimal plan-step card styling (dim border, status-tinted title)
* feat(tui): auto-hide context sidebar when it has no agents or plan
* feat(tui): widen chat reading column to fill wide terminals with breathing room
* fix(tui): toggle context sidebar silently (no transcript notice)
* fix(tui): mark plan complete when a finished task forgot the final update_plan
* fix(tui): remove the lime streaming-text glow (render responses in static ink)
* fix(tui): remove the shaded panel band behind tool-result cards
* fix(tui): Ctrl+B hides the plan entirely (no pinned-panel fallback)
* style(tui): gofmt-align model struct fields
* feat: narrate the build as a clean story (hide plumbing cards, live plan progress, narration prompt, write_file line counts)
* feat(tui): render agent narration bright (ink) so the build reads as a story
* feat(agent): demand an elaborate, structured closing summary (not a single line)
* feat(tui): mark agent narration with a leading bullet (selection-aligned)
* feat: post-turn recap line (※ recap:) with /config recaps on|off toggle
* feat(tui): show 'still generating…' during quiet stretches + count tool-call streaming
* feat(tui): ACTIVITY panel under PLAN + push the model to step the plan per file
* fix(tui): show the quiet 'still generating' hint only when the sidebar is hidden
* feat(tui): code preview in write/edit/apply cards (card-only, no model token cost)
* fix(tui): address CodeRabbit review — stale plan timestamps, stale explanation, Ctrl+B persistence, UTF-8 truncation
* fix: address CodeRabbit re-review round 2 (7 actionable + 2 nitpicks)
* chore: drop accidentally-committed WIP internal/cli/dryrun_test.go
It is an unrelated, pre-existing dry-run test that does not compile (references a
non-existent agent.Options.DryRun field, an unused import, and out-of-scope
sandbox symbols). It was swept into the prior commit by 'git add -A' and is not
part of this PR's scope.
|
||
|
Vasanth T
|
f9a26e7148 |
feat(tools): show red/green diff preview on write_file and edit_file (#318)
write_file/edit_file only reported "Created X (8462 bytes)" / "edited X", so the user never saw WHAT changed — no code preview, no red/green. The TUI already renders a colored diff card whenever a tool's output looks like a unified diff (diffFirstToolBodyRenderer + looksLikeDiff), so the fix is purely tool-side: emit a diff. - edit_file appends a unified diff (old vs new) after its summary line → red/green change preview. - write_file appends a unified diff of prior vs new content: a fresh create previews as all-green additions, an overwrite as red/green. It reads the prior bytes before writing only when the file existed. - boundedUnifiedDiff (via the already-vendored go-udiff, now a direct require) caps the emitted diff at 48KB so a large generated file can't flood the transcript or balloon persisted session events; past the cap only the summary shows. The summary stays the first output line so any consumer that reads it (swarm/Task result bubbling) is unaffected. Tests: edit emits @@/-/+ ; new-file emits additions-only; overwrite emits red/green. Full tools/tui/agent/swarm suites green. |
||
|
Anandan
|
644f61fa58 |
Fix shared temp roots in sandbox scope (#314)
Treat platform temp directories as default writable roots so file tools, shell commands, and sandbox profiles share the same temp view. On Linux, bind host temp roots instead of mounting an isolated tmpfs at /tmp so artifacts created under /tmp remain visible across tool and shell calls. Tests: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/tools ./internal/cli ./internal/agent |
||
|
Anandan
|
212f1a0ff8 | Fix token accounting and usage display (#304) | ||
|
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 |
||
|
KRATOS
|
b776a9281f |
fix(sandbox): grant ancestor metadata so cd into the workspace works (#302)
* fix(sandbox): grant ancestor metadata so cd into the workspace works macOS seatbelt granted file-read* on each read root via (subpath "<root>"), which covers the root and its descendants but NOT its parent directories. An absolute cd (and any path the kernel canonicalises) must stat each ancestor to traverse to a deeply-nested workspace like /Users/me/Downloads/app — those parents weren't granted, so seatbelt denied the chdir and surfaced it as the misleading 'cd: <path>: Not a directory' (ENOTDIR). This blocked the agent from cd-ing into its own workspace (e.g. 'cd /abs/workspace && go test'). Fix: grant file-read-metadata + file-test-existence on the path-ancestors of each read root (metadata only — ancestor contents stay unreadable). Same pattern already used for /opt/homebrew, /usr/local, /System/Volumes/Data/private. Verified end-to-end: 'cd /abs/workspace && ...' now succeeds under the real generated profile. go build/vet/test green. * fix(sandbox): skip path-ancestors for filesystem roots (invalid SBPL) Follow-up to the ancestor-metadata traversal fix. A read root of "/" has no ancestors, and (path-ancestors "/") is invalid SBPL — sandbox-exec aborts with exit 65 and the sandboxed command can't launch at all. This was reachable via the compat builder (ReadRoots=["/"] flipped to restricted under EnforceWorkspace) and any degenerate "/" workspace. seatbeltAncestorMetadataRule now skips any root that is its own parent (filepath.Dir(clean)==clean), neutralizing "/" and volume roots regardless of entry point. Verified: the compat profile now compiles under sandbox-exec (exit 0). Found by adversarial verification of the prior commit. * fix(sandbox): grant the CLT toolchain so git/clang/make work On macOS, /usr/bin/git (and clang, make, etc.) are thin stubs that resolve the real binary under the active developer dir — usually /Library/Developer/CommandLineTools. The sandbox's platform read roots didn't include it, so the stub couldn't reach the real tool and failed with the misleading 'xcode-select: No developer tools were found' error. Add /Library/Developer (read-only) to the platform read roots. * fix(sandbox): allow reading the user's global git config git reads identity/config from the user's global git config, but the sandbox confines reads away from HOME, so it failed with 'unable to access ~/.gitconfig: Operation not permitted'. Grant read-only on the git config FILES specifically (~/.gitconfig and ~/.config/git/config) — not the ~/.config/git directory, which can hold an XDG credential store — so git runs while credentials and the rest of HOME stay unreadable. The path-ancestors traversal grant covers reaching them. * fix(sandbox): let the agent terminate user-owned processes on request The seatbelt profile only allowed signaling the agent's own process group ((target self) (target pgrp)), so a process started by a PREVIOUS session — e.g. a dev server still listening on a port — was in a different group and could not be killed. The agent would thrash through kill/pkill/osascript/sudo, all denied, and never stop the process the user asked it to terminate. Relax to (allow signal). The kernel still enforces UID ownership: a sandboxed command runs as the user, so it can only signal the user's own processes, never root's or another user's. Verified end-to-end: a sandboxed command now kills a different-group user-owned process under the real generated profile. * fix(sandbox): let the agent find processes + guide it to working tools Pairs with the signal fix (8b5018d). Two more gaps blocked process management: - process-info was scoped to (target same-sandbox), so the agent couldn't even inspect processes outside its own sandbox. Relaxed to (allow process-info*). - `ps` is setuid root and macOS sandbox-exec refuses to exec setuid binaries (privilege-escalation guard) — unfixable via profile. `pgrep` needs a blocked system service. So the agent kept thrashing on ps/pgrep/os.kill. Add macOS shell guidance pointing the model at the tools that DO work: lsof to find a process, kill to stop it. With the signal grant, lsof+kill now manages processes reliably (verified end-to-end). * fix(sandbox): grant git config at the seatbelt rule, not the shared profile The git-config read paths (~/.gitconfig, ~/.config/git/config) were added to the cross-platform permissionProfileReadRoots, so they leaked HOME-dependent absolute paths into the platform-agnostic 'sandbox policy --json' snapshot and broke the golden test on every OS (machine-specific paths + Windows separators). Move the grant to seatbeltReadRule (macOS only, where reads are actually confined): the PermissionProfile stays platform-agnostic so the golden is stable, and the seatbelt profile still grants the config files + their ancestor metadata for traversal. Verified: golden test green, full 'go test ./...' green, and git still reads ~/.gitconfig under the real generated profile. * fix(sandbox): grant ancestor metadata for platform read roots too Addresses CodeRabbit review on #302. seatbeltAncestorMetadataRule was passed only fs.ReadRoots + gitConfig, on the assumption that platform read roots are top-level or already covered. /Library/Developer (the CLT toolchain) breaks that: it needs /Library stat-able as an ancestor, so a chdir-style traversal into it (cd /Library/Developer/CommandLineTools) failed with ENOTDIR even though reads succeeded (exec doesn't require ancestor-stat the way chdir does). Include the platform read roots in ancestorRoots when IncludePlatformRoots is set. Verified: cd into the CLT toolchain now succeeds under the real profile; golden + sandbox tests green. Regression test added. |
||
|
KRATOS
|
63c0e278ff |
feat(tui): chat + context sidebar redesign, AGENTS panel, animations (#300)
* feat(tui): chat + context sidebar redesign, AGENTS panel, animations, plan-timer fix
Two-column managed-mode layout: the existing chat (scroll engine, reduced width)
on the left + a new right context sidebar — AGENTS (spawned subagents: status
glyph, name, and a live working detail for running ones, with a running/total
count), the live PLAN, and a token readout. The sidebar is suppressed on the
home screen and under overlays/subchat; alt-screen managed mode, wide terminals.
Also: freeze the plan clock while the agent is idle (an in_progress step left
mid-plan when the agent yields stops ticking instead of counting forever), and a
clean-room animation port (phase scroll indicator + a cosine ripple working
line; the spinner was already animated, frozen under ZERO_REDUCED_MOTION).
Reuses the scroll/viewport engine; inline mode unchanged. New unit tests for the
sidebar, plan-clock freeze, and the pure animation helpers; go build/vet/test green.
* feat(swarm): live AGENTS sidebar + un-defer coordination tools
Sidebar AGENTS section now reflects the swarm/team accurately:
- name each member by a short 1-2 word task (not subagent-N)
- a mild, slow cool pulse on live member names (no per-letter flicker)
- members disappear when swarm_status reports them [done]/[failed]
- hide bogus "specialist not found" tool-misroute attempts
- keep the animation tick alive while sidebar agents exist
Plus the deeper fix so a coordinating model stops misrouting swarm calls
to the specialist tool ("specialist swarm_send not found"): the swarm
COORDINATION tools (swarm_send/status/inbox/collect) un-defer once a
swarm is active, while spawn/schedule/handoff stay discoverable via
tool_search. A new DeferralEligible() interface keeps all swarm tools
counting toward DeferThreshold even when exposed, so un-deferring can
never deactivate deferral / force-expose MCP tools in any config.
* feat(tui): polish pass — hierarchy, status line, motion, sidebar toggle
A grounded polish pass across the TUI (17 items):
- color: spread the gray tiers (AA-safe), lime working-ripple (no semantic
green/amber), calm steady ask-label, dim completed plan-step bodies,
bold-muted sidebar headers + stateful PLAN count
- status line: run-state chip (permission mode + effort) replaces the
duplicated provider; composer rule is model-only; context-fill gauge down to
the narrow tier + a sidebar context-% chip
- footer: persistent idle key-hint (managed mode) + a jump-to-bottom cue
- motion: pulsing streaming caret; running specialist spins; swarm/specialist
agents linger with a fading ✓ before removal (no abrupt pop)
- layout: Ctrl+B toggles the sidebar; padded ' │ ' divider; sidebar gated to
the medium tier so it never starves the chat
- home screen: cwd · branch · model orientation + example prompts
- cleanup: drop dead autoTag, fix 'six swarm tools' miscount
go build/vet/test green; -race clean; no hex outside theme.go.
* feat(tui): reading column for the chat transcript
Cap transcript body rows at a readable ~90-col measure and indent them by a
small left gutter, so on a wide terminal the chat text no longer runs
edge-to-edge. Only true transcript rows use the reading column; the frame,
composer, status line, sidebar, title bar, empty state, and prompts keep the
full chatColumnWidth. Streaming text shares the column so it doesn't snap on
finalize. Plus extra vertical spacing before headings and between paragraphs.
Implementation keeps the height cache valid (rows render at contentWidth, which
the width-keyed cache already captures) and the gutter is a horizontal post-pad;
each selectable line's textStart is shifted by the gutter so click-to-select and
the selection highlight stay aligned. Mouse card/toggle hit-testing is Y-based,
so it's unaffected. go build/vet/test green; -race clean.
* fix(tui): plain cancellation marker + copyable card/toggle rows
- 'Run cancelled.' renders as a plain amber '⊘ Run cancelled.' line instead of a
heavy box (other system notices keep their box).
- Copy root cause: the app's text-selection skipped 'button' rows (collapsible
toggles, specialist cards) — they carried no selectable text, so a drag-select
omitted exactly that (dull) content. Fix: those rows now carry their plain text
+ start, so a selection dragged THROUGH them copies their content; a direct
click still toggles/drills (resolved on press, before selection). The selection
highlight no longer excludes them. Permission buttons stay excluded.
- Surface Ctrl+E in the idle hint — it releases the mouse for native terminal
selection, which copies everything (the zero-risk full-copy path).
go build/vet/test green; -race clean.
* fix(tui): render all system notices as plain lines, not boxes
Every system notice (mouse-mode changes, mode set, run cancelled, etc.) now
renders as a plain marked line instead of a bordered box: a faint '·' marker +
muted text for info, amber '⊘' for a run cancellation. Multi-line notices keep
the marker on the first line and indent the rest. Command cards and error rows
keep their boxes. go build/vet/test green.
* fix(tui): separate think→act tool groups with a blank line
Reasoning headers (Thought for X) interleave the tool cards (tool → thought →
tool …), which broke the existing tool-card→tool-card gap rule, so consecutive
list_directory/read_file/update_plan groups packed into a dense wall. Now a
reasoning header that follows a tool card gets a blank line above it, so each
think→act group reads as a distinct block. go build/vet/test green.
* feat(tui): colour markdown emphasis + headings for readability
Assistant prose now reads as three tiers instead of one flat ink colour:
- body text stays ink
- bold / important words render in the brand accent (lime) so they pop
- markdown headings render accent + bold + underline as a clear top tier
go build/vet/test green.
* fix(tui): calmer prose colours + a working-phase label
- Revert the accent colour on bold/important words: emphasis is weight-only now
(no colour), dark-mode-friendly.
- Headings drop the bright lime — distinguished by ink + bold + underline instead
of a saturated accent colour.
- Working status line gains a phase label ('writing' while the answer streams,
'thinking' otherwise) so a long, output-less step reads as live progress
('Working · thinking · 55s') rather than a frozen screen.
go build/vet/test green.
* fix(tui): paint selection highlight in the gutter-shifted coordinate
Mouse text-selection looked like it started in the wrong place: the highlight
landed a few cells right of where you clicked, so selecting/copying felt
impossible to aim.
Cause: the row renderers painted the selection highlight on the UNshifted,
unpadded line, then the body item applied the reading-column gutter (pad lines +
shift selectable textStart) afterward. But the stored selection points are in the
shifted/displayed coordinate (that's how the mouse maps them), so the highlight
was computed against unshifted textStart and landed gutter-cells off. (The copied
text was actually correct — extraction uses the shifted selectable — but the
visual selection was wrong, which is what made it un-aimable.)
Fix: stop highlighting inside the row renderers; add finalizeTranscriptBodyRow,
which pads + shifts THEN paints the highlight, so it's computed in the same
coordinate the mouse uses and lands exactly on the click. Also applies the
highlight uniformly to all rows + the interim block. Regression test added.
|
||
|
KRATOS
|
bd7cc5665f |
fix: behavioral-audit follow-ups (exec -p, cron/version/specialist UX, update_plan coercion, Makefile) (#286)
From the 2026-06-21 behavioral audit. Each verified by tests + the real binary. - exec: accept the advertised `-p`/`-p=` short flag in the exec subcommand parser; previously only `--prompt` worked even though top-level help advertises `-p`, so the documented `zero exec -p "..."` form errored (internal/cli/exec_parse.go). - version: `version --help` prints a usage line instead of the version string (internal/cli/app.go). - specialist: `specialist edit <builtin>` explains builtins are read-only instead of the misleading "specialist not found" (internal/cli/specialist.go). - cron: `pause`/`resume`/`rm` now print a success confirmation (rm's verb arg was previously dead) (internal/cli/cron.go). - update_plan: coerce non-canonical statuses (e.g. "done", "in-progress") to the nearest canonical value instead of failing the whole call, and keep at most one in_progress item (internal/tools/update_plan.go + tests). - build: add a Makefile (build/test/lint/...) backing AGENTS.MD's `make`/`make lint`. Investigated but intentionally NOT changed (guarded by existing tests): - write_stdin AdvertiseInAuto — TestCoreToolsExposeShellTools asserts it stays visible in auto mode for polling/interrupts. - --list-tools not resolving config (so tool_search is absent there) — TestRunExecListsMCPToolsWithoutProviderResolution asserts list-tools must not resolve provider config. Out of scope: LICENSE (owner decision), provider-resolve startup crash (PR #283), OAuth default storage + MCP auto-reconnect (design choices). |
||
|
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 (
|
||
|
Vasanth T
|
bfbdbb10f6 |
test(tools): make write_stdin "\x03" the trigger in the interrupt test (#273)
The de-flake infra (resilientTempDir + deterministic <-session.done wait) is already on main, but the test still called session.terminate() directly before write_stdin "\x03" — so the session was already reaped when the code under test (exec_command.go's Ctrl-C branch) ran, and the assertion no longer guarded it: the test would pass even if that branch were deleted (caught in review). Capture the session handle before the interrupt (write_stdin removes a finished session from the manager), then make write_stdin "\x03" the operation that terminates, then wait on session.done. Keeps the de-flake; restores the coverage. Verified by mutation: removing session.terminate() from the Ctrl-C branch makes this test fail. Runs 15x locally without flaking. |
||
|
Vasanth T
|
b2bb6c4b41 |
feat: five capability features from the reference-agent comparison (#276)
* feat(commands): user-defined slash commands from markdown files
Drop a markdown file at `.zero/commands/<name>.md` (project, shared with the
team) or `<userConfigDir>/zero/commands/<name>.md` (personal); typing
`/<name> args` expands its body template and submits it as a normal prompt.
Optional YAML frontmatter (`description:`, `model:`, `agent:`) feeds help text
and routing. This turns Zero's model-pulled skills into user-invokable,
repo-checked-in team workflows (`/release v1.2`, `/fix-flaky <test>`).
New internal/usercommands package: Load() scans the project + user dirs
(project shadows user on name collision, mirroring specialist scope
precedence), validates names to a safe slash-command shape, and parses
frontmatter with a self-contained parser. Expand() substitutes $ARGUMENTS,
$1..$9, and $$ (literal $); a placeholder-free template gets the raw args
appended so a trivial command still receives input.
Wiring: model loads commands at construction; the commandUnknown dispatch
checks the user set before reporting "unknown"; autocomplete lists user
commands alongside builtins. Builtins win on collision (listed first, shadow at
dispatch). Tests cover load/precedence/name-validation, the placeholder
substitution table, and the TUI dispatch + autocomplete + still-unknown paths.
* feat(init): guided /init that generates AGENTS.md seeded with repo facts
`/init` (TUI) and `zero init` (headless) investigate the repo and write a
concise AGENTS.md so future runs start with project context. Zero already has
the AGENTS.md loader and the repoinfo scanner, but the file only existed if a
user hand-authored it — this activates the machinery in one command.
New internal/agentinit.BuildPrompt seeds the bootstrap prompt with
repoinfo.Collect() output (primary language, build/test tools, CI, workspace
layout, git) formatted as a fact block, then instructs the agent to investigate
the gaps and write a tight AGENTS.md (build/test commands, conventions, things
to avoid). Collection failure is non-fatal — the agent investigates from
scratch. This is one better than a pure-prompt /init: the agent starts from
known facts instead of re-deriving them.
TUI: /init command + handleInitCommand reuse launchPrompt (standard run, tools,
AGENTS.md write path). Headless: `zero init` builds the prompt and forwards it
through the existing exec machinery, so all exec flags (--model, -w, --add-dir)
work unchanged. Tests cover FormatFacts (rendered/omitted fields), BuildPrompt,
and the TUI dispatch.
* feat(tools): lsp_navigate — semantic code navigation via the language server
A read-only, model-callable tool exposing LSP navigation: jump-to-definition,
find-all-references, find-implementations, and workspace_symbol search. The
agent can answer "where is X defined / who calls X / what implements this
interface" precisely, where grep collides on common names or misses indirect
references — cutting wasted read/grep turns and wrong-edit risk on large repos.
Zero already runs language servers for diagnostics, so this adds the
request/response navigation methods on top: Manager.Navigate syncs the file
(didOpen) then issues textDocument/definition|references|implementation or
workspace/symbol via the existing client.Call, decoding all three result shapes
(Location, []Location, []LocationLink). 1-based file:line:col in and out
(converting LSP's 0-based positions); the tool has its own lazily-started
manager (servers start on first use, reused across calls).
Opportunistic: a file type with no installed server, or a server lacking the
capability, degrades to a clear "unavailable — fall back to grep" message, not
an error. Registered in CoreReadOnlyToolsScoped. Tests cover the response-shape
decoding (incl. LocationLink + null), symbol decoding/kinds, unknown-op and
unsupported-extension degradation, and the tool's arg validation + read-only
safety.
* feat(agent): prune stale tool output before paid summarization
Long, dump-heavy sessions accumulate large read_file/grep/glob/bash tool
results the model has already acted on; their bodies are dead weight the loop
otherwise keeps paying an LLM summarizer to compress. Add a zero-cost prune
stage inside maybeCompact, before the summarizer: when over threshold, replace
the bodies of OLD, large tool results with a compact placeholder, recompute
size, and only fall through to the paid Compact() if still over.
pruneStaleToolOutput walks newest-first, protects the last preserveLast
messages plus a trailing 40k-token window of tool output (most likely still in
use), and prunes only tool results older than that and larger than 200 tokens —
gated so the whole pass is a no-op unless reclaimable output exceeds 20k tokens.
It keeps the tool message and its ToolCallID (provider replay stays valid; the
model knows what was there and can re-run the tool), never touches non-tool
messages, never re-prunes a placeholder (idempotent), and copies-on-write so it
never mutates the caller's slice. Recent turns are preserved verbatim — no
paraphrase loss, no token or latency cost.
Tests cover the reclaim gate, recent-window protection, idempotence,
non-tool-message safety, ToolCallID/role preservation, and no-input-mutation.
* feat(agent): reconnect on a transient model disconnect instead of ending the run
A long autonomous task (a big refactor, a swarm member, a headless/cron run)
should survive a single transient upstream hiccup rather than dying and
re-burning every token on a restart. When the initial StreamCompletion connect
fails with a disconnect-shaped error, streamWithReconnect re-issues the SAME
request with exponential backoff up to 2 times.
It retries only the connect — before any content is forwarded — so no
already-streamed OnText is ever duplicated. A context-limit error (the
compactor's job), an image rejection, a non-disconnect error, or a cancelled
context is returned immediately without retry, so the reconnect never fights
the existing handlers. Disconnect classification matches EOF / connection
reset|refused|closed / broken pipe / timeout / 502 / 503 / server-closed.
Reconnect attempts surface through OnReasoning ("connection lost —
reconnecting N/max"), a non-content channel that never corrupts the answer
text; silent when there's no reasoning sink.
Tests: recover-from-transient, give-up-after-max, no-retry-on-context-limit,
stop-on-context-cancel, the disconnect-vs-not classifier, backoff growth, and
the notice routing.
* fix(specialist): add lsp_navigate to knownToolNames
The new lsp_navigate tool was added to the core read-only registry but not to
specialist.knownToolNames, which mirrors the core tool set — so
TestKnownToolNamesMatchCoreRegistry failed (the list drifted from the registry).
Add it to the read-only group to match.
* fix(tools): confine lsp_navigate to the workspace (close arbitrary-file-read)
Review caught that lsp_navigate dropped the PathScope every sibling read-only
tool (read_file/glob/grep) carries, and its readFile joined or used the
model-supplied path verbatim — so `path:"/etc/passwd"` (absolute) or
`path:"../../etc/..."` (escaping) read arbitrary files off disk AND opened them
into the language-server subprocess, reachable via indirect prompt injection.
Add NewScopedLSPNavigateTool(workspaceRoot, scope) (NewLSPNavigateTool now
delegates with nil scope), resolve the path through resolveScopedReadPath BEFORE
reading or building the LSP request — the same confinement the neighbouring
tools enforce — and hand the LSP manager the resolved absolute path so it never
opens an out-of-workspace file. Errors echo only the relative path, never an
absolute one. Registered via the scoped constructor in CoreReadOnlyToolsScoped.
Regression test asserts `/etc/passwd` and `../../` are rejected for both the
position and workspace_symbol ops; verified by mutation (bypassing the scope
resolution makes the test fail).
|
||
|
KRATOS
|
a981120432 |
Audit tail: remaining medium/low correctness fixes (CLI, config, OAuth, tools, LSP, TUI) (#275)
* audit tail: fix remaining medium/low correctness findings A sweep of the remaining audit findings across CLI, config, OAuth, tools, LSP, and the TUI. Each fix has a regression test; build/vet/gofmt clean and the touched packages' suites pass. CLI - `zero -p "<prompt>"` now forwards the prompt as the inline `--prompt=<value>` form so a prompt whose first character is a dash (e.g. `zero -p "-foo"`) is taken verbatim instead of being rejected as a missing flag value (matches the cron path). Config - mergeProfile no longer drops parseThinkTags across profile merges; an explicit value carries over and a nil (unset) never clobbers an existing one. OAuth - Refresh carries the existing token_type forward when the refresh response omits it (previously silently lost). - RefreshScheduler.Stop resets started/done so the scheduler can be started again instead of being permanently inert after the first Stop. Tools - apply_patch header parsing takes the whole post-prefix value (dropping a trailing tab timestamp and undoing git's C-style quoting) instead of splitting on spaces, so a diff target whose name contains spaces is recognized. LSP - Server.Shutdown honors its context (kills immediately on cancel rather than waiting out the full grace window), and Manager.Shutdown shuts servers down concurrently so total shutdown is bounded by one grace window, not N×grace. TUI - /theme auto re-probes the terminal background (RequestBackgroundColor) instead of reusing a stale startup reading, so switching to auto re-detects light/dark. - Specialist card: wire the tool-call count from progress events (was always 0) and omit the token segment when the total is 0 rather than show "0 tokens". - Resume: extract the tool-call row id the same way as the result row and the specialist pre-pass (toolCallId first), so call/result dedup and the failed-Task visibility rule key on the same string. - Plan panel: height takes an injected clock (testable, consistent with render), and duplicate-content steps inherit distinct prior timestamps positionally. * tui: test that /theme auto re-probes the terminal background (M17) The M17 fix lives in handleSubmit's command dispatch (`return m, tea.RequestBackgroundColor` when the mode is auto), which no test exercised — only the handleThemeCommand helper was covered. Add a dispatch-level test: /theme auto must return the background re-probe command, /theme dark must not. Verified to fail if the dispatch is reverted to `return m, nil`. |
||
|
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 |
||
|
Vasanth TandClaude Opus 4.8
|
c572791edf |
feat: clipboard image paste + provider 400 handler for image rejection (#268)
* fix(agent): abort on image-rejection 400s instead of retrying in a loop
* feat(imageinput): add ReadClipboardImage for OS clipboard image extraction
* feat(tui): clipboard image paste — detect and attach screenshots from OS clipboard
* fix(imageinput): use temp file for Windows clipboard image (PowerShell stdout can't emit raw binary)
* test: handle both image-present and image-absent clipboard states
* fix(vision): add kimi-k2 and qwen-vl to vision-capable name heuristic
* fix(tui): render image errors as red error rows instead of grey system notes
* feat(vision): check models.dev InputModalities for vision support instead of only name heuristics
The vision gate now checks three sources:
1. Curated model registry (catalog authority + name heuristic)
2. Discovered model list from models.dev (InputModalities contains 'image')
3. Name heuristic fallback for unknown models
This means custom/ollama/cloud models (kimi-k2, qwen-vl, etc.) are correctly
detected as vision-capable when models.dev reports them with image input
modality — no more manual name matching needed.
* chore: gofmt clipboard_test.go
* fix(imageinput): remove shell injection in the Linux clipboard reader
The Linux clipboard reader built shell commands by concatenating a
clipboard-derived MIME type into `sh -c`:
exec.Command("sh", "-c", "wl-paste --type "+t+" 2>/dev/null")
exec.Command("sh", "-c", "xclip ... -t "+t+" -o 2>/dev/null")
`t` comes from `wl-paste --list-types` / `xclip TARGETS` and is only filtered by
`HasPrefix(t, "image/")`, so a hostile clipboard offerer could register a target
like `image/png; rm -rf ~` — it passes the prefix check and then executes
through the shell. A real (if low-likelihood) local code-exec vector.
Drop the shell entirely: every helper now runs via exec.Command(prog, args...)
with the MIME type passed as a discrete argument (matching the existing no-shell
pattern in pdf.go). Stderr is discarded by not assigning it. Factored into
runClipboardStdout + imageMIMETypes helpers.
Test: imageMIMETypes filters to image/* lines and returns even an
injection-shaped type verbatim (safe now that it never reaches a shell).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* test(tools): de-flake TestWriteStdinInterruptTerminatesSession
The test had two Windows-CI flakes that reddened unrelated PRs:
1. It relied on write_stdin's 1000ms yield window being long enough for the
async SIGKILL + reap to land before asserting the session was no longer
running. On a loaded runner the reap exceeded that window, so write_stdin
reported "still running" and the assertion failed. Fix: after terminating,
wait on session.done deterministically (30s safety timeout that fails loudly
if the kill genuinely hangs); the common case returns the instant the
process exits.
2. The SIGKILL'd child had the test's t.TempDir() as its cwd, and Windows may
not release the directory handle the instant the process is reaped — so
t.TempDir()'s immediate RemoveAll on cleanup failed with "being used by
another process". Fix: resilientTempDir retries RemoveAll for up to 5s
(best-effort; a leaked temp dir never fails the test).
Verified 20x locally; full internal/tools suite green.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
|
||
|
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 |
||
|
Vasanth T
|
b237e9849a |
feat(tui): sticky plan panel + specialist task cards + task table overlay (#255)
* feat(tui): sticky plan panel + specialist task cards + task table overlay Three new TUI surfaces that give users live visibility into agent progress and specialist delegation, so long turns and subagent work never look frozen. Sticky plan panel (plan_panel.go): - Pinned between the title bar and transcript body, shows the agent's step-by-step plan from the update_plan tool as a live checklist. - Header with spinner + progress count + elapsed; text progress bar; per-step list with status icons (✓ completed, ⠹ in_progress, ○ pending, ✗ failed) and individual step timings. - Auto-collapses to a one-line green summary 30s after completion. Ctrl+P toggles expand/collapse. - Plan state synced from update_plan tool results via CurrentPlan(); timestamps preserved across the tool's full-replacement updates by matching on content. Specialist cards (specialist_card.go): - New rowSpecialist transcript row kind; EventSpecialistStart/Stop now handled in session.go hydration (previously fell through to default and were silently dropped on resume). - Animated card with spinner + name + task + elapsed + token count + tool call count. Blue while running, green when done, red on error. - specialistTracker tracks live state; Task tool calls/returns are intercepted in the OnToolCall/OnToolResult callbacks to start/complete tracking and emit card rows. - Height cache marks running specialist rows unstable so the spinner animates. Task table overlay (task_table.go): - Ctrl+G toggles a full-width overlay showing all specialists in the session as a sortable table: #, Specialist, Task, Status, Time, Tokens, Tools. - Summary bar with running/completed/error counts, total tokens, aggregate elapsed. - Arrow keys navigate rows; Esc closes the overlay. Tests: plan_panel_test.go (7 tests), specialist_card_test.go (11 tests), task_table_test.go (8 tests). All existing TUI tests pass. * fix(update_plan): expose structured nested item schema so models stop sending bad arguments The plan property's Items schema was omitted because of an outdated comment claiming the serializer drops nested Properties/Required. The serializer (propertyToRuntimeMap) already handles them recursively. Without the nested schema, weaker models guessed the item structure from a text description and sent malformed arguments, hitting the 4-strike guardrail halt. Now exposes Items as an object with content (string, required), status (enum), and notes (string). * fix(tui): route plan + specialist updates through message sink, not stale closures The OnToolCall/OnToolResult callbacks captured model by value, so m.plan.updateFromItems() and m.specialists.start/complete() mutated stale copies that never reached the live model. Introduced planUpdateMsg, specialistStartMsg, and specialistCompleteMsg tea.Msg types sent through runtimeMessageSink, handled in updateModel() where they mutate the live model. * feat(tui): subchat drill-in for specialist sessions Enter on a task table row loads the child session's events and swaps the transcript view to show the subagent's full conversation (user messages, agent text, tool calls, results). A nav bar at the top shows the specialist name/task and a back hint. ArrowUp or Esc pops back to the main chat at the saved scroll position. - subchat.go: subchatState with enter/exit, renderSubchatNavBar - transcript_selection.go: transcriptBodyItemsFromRows renders arbitrary rows (reuses the same rendering pipeline as the main transcript) - model.go: Enter on task table row triggers drill-in; ArrowUp/Esc exits; transcriptView() short-circuits to child rows when subchat is active * fix(ci): strip CRLF line endings and BOM from plan_tool_test.go and update_plan.go gofmt and git diff --check failed on ubuntu CI because PowerShell's Set-Content wrote UTF-8 with BOM and CRLF. Stripped both to plain LF without BOM. * feat(tui): add renderLeftRuleCard helper for borderless specialist cards * feat(tui): switch specialist cards to left-rule rendering, drop hint line * feat(tui): add renderSpecialistSummary one-line rollup for specialist cards * fix(tui): correct slice offset in renderSpecialistSummary and add pluralization test The muted tail was sliced at summary[len(spinnerView)] but the spinner sits at byte offset 2 (after the 2-space indent), so the slice cut into the spinner's UTF-8 continuation bytes and dropped the indent. Slice at 2+len(spinnerView) instead. Also add a test asserting '2 errors' is present and '2 error' (singular) is absent when two specialists errored. * refactor(tui): remove task table overlay — cards carry everything now * feat(tui): render specialist summary line above cards in transcript * feat(tui): mark specialist card lines as clickable selectable rows * feat(tui): click specialist card to drill into subchat, Enter as keyboard fallback * tui: clear selectedSpecialistID after Enter drill-in Fixes Task 7 review issue: selectedSpecialistID persisted after Enter drill-in, causing a subsequent Enter to re-drill into the specialist's subchat instead of submitting the composer. Clear it immediately after subchat.enter succeeds so the keyboard fallback is one-shot per click; click re-sets it, so click->Enter still works. * chore: gofmt model.go + test files * chore: gitignore .superpowers/ and docs/superpowers/ — local skill artifacts * fix(tui): remove Enter keyboard fallback for subchat drill-in — mouse click only * fix(tui): reconcile specialist childSessionID from tool call ID to real session ID on completion * feat(specialist): add Progress callback field to RunOptions and TaskRunOptions * feat(specialist): stream child stdout line-by-line via Progress callback * feat(agent): wire OnToolProgress callback for specialist child events * feat(tui): show live tool-call progress in specialist cards * fix(swarm): update RunChild mock signature for new progress callback param * fix(tui): render plan panel inline in transcript, not pinned at top * chore: gofmt model.go, specialist_card.go, specialist_click_test.go * fix(tui): address PR review - plan panel without specialists, stream reader safety, resume dedup + payload keys |
||
|
KRATOS
|
b64a9d3de9 |
tools: tool_search redirects eager-tool lookups instead of "no tools matched" (#259)
When the model calls tool_search for a tool it already has eagerly (e.g. select:Task or select:update_plan), resolveExact/noMatchMessage only ever consulted the DEFERRED set, so it fell through to a misleading "No tools matched. Available: <deferred tools>". That implies the tool does not exist, and weaker models (e.g. cloud deepseek/minimax) loop retrying tool_search instead of just calling the tool. tool_search now recognizes eager tools (registered, not deferred, passing the operator filters) and, for a select: or keyword query that names one: - with no deferred match: redirect — "<tool> is already in your tool list — call it directly" instead of "no tools matched"; - mixed with a deferred match: load the deferred tool AND append the note; - a genuinely-unknown name still gets the honest "no tools matched". Surgical: only the error/redirect path changed; loading deferred tools is unchanged. NewToolSearchTool already holds the full registry — it just wasn't using it to recognize eager tools. Tests: eager redirect via select: and keyword, mixed load+note, and unknown-stays-honest; all existing tool_search tests pass. |
||
|
KRATOS
|
eed61cf8ec |
tools: defer swarm tool schemas + keyless SearXNG web_search backend (#258)
Two fixes for the raised issues — the idle tool-schema prefix and native web search.
Idle prefix — make the swarm tools deferred-eligible:
- The 7 swarm_* tools are an advanced, rarely-first-move feature the base system
prompt does not name, so they now implement Deferred() and load on demand via
tool_search instead of shipping full schemas in the eager per-request prefix.
Core tools and specialist Task stay eager (no friction on common operations).
- partitionTools keys off the shared tools.IsDeferred check exactly as it does for
MCP tools, so this reuses the existing, proven mechanism. Measured ~470 fewer
prompt tokens per request in a swarm-enabled (--auto high) config, with no
behavioral loss — swarm still works, just loaded when needed.
Web search — keyless SearXNG backend:
- web_search gains a "searxng" provider: ZERO_WEBSEARCH_PROVIDER=searxng with a
SearXNG URL queries GET /search?format=json with NO API key, parsing the
{results:[{title,url,content,score}]} shape. Self-host or point at any SearXNG
instance for native search without baking a shared key or a flaky default.
Tests: swarm tools are deferred-eligible; the SearXNG backend issues the GET, sends
no auth header, maps fields, and respects the limit.
|
||
|
Anandan
|
ac6addaa83 |
Align sandbox permission blocks and backend states (#253)
* Prompt for shell network permissions Route network-risk shell calls through the existing permission prompt instead of hard-denying them at sandbox preflight. Approved network prompts apply the current turn/session request-permissions overlay so the effective sandbox policy is widened only after user approval. Move network command detection into the shell AST analyzer for package installers, gh/git network subcommands, and python http.server so quoted process patterns such as pkill -f "python3 -m http.server 8000" are not misclassified as network activity. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestAnalyzeCommand|TestClassifyDoesNotFlagQuotedHttpServerPattern|TestEvaluateExemptsNetworkToolsFromDenyByDefault|TestEngineClassifiesNetworkAndDestructiveShellCommands|TestGrantRequestPermissionsNetworkOverlaysPolicyForTurn' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent -run 'TestRunPersistentCommandPrefixStillPromptsForNetwork|TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tools -run 'TestBash|TestRegistryRunsBashThroughSandboxEngine' Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Auto-allow native-sandboxed shell commands Remove the old hidden bash auto-allow environment/config path. Ordinary shell commands now use the active native sandbox as the permission boundary by default, while network, destructive, and workspace policy checks still run first. Policy-only and disabled sandbox backends continue to require approval for shell commands, so unsandboxed execution is not silently allowed. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestSandboxedBash|TestShellSandboxActive|TestEvaluateExemptsNetworkToolsFromDenyByDefault|TestGrantRequestPermissionsNetworkOverlaysPolicyForTurn|TestAnalyzeCommand|TestClassifyDoesNotFlagQuotedHttpServerPattern' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tools -run 'TestBashAuto|TestBashStillPromptsWithoutActiveSandbox|TestRegistryRunsBashThroughSandboxEngine' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli -run 'TestApplyConfiguredSandboxPolicy|TestApplyConfiguredAutonomyCeiling' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent -run 'TestRunPersistentCommandPrefixStillPromptsForNetwork|TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/daemon -run 'TestScrubWorkerEnvRemovesReentrancyMarkers' Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Prompt for destructive shell approval Remove the separate DenyDestructiveShell policy layer from active sandbox decisions. Destructive shell classification now produces an approval prompt in ask/auto flows instead of a sandbox hard-deny, while no-approval paths remain blocked. Drop the removed destructive-shell policy field from sandbox policy output, backend capabilities, snapshots, and Windows golden data. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent -run 'TestRunPromptsForDestructiveShellInsteadOfSandboxDeny|TestRunPersistentCommandPrefixStillPromptsForNetwork|TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestEngineClassifiesNetworkAndDestructiveShellCommands|TestBackendCapabilitiesReflectDisabledPolicy' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli -run 'TestRunSandboxPolicy|TestEffectiveSandboxPolicy' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/zerocommands -run 'TestSandboxPolicySnapshotFromPolicyFillsEffectiveMode' Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Remove web-tool network policy switch Remove the old EnforceToolNetwork policy option and the NetworkHostAllowed per-tool gate so sandbox network policy only controls sandboxed shell egress. Keep web_search and web_fetch on their own tool permission and URL safety paths, with tests proving deny/scoped shell network policy no longer blocks in-process web tools. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestEvaluateExemptsNetworkToolsFromShellNetworkPolicy|TestEngineAllowsNetworkSideEffectWhenShellPolicyBlocksNetwork|TestGrantRequestPermissionsNetworkOverlaysPolicyForTurn|TestScopedNetworkGateInEvaluate' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tools -run 'TestWebSearchRunWithSandbox|TestWebFetchRunWithSandbox|TestRegistryRoutesWebFetchThroughSandboxPath' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent -run 'TestRunPersistentCommandPrefixStillPromptsForNetwork|TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant' Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Prompt for outside-workspace file access Convert recoverable outside-workspace file path violations into permission prompts instead of hard-denying in ask/auto modes. Approved prompts now grant the requested filesystem scope through the existing request_permissions overlay for the current turn or session, so the follow-up sandbox evaluation and scoped file tools use the same policy path. Keep unsafe mode, apply_patch escapes, symlink traversal, and non-recoverable path cases hard-denied. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestEngineAutoAllowsWorkspaceFileMutationTools|TestEngineDeniesOutOfWorkspacePaths|TestGrantRequestPermissionsReadDoesNotGrantWrite|TestGrantRequestPermissionsSessionPersists' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent -run 'TestRunPromptsAndAllowsOutsideWorkspaceWrite|TestRunSessionAllowsLaterOutsideWorkspaceWrite|TestRunAppliesSandboxEvenInUnsafeMode' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tools -run 'TestRegistryAppliesSandboxBeforeToolExecution|TestRegistrySandboxGatesPathAliasKeys|TestRegistryAllowsPromptToolWithPersistentSandboxGrant' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli -run 'TestRunSandboxCheckJSONDeniesOutOfWorkspaceWrite' Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Fix Windows network prompt test fixture Use a Windows curl.cmd shim and cmd.exe PATH syntax for TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant while keeping the POSIX fake curl fixture for Linux and macOS. This keeps the test focused on network prompt approval and turn network grants instead of relying on a POSIX shell script being executable on Windows. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent -run 'TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant|TestRunPersistentCommandPrefixStillPromptsForNetwork' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestClassifyASTCatchesNetworkProgramsRegexMisses|TestClassifyFlagsPipedInstallerVariants' Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Remove sandbox autonomy ceilings Sandbox permission decisions now match grants by tool and scope only, without MaxAutonomy or request autonomy checks. Persistent/session grants, permission approvals, workspace-write auto-allow, and native-sandbox shell auto-allow no longer route through an autonomy ceiling. Remove the sandbox.maxAutonomy config and CLI surfaces, including grant --auto, policy output, sandbox check autonomy input, and sandbox grant snapshots. MCP permission autonomy is intentionally unchanged because it is a separate MCP permission model. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/agent ./internal/tools ./internal/zerocommands ./internal/config ./internal/cli ./internal/tui Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Fail closed without native sandbox Remove AllowPolicyOnlyRunner from sandbox policy and snapshots. A restricted/managed command plan now errors when native sandbox support is unavailable instead of running through a policy-only direct runner. Policy-only backend data remains as diagnostics for capability/reporting paths, but it no longer authorizes command execution when a platform sandbox is required. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/zerocommands ./internal/cli ./internal/tui Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/tools Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Remove scoped shell network policy Drop the domain-scoped shell network mode and the local scoped-egress proxy path from the active sandbox model. Shell network policy now serializes and evaluates only deny or allow. Request-permissions network grants widen to allow without domain lists, and platform profiles consume the binary mode directly. The Linux helper no longer accepts proxy-route flags, Windows network hashes only the binary mode, and web tool tests assert they remain separate from shell network policy. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/tools ./internal/notify Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Align permission grant ordering and prompt text Move persistent and session grant matching ahead of recoverable sandbox prompts while keeping explicit denies and non-recoverable path violations fail-closed. Clean TUI permission transcript/card text so normal prompts show action, reason, scope, and readable block details instead of raw permission/risk/violation key-value metadata. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/tui ./internal/agent ./internal/tools Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Remove policy-only sandbox terminology Replace the unavailable native-backend diagnostic path with explicit unavailable support/backend states, and stop presenting missing native isolation as a policy-only fallback. Rename structured sandbox denial metadata from violation to block across permission events, stream-json, snapshots, TUI rendering, and sandbox-check output so user-facing flow matches blocked action language. Update README, work split docs, smoke test selectors, and Windows sandbox policy golden output for the new backend/support names and block payloads. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/agent ./internal/tools ./internal/tui ./internal/cli ./internal/doctor ./internal/zerocommands ./internal/streamjson Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Fix sandbox review follow-ups Fail closed for unknown network modes, keep network prompts ahead of bash allow grants, preserve v1 grant-file reads, and classify unparseable commands with obvious network programs as network-risk. Bump stream-json schema to v2 and document the block metadata field after the violation-to-block rename. Tests: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/streamjson ./internal/specialist ./internal/cli -run 'TestExec.*Stream|TestStreamJSON|TestExecProtocol|TestExecReadsStreamJSONImages|TestExecAcceptsImageOnlyStreamJSONMessage|TestExecInputFormatStreamJSON' Tests: GOCACHE=/tmp/zero-go-cache go test ./... Tests: scripts/sandbox-smoke.sh Tests: git diff --check |
||
|
KRATOS
|
03784268ac |
token efficiency: lossless cost accounting + minified file reads (read_minified_file) (#256)
* token efficiency: lossless cost accounting + minified file reads Two complementary improvements to how Zero handles tokens. ACCOUNTING — make cost measurement lossless and correct: - Carry all five token dimensions end-to-end (input, cache-read, cache-write, output, reasoning) in one Usage; add CacheWriteTokens (cache creation), which the Anthropic adapter previously parsed then discarded. - Price cache-writes at their premium rate (Anthropic 1.25x input, derived per model). Regression-safe: a model without the rate bills cache-write at the input rate exactly as before. - Persist cache-read/cache-write/reasoning in EventUsage so `zero usage report` reconstructs the SAME cost as the live tracker instead of over-pricing cache-heavy sessions. A single EventUsagePayload writer is paired with the reader so the JSON keys can't drift. - Add a live context-window gauge to the status bar (used/window + percent, graded green/amber/red) using the last request's input tokens. REDUCTION — read_minified_file, a dense token-cheap file view: - New internal/minify package + read_minified_file tool returning a comment-free, line-number-free view: Go via the real go/parser AST; C/Java/CSS/SCSS/LESS/ Python via a string-aware comment lexer (handles strings, char literals, triple-quotes, Java text blocks, Python f-strings incl. 3.12 same-quote nesting). Every other language falls back to whitespace-only, so the stripper can never corrupt content. - ~32% fewer tokens on a realistic multi-file read; +~102 fixed tokens for the tool schema, recovered many times over by a single subsystem read. Tests: cache-write pricing + fallback, lossless persistence round-trip, the context gauge, comment-stripping golden cases per language (comment-chars in strings, Java text blocks, Python f-string nesting, CSS has no //), and the unsupported-language safe fallback. gofmt / vet / build / -race all green. * specialist: register read_minified_file in the tool registry + read-only sets Smoke caught TestKnownToolNamesMatchCoreRegistry drifting once read_minified_file joined the core read-only tools: the specialist tool allowlist (knownToolNames), the read-only/edit/execute categories, and the read-only auto-approve set did not list it. It is a read-only companion to read_file, so it belongs alongside read_file in all of them; updated the default-allowlist test accordingly. |
||
|
Anandan
|
f00e83a5ed |
Add session command prefix approvals (#250)
Bash permission prompts can now expose a validated session-scoped prefix approval. Prefixes are parsed from simple shell commands or an explicit prefix_rule argument, with redirects, control operators, assignments, expansions, globs, and broad launcher prefixes rejected before the option is shown. Approved prefixes are stored in the session sandbox engine and only skip future matching bash prompts; commands still execute through the existing sandbox backend. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/tui Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/sandbox ./internal/tui ./internal/tools Tested: GOCACHE=/tmp/zero-go-cache go test ./... |
||
|
Anandan
|
2cf6fd2334 |
Add request permissions flow (#248)
* Add request permissions flow Adds a runtime-handled request_permissions tool that can ask for turn or session filesystem/network grants and return the granted profile as structured JSON. Temporary turn grants update the shared sandbox scope and effective policy, then clean up at run end; session grants persist in memory for the session. Read grants now stay read-only through tool path resolution, policy evaluation, and native sandbox profile construction. The TUI renders request_permissions with grant-specific choices, including strict review, and Esc continues without granting permissions. Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Align request permission grants with prompts Display and return the normalized grant roots that the sandbox actually applies, so file-like permission requests no longer show a narrower path than the directory root being granted. Also makes the network prompt explicit about all outbound access and relabels the strict option as a model review request instead of implying enforced command review. Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Retry transient OAuth secret read locks Windows can briefly deny reads of the freshly published token secret while concurrent OAuth secret creation converges. Treat Windows sharing and lock violations as transient and retry those reads before returning an error. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/oauth Tested: GOCACHE=/tmp/zero-go-cache GOOS=windows GOARCH=amd64 go test -c ./internal/oauth -o /tmp/zero-oauth-windows.test.exe Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/agent ./internal/tui ./internal/tools ./internal/specialist ./internal/oauth Tested: GOCACHE=/tmp/zero-go-cache go test ./... |
||
|
KRATOS
|
995b9086c7 |
feat(subagent): proactive specialist delegation + swarm execution fixes (#243)
* feat(subagent): proactive specialist delegation + swarm execution fixes Fixes: - Task tool dropped the parent's permission mode, so every delegated sub-agent ran at --auto high regardless of the parent. Forward PermissionMode so a child never exceeds its parent's authority. - Swarm members never executed: swarm_spawn accepts agent types teammate/subagent, but the launcher looked those up as specialist names (only worker/explorer/code-review exist), so every member failed with "specialist not found". Run each member from its swarm definition via an inline manifest on the executor. Enhancements: - Auto-delegation: the orchestrator system prompt now lists the available specialists and when to use each, nudging it to offload read-heavy work to a sub-agent so large tool output stays out of the main context. - @-mention: a leading "@<specialist> <task>" in the TUI composer delegates to that specialist (transcript keeps the verbatim mention); the @ palette suggests specialists on the first token, file references mid-message. - swarm.maxTeamSize config caps concurrent members per team (0 => default 8; negative rejected), honored in user and project config. - Proactive spawning: read-only specialists (explorer/code-review) auto- approve via a new args-aware tool permission (tools.ArgsPermissioner), so the agent delegates exploration off a normal prompt without an approval prompt; write-capable specialists, resume, and unknown names still prompt. The exec gate registers specialist tools at medium/high autonomy so a non-trivial headless run can auto-delegate read-only too. * fix(specialist): fail-safe child autonomy when PermissionMode is unset specialistAutonomy mapped an empty PermissionMode to "--auto high" (unsafe), silently re-introducing the privilege escalation the rest of this PR fixes: an orchestrator that forgets to wire PermissionMode would get an unsafe child. The Task tool and swarm both now propagate a resolved mode, so "" only occurs for a caller that omitted it -- default that case to "low". Only an explicit unsafe parent now yields --auto high; everything else (incl. unset/unknown) is fail-safe low. Updates the stale "back-compat" comments and the tests that documented the old behavior. Addresses the non-blocking follow-up from PR review. |
||
|
Anandan
|
2d62387e08 |
Align permission session approvals (#244)
* Align permission session approvals Add in-memory session grants, workspace-write auto-allow for built-in file mutation tools, and dynamic permission choices for once/session/future approvals. Persist future approvals only when a grant store is configured, while keeping session approvals scoped to the active engine. Tested: GOCACHE=/tmp/zero-go-cache go test ./... * Guard workspace write auto-allow Exclude protected workspace metadata from built-in file-tool auto-allow so writes under .git, .zero, and .agents re-prompt instead of silently running. Derive apply_patch targets from patch headers inside the sandbox engine so patch-body escapes and symlink traversal are denied before workspace auto-allow. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestEngineAutoAllowsWorkspaceFileMutationTools|TestEngineDoesNotAutoAllowProtectedMetadataWrites|TestEngineDeniesApplyPatchEscapesFromPatchBody|TestEngineSessionGrantDoesNotMatchSiblingFile|TestEngineDeniesAutoAllowedSymlinkEscape|TestEngineDeniesWorkspaceSymlinkTraversal|TestDeriveScope' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/agent ./internal/tui ./internal/tools Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check |
||
|
Anandan
|
a1b9bbb07c |
Align permission prompts for web tools (#242)
Treat hosted web_search as an allowed read-only network discovery tool so normal searches do not stop on a permission prompt, while retaining sandbox network enforcement when tool-network policy is enabled. Add host-scoped persistent grants for web_fetch URLs so an always-allow decision is bounded to the requested host instead of becoming a blanket tool-wide fetch grant. Update permission prompt and transcript rendering to show network targets, scoped always labels, and preserved scope metadata across live rows, render caching, and session hydration. Tested: GOCACHE=/tmp/zero-go-cache go test ./... |
||
|
Anandan
|
4cb1f10952 |
Add sandbox architecture baseline (#240)
* Add sandbox architecture baseline Record target backend names, enforcement metadata, legacy sandbox entrypoints, and policy JSON baseline coverage for the sandbox rewrite. Add a focused sandbox smoke script with cross-platform compile checks and an opt-in real backend smoke path. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestTargetBackendForPlatformBaseline|TestLegacySandboxEntrypointsAreExplicitAndExist|TestBackendPlanCarriesPhase0ManagerFields|TestCommandPlanCarriesSandboxMetadata|TestPolicyOnlyDisabledFailClosedForTargetPlatforms|TestSelectBackendChoosesPlatformAdapterWithFallback|TestBackendBuildPlanDocumentsBestEffortIsolation|TestBackendCapabilitiesReflectDisabledPolicy' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli -run 'TestRunSandboxPolicyInspectTextAndJSON|TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields|TestRunSandboxPolicyEffectiveTextAndJSON' Tested: scripts/sandbox-smoke.sh Tested: git diff --cached --check * Introduce sandbox manager and permission profile Add a normalized permission profile and sandbox manager execution request that owns target backend selection, enforcement metadata, and fail-closed validation. Route command planning and sandbox policy output through the manager boundary while keeping legacy adapters explicit as temporary integration points. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestPermissionProfileFromPolicyBuildsWorkspaceWriteProfile|TestPermissionProfileFromDisabledPolicyDoesNotRequirePlatformSandbox|TestSandboxManagerBuildsExecutionRequestFromProfile|TestSandboxManagerFailsClosedWhenNativeRequiredAndPolicyOnlyDisabled|TestTargetBackendForPlatformBaseline|TestBackendPlanCarriesPhase0ManagerFields|TestCommandPlanCarriesSandboxMetadata|TestPolicyOnlyDisabledFailClosedForTargetPlatforms' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli -run 'TestRunSandboxPolicyInspectTextAndJSON|TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields|TestRunSandboxPolicyEffectiveTextAndJSON' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tools -run 'TestRegistryRunsBashThroughSandboxEngine|TestBashToolReportsDisabledPolicyOnlyRunner|TestBashToolBuildsWrappedSandboxExecCommand' Tested: scripts/sandbox-smoke.sh Tested: git diff --cached --check * Move sandbox backend selection into manager Make SandboxManager own platform backend selection and keep SelectBackend as a compatibility shim for existing CLI and TUI wiring. Add tests proving manager-owned selection and the compatibility wrapper agree. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestSandboxManagerSelectsPlatformBackend|TestSelectBackendDelegatesToSandboxManagerSelection|TestSelectBackendChoosesPlatformAdapterWithFallback|TestLegacySandboxEntrypointsAreExplicitAndExist|TestSandboxManagerBuildsExecutionRequestFromProfile|TestSandboxManagerFailsClosedWhenNativeRequiredAndPolicyOnlyDisabled' Tested: scripts/sandbox-smoke.sh Tested: git diff --cached --check * Route command planning through sandbox manager Make SandboxManager build command plans from sandbox execution requests, with backend-specific wrapping isolated behind a temporary adapter. Move Engine.BuildCommandPlan to a single manager boundary and mark buildLegacyCommandPlan as the explicit delete target for later backend work. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestSandboxManagerBuildsCommandPlanThroughTemporaryAdapter|TestSandboxManagerBuildsDegradedPolicyOnlyCommandPlan|TestBuildCommandPlanWrapsBubblewrap|TestBuildCommandPlanWrapsSandboxExec|TestBuildCommandPlanUsesPolicyOnlyFallback|TestBuildCommandPlanCanRejectPolicyOnlyFallback|TestLegacySandboxEntrypointsAreExplicitAndExist|TestSandboxManagerBuildsExecutionRequestFromProfile|TestSandboxManagerFailsClosedWhenNativeRequiredAndPolicyOnlyDisabled' Tested: scripts/sandbox-smoke.sh Tested: git diff --cached --check * Add Linux sandbox helper command boundary Introduce the zero-linux-sandbox command surface and typed permission-profile argv builder used by the upcoming Linux helper rewrite. Keep the helper fail-closed until enforcement is wired, and add parser tests plus smoke compilation for the new helper package. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestBuildLinuxSandboxCommandArgsSerializesPermissionProfile|TestParseLinuxSandboxHelperArgs|TestRunLinuxSandboxHelperFailsClosedUntilEnforcementIsWired' Tested: scripts/sandbox-smoke.sh Tested: git diff --cached --check * Fix sandbox policy golden path aliases Normalize both original and symlink-resolved temp paths before comparing sandbox policy JSON goldens, so macOS /private/var aliases do not fail the smoke job. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli -run 'TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields|TestRunSandboxPolicyInspectTextAndJSON|TestRunSandboxPolicyEffectiveTextAndJSON' Tested: scripts/sandbox-smoke.sh Tested: git diff --cached --check * Fix Windows sandbox smoke test assumptions Normalize JSON-escaped Windows paths in the sandbox policy golden test and mark the fake bubblewrap backend as Linux in the command metadata test. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli -run 'TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields|TestRunSandboxPolicyInspectTextAndJSON|TestRunSandboxPolicyEffectiveTextAndJSON' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestCommandPlanCarriesSandboxMetadata|TestBackendPlanCarriesPhase0ManagerFields|TestPolicyOnlyDisabledFailClosedForTargetPlatforms' Tested: scripts/sandbox-smoke.sh Tested: git diff --cached --check * Stabilize sandbox policy golden test Compare the Windows sandbox policy golden as decoded JSON after path tokenization so platform-specific byte formatting does not fail smoke tests while preserving the same field-level regression coverage. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli -run TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli -run 'TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields|TestRunSandboxPolicyInspectTextAndJSON|TestRunSandboxPolicyEffectiveTextAndJSON' Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Route Linux sandboxing through helper Move Linux command planning to invoke zero-linux-sandbox with the permission profile, command cwd, and command argv instead of exposing bubblewrap construction from the runner boundary. The helper now builds the bubblewrap argv, binds its own executable for the inner stage, applies the seccomp/no-new-privs step before exec, and preserves the original command cwd path. Package Linux releases with zero-linux-sandbox next to zero so release builds use the helper directly, while local development can fall back to go run for the helper. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli ./internal/tools ./cmd/zero-linux-sandbox Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/release ./cmd/zero-release Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Normalize Linux helper cwd test paths Compare native paths in the helper cwd regression test so Windows smoke does not fail on path separator formatting while preserving the same helper behavior assertion. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run TestLinuxHelperPlanPreservesRealExtraRootCwd Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Add Linux Landlock helper enforcement Implement the zero-linux-sandbox Landlock helper path with fail-closed profile validation, Landlock filesystem rules, and seccomp network-deny enforcement for the fallback backend. Broaden the Linux real smoke coverage to cover helper write allow/deny, deny-read, temp writes, metadata protection, network deny, and direct Landlock fallback behavior. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: scripts/sandbox-smoke.sh Tested: GOCACHE=/tmp/zero-go-cache go test ./cmd/zero-linux-sandbox ./internal/cli ./internal/tools ./internal/release ./cmd/zero-release Tested: git diff --check * Route Seatbelt profiles through permission profiles Build macOS sandbox-exec profiles from the shared PermissionProfile so read roots, write roots, temp access, deny-read, deny-write, and protected metadata are expressed by the platform profile instead of legacy write-root-only inputs. Keep the existing sandbox-exec adapter path as a compatibility wrapper while adding target macos-seatbelt capability handling. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: scripts/sandbox-smoke.sh Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli ./internal/tools Tested: git diff --check * Stabilize Seatbelt write carveouts Emit protected metadata and read-only write-root carveouts as explicit Seatbelt deny rules after the broad write allow. This preserves deny precedence while avoiding fragile allow-rule predicates in real sandbox-exec runs. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Expand macOS Seatbelt runtime allowances Restore the platform startup allowances needed by sandbox-exec-wrapped commands under restricted filesystem profiles: executable mapping, system metadata reads, pty/device access, preferences, IPC, and standard library/runtime roots. This keeps project writes constrained to the PermissionProfile roots while preventing macOS shells from aborting before user commands run. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Normalize Seatbelt deny expectations in tests Build Seatbelt deny-rule expectations from normalized profile paths so the metadata ordering test matches production behavior on Windows and Unix hosts. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: scripts/sandbox-smoke.sh Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go test -c ./internal/sandbox -o /tmp/zero-sandbox.test.exe Tested: git diff --check * Add Windows sandbox runner command planning Select the windows-restricted-token backend only when the Windows sandbox command runner is discoverable, and keep the missing-runner path as an explicit policy-only downgrade. Add the Windows runner argument contract for command cwd, workspace roots, permission profile JSON, child environment JSON, restricted-token level, and payload command parsing. Update sandbox policy output and tests to describe the missing Windows runner instead of an unimplemented adapter. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go test -c ./internal/sandbox -o /tmp/zero-sandbox.test.exe Tested: scripts/sandbox-smoke.sh Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Add Windows sandbox command runner Add the zero-windows-command-runner executable and wire windows-restricted-token plans to pass sandbox home, command cwd, workspace roots, permission profile JSON, child env JSON, sandbox level, and command argv. Persist Windows capability SIDs under the resolved sandbox home, select read-only or writable-root capability SIDs from the permission profile, and create a restricted token before launching the payload with CreateProcessAsUserW. Package the Windows runner next to zero.exe and extend sandbox smoke coverage to cross-build it. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/release ./cmd/zero-windows-command-runner ./cmd/zero-release Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-command-runner.exe ./cmd/zero-windows-command-runner Tested: scripts/sandbox-smoke.sh Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Plan Windows sandbox ACL operations Derive a deterministic Windows ACL operation list from the restricted permission profile, including writable root allows, protected metadata deny-write entries, explicit deny-write paths, and deny-read materialization targets. The plan maps workspace and extra writable roots onto the persisted capability SIDs so the elevated setup helper can apply the same contract without reinterpreting the profile. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go test -c ./internal/sandbox -o /tmp/zero-sandbox.test.exe Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-command-runner.exe ./cmd/zero-windows-command-runner Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Add Windows sandbox setup boundary Add the zero-windows-sandbox-setup command, setup argument parsing, deterministic ACL plan hashing, and a setup marker contract that can detect profile changes requiring refresh. Make Windows native backend discovery require both the command runner and setup helper, package the setup helper with Windows releases, and include it in sandbox smoke builds. The setup command remains fail-closed until the Windows ACL applier is wired, so it cannot create a setup-complete marker without applying enforcement. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/release ./cmd/zero-windows-sandbox-setup Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go test -c ./internal/sandbox -o /tmp/zero-sandbox.test.exe Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-command-runner.exe ./cmd/zero-windows-command-runner Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-sandbox-setup.exe ./cmd/zero-windows-sandbox-setup Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Apply Windows sandbox ACL setup Implement the Windows ACL setup path with grouped native DACL updates, deny-read materialization, existing DACL snapshots, and rollback for partial application failures. Write the setup marker only after ACL setup succeeds, roll back applied ACLs if marker persistence fails, and make the Windows command runner validate the setup marker before creating the restricted token. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./cmd/zero-windows-sandbox-setup ./cmd/zero-windows-command-runner Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go test -c ./internal/sandbox -o /tmp/zero-sandbox.test.exe Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-command-runner.exe ./cmd/zero-windows-command-runner Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-sandbox-setup.exe ./cmd/zero-windows-sandbox-setup Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Fail closed for Windows network policies Add Windows network policy validation that allows only explicit network-allow until native Windows network enforcement is implemented. Make the Windows command runner reject deny, scoped, and unset network modes before launching a restricted-token process so default network-deny cannot silently run open. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./cmd/zero-windows-command-runner Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go test -c ./internal/sandbox -o /tmp/zero-sandbox.test.exe Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-command-runner.exe ./cmd/zero-windows-command-runner Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Track Windows network setup state Add a canonical Windows network-policy hash and persist it in the Windows sandbox setup marker so setup refresh can detect network mode, domain, and proxy-requirement changes. Bump the setup marker schema and add tests for domain-order normalization plus network-change marker invalidation. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go test -c ./internal/sandbox -o /tmp/zero-sandbox.test.exe Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-command-runner.exe ./cmd/zero-windows-command-runner Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-sandbox-setup.exe ./cmd/zero-windows-sandbox-setup Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Add sandbox setup CLI command Add zero sandbox setup to resolve the active sandbox policy, derive the Windows setup helper next to the command runner, and invoke it with the shared Windows setup argument contract. Return text or JSON setup results, no-op cleanly for non-Windows native backends, and add tests using an injected helper runner. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/cli ./internal/sandbox Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go test -c ./internal/cli -o /tmp/zero-cli.test.exe Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go test -c ./internal/sandbox -o /tmp/zero-sandbox.test.exe Tested: GOOS=windows GOARCH=amd64 GOCACHE=/tmp/zero-go-cache go build -o /tmp/zero-windows-sandbox-setup.exe ./cmd/zero-windows-sandbox-setup Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: scripts/sandbox-smoke.sh Tested: git diff --check * Stabilize Windows ACL canonical path test * Add TUI sandbox setup command * Gate TUI sandbox setup command * Clarify sandbox doctor setup status * Add Windows WFP network setup Adds a Windows network enforcement plan with stable WFP provider, sublayer, and filter specs for deny-mode sandbox identities. The setup helper now installs or removes those filters transactionally before writing the setup marker, and the marker tracks the network enforcement plan so stale filters require refresh. Scoped Windows network mode remains fail-closed until the proxy exception path is implemented, so it cannot silently become open network. Diagnostics now report native Windows network enforcement when the restricted-token backend is active. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestValidateWindowsNetworkPolicyAllowsNativeModes|TestValidateWindowsNetworkPolicyFailsClosedForScoped|TestBuildWindowsNetworkPlanForDenyUsesCapabilityIdentity|TestBuildWindowsNetworkPlanFailsClosedForScoped|TestWindowsNetworkPlanHashIsStableAcrossEntryOrder|TestWindowsSandboxSetupMarkerRefreshesWhenNetworkChanges|TestSelectBackendChoosesPlatformAdapterWithFallback' Tested: GOCACHE=/tmp/zero-go-cache GOOS=windows GOARCH=amd64 go build -o /tmp/zero-windows-sandbox-setup.exe ./cmd/zero-windows-sandbox-setup Tested: GOCACHE=/tmp/zero-go-cache GOOS=windows GOARCH=amd64 go build -o /tmp/zero-windows-command-runner.exe ./cmd/zero-windows-command-runner Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/cli ./internal/doctor Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: GOCACHE=/tmp/zero-go-cache scripts/sandbox-smoke.sh Tested: git diff --check * Add Windows sandbox network smoke Adds a gated Windows real sandbox smoke test that runs the compiled setup helper and command runner, verifies sandboxed writes still work, and proves network=deny blocks a loopback TCP connection from a restricted-token command. The sandbox smoke script now runs this test on Windows using the helper binaries it builds in its temporary directory. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox -run 'TestValidateWindowsNetworkPolicyAllowsNativeModes|TestBuildWindowsNetworkPlanForDenyUsesCapabilityIdentity' Tested: GOCACHE=/tmp/zero-go-cache GOOS=windows GOARCH=amd64 go test -c -o /tmp/zero-windows-sandbox.test ./internal/sandbox Tested: GOCACHE=/tmp/zero-go-cache GOOS=windows GOARCH=amd64 go build -o /tmp/zero-windows-sandbox-setup.exe ./cmd/zero-windows-sandbox-setup Tested: GOCACHE=/tmp/zero-go-cache GOOS=windows GOARCH=amd64 go build -o /tmp/zero-windows-command-runner.exe ./cmd/zero-windows-command-runner Tested: GOCACHE=/tmp/zero-go-cache scripts/sandbox-smoke.sh Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Remove legacy sandbox runner paths Drop the old direct bubblewrap adapter, the standalone zero-seccomp wrapper, legacy adapter metadata, and the Phase 0 legacy inventory now that command planning goes through the sandbox manager backends. Switch platform selection and tests to target backend names: linux-bwrap, macos-seatbelt, windows-restricted-token, and explicit policy-only degradation. Keep policy as the shared permission/profile and diagnostics surface, but stop exposing legacy adapter fields in CLI policy output and tool metadata. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/cli ./internal/tools ./internal/zerocommands ./internal/doctor Tested: GOCACHE=/tmp/zero-go-cache scripts/sandbox-smoke.sh Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Address sandbox baseline review blockers Restore sandbox.blockUnixSockets as a resolved config and policy field, route it into the Linux helper as --block-unix-sockets, and keep zero-seccomp as a Linux compatibility wrapper packaged with release artifacts. Add the missing legacy-entrypoint test referenced by the smoke script, extend helper/release/config tests, and make CLI startup tests hermetic by stubbing MCP registration where they assert quiet output. Document the compatibility path, release helper scope, and the already-resolved #237 prompt-file overlap in the PR body. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/cli ./internal/config ./internal/release ./cmd/zero-seccomp -run 'TestLegacySandboxEntrypointsAreExplicitAndExist|TestSandboxManagerBuildsCommandPlanThroughLinuxHelper|TestBuildLinuxSandboxCommandArgsSerializesPermissionProfile|TestParseLinuxSandboxHelperArgs|TestBuildLinuxSandboxBwrapArgsWrapsInnerSeccompStage|TestApplyConfiguredSandboxPolicyDiagnosticsFlags|TestResolveSandboxBlockUnixSocketsFromFile|TestCopyPackageFilesStagesLinuxSandboxHelper' Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: GOCACHE=/tmp/zero-go-cache scripts/sandbox-smoke.sh Tested: git diff --cached --check Tested: local leak scan found no matches |
||
|
KRATOS
|
efb6b79164 |
web_search robustness + chat UX: collapse, clipboard, scroll-pin, proactive search (#218)
* web_search: fix #209 follow-ups (tolerant score, render gate, redaction, FQDN) Verified-and-fixed defects from a review of #209: - Tolerant score decode (lenientScore): a stringified/odd/null "score" no longer makes json.Unmarshal discard the entire response; it degrades to absent. - Score render gate rounds first (>= 0.01) so tiny positives and negatives no longer print a noisy "score 0.00". - Domain-filter error now goes through redactWebSearchText, matching the other error paths. - Trailing-dot FQDN symmetry: "react.dev." and "react.dev" normalize the same on both the allowlist and result sides. Left as-is (intended, tested): scheme-less host:port allowlist entries. Includes mixed-type domains-array tests. * web_search: only register the built-in tool when a backend is configured When no search backend is set (ZERO_WEBSEARCH_BASE_URL / ZERO_WEBSEARCH_BACKEND), CoreNetworkTools no longer offers the built-in web_search. Previously it was always registered, so a model with an MCP search tool (e.g. Exa) would burn several calls + high-risk permission prompts on the built-in tool — which can only return "no backend configured" — before falling back to the working MCP tool. Now: no backend -> only web_fetch (+ any MCP search tool) is offered, so the model uses the MCP search directly. A configured backend behaves exactly as before. Tests: registry/web_search/specialist tests that asserted web_search is always in CoreTools now configure a backend; added a test that it is omitted when none is set. (Pre-existing, unrelated: internal/cli doctor/exec tests fail in the sandbox on origin/main too — network/provider-dependent, not touched here.) * tui: copy selected transcript text to the native OS clipboard Selecting transcript text auto-copies on mouse release, but the copy went only through OSC52 (ansi.SetSystemClipboard), which macOS Terminal.app doesn't support at all and iTerm2 / some Windows Terminal builds have off by default — so the selection silently never reached the clipboard. Copy via the native clipboard (pbcopy / clip.exe / xclip) first, which works on local terminals regardless of OSC52, and fall back to OSC52 for remote/SSH sessions where no local clipboard utility is reachable. atotto/clipboard was already in the module graph (indirect); promoted to a direct import — no new dependency. Gates: gofmt/vet/build(host+linux+windows)/tui test -race/staticcheck all pass. * tui: collapse long tool output by default with click-to-expand Long, noisy tool output (web-search / MCP / read dumps) flooded the transcript. Now a tool result whose output exceeds the live body cap renders collapsed by default — head + a "▸ N lines — click to expand" footer — and you click the card (while it is live) to expand (▾) / collapse. Mirrors the existing reasoning-row collapse, so collapsed rows also flush to scrollback clean. Scoped to avoid hiding what matters: - diff tools (edit_file / apply_patch / write_file) never collapse — their body must stay reviewable (matches the deeper flush cap intent); - the uncapped detailed transcript view (Ctrl+O, bodyCap==0) never collapses — it still shows full output; - short output (<= cardBodyMaxLines) stays inline as before. Reuses transcriptRow.expanded + the row toggle (toggleReasoningRow generalized to toggleTranscriptRow); the card head is a clickable toggle line. Running tool cards (spinner) are unchanged. Tests: collapse/expand, short-output inline, diff tools never collapse, toggle, and the head exposes a click toggle; existing diff + detailed-view tests updated. Gates: gofmt/vet/build(host+linux+windows)/tui -race/staticcheck all clean. * cli: fall back to a usable saved provider instead of re-running onboarding If the active provider lacked a credential (e.g. it relies on an env var that isn't set, or an OAuth login that isn't stored), the TUI forced first-run onboarding on every launch — even when other saved providers were fully configured. Each onboarding then appended yet another provider. Now, when the active provider isn't usable, fall back to the first usable saved provider (prefer a remote keyed one over a local endpoint) for the session, and skip onboarding. Onboarding only runs when no configured provider is usable — a genuinely fresh setup. Saved logins persist; switch the active provider with `zero provider use <name>`. Tests: firstUsableProvider prefers remote keyed, falls back to local, accepts a keyless local proxy, and reports none-usable. Gates: gofmt/vet/build (host+linux+windows)/staticcheck clean; the only failing cli tests (doctor/exec, network-dependent) are pre-existing on origin/main. * tui: hold the scroll position while output streams in The chat viewport scroll offset is measured from the bottom, so when the transcript grew (a streaming turn) the window followed the new bottom and yanked the user off whatever they had scrolled up to read. syncChatScroll now pins the viewport: while the user is scrolled up, it bumps the offset by however many lines the body grew, so the absolute view holds where they are reading; at the bottom (offset 0) it follows the tail as before. Hooked into the single Update funnel; only the scrolled-up path renders the body, so the common (at-bottom) case stays cheap. Tests: pins by the growth delta while scrolled up; resets/follows at the bottom. Gates: gofmt/vet/build(host+linux+windows)/tui -race/staticcheck clean. * agent: search the web proactively instead of answering "I don't know" When a web search/fetch tool is available and the user asks about something external the model is unsure of or that may be current (a product, library, company, price, version, recent event), the system prompt now tells it to search and read results before answering, rather than replying that it doesn't know without checking. Addresses the model saying "I don't know about that" until the user explicitly asked it to search. Tests: prompt includes the new guidance ("search the web and read"). Existing prompt assertions use Contains, so the addition is non-breaking. * tui: show clean labels for MCP tool cards (mcp_exa_web_search_exa → web search) * agent: sharpen web-search guidance (disambiguate, refine, scale) Strengthen the search rule so the model searches before answering about an external entity/product/library/model/version/release it doesn't recognize, doesn't assume the most common meaning of an ambiguous name (disambiguates by the conversation's domain), and refines + re-searches when results look off-topic — rather than confidently answering the wrong interpretation (e.g. reading "Fable 5" as a video game instead of an AI model). Also: don't web-search timeless facts or workspace-answerable questions (use the file tools), keep queries short, and scale the number of searches to the question. Tests: prompt includes the new guidance ("search the web before answering", "do not recognize"). Existing assertions use Contains, so non-breaking. * test: gofmt setup_fallback_test.go (comment alignment) * Address review feedback on web_search + chat UX Resolve CodeRabbit review comments on this PR: - firstUsableProvider: skip a profile whose CatalogID no longer resolves *and* that has no explicit BaseURL (no endpoint → unusable). A stale CatalogID with a BaseURL still works as a custom endpoint and is kept. - providerProfileIsLocal: classify by parsed URL hostname (exact localhost / 127.0.0.1 / ::1) instead of substring matching, so hosts like "notlocalhost.com" are no longer misread as local. - lenientScore: reject non-finite scores (ParseFloat accepts "NaN"/"Inf") so the documented score filter actually holds and the renderer never emits a non-finite value. - syncChatScroll: shift the pinned from-bottom offset by the signed line delta so the view also holds when the body shrinks (card collapse, transcript clear), clamped at zero; previously only growth was handled. - copyTranscriptSelectionCmd: report "Copy failed" when both the native clipboard and the OSC52 fallback fail, instead of claiming success; keep the selection so the user can retry. - Drop vestigial ZERO_WEBSEARCH_BACKEND test env: the native backend was removed (MCP path), so the var is read by no product code and the references misrepresented it as a config knob. Adds tests for the local-host parsing, the catalog/BaseURL skip, and the non-finite score rejection. |
||
|
|
de5d402e1f |
web_search: add domains allowlist + score field (#209)
* web_search: add domains allowlist + score field The web_search tool now accepts an optional 'domains' array that filters results to the listed hostnames before they reach the model. This is the zero-only prompt-injection defense that none of the major CLIs ship: a model that the user has constrained to a known-good set of domains cannot be tricked into fetching arbitrary pages from search results. Also surfaces the provider's optional 'score' field on each result row when present (zero/absent scores are not rendered, to keep the common case tidy). The 'score' field is parsed from the generic JSON shape already understood by the httpSearchBackend. Filters are applied after the search returns, before the empty-result short-circuit, so a non-empty allowlist that ate every hit surfaces as a clear 'no results matched domains' error rather than a misleading 'no results' message. Co-Authored-By: Claude <[email protected]> * web_search: fail closed on all-invalid domains, strip :port on canonicalize CodeRabbit review fixes: 1. canonicalizeWebSearchHost: switch parsed.Host -> parsed.Hostname() so an allowlist entry like 'https://react.dev:443/path' normalizes the same as 'react.dev'. The previous code left ':443' in the string, which silently broke every match against a result URL on the default port. 2. stringListArgWebSearch: return a 'provided' bool so Run can distinguish 'no allowlist' from 'allowlist that normalized to empty'. The previous code returned len(domains)==0 in both cases, and Run fell through to the unfiltered path — defeating the prompt-injection defense the parameter exists to provide. Now we error fail-closed. 3. Tests: - TestWebSearchDomainsFilterRejectsAllInvalidInputs: locks in the fail-closed contract for all-whitespace, all-empty, embedded-space, and mixed-invalid allowlists. - TestWebSearchDomainsFilterAcceptsHostPort: locks in the Hostname()-strips-port fix for the scheme'd case and asserts that scheme-less 'host:port' is rejected. - TestCanonicalizeWebSearchHostStripsPort: the direct unit-level regression test for canonicalizeWebSearchHost. Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Vasanthdev2004 <[email protected]> Co-authored-by: Claude <[email protected]> |
||
|
KRATOS
|
671af594fb |
feat(sandbox): harden internal/sandbox — re-entrancy guard, SOCKS5 egress, sandbox-aware search, fine-grained path lists (#199)
* feat(sandbox): harden internal/sandbox with four opt-in capabilities
All four are off by default and fail closed; existing behavior is unchanged
when the new policy fields are empty / ZERO_SANDBOXED is unset.
1. Re-entrancy guard — ZERO_SANDBOXED=1 marks every wrapped command; a nested
BuildCommandPlan returns a pass-through plan (no double bwrap/sandbox-exec,
no second egress proxy). New: EnvSandboxed, IsAlreadySandboxed().
2. SOCKS5 egress — the scoped-egress proxy also serves SOCKS5 CONNECT on a
second loopback port through the SAME authorize()/domainAllowed() gate.
ProxyEnvWithSocks points ALL_PROXY at socks5://<addr>; HTTP(S)_PROXY stay on
the HTTP listener; the sandbox-exec profile allows both proxy ports.
Bubblewrap scoped still collapses to deny.
3. Sandbox-aware search — ReadExclusionGlobs(policy, scope) returns ripgrep
--glob '!<pat>' for DenyRead subtrees; grep/glob skip read-denied subtrees
at walk time via Engine.ReadPathExcluded/ReadDirExcluded when sandboxed.
4. Fine-grained path lists — Policy.AllowRead/DenyRead/AllowWrite/DenyWrite.
Read = deny-then-allow (more-specific AllowRead re-includes); Write =
DenyWrite > workspace/scope > AllowWrite > deny. AllowWrite is reflected in
the OS write binds; DenyWrite is emitted as sandbox-exec deny rules. Each
entry is home-expanded, made absolute, and symlink-resolved.
Tests: internal/sandbox/{reentrancy,egress_socks,pathlists}_test.go and
internal/tools/read_exclusions_test.go.
Gates: gofmt, go vet, go build, go test (incl. -race), staticcheck (no new
findings), govulncheck (0), deadcode -test=false (no new unreachable funcs).
* sandbox: address review — SOCKS5 method negotiation + cache read-exclusion roots
- egress_socks: respect SOCKS5 method negotiation (RFC 1928). The proxy now
inspects the client's offered methods and replies 0xFF ("no acceptable
methods") when no-auth was not offered, instead of forcing no-auth.
- pathlists/engine: resolve DenyRead/AllowRead roots ONCE per search walk via a
new ReadExclusions matcher (Engine.ReadExclusions), replacing the per-path
ReadPathExcluded/ReadDirExcluded that re-ran Abs/EvalSymlinks for every visited
path during a grep/glob walk.
Re-entrancy guard left as-is by design: ZERO_SANDBOXED is an internal marker set
only on commands zero wraps; cross-process re-entrancy detection legitimately
relies on the inherited env, and a manually-exported flag is a self-opt-out, not
an external bypass.
Tests: SOCKS no-acceptable-methods rejection; nested-AllowRead descent via the
matcher. Gates: gofmt, vet, build, test (incl. -race), staticcheck (no new
findings), govulncheck (0), deadcode (no new unreachable funcs).
* sandbox: fix windows smoke test + symlink-resolve DirExcluded prefix
- pathlists_test: TestSandboxExecProfileEmitsDenyWriteRule now builds the
expected deny clause via sandboxProfileString(secret), so it matches the
profile's own escaping (Windows doubles backslashes). Fixes the
windows-latest smoke failure; macOS/Linux unaffected (slash paths, no-op
escaping).
- pathlists: ReadExclusions.DirExcluded now EvalSymlinks-resolves the candidate
path before the nested-AllowRead prefix check, matching the resolution applied
to allowRoots — otherwise a denied dir reached THROUGH a symlink fails to match
a nested AllowRead root and is wrongly skipped, dropping a re-included subtree.
Added TestDirExcludedResolvesSymlinkPrefix (skips where symlinks are
unavailable).
Gates: gofmt, vet, build, test (incl. -race), staticcheck (no new findings),
govulncheck (0), deadcode (no new unreachable funcs).
* sandbox: strengthen re-entrancy guard with a corroborating backend marker
Addresses review feedback that the re-entrancy guard trusted a single ambient
env var. IsAlreadySandboxed now requires BOTH correlated markers that
sandboxEnvironment sets together on every wrapped command — ZERO_SANDBOXED=1 AND
a non-empty ZERO_SANDBOX_BACKEND (new EnvSandboxBackend const) — so a lone
stray/hand-exported ZERO_SANDBOXED=1 no longer forces an unsandboxed
pass-through. Pass-through (direct) plans set neither marker, so genuine nesting
still detected and double-wrapping still avoided.
Tests updated: TestIsAlreadySandboxed now asserts each marker alone is
insufficient and both together are required; the re-entrancy and wrapping plan
tests set/clear both markers explicitly.
Gates: gofmt, vet, build, test (incl. -race), staticcheck (no new findings),
govulncheck (0), deadcode (no new unreachable funcs).
* sandbox: don't subject first-party web tools to the network policy by default
The sandbox was reported as too strict: web_search (and web_fetch) were blocked
with "network access is disabled by the sandbox policy" because the default
policy is network=deny and the in-process web tools were gated by the same
NetworkHostAllowed check as the sandboxed shell's egress.
That network policy exists to confine the sandboxed SHELL's egress, which these
in-process tools don't use. New Policy.EnforceToolNetwork (off by default) makes
NetworkHostAllowed exempt web_search/web_fetch unless an operator opts in. The
sandboxed-shell egress decision (Evaluate / effectiveNetworkMode) is unchanged,
and the tools keep their own SSRF/private-IP, port, redirect, and redaction
safeguards. Set EnforceToolNetwork to hold the tools to the allow/scoped/deny
policy as before.
Tests: engine default-exempt + enforced-table cases; web_search default-allows
and enforced-when-flag; web_fetch enforced cases set the flag. Gates: gofmt, vet,
build, test (incl. -race), staticcheck (no new findings), govulncheck (0),
deadcode (no new unreachable funcs).
* sandbox: re-review fixes — disabled policy inert; path lists independent of EnforceWorkspace
- engine: ReadExclusions() / ReadExclusionGlobs() now return inert results under
ModeDisabled, so a disabled policy never filters grep/glob (parity with
Evaluate, which already allows everything when disabled).
- engine/pathlists: the fine-grained path lists (DenyRead/DenyWrite, with
AllowRead/AllowWrite) now apply in Evaluate whenever the sandbox is enforcing,
independent of EnforceWorkspace — matching the grep/glob path that already
honors DenyRead directly. The workspace boundary (scope.validate) stays gated
by EnforceWorkspace via a param threaded through validatePathWithPolicy /
validateWritePath; DenyWrite wins regardless.
- reentrancy_test: TestSandboxEnvironmentMarksSandboxed now asserts BOTH
correlated markers (ZERO_SANDBOXED=1 and ZERO_SANDBOX_BACKEND) so dropping
either would fail the test.
Tests added: path lists enforced with EnforceWorkspace off; ReadExclusions inert
under ModeDisabled. Gates: gofmt, vet, build, test (incl. -race), staticcheck
(no new findings), govulncheck (0), deadcode (no new unreachable funcs).
* sandbox: enforce path lists even without a workspace root
Re-review follow-up: the Evaluate path-validation loop was still guarded by
request.WorkspaceRoot != "", so an engine built without a workspace root skipped
DenyRead/DenyWrite/AllowRead/AllowWrite even for absolute paths. The loop now
runs unconditionally; the workspace boundary (scope.validate) stays gated on
having a root (enforceWorkspace = EnforceWorkspace && WorkspaceRoot != ""), while
the path lists match absolute paths regardless. An unanchorable relative path
(no workspace root) fails closed when anything is configured to enforce, and is a
no-op otherwise (unchanged from prior behavior).
Test: TestEvaluatePathListsWithoutWorkspaceRoot. Gates: gofmt, vet, build, test
(incl. -race), staticcheck (no new findings), govulncheck (0), deadcode (no new
unreachable funcs).
* sandbox: base no-workspace fail-closed on normalized (enforceable) path lists
policyHasPathLists now resolves the lists (resolvePolicyPaths/
resolveWriteRootPaths) instead of counting raw config, matching how the rest of
the file normalizes them. A typo or non-existent entry (dropped during
resolution) therefore no longer makes validatePathWithPolicy fail-close every
relative request when there is no workspace root.
Gates: gofmt, vet, build, test (incl. -race), staticcheck (no new findings),
govulncheck (0), deadcode (no new unreachable funcs).
|
||
|
KRATOS
|
474f494b93 |
Deep-audit remediation + sandbox hardening (batches 1–35) (#189)
* audit batch 1: redact nested secrets fully; count image tokens in compaction estimate
- secrets.Redact: replace longest matches first so a containing PRIVATE KEY
block is redacted before a nested shorter match corrupts its exact string
(audit medium internal/secrets/scanner.go:81) + regression test.
- agent compaction estimateTokens: add a flat per-image token cost so an
image-heavy context trends toward compaction instead of reading as ~0
(audit medium internal/agent/compaction.go:58) + test.
- ledger: re-verify against current main; mark scanner.go:35 and
redaction.go:362 already-fixed-on-main, record this batch's fixes.
* audit batch 2: worktrees defaultRunGit separates stdout/stderr
Capture git stdout and stderr into separate buffers instead of CombinedOutput,
so parsed rev-parse output is not polluted by git warnings and CommandResult.Stderr
carries the real error text (audit medium internal/worktrees/worktrees.go:247) + test.
* audit: re-verify all 87 pending findings against current main
Independent agent per file re-checked each open/partial finding against the
actual current code (the 06-11 ledger was verified on a since-diverged branch),
with an adversarial second pass on every already_fixed verdict. Authoritative
result: 78 genuinely pending (66 still_open + 12 partial); 9 rows were already
fixed on main and are reclassified. Full table in 2026-06-13-reverification.md.
* audit batch 3 (agent): cancellation identity, always-allow w/o sandbox, compaction usage, dedup
- loop.go: check ctx.Err() before collected.Error so a cancelled run returns
context.Canceled (errors.Is holds) instead of a plain stringified error.
- loop.go: 'Always allow' with no sandbox engine now allows THIS call instead of
denying it (persistence is skipped when there's nowhere to persist).
- loop.go: remove a duplicate schema["items"] assignment in propertyToRuntimeMap.
- compaction.go: forward OnUsage (token accounting) through the summarizer while
still omitting OnText, so compaction's token cost is counted in usage/budgets.
- tests for all three behavioral fixes (each fails pre-fix).
* audit batch 4 (cli): list-tools -o json, interrupted -o json terminal, surface session-record failures
- exec --list-tools -o json now emits a JSON tool listing instead of falling
through to the human-readable text (audit medium exec.go:182) + test.
- interrupted run with -o json now emits a terminal error+done on stdout instead
of only printing to stderr, so a JSON consumer sees a clean end (medium exec.go:500) + test.
- execSessionRecorder: surface a latched session-append failure to stderr once at
run end (deferred) instead of silently dropping it (medium exec_sessions.go:114/115) + test.
- left app.go trailing-arg ignore as-is: documented-intentional, TUI has no argv prompt.
* audit batch 5 (zerogit): status -z porcelain (renames + non-ASCII paths); document env-drop on plain Runner
- parseStatus: switch `git status` to `--porcelain -z` and parse the
NUL-delimited form (audit medium zerogit.go:212). Previously the default
--short output was split on newlines, so a rename `R old -> new` was kept as
one bogus path and a non-ASCII/whitespace filename arrived C-quoted+escaped
(e.g. `"caf\303\251.txt"`). The -z form never quotes paths and emits a
rename/copy as `XY <dest>\0<src>`, so we now record the destination verbatim
and consume the source field. + TestParseStatusZHandlesRenamesAndSpecialPaths.
- resolveRunners: document that the plain-Runner EnvRunner adapter intentionally
drops env (audit low zerogit.go:417). The branch is reached only when a caller
supplies a custom Runner without a RunGitEnv — in practice fake-Runner tests.
Production leaves both nil and gets defaultRunGit/defaultRunGitEnv, which honor
GIT_INDEX_FILE, so snapshot-diff isolation holds. Routing env calls to real git
instead would break the fake-Runner test pattern and fix nothing in production.
- update Inspect/Commit mock tests to the -z status format and the new command.
* audit batch 6 (config): reject negative maxTurns; drop dead contract-gap helpers
- FileConfig.UnmarshalJSON now rejects a negative maxTurns instead of letting the
`MaxTurns > 0` merge gates silently drop it and fall back to the default,
hiding the misconfiguration (audit medium resolver.go:136). 0 is left as the
"unset" sentinel (omitempty) and still falls back to the default; the CLI
--max-turns flag continues to reject 0 too since there it is distinguishable
from absent. + TestResolveRejectsNegativeMaxTurns / ...ZeroFallsBackToDefault.
- remove dead config/contracts.go (ContractGap, DefaultContractGaps,
FindContractGapsByMilestone, ContractOwnerRuntime) and its test — referenced
only by that test, never by production (audit low contracts.go:12).
- NOT changed: ResolveMCP intentionally does not run the provider command
(TestResolveMCPDoesNotRunProviderCommand guards this), so the "ZERO_PROVIDER_COMMAND
MCP servers dropped" row is by-design, not a defect.
* audit batch 7 (mcp): bound captured MCP server stderr
connectStdio attached a plain bytes.Buffer as cmd.Stderr that is only ever read
on initialize failure, yet it kept growing for the entire process lifetime as a
long-lived server logged to stderr (audit medium client.go:98). Replace it with
a concurrency-safe boundedBuffer capped at 64 KiB that keeps the head (enough for
init-error diagnostics) and discards the rest, reporting full write lengths so
os/exec never sees a short write. + TestBoundedBufferCapsRetainedBytes.
Left as-is (not defects worth the risk in the MCP client):
- client.go writer.write under client.mu: already designed to release the lock
before the unbounded response wait; the residual concern is a server that stops
draining stdin, an involved async rewrite for a theoretical deadlock.
- readLoop not answering server-initiated requests (ping): responding safely
needs re-locking around the shared writer; no configured server relies on it.
* audit batch 8 (tools/providers): read_file truncation marker; drop dead provider branch
- read_file: when a max_lines cut occurs, append an explicit "[truncated: N more
line(s)... set start_line=X to continue]" marker to the output. Previously only
the Result.Truncated flag was set, which is invisible in the rendered text, so
the model could not distinguish a cut read from a complete one (audit low
read_file.go:89). + TestReadFileToolMarksTruncation.
- providers/factory: simplify `if !explicitProvider || isImplicitOpenAI(...)` to
`if !explicitProvider` and delete the now-unused isImplicitOpenAI. The clause
was dead: explicitProvider==true implies ProviderKind or Provider is set, but
isImplicitOpenAI required both empty, so it could never contribute a case
(audit low factory.go:118). Behavior-preserving; providers tests still green.
* audit batch 9 (sessions): Tree builds from a single List() snapshot (no N+1 disk scans)
Store.Tree recursed via ListChildren per node, and each ListChildren ran a full
store.List() that re-read every metadata.json from disk — O(nodes * total-sessions)
reads for one tree (audit medium lineage.go:140). Snapshot all sessions once, index
children by parent in memory, and recurse over that. Child ordering is factored into
sortChildSessions so ListChildren and Tree stay identical; path-scoped cycle
detection (delete(seen,...)) is preserved. Existing tree/lineage tests still pass.
Left as-is: store.go timestamp() second precision (RFC3339) — switching to
RFC3339Nano would break lexical UpdatedAt ordering against existing second-precision
records ("...00.5Z" sorts before "...00Z"), so it is not a safe one-line change.
* audit batch 10 (specialist): serialize accounting dedup-then-append
recordSpecialistStop and appendSpecialistUsageRollup checked for an existing event
(specialistEventExists → ReadEvents) and then appended under SEPARATE store locks,
so two concurrent finishers — a foreground onExit racing a TaskOutput poll, or a
background reaper — could both pass the dedup check and write duplicate stop/usage
events (audit medium accounting.go:72). Guard each check-then-append pair with a
package-level accountingMu (Executor is used by value, so a struct field would not
be shared). + TestRecordSpecialistStopDedupesUnderConcurrency (passes under -race).
* audit batch 11 (zeroruntime/gemini): O(1) text accumulation; gemini cached-token counts
- zeroruntime CollectStreamWithOptions accumulated assistant text with
`collected.Text += event.Content`, reallocating the whole string on every chunk
(O(n^2) over a long streamed response). Accumulate in a strings.Builder on the
collector and materialize it once in flush, the single finalization point hit
before every return (audit low helpers.go:104). Existing collect tests still pass.
- gemini decoded usageMetadata without cachedContentTokenCount, so a cached-prompt
response reported zero cached input tokens while the other providers report them
(audit medium gemini/types.go:88). Decode the field and thread it through
streamState into NormalizeUsage as CachedInputTokens. + extended
TestStreamCompletionEmitsTextUsageAndReasoningTokens.
* audit batch 12 (specialist): parse child stream without a line-length cap; drop dead helper
- ParseStream used a bufio.Scanner capped at 1 MiB, so one large stream-json line
from a child (a big tool result or final answer) hit bufio.ErrTooLong and aborted
the entire specialist run (audit medium streamer.go:43). Switch to a bufio.Reader
with ReadString, which has no per-line limit; the child is our own trusted
subprocess, so its output length is the legitimate bound. Line numbering and
blank-line/EOF handling are preserved. + TestParseStreamHandlesLineLargerThan1MiB.
- remove dead formatTaskOutput (output_tool.go:244): zero callers and zero tests;
it only forwarded to formatTaskOutputSummary, which is the one actually used.
* audit batch 13 (background/specialist): kill background tasks as a process group
Background children were launched in the parent's process group and terminated by
signalling only the single PID, so any process the task forked was orphaned and
outlived a cancel; the signal also risked hitting a recycled PID after the child
was reaped (audit medium process_posix.go:38/50).
- ConfigureChildProcessGroup (Setpgid on POSIX, no-op on Windows where taskkill /T
already tree-kills) is applied at launch so the child leads its own group.
- terminateProcess now SIGTERM/SIGKILLs the negative PID (the whole group), with a
pid>1 guard so a bogus 0/1 can never expand to "our own group" or "everything",
and treats ESRCH as already-gone. Liveness polls the group. Targeting a group
(rarely recycled vs. a bare PID) also shrinks the reaped-then-recycled window.
- specialist launchBackgroundProcess opts every child into its own group; on a
SetPID failure it now kills the child + group, marks the task errored, and records
the stop (deduped) instead of leaking an untracked orphan.
- tests launch as group leaders; new TestTerminateProcessKillsForkedChildren proves
a forked grandchild dies with the group.
* audit batch 14 (sandbox): quote-aware shell segment splitting
splitShellSegments used a quote-unaware strings.Replacer, so a shell operator
inside quotes was treated as a separator: `git commit -m "use top | less"` split
into a `less` segment and `echo "a; vim b"` into a `vim` segment, both falsely
flagged as interactive (audit medium safe_command.go:138).
Replace it with a quote-aware scanner that mirrors the shell: single quotes make
everything literal; double quotes keep $(...)/`...` substitution active but treat
|, ;, && as literal; unquoted text splits on the same operator set as before
(;, |, ||, &&) plus substitution boundaries. Real unquoted operators still split,
so an interactive program behind a separator — or inside a live `"$(...)"`
substitution — is still caught (no new false negatives, the security-relevant
direction). + TestDetectInteractiveQuoteAwareSeparators covering both directions.
* audit batch 15 (cron): collapse DST fall-back repeated hour to one fire
On a fall-back day the clock repeats an hour (e.g. 01:30 EDT then 01:30 EST), so
Next("30 1 * * *", <01:30 EDT>) returned the same-day 01:30 EST — firing a daily
job twice (audit medium schedule.go:171, which previously guarded only the
spring-forward gap).
Next now skips a match whose local wall-clock minute equals `after`'s but whose
absolute time is later (the repeated-hour duplicate) and keeps searching. Normal
forward search always advances the wall-clock, so this only triggers on the
fall-back repeat; when the scheduler passes the last fire time as `after`, the
repeated hour collapses to a single fire. + TestNextDSTFallBackDoesNotRepeatHour
(spring-forward termination test still passes).
Note: Next is stateless, so it dedupes relative to `after`; a caller that passes
an arbitrary mid-hour "now" instead of the last fire time is out of scope here.
* audit batch 16 (mcp): skip SSE notifications before the response; larger SSE buffer
- decodeSSERPCMessage returned the first non-empty `message` event unconditionally,
so a server-initiated notification/request arriving before the response made the
caller see an id mismatch and fail (audit medium network_client.go:621). It now
skips any message carrying a method (notifications/requests have one; a response
does not) and returns the first real response; the caller's rpcIDMatches still
validates the id. + TestDecodeSSERPCMessageSkipsNotifications.
- raise the per-event SSE scanner cap from 1 MiB to a bounded 8 MiB so a large but
legitimate MCP message no longer hits bufio.ErrTooLong and fails the request
(audit medium network_client.go:637); still bounded against an unbounded server.
Out of scope (kept): full reconnect of a persistent SSE stream after a fatal read
error — a larger change than this remediation.
* audit batch 17 (sessions): fsync session metadata before the atomic rename
writeMetadata wrote the temp file with os.WriteFile (no sync) and renamed it into
place, so a crash right after the rename could expose a metadata file whose bytes
were never flushed — a torn or empty file that corrupts the whole session (audit
medium store.go:632). Write+fsync the temp via a new writeFileSync helper before
renaming, closing that window. + TestWriteFileSyncRoundTrips.
Deliberately scoped:
- Event-log appends (appendEventLocked) are NOT fsynced per append: that would add
a disk flush to every event (every tool call), and a hard-crash loss of the log's
unsynced tail is acceptable, whereas torn METADATA is not. Metadata is the
integrity-critical part and is what this syncs.
- Directory fsync is omitted for cross-platform simplicity (it errors on Windows);
the file fsync already prevents the torn-contents failure, the serious case.
Left for follow-up (in this bucket but not safe/clear wins here):
- hooks append sequence still re-reads the file per append; the in-memory cache is
cross-process-unsafe and the safe last-line read is fiddly for a low-volume log.
- Fork event re-append/de-dup is nuanced usage-accounting and needs its own change.
* audit batch 18 (agent): count tool-defs in compaction estimate; don't deny an always-allow on persist failure
- maybeCompact's estimate omitted the tool definitions, which ride on every
request, so the real input could exceed the model limit while the message-only
estimate stayed under threshold and compaction never fired (audit medium
compaction.go:58). partitionTools is now computed before maybeCompact (it does
not depend on the messages) and the exposed tools' estimated tokens are added to
both the threshold check and the post-compaction shrink check. + estimateToolDefTokens
and TestEstimateToolDefTokensCountsDefinitions.
- PermissionDecisionAlwaysAllow denied the call when persistPermissionGrant failed,
overriding what the user explicitly allowed (audit medium loop.go:468). Persisting
is now best-effort: a failure (or no sandbox engine) just means the grant is not
remembered and the user is re-prompted next time, not denied now. This also drops
the dead requestEvent.GrantMatched/Grant writes (audit low loop.go:473) — the
emitted event derives them from the sandbox decision via buildPermissionEvent.
Left intentional (documented, not defects): the reactive-retry path omits OnText to
avoid duplicating mid-stream text (loop.go:183); OnContext/MeasureContext is a
complete, tested context-breakdown scaffold awaiting a consumer (types.go:183).
* audit batch 19 (usage): price escalated usage from the event's own model
exec persists payload["model"] on --allow-escalation runs (the model can change
mid-run), but BuildReport priced every event from the session's Metadata.ModelID
and never read the payload, so usage from an escalated model was mis-priced at the
base model (audit medium report.go:17). usageEventPayload now carries Model and
BuildReport prefers it, falling back to the session model when absent (older
events stay byte-identical and behave as before). + TestBuildReportPricesFromEventModelWhenPresent.
* audit batch 20 (sessions): don't copy usage events into a fork (no double-count)
Fork re-appended every parent event, including provider_usage, into the child, so
a usage report aggregating the parent and the fork double-counted the parent's
usage (audit medium store.go:387). Skip EventUsage when copying — it already
counted against the parent and is not part of the conversation history a fork
replays — and record the actual copied count in the fork marker. + TestStoreForkSkipsUsageEvents.
* audit batch 21 (cli/skills): warn on duplicate skill names instead of silently dropping
skills.Load deduplicates same-named skills (first directory wins) and records the
dropped collisions via skills.Duplicates, but nothing surfaced them, so a shadowed
skill just vanished with no signal — and Duplicates had no production caller (audit
low skills.go:91). `skills list` now reports each collision to stderr (keeping
stdout/--json clean), using Duplicates. + TestRunSkillsListWarnsOnDuplicateNames.
Note: skills.Get is kept — it is a by-name accessor used throughout the package's
tests, not dead code.
* audit batch 22 (cli/cron): capture the run error from stream-json stdout
Cron jobs run exec with --output-format stream-json, so a failure is reported as an
`error` event on STDOUT, but the run record's Error was set only from stderr — so a
failed job often recorded an empty/uninformative error while the real message sat
unread in outBuf (audit low cron_run.go:150). On a non-zero exit, scan the stdout
stream-json for the error event's message and prefer it, falling back to stderr.
+ extractStreamJSONError and TestExtractStreamJSONError.
* audit batch 23 (sandbox): richer macOS seatbelt profile (signal + mach-lookup)
The generated sandbox-exec profile was default-deny with only process/sysctl/file
allowances, so two real gaps remained on macOS (sandbox quality report H1):
- A sandboxed script could not signal the children it spawned — `kill` returned
"Operation not permitted" under seatbelt, breaking common `cmd & ... kill`,
test-runner, and timeout patterns. Add `(allow signal (target self) (target pgrp))`,
which stays scoped to the command's own process group.
- XPC to system daemons was denied, so keychain (securityd/trustd), user/group
lookup (opendirectoryd), preferences (cfprefsd), network config
(SystemConfiguration), launch services, and the pasteboard all failed. Add a
curated `(allow mach-lookup ...)` allowlist (`sandboxMachServices`).
Neither touches the file-write or network rules, so the workspace boundary is
unchanged — verified on a real macOS host: `security`/`scutil`/`pbpaste`/self-kill
now succeed while writes to $HOME and /etc are still denied. iokit-open was tried
and dropped as unnecessary. + unit assertions (TestSandboxExecProfileGrantsSignalAndMachLookup)
and extended the darwin integration test with a self-kill case and an
outside-workspace deny check.
* audit batch 24 (sandbox/tools): remove dead OnSandboxDecision hook
RunOptions.OnSandboxDecision had no setter anywhere, yet registry.RunWithOptions
spawned a goroutine (with panic recovery) for it on every sandboxed tool call —
pure per-call overhead for an unreachable callback (sandbox report M1). The sandbox
decision is already surfaced to observers via the permission event
(buildPermissionEvent reads the decision), so this hook is redundant; delete the
field and its call site.
* audit batch 25 (sandbox/cli): exact-path grants — create and revoke by path
The engine already matched ScopeFile grants and DeriveScope already produced them
from path/file tool args, but the CLI could neither create one explicitly nor
revoke a single one (sandbox report L3 / prompt 7). Add:
- GrantStore.RevokePath(tool, path): revokes only the grant whose scope matches the
given file/dir, leaving tool-wide and other-path grants intact (path canonicalized
to absolute like stored scopes). + TestGrantStoreRevokePathRemovesOnlyMatchingScope.
- `zero sandbox grants allow <tool> --path <p>`: persists an exact-file (ScopeFile)
grant, path resolved to absolute so it matches lookups and revokes.
- `zero sandbox grants revoke <tool> --path <p>`: revokes just that grant (default
still revokes all grants for the tool). + TestRunSandboxGrantsCreateAndRevokeByPath.
The TUI permission-card "Allow always (exact path)" option is intentionally left for
the TUI feature branches (it overlaps #187/#188); the engine + CLI now cover the
full create/match/revoke round-trip for file-scoped grants.
* audit batch 26 (sandbox): Engine.Precheck — report violations before execution
Add Engine.Precheck(ctx, request) []Violation, which reports the sandbox
violations that would block a tool request before it runs (sandbox report / prompt 6).
It reuses Evaluate so policy is not duplicated: an allowed or merely-prompted
request yields no violations; a denied request yields its violation (via the shared
violationsFromDecision helper, with a ViolationPolicyDenied fallback for a deny that
carries no structured violation). A nil engine yields nothing.
+ TestEnginePrecheckReportsViolationsBeforeExecution.
Wiring Precheck into the batch-confirmation prompt is intentionally left to the TUI
branch: it touches prompt rendering, and short-circuiting the agent loop's deny path
would skip the before/after-tool hooks that currently fire around a policy-denied
call. The engine API is in place for that consumer.
* audit batch 27 (sandbox): scoped-egress domain prompt with timeout + caching
The scoped-egress proxy failed closed on any host outside the allowlist. Add an
opt-in DomainPrompt callback (prompt 8): an UNKNOWN host (neither allowed nor
explicitly denied) is authorized at request time via the callback instead of being
refused outright. An explicitly denied host is still refused without prompting; the
callback is bounded by PromptTimeout (default 60s, timeout/error => deny); decisions
are cached for the proxy's lifetime so a host is prompted at most once. With no
callback set the strict fail-closed behavior is unchanged. Wired through both the
CONNECT and HTTP handlers via authorizeTarget.
+ TestEgressProxyAuthorizeDomainPrompt / ...TimeoutDenies / ...NilPromptFailsClosed.
The user-facing prompt that supplies the callback is left to the permission UI (TUI)
branch; the egress mechanism (callback + timeout + cache) is in place and tested.
* audit batch 28 (sandbox): AST-based shell analysis (AnalyzeCommand)
Add AnalyzeCommand(script) AnalysisResult, a static shell analyzer built on the
mvdan.cc/sh/v3 parser (prompt 12). Walking the parsed command tree makes it a more
precise second opinion than the regex detector: a program name only counts when it
is an actual command, so `echo "vim ..."`, `printf 'open with vim'`, and
`git commit -m "rm -rf /"` are clean, while real commands are flagged —
interactive (editors/pagers + REPLs, with the same -e/script suppression as
nonInteractiveREPLFlags), destructive (rm -rf/-fr, dd, mkfs, shred, ...), and
network (curl/wget/ssh/nc/...). An unparseable script yields TooComplex so callers
can treat it as higher-risk. + TestAnalyzeCommand (16 cases) / ...EmptyIsClean.
Dependency: mvdan.cc/sh/v3 pinned to v3.11.0 — compatible with the existing go 1.24
directive (the latest v3.13.1 would have bumped the module to go 1.25). It is kept
as a standalone analyzer for callers to consult; feeding it into Engine.Evaluate is
deliberately not done here, since that would change allow/deny classification.
* audit batch 29 (sandbox): unify proxy-env generation (ProxyEnv)
Export ProxyEnv as the single source of truth for proxy-env injection (prompt 9
req 1), replacing the unexported scopedProxyEnv used by the sandboxed-shell path.
+ TestProxyEnv.
Investigation that bounds the rest of prompt 9 (documented in the ProxyEnv doc):
- web_fetch already honors HTTP_PROXY/HTTPS_PROXY — its transport clones
http.DefaultTransport, whose Proxy is http.ProxyFromEnvironment.
- MCP children already inherit the parent env (mergeProcessEnv appends os.Environ),
so they pick up any proxy vars the agent has.
So the only missing link to scope FetchUrl/MCP is a SESSION-level egress proxy whose
address reaches the agent process. That is deliberately NOT wired here: doing it
safely requires allowlisting the active LLM provider's own domain first, or the
agent's provider calls would route through the scoped proxy and be denied — a
correctness pitfall that also can't be verified end-to-end on this host. Flagged
rather than shipped blind.
* audit batch 30 (sandbox): Linux seccomp Unix-socket block — UNVERIFIED, needs Linux CI
Adds a pure-Go (no cgo, no vendored binary) seccomp BPF filter that denies
socket(AF_UNIX) with EPERM on x86-64/arm64, closing the Unix-socket gap bubblewrap's
filesystem/network isolation leaves open (prompt 11):
- internal/sandbox/seccomp.go: the classic-BPF program as a platform-neutral
[]sockFilter, built and UNIT-TESTED (structure + every jump offset in range — the
classic way a hand-written BPF filter goes silently wrong).
- seccomp_linux.go: ApplyUnixSocketBlock installs it via prctl(NO_NEW_PRIVS) +
prctl(SET_SECCOMP, MODE_FILTER). seccomp_other.go is a no-op everywhere else.
- cmd/zero-seccomp: a tiny linux-only exec wrapper that applies the filter then
execs the real command (the standard way to install seccomp in a child with
os/exec); degrades gracefully (warns + runs) if the kernel lacks seccomp.
⚠️ HONEST CAVEAT: I developed this on macOS and CANNOT run the actual AF_UNIX block
here. darwin build, `GOOS=linux` cross-compile, vet, and the structure test all pass,
but the unit test verifies the program's SHAPE, not that the kernel actually blocks
AF_UNIX — that MUST be verified on Linux CI (create a Unix socket under the wrapper
and assert EPERM). Safe to ship as-is: it is linux-only, no-op on every other
platform, and not wired into the bwrap command yet (opt-in prefixing the sandboxed
command with zero-seccomp is the remaining integration), so it cannot regress any
current path.
* audit batch 31 (sandbox): macOS denial log monitor (opt-in, default off)
Re-adds the macOS seatbelt-denial monitor as an opt-in feature (prompt 10). When
Policy.MonitorDenials is set, the sandbox-exec profile tags its default-deny with a
marker and bash tails `log stream` for that tag during the command, appending any
captured denials as a <sandbox_violations> block to the command's stderr so the
model can see what was blocked.
- DenialMonitor (darwin) tails/parses tagged denials with a capped, deduped ring
buffer and noise filtering; no-op stub on other platforms. parseSandboxDenyLine is
unit-tested. + profile-tag test + appendSandboxViolations test.
- Default OFF: with MonitorTag empty, StartDenialMonitor is a no-op and the command
path is byte-for-byte unchanged, so there is no regression or per-command overhead
unless enabled.
CAVEAT: on at least one macOS host (verified live) seatbelt denials are NOT delivered
to the `log stream`-queryable unified log, so the monitor captures nothing there. It
is the standard mechanism and works on macOS versions that do log denials; shipping
it default-off means it never affects whether a command runs.
* audit batch 32 (sandbox): wire seccomp Unix-socket block into bubblewrap (opt-in)
Completes prompt 11: the zero-seccomp helper (which installs the AF_UNIX seccomp
filter) is now actually invoked by the sandbox instead of only existing as a
standalone binary.
- bubblewrapCommandPlan, when Policy.BlockUnixSockets is set, ro-binds the
discovered zero-seccomp helper into the sandbox at its host path and prefixes the
command with it so the filter is installed before the real argv runs.
- findSeccompHelper discovers the helper next to the running executable first, then
on PATH; it returns "" when absent, so the sandbox degrades gracefully (runs
without the extra filter — bubblewrap's fs/net isolation still applies) rather
than failing the command. Exposed as a package var so the wiring is unit-tested
on any OS.
- Reachable from config: SandboxConfig.BlockUnixSockets and .MonitorDenials now map
onto the policy in applyConfiguredSandboxPolicy (opt-in only — config can turn a
hardening flag on, never off). Both default OFF.
- Tests: helper-prefix present when enabled / absent by default; config flags apply
independently and stay off when omitted.
Default OFF and Linux-only effect; the runtime seccomp enforcement still needs a
Linux host/CI to exercise end-to-end (the wiring and BPF program are unit-tested).
* audit batch 33 (sandbox): enforce scoped/deny network policy in web_fetch & web_search
Completes prompt 9: the built-in network tools now honor the SAME allow/deny/scoped
policy the sandboxed shell egress proxy enforces, instead of bypassing it. Before
this, web_fetch/web_search would reach any public host even when the policy was
NetworkDeny or NetworkScoped — the egress proxy only constrained shell commands.
- New shared gate Engine.NetworkHostAllowed(host) reuses the egress domain matcher
(domainAllowed/normalizeDomains/effectiveNetwork) as the single source of truth:
allow → all hosts; deny → none; scoped → only AllowedDomains minus DeniedDomains
(empty allowlist collapses to deny). Disabled policy / nil engine impose nothing.
- web_fetch is now sandbox-aware: the host is checked before the request AND on every
redirect, so a denied/unlisted host fails closed before any dial. Plumbed via a
networkPolicyGate so the no-sandbox path is byte-for-byte unchanged.
- web_search is sandbox-aware: the configured backend's endpoint host is checked
before the query leaves the machine; blocked under deny, and under scoped unless the
backend host is allowlisted.
- The registry already routes sandbox-aware tools through RunWithSandbox when an
engine is present, so this activates automatically wherever the sandbox is wired.
- Tests: engine gate matrix (deny/allow/scoped/denylist/empty/disabled/nil + host:port);
web_fetch & web_search block denied/unlisted hosts without dialing and allow listed
ones; endpoint-host parsing.
* audit batch 34 (sandbox): prove network-policy wiring end-to-end + document opt-in flags
Verification follow-up to batches 31-33; no behaviour change.
- Adds a registry-level test that exercises the FULL chain for the scoped-network
enforcement: registry.RunWithOptions -> engine.Evaluate (passes a scoped,
egress-capable sandbox-exec backend) -> routes web_fetch through RunWithSandbox ->
per-host allowlist blocks an unlisted host before any dial. This proves the
Tool-interface value actually satisfies sandboxAwareTool at the registry boundary,
not just in a direct method call.
- Documents the two opt-in hardening flags (sandbox.blockUnixSockets,
sandbox.monitorDenials) and the scoped web_fetch/web_search behaviour in the README
so the config keys are discoverable. Both remain off by default.
Confirmed wiring: agent loop forwards options.Sandbox to the registry; the registry
routes sandbox-aware tools (web_fetch, web_search) through RunWithSandbox when an
engine is present; MonitorTag is set only on the macOS sandbox-exec plan; the
seccomp helper prefix is added only on the bubblewrap plan when BlockUnixSockets is
set; both config flags map onto the policy via applyConfiguredSandboxPolicy.
* audit batch 35 (sessions): fix Windows CI failure in TestWriteFileSyncRoundTrips
The exact-permission assertion (perm == 0o600) added in batch 17 fails on
windows-latest because Windows does not honor Unix permission bits: Go reports
0o666 for a writable file regardless of the mode passed to OpenFile. This was the
sole cause of the failing Smoke (windows-latest) CI job; macOS and Ubuntu passed.
Guard the perm check with runtime.GOOS != "windows" so it still verifies the mode
on the platforms that enforce it, while the round-trip (write + read-back) is still
asserted on every platform.
* audit batch 36: address CodeRabbit review (5 findings)
- background/process_posix: after SIGKILL, poll until the process group is
actually gone (SIGKILL is async) and return an error if it survives past a
second deadline, instead of returning nil while descendants still race.
- sandbox/safe_command: only treat ')' as a substitution boundary when it closes
an active $(...). A literal ')' (e.g. echo "a) less") no longer splits into a
fake `less` segment and is no longer misflagged as interactive. Depth-tracked so
nested $(...) still isolate their inner command. Direct splitShellSegments test
(fails pre-fix: yields ["echo \"a", "less\""]).
- cron/schedule: the DST fall-back collapse now only applies when `after` sits
exactly on the minute boundary (its "last fire time" form). With sub-minute
precision (e.g. 01:30:30 EDT) the first 01:30 fire already passed, so the repeated
01:30 EST is the legitimate next fire and is returned — restoring strictly-after
semantics. New test fails pre-fix (skipped to next day).
- config/types: maxTurns error message now reads "must be >= 0" to match the check
(0 is accepted; only < 0 is rejected). Negative-maxTurns test asserts the message.
- docs/audit: header date/branch metadata now matches the body (2026-06-13,
fix/audit-batch) instead of the stale 2026-06-11/fix/audit-critical-high snapshot.
Verified: gofmt clean; go vet clean; host + linux + windows builds; affected suites
green. Each behavioural fix ships with a test confirmed to fail without the change.
* audit batch 37: address CodeRabbit re-review (12 findings)
Findings on the audit code from batches 21-36; each behavioural fix ships with a
regression test (several confirmed to fail without the change).
sandbox/analyzer (AnalyzeCommand):
- Unwrap launcher prefixes (sudo/env/nice/bash -c/...) and recurse into `sh -c`
payloads so `sudo rm -rf`, `env curl …`, `bash -c 'vim x'` classify on the real
command, not the launcher. Mirrors DetectInteractiveCommand.
- Honor rm's `--` end-of-options marker so `rm -- -rf` (delete a file named -rf) is
not flagged destructive.
sandbox/engine: NetworkHostAllowed now shares one backend-aware effective-mode
helper with Evaluate, so scoped collapses to deny when the backend can't enforce
scoped egress — the per-tool gate can no longer permit traffic the engine-level
decision would fail closed.
sandbox/egress: cache domain-prompt decisions by host:port, not host, so one
"allow" can't authorize other ports on the same host.
sandbox/safe_command: the segment splitter is now escape-aware (\| , \" ) so an
escaped operator/quote can't manufacture a fake interactive segment.
sandbox/log_monitor (darwin): Wait() the `log stream` child after draining stdout
so the cancelled process isn't leaked as a zombie.
tools/web_search: confine the search backend to same-host redirects (with a limit)
so a redirect chain can't egress to a host the scoped/deny policy never authorized.
cli/sandbox: an explicit empty --path (`--path=` / `--path ""`) now fails closed
instead of silently widening an allow to tool-wide or a revoke to all-grants.
cli/skills: warn on stderr when duplicate-skill detection fails instead of
swallowing the error.
specialist/exec: don't mask a failed orphan-kill — join the TerminateProcess error
with the SetPID error so a surviving process is visible.
background/process_posix: resolve the PGID and only group-signal when pid leads its
own group; otherwise signal the individual PID. Avoids both a false-success on a
non-leader ESRCH and the hazard of signalling an unrelated (possibly our own) group.
Verified: gofmt + go vet clean; host + linux + windows builds; full go test ./...
green.
* audit batch 38: address CodeRabbit re-review round 2 (3 findings)
Follow-ups to batch 37, each with a regression test:
- safe_command (splitter): a ')' is now only a substitution boundary when it
closes an active, UNQUOTED $(...). A quote-state stack saved at each '$(' lets a
substitution run in a fresh quoting context and restores the outer quoting on
its matching ')'. Fixes `echo $(printf "a) less")` splitting into a fake `less`
segment, while still isolating the inner command of `echo "$(vim x)"`.
- egress: deduplicate in-flight domain prompts per host:port. Concurrent requests
for the same unknown target now wait on the first prompt and reuse its cached
decision instead of each firing a prompt (a prompt storm) and racing to write
the final decision. Race-tested with 25 concurrent authorizers → one prompt.
- analyzer + safe_command: wrapper option-value consumption is now wrapper-specific
(wrapperValueOptionsByProg) instead of a global option set. A valueless flag like
`sudo -n` no longer swallows the real payload, so `sudo -n rm -rf` /
`sudo -n curl …` are correctly classified destructive/network; `sudo -u root vim`
still consumes its value. Shared by the AST analyzer and the regex detector so
they can't diverge.
Verified: gofmt + go vet clean; host + linux + windows builds; go test ./... and
-race on sandbox/background all green.
* audit batch 39: address CodeRabbit re-review round 3 (12 findings)
All verified against current code; each behavioural fix has a regression test.
Security / correctness:
- tools/web_search: redirect policy now also refuses a scheme change, blocking a
same-host https→http downgrade that would send the query + bearer token over
plaintext.
- sandbox/safe_command: a ')' is only a substitution boundary when it closes an
active UNQUOTED $(...), and backticks now get the same quote-state save/restore
as $( — so `echo "`a | less`"` and `echo "$(true | less)"` no longer hide the
inner pager from interactive detection.
- sandbox/analyzer + safe_command: wrapper option-value consumption stays
wrapper-specific, and unwrapping no longer aborts on a dynamic ($x) wrapper arg
once a wrapper is active, so `env "$opts" curl` / `sudo "$x" rm -rf` are still
classified.
- sandbox/egress: in-flight prompts deduped per host:port (carried from batch 38);
the prompt callback is now cancellable (context cancelled on timeout) so a stuck
prompt can abort instead of leaking; signature threaded through.
- sandbox/runner: per-plan unique denial tag (pid+counter) so concurrent monitored
sandbox-exec runs can't ingest each other's denials; helper selection now
requires an executable regular file so a non-exec sibling degrades gracefully.
- mcp/network_client: aggregate SSE event-size cap (not just per-line) so a server
can't grow memory with many unterminated data: lines.
- specialist/accounting: a previously-recorded stop/usage with an empty runId is a
catch-all and now suppresses a later runId-bearing duplicate.
- sessions/lineage: Tree fetches the root via Get() first so a corrupt root
surfaces its real error instead of degrading to "not found".
Docs/tests:
- background/terminate: docstring now states the leader-only group-kill (PID-only
fallback for non-leaders); test child-liveness probe uses ESRCH specifically.
- cli/sandbox: grant usage strings include --path.
Verified: gofmt + go vet clean; host + linux + windows builds; go test ./... and
-race on sandbox/background all green.
* audit batch 40: address CodeRabbit re-review round 4 (8 fixed, 1 skipped)
Verified each against current code.
Fixed:
- sessions/lineage: PANIC FIX — store.Get returns (nil, nil) for a missing
session, so the batch-39 root prefetch could nil-deref. Guard root == nil and
return a not-found error. Regression test added (was a panic).
- agent/compaction: the reactive recover() now records lowWaterMark in the SAME
combined (messages + tool-defs) token domain maybeCompact uses, so the proactive
shrink-guard compares like with like. recover() takes tools; callers pass
request.Tools.
- mcp/network_client: SSE aggregate counter no longer counts a joining newline for
the first data line (only when prior content exists), avoiding premature rejects.
- sessions/store: fsync the parent directory after the metadata rename so the
rename is durable across a crash (no-op on Windows, which can't fsync a dir).
- sandbox/egress: parse the target port with strconv.Atoi instead of
net.LookupPort (no context-unaware resolver use).
- sandbox/runner: escape the per-plan denial tag through sandboxProfileString when
embedding it in the seatbelt profile.
- sandbox/seccomp_linux: guard against an empty filter program before taking
&kernelFilters[0] (defensive; the program is fixed non-empty today).
- cli/cron_run_test: assert last-error-wins with two error events.
- background/terminate, cli/sandbox usage, process_posix_test ESRCH (carried).
Skipped (with reason):
- zerogit worktree-rename (Y='R') test: git porcelain v1 -z only emits the
rename-source field when the INDEX column is R/C; the worktree column never
carries 'R' with a source field in v1, so the fixture would test output git
doesn't produce, and making parseStatus consume on code[1]=='R' would risk
mis-consuming the following entry for a plain ` R` status. Left as-is.
Verified: gofmt + go vet clean; host + linux + windows builds; go test ./... and
-race on sandbox/background all green.
* audit batch 41: address CodeRabbit re-review round 5 (4 findings)
All verified against current code; code fixes have regression tests.
- agent/compaction: summarizeWithFallback's reduce pass no longer swallows ALL
errors. A context-limit error still falls back to the joined partial summaries
(documented "extreme" case), but a non-context failure (auth/network/provider)
now surfaces unchanged, per the function's contract. Two tests (propagate vs.
joined-fallback); the propagate test fails without the fix.
- sandbox/safe_command + analyzer: wrapper value-consuming options now include the
long spellings (sudo --user root, env --unset NAME, timeout --signal …), so a
long flag's value is no longer mis-read as the program (interactive/destructive/
network detection bypass). A `--flag=value` token carries its own value and never
consumes the next token. Tests for space and = forms in both the AST analyzer and
the regex detector.
- docs/audit: the status-ledger summary table's open/partial cells (64/14) were the
lone outlier — the ledger's own narrative and 2026-06-13-reverification.md both
state 66/12. Corrected to 66/12 (66+12+97 = 175); derivation: body 69 open + 18
partial minus the 9 already-fixed-on-main (3 open + 6 partial) = 66/12 = 78
pending, fixed 88+9 = 97. Added a comment documenting the summary-vs-body
relationship. The reverification doc already states 66/12, so both now agree.
Verified: gofmt + go vet clean; host + linux + windows builds; full go test ./...
green.
* audit batch 42: address CodeRabbit re-review round 6 (1 fixed, 1 kept w/ proof)
CodeRabbit re-surfaced two "duplicate" refinements; CI was already green (its 6th
pass was a COMMENT with no new findings).
- cli/sandbox_test (TestRunSandboxGrantsRejectsEmptyPath): now seeds a tool-wide
grant and asserts grant-store immutability after EACH rejected empty-`--path`
call (reflect.DeepEqual before/after), so a mutation in one call can't be masked
by a later one — and a buggy "revoke all" is actually observable (it's a no-op on
an empty store). Stronger than the suggested patch.
- zerogit.parseStatus: KEPT consuming the rename source on the index column only
(code[0]), with an explanatory comment. Verified empirically that git porcelain
v1 -z reports a rename/copy (and emits the extra source field) only in the index
column: `git mv` → "R new\0old\0"; a worktree-only rename → " D old\0?? new\0"
(delete + untracked, never "R" in code[1]). Consuming on code[1]=='R'/'C' would
match no real git output and would only risk mis-consuming the next entry on
malformed input, so the suggestion is declined as incorrect for v1.
Verified: gofmt + go vet clean; host + linux + windows builds; affected suites green.
|
||
|
KRATOS
|
5e68c78824 |
wedge stage 10: distribution (skill/plugin install, tools make) (#184)
* stage 10: distribution (skill/plugin install, tools make)
Add a file-backed distribution surface so skills and plugins can be
installed from a git URL or local path, and new plugin-tools can be
scaffolded. No central registry: a source is just a git URL or path.
- internal/skills/install.go: Install/Remove/Info + skills.lock. Validates
SKILL.md frontmatter via the existing parser, copies it into the skills
dir, and records a sha256 content hash. Reinstall surfaces the hash
change; a clash with a different source is refused without --force.
- internal/plugins/install.go: same flow validated against the plugin
manifest schema, copying the whole plugin tree (minus .git) so the
plugin is activatable. plugins.lock records source + hash.
- internal/tools/scaffold.go: generate a prompt-gated plugin-tool skeleton
(manifest + runnable shell/node/python entry stub) in the toolbox dir.
- CLI: zero skill {add,info,remove}, zero plugin {add,remove}, and a new
zero tools {make,list}; wired in app.go/extensions.go/skills.go.
Safety: fetch honors the process environment (proxy/egress) and disables
git credential prompts; install never executes fetched content (skills are
markdown; plugin install scripts are never run) and installed plugins still
go through normal activation with permission gating.
* stage 10: make no-usable-name install test cross-platform
The test used a whitespace-only source dir name to force the no-usable-name rejection, which is not creatable on Windows (Smoke failure). Switched to a dir base that validSkillName rejects on every platform (contains "..") while staying creatable on Windows; the blank frontmatter still falls back to the unusable dir base, exercising the same rejection.
* stage 10: address review (tree hash, tools-list diagnostics, --json guards)
* stage 10: make canonical-source tests cross-platform (Windows)
Two Windows Smoke failures from the canonical-source review fix:
- TestInstallSameLocalSourceDifferentSpellingIsNotAClash (skills + plugins) used filepath.Rel(cwd, tempdir), which errors on Windows when the temp dir (C:) and repo (D:) are on different drives. Switched to a same-directory alternate spelling (redundant /./), which still exercises canonicalSource without a cross-drive relative path.
- TestRunSkillAddInfoRemove asserted the raw source path, but canonicalSource resolves symlinks/8.3-short-paths, so the stored+displayed source differs on Windows. Normalize the source via EvalSymlinks up front so the assertion matches on every platform.
* stage 10: address CodeRabbit nitpicks (variadic append, header comment)
|
||
|
KRATOS
|
59c332638d |
wedge M2: MCP server resources/prompts + sandbox scoped egress + MCP OAuth (#179)
* wedge stage 06: MCP server resources + prompts
Advertise resources and prompts capabilities in the initialize response and
handle resources/list, resources/read, prompts/list, and prompts/get alongside
the existing tools methods.
Resources enumerate in-scope workspace files via the shared workspace scanner
(reusing its gitignore-style exclusions) and read them read-only with strict
scope enforcement: traversal and out-of-scope absolute paths are rejected with
a JSON-RPC error and no contents, and binary files come back as base64 blobs.
Prompts expose a curated set of templates with simple {{arg}} substitution;
unknown prompt names return an error.
* wedge stage 07: sandbox scoped egress + auto-allow-bash
Add a middle-ground network mode between full allow and full deny:
- types.go: NetworkScoped network mode; Policy.AllowedDomains /
Policy.DeniedDomains; Policy.AutoAllowBashWhenSandboxed plus its
ZERO_SANDBOX_AUTO_ALLOW_BASH env surface (off by default).
- egress.go: in-process loopback (127.0.0.1:0) filtering proxy handling
HTTP CONNECT (HTTPS tunnel) and plain HTTP. Allows only AllowedDomains
minus DeniedDomains via exact-or-subdomain matching (github.com matches
api.github.com but not notgithub.com or github.com.evil.com); deny wins.
Denied targets get 403 / refused CONNECT. Decisions are logged through
the repo redaction so query tokens never hit the audit trail. Fails
closed: empty allowlist is rejected at construction.
- runner.go: NetworkScoped starts the proxy and routes the sandboxed
process through it (HTTP(S)_PROXY/ALL_PROXY + NO_PROXY into the sandbox
env; Linux keeps --unshare-net; macOS profile denies general network
but permits localhost:proxyPort). Empty-allowlist scoped collapses to
deny (effectiveNetwork). Any proxy-start error denies the build rather
than falling back to open network. NetworkAllow/NetworkDeny unchanged.
- engine.go / registry.go / bash.go: honor AutoAllowBashWhenSandboxed -
auto-allow bash without a prompt only when a wrapping sandbox is active
for the command; ignore the flag when the sandbox is inactive. Preflight
network gate now uses effectiveNetwork so a misconfigured scoped policy
still fails closed. Bash defers plan.Cleanup() to shut the proxy down.
Default behaviour is unchanged for existing NetworkAllow / NetworkDeny
users and for the off-by-default auto-allow flag.
* wedge stage 08: MCP OAuth client
Add an OAuth 2.0 + PKCE authorization-code flow so ZERO can connect to
remote MCP servers that require OAuth.
- internal/mcp/oauth.go: authorization-server metadata discovery with
config fallback/override, S256 PKCE, loopback redirect listener, state
(CSRF) validation, code+verifier token exchange, refresh, and optional
dynamic client registration. Tokens are never logged.
- internal/mcp/oauth_store.go: per-server token persistence in a 0600
JSON file under the config dir, with a redaction-safe status summary
that omits the token material.
- internal/mcp/network_client.go: OAuth-configured servers get a round
tripper that attaches the bearer token and, on 401, refreshes once and
retries; refresh failure surfaces an actionable re-login message.
- internal/mcp/config.go + internal/config: an MCP server entry may
declare auth: "oauth" with optional client registration / explicit
endpoints. Non-OAuth servers are unaffected.
- zero mcp oauth {login,logout,status} wired into the CLI surface;
status reports presence/expiry, never the token.
* wedge M2: address review — 2 security fixes, portability, robustness (#179)
Security (Critical):
- resources: resolve symlinks BEFORE the scope check (and resolve roots) so an
in-workspace symlink to an out-of-scope file (e.g. link -> /etc/passwd) is no
longer readable via resources/read. resources/read echoes the requested URI
(not the resolved path) so list/read stay consistent.
- sandbox egress: authorize the dialed URL host, not the client Host header, so
'GET http://denied/ ... Host: allowed' can't bypass the allowlist.
Robustness/correctness:
- prompts: reject prompts/get that omits a required argument; tokenized
substitution so {{...}} inside supplied values is preserved (not stripped).
- resources: percent-encode/decode file URIs via net/url (spaces, #, %).
- oauth: one timeout-scoped context bounds discovery + registration + callback +
exchange; joinWellKnown preserves issuer path segments (RFC 8414).
- cli: 'mcp oauth login' rejects --json instead of silently ignoring it; the
login test is bounded so a missed callback fails fast instead of hanging.
Portability: XDG token-store path test uses t.TempDir() so it passes on Windows
(fixes the Windows Smoke failure).
Adds regression tests for the symlink escape, Host-header bypass, required-arg
rejection, and placeholder preservation. Full go test ./... + -race green.
* wedge M2: fail closed on unenforceable scoped egress (review #179)
Addresses the two P1 scoped-egress enforcement gaps from review:
1. Policy-only fallback ran NetworkScoped with unrestricted networking. On Windows / any host without a native backend, 'network: scoped' with allowedDomains silently got open host networking because Evaluate permitted network-risk tools (expecting a proxy) while BuildCommandPlan ran a direct plan with no proxy. Evaluate now collapses scoped to deny when the active backend cannot enforce egress, so network-risk tools fail closed.
2. Bubblewrap 'scoped egress' could not reach the proxy. --unshare-net isolates the netns and there is no bridge to the host-loopback proxy (the comments claimed one; no code added it), so allowlisted egress never worked. Bubblewrap no longer starts an unreachable proxy: scoped collapses to deny (--unshare-net, no proxy env), matching Evaluate. Comments corrected; a real relay (e.g. slirp4netns) is left as future work.
Mechanism: new Backend.ScopedEgress capability (true only for sandbox-exec, which shares the host net under a seatbelt profile restricting outbound to the proxy port) + Backend.EnforcesScopedEgress(). sandbox-exec scoped egress is unchanged. Regression tests cover enforcing vs non-enforcing backends in Evaluate and the bubblewrap fail-closed plan.
|
||
|
KRATOS
|
ae4ce8aae2 |
wedge build: web_search + LSP client/diagnostics (stages 01-03) (#175)
* stage 01: web_search tool Adds a web_search tool alongside web_fetch: discovers URLs via a pluggable searchBackend (env-configured generic JSON backend; swappable for Exa/You/ Tavily/Brave) and formats ranked results as a numbered list. Registered in CoreNetworkTools(). Deviation from the stage spec: the spec said readOnlySafety, but the repo enforces (TestCoreNetworkToolsExposePromptMetadata) that every core network tool is prompt-gated network egress — so web_search matches web_fetch's posture instead of being auto-allowed. Necessary fallout from adding the tool: knownToolNames (+web_search), the network-tools metadata test (now covers both), and a stale 'unknown tool' fixture that had used web_search. * stage 02: LSP client foundation New internal/lsp package: a direct (self-spawned, stdio) Language Server Protocol client so unattended runs see real compiler diagnostics without an open editor. - client.go: JSON-RPC 2.0 with LSP Content-Length framing; concurrent Call/Notify, id-matched responses, server->client request auto-reply, notification handler, context cancellation, read-loop teardown. - server.go: process lifecycle (spawn via os/exec, initialize/initialized handshake, graceful shutdown+exit with kill fallback). Handshake factored out (performInitialize) for pipe-based testing. - registry.go: extension -> server command (.go/.ts/.tsx/.js/.jsx/.py/.rs), languageId map, PATH availability via exec.LookPath. - types.go: minimal LSP types + PathToURI/URIToPath (Windows-drive aware). Stdlib-only; tested over in-memory pipes with a stub server (no real gopls), including -race for the concurrent id router. * stage 03: LSP document sync + diagnostics Makes the LSP client produce actionable signal so ZERO can ask 'did my edit compile?' headlessly. - documents.go: per-server session with document lifecycle (didOpen/didChange full-sync/didClose), a URI->diagnostics store fed by publishDiagnostics, and WaitForDiagnostics with a publish-baseline + 300ms quiet debounce (servers never signal 'analysis complete'). - diagnostics.go: FormatDiagnostics (path:line:col: severity: message, 1-based), HasErrors, FilterBySeverity, severity labels. - manager.go: Manager — one lazily-started, reused server per language (keyed by binary), routes a file to the right server, Check(ctx,path,text) -> settled diagnostics, graceful Shutdown; unknown extension -> (nil,nil), not an error. Concurrency-safe; tested under -race over in-memory pipes with a stub server (no real gopls). FormatDiagnostics takes the path explicitly since a Diagnostic carries only a range, not its document. * wedge: address review on #175 (LSP robustness + web_search provider) - lsp/client.go: Close() now disables the client — Call/Notify reject after close instead of leaking a pending entry / writing to a dead conn. - lsp/documents.go: serialize same-document syncs through the Notify write via a per-URI lock and commit version/open only on success; drop delayed publishDiagnostics for a superseded (older positive) version; WaitForDiagnostics now reports whether a fresh publish arrived. - lsp/manager.go: Check returns (nil,nil) when no publish newer than baseline arrives (don't return a stale prior result for new text); Shutdown joins and returns per-server shutdown errors instead of always reporting success. - lsp/diagnostics.go: FormatDiagnostics collapses embedded whitespace so each diagnostic stays one physical line. - tools/web_search.go: forward ZERO_WEBSEARCH_PROVIDER in the request so the config knob isn't inert. Adds tests: close-disables-client, stale-version drop, multi-line message collapse, Shutdown error propagation, httptest backend (provider+parse). * wedge: degrade gracefully when an LSP server binary is missing (#175 review) Manager.Check translated a missing-binary start error (exec.ErrNotFound) into (nil,nil) so a configured extension on a machine without gopls/ pyright/rust-analyzer degrades exactly like an unsupported extension — LSP diagnostics are an opportunistic layer, not a hard dependency. Other start failures still surface. Adds a test injecting an exec.ErrNotFound starter. * wedge stage 04: self-correct loop (#176) * stage 04: self-correct loop After a successful mutating tool call, verify the changed files (LSP diagnostics + project tests) and feed failures back to the model to fix — bounded and autonomy-gated, so an unattended run fixes its own mistakes without looping forever. - internal/agent/selfcorrect.go: SelfCorrector with injectable diagnosticsChecker (lsp.Manager adapter) and projectVerifier (verify.DetectPlan+Run); AfterEdit produces a redacted, model-facing corrective message; per-run attempt ceiling; low autonomy reports without auto-attempting; medium/high auto-correct. - internal/agent/loop.go: invoke AfterEdit after a successful mutating tool call (ChangedFiles non-empty), appending feedback to messages. nil SelfCorrect (the default) is a no-op — read-only tools never trigger it. - internal/agent/types.go: add Options.SelfCorrect *SelfCorrector (the explicit options-struct change). Single-pass verify per call (the agent loop + model edits provide the multi- attempt cycle, not selfverify's same-command re-runs). Tested with fakes (no gopls/shell): fail-then-pass, max-attempts abort, read-only skip, low-autonomy report-only, LSP-only failure, secret redaction; -race clean. CLI activation (a --self-correct flag building the SelfCorrector in exec.go) is intentionally left out to honor this stage's file scope — the engine is exported and ready to wire. * wedge: propagate file-read error in self-correct LSP checker (#176 review) CodeRabbit/golangci-lint (nilerr) flagged lspDiagnosticsChecker.Check returning (nil, nil) after os.ReadFile fails. Propagate the error instead; the sole caller (inspect) already skips files whose Check errors, so a deleted/unreadable file still degrades quietly while future callers can decide. Adds a regression test. * wedge: surface non-degradable verify errors in self-correct (#175 review) SelfCorrector.inspect silently continued on every checker.Check error and ignored verifier errors, so a real LSP startup/sync failure, an unreadable changed file, or a failed plan detection produced report.Failed=false -> OutcomePassed with no signal. Manager.Check already degrades missing servers to (nil,nil) and DetectPlan returns an empty plan (not an error) when nothing is detected, so a non-nil error from either is a genuine failure. Record such errors in CorrectionReport.InspectErrors, fail the pass, and surface them in the model feedback. Adds regression tests for both paths. * wedge: drop third-party search-provider names from web_search comments Keep code/comments provider-neutral: describe the backend interface as 'any hosted search API' rather than naming specific services. * wedge: defer self-correct feedback until after the tool-call batch (#175 review) Self-correct feedback was appended as a user message inside the per-tool-call loop, so a multi-tool-call turn whose first (mutating) call failed verification produced: assistant tool_use batch -> tool_result 1 -> user feedback -> tool_result 2. A user message interleaved between tool_results breaks strict provider replay and later session replay/compaction. Accumulate feedback during the batch and append it once after the loop (same after-batch pattern already used for mid-run model escalation), so every advertised tool call keeps its tool_result contiguous. Adds a two-tool-call regression test asserting the feedback lands after both tool_results. * wedge: batch self-correct AfterEdit once per turn over union of changes (#175 review) Deferring only the feedback emission still ran SelfCorrect.AfterEdit once per mutating tool call, which (a) consumes the per-run attempt budget multiple times in a single turn and (b) verifies an intermediate edit that a later call in the same batch already superseded — AfterEdit is documented as one per-run instance holding attempt state, so this is a correctness issue, not just transcript shape. Aggregate every mutating tool's ChangedFiles during the batch and call AfterEdit once afterward over the deduped union, appending its feedback once (still after all tool_results, preserving contiguity). Adds dedupeStrings and a regression test asserting two writes in one turn consume exactly one attempt and emit one feedback message. |
||
|
KRATOS
|
06ea4f004b |
Edit safety: file read/write tracking, external-modification detection, post-apply hooks (#172)
* Edit safety: file version tracking, conflict detection, hook wiring
Phase 1 — FileTracker (internal/tools/file_tracker.go): per-session
map[absPath]{sha256, size, mtime}; CheckConflict flags drift only against
a known baseline, so first-touch writes are unaffected. nil = no-op.
Phase 2 — external-modification detection: read_file records the
whole-file baseline; edit_file and write_file --overwrite refuse to
clobber a file whose on-disk content drifted from it and tell the model
to re-read first, then re-baseline to what they wrote. apply_patch
forgets the files it rewrote (git apply has its own context guard).
Threaded via RunOptions/agent.Options FileTracker, built once per session
in the exec, spec, and TUI paths.
Phase 3 — post-apply validation via hooks: the dormant hooks.Dispatcher
is wired into executeToolCall. beforeTool hooks can veto a call
(DenialHookBlocked); afterTool hook output is scrubbed and surfaced to
the model. DispatchOutcome now collects hook stdout/stderr. Per-session
dispatcher built from merged user+project hooks.json; fails open and runs
nothing until a hook is configured.
* edit-safety: address review — fail closed on read, redact hook reason
- write_file overwrite: if a tracked file cannot be re-read to verify it,
fail CLOSED (refuse the overwrite) instead of silently proceeding — the
read error previously skipped the conflict check and could clobber an
externally modified or replaced file.
- blockedByHookResult: scrub outcome.Reason through redaction before placing
it in model-visible ToolResult.Output; this intercepted path bypassed the
registry's normal output-redaction boundary.
- TestHashContentIsStableAndDistinguishing: compare separately stored hashes
so the stability check is not a same-expression comparison (SA4000).
* edit-safety: flag ToolResult.Redacted when hook output is scrubbed
appendHookFeedback now reports whether scrubbing changed the afterTool
feedback, and blockedByHookResult reports whether it scrubbed the reason;
both set ToolResult.Redacted so the hook-intercepted paths match the
registry's redaction contract. Adds tests asserting a secret-bearing hook
reason/feedback is scrubbed and flips Redacted.
|
||
|
KRATOSandClaude Fable 5
|
e381d8eab1 |
Additional write roots: --add-dir flag, global config key, /add-dir TUI command (#162)
* docs: spec for additional write roots (--add-dir / /add-dir) Co-Authored-By: Claude Fable 5 <[email protected]> * docs: implementation plan for additional write roots Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: add Scope type for multi-root write access Co-Authored-By: Claude Fable 5 <[email protected]> * docs: write roots honored from global config only, union merge semantics Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: fix multi-root violation reporting and pin prefix-alias behavior - validate: prefer ViolationSymlinkTraversal over ViolationOutsideWorkspace when all roots deny; return original requestedPath in violation; append --add-dir hint only on outside_workspace results; extend doc comment to state that a symlink resolving inside any root is allowed. - normalizePrefixForRoot: dedupe insideRoot check via pathWithinRoot (Fix 3); widen EvalSymlinks comment to cover jump-over case; add POSIX-only note. - WorkspaceRoot: add doc comment noting immutability/lock safety (Fix 4). - Tests: pin multi-root traversal-preferred behavior, deterministic alias test (TestValidateResolvesAliasedPathPrefixes), Add symlink normalization test, and tilde-expansion error path test. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: make engine path validation and risk scope-aware Thread *Scope through EngineOptions/Engine so Evaluate and Classify use multi-root scope validation instead of single-root validateWorkspacePaths, letting extra write roots (--add-dir / /add-dir) be honoured by the policy gate and risk classifier. Remove now-dead validateWorkspacePaths helper. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: pin override-root scope isolation, tidy Evaluate scope handling Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: widen seatbelt/bubblewrap profiles and command cwd to scope roots Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: pin extra-root chdir behavior and document bind invariants Co-Authored-By: Claude Fable 5 <[email protected]> * tools: resolve paths against the shared write scope Add PathScope interface and scoped resolver helpers to workspace.go, then thread a scope field through all 8 file tools (read_file, list_directory, glob, grep, write_file, edit_file, apply_patch, bash). Each existing constructor delegates to a new NewScoped* variant; nil scope is byte-identical to the original behavior. Registry gains CoreReadOnlyToolsScoped, CoreWriteToolsScoped, CoreShellToolsScoped, and CoreToolsScoped. Co-Authored-By: Claude Fable 5 <[email protected]> * tools: restore fail-closed write-target symlink checks, share prefix normalization * tools: report absolute paths for extra-root changes, pin scoped traversal denial - resolveScopedPath and resolveScopedTargetPath now return the absolute path as the second value when the matched root is an extra (non-workspace) root, eliminating ambiguity in ChangedFiles, cwd meta, and display summaries downstream - Add TestScopedWriteRefusesSameRootSymlinkTraversal to pin that same-root symlink traversal within an extra root is denied (pre-existing behavior) - Add TestScopedWriteReportsAbsolutePathForExtraRoot to enforce the new absolute-path contract for ChangedFiles on extra-root writes - Extend doc comments on PathScope, ChangedFiles, changedFilesFromPatch, and both scoped resolvers to capture the workspace-vs-absolute invariant Co-Authored-By: Claude Fable 5 <[email protected]> * config: add sandbox.additionalWriteRoots (global config only, union merge) * cli: add repeatable --add-dir flag wiring scope into registry and engine Co-Authored-By: Claude Fable 5 <[email protected]> * cli: forward --add-dir across dispatch paths, pin scope wiring with tests Co-Authored-By: Claude Fable 5 <[email protected]> * cli: pin --add-dir dispatch forwarding end-to-end, close guard gaps Review fixes for the --add-dir dispatch commit: - Add TestRunAddDirDispatchForwardsGrantIntoExecScope: drives runWithDeps through both the "exec" and "-p" dispatch shapes with a provider that calls write_file inside the extra root, asserting the grant reaches the exec sandbox scope (and a negative control without --add-dir is denied fail-closed). Mutation-verified: dropping addDirFlagArgs forwarding from either case fails the test. - Remove help/version from the --add-dir allowlist so any non-TUI/ non-exec leading --add-dir hard-errors, matching the stated requirement; extend the rejection test with those cases. - Fail loud when --add-dir is hidden behind a stray non-flag arg on the --skip-permissions-unsafe path instead of silently dropping the grant. - Reject flag-like values in the inline --add-dir= spelling to match the space form (a directory named -foo stays reachable as ./-foo). - Move TestExecScopeReRegistrationSwapsCoreToolsByName out of the parse test file into the new exec_scope_test.go alongside the e2e test. Co-Authored-By: Claude Fable 5 <[email protected]> * tui: add /add-dir command for session write-root grants Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: surface write roots in zero sandbox status and plan snapshots Co-Authored-By: Claude Fable 5 <[email protected]> * cli: surface write-roots error in --effective --json, normalize fallback root Review fixes for the Task8 observability range: - runSandboxPolicyEffective JSON payload gains writeRootsError (omitempty) so --json consumers see the same fail-soft signal as the text write_roots_error line instead of a misleading workspace-only writeRoots list. - The fail-soft fallback now derives its write roots from a workspace-only Scope (which cannot fail) so the error path renders the same symlink-resolved root as the success path. - TestRunSandboxPolicyEffectiveWriteRootsFailSoft gains a --json leg asserting writeRootsError names the stale root with a workspace-only fallback; the configured-roots test asserts the key is absent for valid roots. - SandboxPlanSnapshot doc comment reworded to match the converter: no current builder populates WriteRoots. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: seatbelt integration test for extra write roots Co-Authored-By: Claude Fable 5 <[email protected]> * docs: document --add-dir and /add-dir write-root grants Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: pass volume-qualified paths through prefix normalization verbatim NormalizePrefixForRoot's POSIX component walk mangled Windows drive paths (C:\Users -> C:Users), which downstream single-root checks treated as relative and joined under the workspace — failing the policy gate OPEN on Windows. Volume-qualified paths now bypass the walk entirely (the macOS /var alias problem it solves has no Windows equivalent) and windows-gated tests pin both the verbatim pass-through and the engine denial. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: make prefix normalization volume-aware for Windows alias resolution The previous commit disabled NormalizePrefixForRoot on Windows, which broke it the other direction: a workspace created under an 8.3 short path (C:\Users\RUNNER~1\...) resolves via EvalSymlinks to its long form (runneradmin), so raw short-form requests escaped the long-form root and legitimate extra-root writes were denied. This is the same alias problem as macOS /var -> /private/var, which the helper already solves. Start the component walk from the volume root (C:\ or //host/share/) instead of "/" so it resolves the prefix on Windows too; POSIX VolumeName is empty so the walk reduces to the original "/"-rooted behavior byte-for-byte. Windows test now pins allow-inside-root + deny-outside both directions. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: address PR review findings for additional write roots Resolve the CHANGES_REQUESTED review on PR #162: - engine: derive workspaceRoot from scope.Roots()[0] when NewEngine is given a Scope without an explicit WorkspaceRoot, so Evaluate's EnforceWorkspace/classification guards no longer silently skip (+ regression test). - tools/workspace: scopedRoots now fails closed (returns an error) when a non-nil PathScope exposes empty Roots(), instead of returning success with an empty path; threaded through the scoped resolve/recheck helpers and resolveGrepRoot. - tools/glob: emit absolute matches when cwd resolves into an extra root so results can't be fed back and resolve to a same-named workspace file (+ regression test). - tools/bash,grep,apply_patch: schema descriptions mention granted extra roots (relative -> workspace, absolute -> granted root). - docs/plan: fix markdownlint heading increment, malformed rule+heading, and bare code fence. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * tools/grep, tui: address PR review for additional write roots - grep now emits absolute, symlink-resolved paths for matches in a granted extra (non-workspace) root, mirroring the glob fix. A bare workspace-relative name like "report.go" otherwise resolves back under the workspace when fed to read_file/edit_file and hits the wrong file when the name exists in both roots. Covers content and files_with_matches output. - tui: only reset chatScrollOffset for a real submission, not an empty Enter (a no-op), so the viewport is no longer yanked to the bottom when nothing was submitted. Real submissions still snap back. - glob: schema description now mentions granted extra roots, matching the bash/grep/apply_patch wording so the --add-dir/-add-dir flow is discoverable. Regression tests: TestScopedGrepReturnsAbsoluteMatchesForExtraRoot, TestEmptySubmitKeepsChatScrollOffset. * docs: add /add-dir to the TUI commands quick-reference table The command was documented further down but missing from the scannable commands table, so users browsing the table wouldn't discover the additional-write-roots flow. --------- Co-authored-by: Claude Fable 5 <[email protected]> |