mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-10 05:01:28 -10:00
ci/coderabbit-auto-approvals
76
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 zzucs
|
63962081cf |
fix(sessions): capture freeform apply_patch checkpoints (#1183)
* fix(sessions): capture freeform apply_patch checkpoints Freeform apply_patch arguments are raw patch text, but both checkpoint paths passed them to json.Unmarshal and dropped the error, so they found no mutation targets and recorded no checkpoint. The tool still executed, leaving rewind unable to restore the edit. Resolve targets through a shared agent.CheckpointTargets helper that prepares freeform input on the registered built-in apply_patch, exactly as executeToolCall does. This keeps absolute patch headers workspace-relative (rewriting them and setting cwd) instead of handing absolute paths to the checkpoint layer. Non-freeform calls now use the same tolerant decoder as execution, so the two cannot disagree. Covers the CLI, spec-draft, and TUI capture paths. Fixes #1162 * test(sessions): canonicalize freeform absolute-header fixtures t.TempDir returns the symlinked /var spelling on macOS and short names on Windows, while the apply_patch tool resolves the physical root. The absolute-header cases must build the header from the resolved root, as the repo's cross-platform test guidance requires. |
||
|
zzucs
|
1e848492e5 |
fix(sessions): preserve image attachment summaries on resume (#1152)
* fix(sessions): preserve image attachment summaries on resume Persist TUI-compatible attachment counts for ACP and exec user events. Restore readable markers in live history, session replay and resumed prompts without storing or resending historical image data. Cover image-only and mixed prompts, restart recovery, malformed metadata and the aborted-turn boundary with regression tests. Fixes #1135 * fix(workspaceindex): follow the current module path Use github.com/Twigpine/zero/internal/gitsafe in the ignore scanner and its tests. The upstream module rename preceded the scanner's gitsafe change, leaving these two imports on the old path and preventing the rebased tree from compiling. * fix(sessions): count only images accepted by the model Take ACP and exec attachment counts after the vision capability gate, matching the TUI's persisted attachment semantics. Update the ignored-image regressions to require zero saved images and no historical image marker. Refs #1135 |
||
|
PierrunoYTandClaude Opus 5.5
|
13e2596b2e |
chore: move repository references to Twigpine/zero (#1105)
Update the Go module path, imports, install scripts, docs, workflows, packaging and action defaults from Gitlawb/zero to Twigpine/zero. The npm scope (already @twigpine/zero on main, with its @gitlawb/zero migration notes), LICENSE, action author, the OpenGateway provider and historical CHANGELOG links are unchanged. The X-AIMLAPI-Integration-Repo header keeps Gitlawb/zero: it is what AIMLAPI sees for attribution, so changing it needs a maintainer's yes. docs/INSTALL.md keeps `go install github.com/Gitlawb/zero/...`, because every tagged release so far declares that module path. The Homebrew formula comment names the Twigpine/homebrew-tap tap. Co-authored-by: Claude Opus 5.5 <[email protected]> |
||
|
Lortu | AEGIS Core
|
eebb384676 |
feat(runtime): add process-local per-model RPM admission limits (#1116)
* feat(runtime): add process-local per-model RPM admission limits * fix(config): preserve model RPM through provider fallback * test(config): pin provider-command RPM cap precedence * style(config): gofmt provider-command RPM test |
||
|
Vasanth T
|
6b5852dc18 |
fix(sandbox): say when a session's sandbox enforcement is degraded (#1082)
* fix(sandbox): say when a session's sandbox enforcement is degraded Only `zero doctor` and `zero sandbox policy` reported a degraded sandbox, so a TUI session or a `zero exec` run went ahead with the native sandbox effectively off and nobody saw it. Both now say so once, before the first prompt or tool call, naming the downgrade reason: exec on stderr next to the grant migration notice, the TUI as a system notice when it opens. The line comes from the session's own engine through Backend.BuildPlan, the plan `zero sandbox policy` prints, so the notice and the command that explains it cannot describe two different sandboxes. Only the degraded level speaks: a disabled sandbox is the user's choice, and the unelevated Windows tier still enforces the write jail. Exec tests that expect a quiet stderr now compare against the notice this host prints under the default policy, since they run on the host's own backend and a Linux runner without the helper is degraded while macOS and Windows are not. Fixes #1041 * fix(sandbox): say so when the nesting markers skip wrapping With ZERO_SANDBOXED and ZERO_SANDBOX_BACKEND set, BuildCommandPlan passes every command through unwrapped on the word of an outer sandbox nothing verifies (#727). The notice came from Backend.BuildPlan, which does not model that path, so a native backend reported full enforcement and a session started with the markers ran every command unwrapped in silence. The engine's notice now mirrors the runner and names the markers and the unverified outer sandbox. A disabled sandbox still says nothing. hostSandboxNotice builds its expectation through the engine too, and the tests that pin the notice clear the markers so a suite run inside a Zero sandbox does not change their answer. * fix(tui): say the startup notices again when a session is resumed /resume rebuilds the transcript from an empty one, so the startup notices went with the old transcript and the session it switched to ran under a degraded sandbox without a word. The model keeps the notices and puts them back first, before the resume summary. Rewind and compaction stay in the same session and keep the notice in the scrollback above their divider. Also pins why the notice can be decided once, at startup: grants made during a session only open the network or add allow and deny paths, never the mode, and every mode but disabled already needs the platform sandbox, so no grant reaches a degraded command plan the startup notice missed, and a deny path the backend cannot enforce is refused instead. * fix(tui): say the startup notices again when /new opens a session /new cleared the transcript and added only its own note, so the new session started under a degraded sandbox without a word, the same gap /resume had. It now puts the notices back first, the way /resume does. /clear stays in the same session and, like rewind and compaction, keeps the notice in the scrollback above its divider. * fix(modelregistry): start no background refresh when the fetch is turned off The TUI and exec each started a goroutine that called RefreshModelsDevCache, which reads ZERO_DISABLE_MODELS_FETCH and the cache path when it runs. A test that turned the fetch off through t.Setenv could finish and have the setting restored before that goroutine ran, and the refresh would then fetch into the developer's real cache. StartModelsDevRefresh reads the setting before starting anything, so with the fetch off no refresh exists to outlive the test. * test(cli): keep the degraded-notice tests out of the developer's user state runExecWithSandbox and the TUI launch test left the sandbox grant store and the models.dev cache at their defaults, so they read the developer's sandbox-grants.json and could migrate it, and could refresh the real models cache. Both now run under isolateCLIUserState, and both join TestCLILaunchHelpersLeaveUserStateAlone so the isolation is pinned for each of them. * fix(cli): stop exec when the degraded-sandbox notice cannot be written The notice goes out before any tool runs, and before MCP servers and plugins start through the same sandbox, but a failed write to stderr was ignored, so the run went ahead with reduced isolation and no warning delivered. It now stops with exitCrash, the way the image and reasoning-effort notices in the same function already do. |
||
|
Marlowe
|
0324c1d410 |
fix(mcp): forward image tool results on the existing image channel (#1077)
* fix(mcp): forward image tool results on the existing image channel Recreated from Gitlawb/zero#1012 (approved but unmerged). Original: euxaristia/zero:feat/823-forward-mcp-images * test(mcp): pin the pre-decode encoded cap and MIME sniffing guards Adds the two reviewer-requested tests for the untrusted-input boundary: - TestOverCapEncodedPayloadIsRejectedBeforeDecoding: a payload one byte over EncodedLen(MaxImageBytes) is rejected and the decoder is never called, so a hostile server cannot force arbitrary decode work. - TestDeclaredMimeTypeDisagreeingWithSniffedBytesIsDropped: an HTML payload declared as image/png is sniffed from its bytes and dropped rather than forwarded to the provider as an image. Both mutations (removing the cap, trusting the declared mimeType) were verified to turn their test red locally. |
||
|
Anandan
|
9e609c5242 |
feat(exec): default headless runs to the turn-budget ceiling (#1049)
* feat(config): report whether the tool-turn budget was explicitly set ResolvedConfig.MaxTurns is ambiguous today: a configured 80 and the built-in default 80 are indistinguishable, so a caller that wants a different default cannot tell "user chose this" from "nobody set it". FileConfig now records at each merge gate (config files, provider command, ZERO_MAX_TURNS, CLI overrides) when a source supplies a positive maxTurns, and Resolve surfaces it as MaxTurnsSet. * feat(exec): default headless runs to the turn-budget ceiling The shared 80-turn default is sized for the TUI, where the user can say "continue" and the run resumes. Headless exec has nobody to do that, so benchmark evals truncated at 80 turns mid-implementation. An unset budget now resolves to MaxTurnsCeiling for exec only; any explicit source (flag, mode preset, ZERO_MAX_TURNS, config file) and exec-profile budgets keep their precedence. The TUI default is unchanged. * test(cli,config): cover the headless turn-budget default * refactor(cli): extract the headless turn-budget decision |
||
|
Vasanth T
|
91d921f4c5 |
feat(tui): offer mid-run model escalation behind --allow-escalation (#1019)
* feat(tui): offer mid-run model escalation behind --allow-escalation Mid-run escalation existed but only exec offered it. The TUI already handled every consequence of a switch, re-deriving the compaction threshold "after a mid-run escalate_model switch" and resolving the summarizer against the active profile, while nothing on that surface could cause one: escalate_model was registered by exec alone. This closes that, opt-in, the same conservative way exec answers the question. Opt-in rather than default because escalation moves a run onto a different model, which changes what it costs and which provider sees the conversation. That is the operator's call, and the exec flag already answers it. The switchers are built per turn from m.providerProfile rather than once at launch. A TUI session can change models with /model, so a closure capturing the startup profile would escalate from whatever the session began with instead of what is in force now, and would carry that stale profile's base URL and credential with it. exec has no such problem because its profile cannot change mid-run, which is why the wiring lives in different places on the two surfaces. The nil-contract logic is now shared rather than copied. providers. EscalationSwitchers is the single implementation both surfaces call: the loop swaps only on a non-nil provider, so a (nil, nil) return has to leave everything untouched including the caller's usage attribution, and the session switcher is installed only when the run STARTED optimized so escalation cannot change the transport underneath a session. A second copy of that would only ever have been exercised on one surface. exec's behaviour is unchanged: its six existing escalation tests pass against the shared builder. The tool and the switchers ride on one flag and a test asserts they cannot be separated. Registering escalate_model without wiring a switcher ships a tool the loop silently ignores, so the model reports an escalation that never happened; that is the specific failure worth a guard rather than a comment. Note this does not answer all of #554. Assigning models per phase, plan with one and execute with another, is still not built. This makes the mechanism reachable from the interactive surface, which is the part that was one flag away. Refs #554 * fix(tui): attribute usage after an escalation to the model in force exec reassigns currentModel when the escalation switcher fires; the TUI captured usageModelID once per run and handed the switcher no callback, so every usage event after a mid-run escalation was billed to the model the run started on. The switcher now updates usageModelID, and the final response carries the model per usage event so the batch fallback does not swing the other way and bill the events before the switch to the escalated model. Covered by a run through a scripted escalation that checks both the live and the batch attribution. * fix(cli): accept the root flags in any order Each leading-flag splitter stops at the first token it does not own, so running them once in a fixed sequence made the order load-bearing: `zero --allow-escalation --theme auto` stranded the theme as an unknown command and exited with an argument error instead of launching, and the same happened after --skip-permissions-unsafe. The three splitters now run until they make no progress, at the root and after the unsafe flag, so every ordering reaches the TUI with every flag applied. A --theme written before --skip-permissions-unsafe was dropped on that path; it is kept now, with a later one winning as the last occurrence does. * fix(cli): forward a root --allow-escalation into exec, reject it elsewhere The root flag was stripped before dispatch, so `zero --allow-escalation exec ...` and `zero --allow-escalation -p ...` ran without the opt-in the help text promises, and `zero --allow-escalation version` accepted a flag it could only discard. It now follows --add-dir: re-synthesised into the exec argument list for the exec and -p shapes, so exec parses it as its own flag, and rejected loudly for every other command. The ordering test now generates every permutation of the three root flags, with --skip-permissions-unsafe absent or at any position. * fix(tui): record the model on an escalated run persisted usage events The live usage record carried the model in force; the persisted session payload did not. The usage report rebuilds cost from that payload and falls back to the session-wide model when an event does not name one, so an escalated interactive run was priced end to end at the model it started on. Written only under escalation, matching what exec records under the same flag, so an ordinary run persists the same compact payload as before. * test(cli): isolate the new launch helpers from real user state A temporary working directory isolates project files and nothing else. runWithDeps fills the dependencies a test leaves out with production ones, and the interactive launch path reads user config, opens stores, refreshes the models.dev cache and migrates any inline plaintext API key into the credential store before it reaches an injected runTUI callback. Running these tests on a developer machine therefore rewrote that developer's config.json and wrote a credential file beside it, and the results depended on whatever providers, MCP servers and plugins the machine had. Both new helpers now point every per-user base directory at a throwaway root, pin the models.dev cache path, disable its background fetch, and force the file credential backend so nothing reaches the host keyring. A new test seeds a config with an inline key outside those fixture roots and fails if either helper rewrites it or leaves a file beside it; without the isolation it reports the rewritten config and the credentials.enc files. * test(cli): check each launch helper isolation on its own t.Setenv restores at the end of the test, not when the helper returns, so one test exercising both helpers let the first isolation cover for the second. Each helper now seeds its own emulated user config in a subtest, and removing the isolation from either one fails that subtest. * test(cli): pin the isolation itself, not only what this platform writes The exec helper writes its models.dev cache from a background goroutine, so a footprint check could not prove its isolation deterministically. Each subtest now also asserts the helper moved the per-user paths off the seeded root and disabled the fetch and the keyring backend, which fails for either helper when the isolation call is removed. |
||
|
Anandan
|
1fde941803 |
perf: reduce startup and turn overhead (#932)
* perf(tui): disable duplicate composer cursor blink * perf(tui): stop composer wakeups after idle * refactor(tools): publish immutable registry snapshots * perf(startup): defer optional tool discovery * perf(provider): reuse compatible response sessions * feat(provider): add native patch tool input * fix(provider): recognize saved chat sessions * perf(tui): render only on screen changes * fix(runtime): address performance review findings * fix(usage): expose cache efficiency details * perf(agent): reduce redundant bounded-task turns * fix: harden runtime review edge cases * fix: address runtime review findings * build: restore official Bubble Tea dependency * fix(cli): redact MCP registration errors * fix: harden turn session regressions * fix: keep run tools and patch roots consistent * fix: bind freeform patch writes to scoped roots * test: keep patch fixtures out of worktree * fix(tools): reject mixed-root freeform patches |
||
|
Kevin CodexandClaude Fable 5
|
6645f9744b |
perf(agent): cheap compaction — dedicated summarizer model, input cap… (#532)
* perf(agent): cheap compaction — dedicated summarizer model, input caps, free reactive prune, window re-derive
Compaction summarization was the single most expensive recurring event
in a long run: the full elided middle — verbatim tool bodies included —
went to the session's main model at full price (a ~100k-token call near
a 200k window, recursively multiplied on overflow).
Four changes, each independently safe:
- Dedicated summarizer model. Compaction summaries route to a cheap
model resolved as ZERO_COMPACTION_MODEL > preferences.compactionModel
> a curated default on official endpoints (Haiku 4.5 / Gemini
Flash-Lite / gpt-4.1-mini; custom-endpoint catalogs are unknowable so
they get none). Built lazily on the first paid compaction; ANY
failure permanently falls back to the main provider for the run, so a
misconfigured summarizer can never break compaction. "main" at any
level forces the old behavior.
- Summarizer input caps. renderTranscript clamps tool-result bodies to
2,000 chars and tool-call arguments to 500 (head+tail, rune-safe) —
the summary needs what was done and decided, not raw payloads. Cuts
summarizer input 5-10x on tool-heavy sessions.
- Free reactive prune. recover() now runs the same prune-first stage as
the proactive path: a context-limit error is first answered by
pruning stale tool bodies at zero cost, without consuming the
one-shot reactive budget; the paid summarizer only runs when a later
error finds nothing left to prune.
- Window re-derivation on model switch. Options.ContextWindowFor
(wired to the model registry in exec and the TUI) lets the loop
re-derive the compaction threshold after escalate_model, closing the
documented KNOWN LIMITATION where a switch to a smaller-window model
could overflow and a larger one over-compacted.
## Rebase notes (2026-08-25, onto
|
||
|
|
ff62f73b7f |
fix(secrets,worktrees): fix secret redaction leakage and prune stale worktrees (#855)
* fix(secrets,worktrees): fix secret redaction leakage and prune stale worktrees 1. Prevent trailing redaction leaks in github_token, aws_access_key_id, and google_api_key by adding trailing word boundaries and allowing variable lengths. Refine the openai_key pattern to cleanly distinguish legacy keys and modern prefixed keys (sk-proj-, sk-svcacct-) from ordinary kebab-case phrases. 2. Implement auto-pruning of zero-owned git worktrees older than 24 hours at the start of worktrees.Prepare to prevent indefinite disk space leaks. * fix(secrets,worktrees): close tail-leak edge case and worktree data-loss risk Drop the trailing \b anchor on the four secret patterns whose body class allows "-" (slack_token, google_api_key, the modern openai_key branch, jwt). \b requires a word/non-word transition, so a secret ending in "-" right before a delimiter has none, and the engine backtracked the greedy quantifier to drop that last character instead of failing the match, leaking it. The body character class already provides the real stopping boundary, so the anchor was unnecessary. Fix two issues in worktree Clean flagged in review: - Staleness was decided by the worktree directory's own mtime, which only changes when an entry is added/removed/renamed directly inside it, not when a long-running task edits existing files deeper in the tree. Clean now walks the tree and treats any recently modified entry as live, and also skips any worktree a caller has explicitly locked via git worktree lock. - baseDir ownership used a raw strings.HasPrefix, so a sibling like "<baseDir>-other" would false-match. Replaced with a filepath.Rel path-boundary check. * fix(worktrees): check exit codes on removal, fail closed on inspection errors defaultRunGit deliberately returns a nil error alongside a nonzero CommandResult.ExitCode for a failed git invocation, so the worktree remove call must check ExitCode itself instead of trusting a nil error to mean success. Route it through gitOutput, which already does that. worktreeIsStale treated an inspection failure (an unreadable file, a WalkDir error) the same as "keep walking," which can let an incompletely-inspected worktree be judged stale. Any inspection error now makes it ineligible for removal instead. * fix(secrets,worktrees): catch appended-suffix keys and scope pruning to owned worktrees The scanner's trailing \b anchors made a credential vanish entirely when followed by a word character outside its body class (an appended suffix like AKIA...EXTRA, or ghp_..._suffix): the fixed or unbounded-greedy quantifier had no valid word boundary to land on and the whole match failed, so the real secret reached the redaction output unredacted. Dropping the trailing anchors lets the body class itself stop the match, so the credential prefix still gets redacted even when noise follows it. Also recognize sk-admin- alongside sk-proj-/sk-svcacct- so OpenAI admin keys aren't skipped by the narrowed modern-key branch. Clean pruned any worktree under the caller-supplied BaseDir, but Prepare only ever creates worktrees under a per-repository zero-worktree-<repoKey> subtree of it. Scope pruning to that subtree so a worktree a user manages by hand elsewhere under a shared BaseDir is never force-removed. Also refuse to force-remove a worktree whose mtime looks stale but that still has uncommitted or untracked changes: a task can hold live work while waiting on a model, network, or user for longer than the staleness window without writing to the tree again. * fix(worktrees): make nested-activity test exercise the deep walk it claims to activePath/internal was created by the same MkdirAll as the nested pkg dir but never backdated, so it kept a fresh mtime and worktreeIsStale's walk reported "not stale" as soon as it hit that directory, before ever reaching the freshly-written file two levels deeper. The test passed without actually exercising recursion past the first directory. * fix(worktrees): lock zero-created worktrees and treat ignored files as dirty Prepare never called git worktree lock, so the entry.locked skip in Clean only ever protected worktrees a human locked by hand, never zero's own; a worktree that finished committing and sat idle (e.g. waiting on a slow model or network retry) for more than 24h looked clean-and-stale and got force-removed by the mtime+dirty heuristic alone. Lock every worktree Prepare creates so it gets the same protection. worktreeIsDirty also used git status --porcelain with no --ignored, so a worktree holding only .gitignore-matched task data (credentials, generated drafts, artifacts) reported as clean and got force-removed with --force, silently discarding it. Add --ignored so those files count as dirty too. * fix(worktrees): release Zero's Prepare lock so Clean can reclaim finished worktrees Prepare locks every worktree it creates so Clean's mtime+dirty staleness heuristic never force-removes one Zero is still using, but nothing ever unlocked it, making the automatic disk-space cleanup permanently inert. Add Release (git worktree unlock) and wire it in two ways: zero exec --worktree defers a release once its own run finishes, since that flow's use of the worktree is bound to its own process. zero worktrees prepare hands the path to a longer-lived external caller with no defined end-of-life, so a new zero worktrees release <path> subcommand lets that caller release it explicitly when done. * fix(worktrees): normalize release path, aggregate Clean errors, unlock deleted worktrees Address CodeRabbit's review on the lock-release fix: - zero worktrees release now resolves its path argument to absolute before calling Release, since git worktree unlock matches against the path git recorded at creation, not whatever directory the caller happens to be running from. - Clean now aggregates removal failures with errors.Join instead of overwriting lastErr, so multiple stale worktrees failing removal in the same pass are all reported, not just the last one. - Release falls back to options.Cwd as the git working directory when the worktree path itself no longer exists (e.g. a caller deleted a locked worktree by hand instead of releasing it first), so the orphaned lock can still be cleared. Added regression coverage for all three. * style(worktrees): align aggregation test comments to gofmt output * fix(worktrees,cli): restore reuse lease and scope unlock to owned locks - Prepare re-locks a reused worktree so Clean's staleness heuristic cannot force-remove it while the new caller is still using it; a lock already held by a live external caller is kept in place and reported through the new Result.LockAcquired field. - exec --worktree only releases the lock its own Prepare call acquired and surfaces a failed release on stderr with the affected path instead of discarding the error. - worktrees release wires the resolved workspace root into Options.Cwd so the deleted-path recovery works outside the worktree directory. - The relative-path release test derives its expected value via filepath.Abs, matching the resolution the CLI uses, so macOS /var vs /private/var spellings no longer break it. * fix(worktrees,cli): reject in-use leases, validate before cleanup, add release -C - Prepare rejects a worktree whose lock another run still holds, on both the reuse and the create-race paths, instead of handing a second live caller an unprotected shared checkout whose sole Git lock the first caller's exit would release. - The automatic stale-worktree pruning runs only after the request itself validates, so a rejected command (an invalid --name) has no destructive cleanup side effect; covered end to end with real git in both directions. - worktrees release accepts -C/--cwd naming the source repository, which the deleted-path recovery needs when launched outside the repo (the deleted worktree path is a one-way hash with no way back to its source). * test(worktrees): canonicalize test roots to physical spelling git records worktree paths in physical form, so the CI runners' symlinked (/var -> /private/var) and 8.3-short (RUNNER~1) temp spellings made Clean's containment check skip the test's stale entry and the pruning assertion fail on macOS and Windows. * fix(worktrees): preserve orphaned commits, verify release ownership, canonicalize base dir Four fixes from the latest review round: - Clean now creates a durable ref (refs/zero/orphaned-worktree/<sha>) for a detached worktree's HEAD before force-removing it, when that commit isn't already reachable from any other ref. Prepare always creates worktrees with `worktree add --detach`, so a commit made there had no ref pointing at it once the worktree was deleted, making it immediately eligible for git gc despite never having been merged/pushed elsewhere. - Clean resolves its configured base directory through EvalSymlinks before comparing it against git's reported worktree paths: `git worktree list --porcelain` reports each worktree's PHYSICAL location (resolving symlink components), so a symlinked --worktree-dir made every worktree created under it permanently unprunable. - Release now verifies path has a zero-worktree-<repoKey> ancestor directory component before running `git worktree unlock`, so it can't be used to clear the lock on a worktree a user or another tool manages by hand. The check doesn't need to know which --dir a given Prepare call used (nothing records that against a specific worktree, and the CLI never threads BaseDir through to Release) — the repoKey component is Prepare's actual ownership signature regardless of which directory it was created under. - Route the release command's printed path and exec's release-failure diagnostic through the existing CLI redaction helper, and split the worktrees help text into prepare-specific and release-specific flag sections (release only ever supported -C/--cwd, not the --name/--dir/ --json the shared block advertised). All four have regression tests confirmed to fail without their fix (two using real git worktrees, not just fakeRunner sequences). Build, vet, and gofmt clean on linux/windows/darwin. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(worktrees): recoverable PID leases and reclaimable released worktrees Two review findings on the cleanup lifecycle: - A lock left by an abnormal exit (SIGKILL, crash, power loss) was skipped by Clean forever, recreating the permanent disk leak this PR set out to fix. exec --worktree now records its PID in the lease reason; Clean expires a lease whose recorded owner is provably dead, unlocking only after the staleness, dirty, and HEAD-preservation guards all pass. Human locks and PID-less leases (external `worktrees prepare` owners) remain permanent until explicit release, and any ambiguity in the liveness probe counts as alive. - An explicitly released worktree holding only gitignored residue (node_modules, build output) was skipped at every age. Release is the owner's completion signal, so unlocked entries now block removal only on tracked/untracked changes; expired crashed leases keep the conservative --ignored probe since they never signaled completion. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(worktrees): address review feedback on lease detection and release safety Split dead-lease PID checking into posix/windows implementations so Windows can reliably tell a dead process from a live one. Fix a path canonicalization mismatch in two release tests. Derive release ownership from git worktree list instead of the git-dir parent, which was wrong for repos with a separate git-dir. Refuse to clear a lock that was not taken by Zero in the first place. * fix(worktrees): canonicalize paths for clean/release ownership checks Compare Clean containment and Release ownership against physical path spellings so macOS /var vs /private/var and symlink TMPDIR layouts match git worktree list. Require a registered porcelain entry and a Zero lease reason before unlock; treat an already-unlocked Zero worktree as a no-op. * fix(worktrees,secrets): address jatmn review findings on #632 - Prepare and Release now agree on repoKey regardless of which worktree (main or linked) Prepare runs from, by keying off git worktree list's first entry (always the main worktree) instead of --show-toplevel. A worktree prepared from a linked checkout previously failed its own ownership check on release and its lease could never be cleared. - osProcessAlive on Windows no longer treats every OpenProcess failure as "process is dead": only ERROR_ACCESS_DENIED (a live process this caller lacks rights to query) is now distinguished from a genuinely missing PID, so Clean can no longer force-remove an active worktree whose owning process it simply couldn't query. - The openai_key redaction pattern now recognizes sk-or-v1- (OpenRouter) alongside the existing sk-proj-/sk-svcacct-/sk-admin- prefixes, so hyphenated OpenAI-compatible provider keys are redacted again without reopening the sk-<kebab-case-phrase> false-positive this pattern was narrowed to avoid. - Release/exec --worktree error text is redacted before reaching stderr, matching the already-redacted success path; ownership errors interpolate the caller-supplied path, which could carry a key-shaped segment. - canonicalizePath resolves symlinks through the nearest existing ancestor when the target itself no longer exists, so the documented `release -C` recovery path works again for a worktree deleted by hand under a symlinked --worktree-dir. - Prepare rolls back the worktree `git worktree add` just created if the subsequent lock call fails for a reason other than a concurrent racer, instead of leaking an unleased checkout until Clean's 24h staleness window reclaims it. Not addressed here: the P2 finding that worktree ownership is provable only by a directory-name convention plus a lock-reason prefix, both of which a user can reproduce by hand. A durable per-worktree ownership marker would close that gap, but internal/worktrees has been on main since #70, so a marker requirement could reject worktrees an already-installed zero created before this change existed. Needs a decision on migration before implementing. * fix(worktrees): prove Prepare ownership with a git-admin marker Address review findings that path convention plus lease-reason prefix are forgeable by hand. Prepare now writes a zero-owner marker into the worktree admin dir; Release and Clean require it before force-touching a path. Clean also keys its owned subtree off the main worktree root so linked-checkout calls still prune the same bucket Prepare uses. Tests plant the marker and list the main worktree first so fixtures match production. * fix(cli): complete worktrees release in shell completions * test(worktrees,cli): add coverage for linked-worktree Clean, forged lease rejection, and completions - TestCleanFromLinkedWorktreePrunesStaleWorktree: pins Clean deriving its owned-subtree key from the main worktree root (not the invoking linked checkout's --show-toplevel) so Prepare/Clean run from a linked worktree actually reclaim the worktrees Prepare created there. - TestReleaseRejectsForgedZeroLeaseWithoutOwnershipMarker: pins the ownership-marker requirement against the exact forgery jatmn described - a worktree under the predictable zero-worktree-<repoKey> path, manually locked with a reason that merely starts with the zero lease prefix. - Fix TestPrepareCreatesDetachedGitWorktree: its fake git runner ran out of canned results at Prepare's post-lock ownership-marker write, so writeOwnershipMarker resolved gitDir to "" and os.WriteFile wrote "zero-owner" as a relative path into the test process's real working directory instead of failing loudly. Give it autoAbsoluteGitDir like the other Prepare-exercising tests use and assert on the marker-write call. - completions_test.go: assert `worktrees`/`worktree` completions include `release` alongside `prepare`. * fix(redaction,worktrees): sort extra secret values by length descending and canonicalize worktree paths * fix(secrets,worktrees): restore Anthropic key redaction and handle legacy worktree cleanup * fix(secrets): add bash output redaction regression test for Anthropic API keys * fix(worktrees): touch worktree mtime on reuse to prevent stale pruning When Prepare reuses an existing worktree, it now touches the directory to refresh its mtime. This prevents Clean from force-removing a long-running but idle worktree (e.g. waiting on a model) that has no recent file changes but is still actively in use. Addresses the data-loss risk flagged in the PR review where mtime-only staleness + --force removal could discard live worktrees. Co-authored-by: cairn-code Co-Authored-By: cairn-code <[email protected]> * fix(worktrees): properly handle os.Chtimes error on reused worktree path Co-Authored-By: cairn-code <[email protected]> * fix(worktrees,secrets): unlock using porcelain entry path, set RepoRoot to primaryRoot, and add anthropic_key pattern Unlock using matched porcelain entry path, set Result.RepoRoot to primaryRoot in Prepare, add dedicated anthropic_key secret pattern, and canonicalize fake-runner test paths. Refs #632 * fix(tools): correct redaction placeholder assertion in Anthropic key test TestFormatBashOutputRedactsAnthropicKey checked for the openai_key placeholder instead of anthropic_key, a copy-paste leftover. The redaction itself was already correct; only the assertion was wrong. * test(worktrees): physicalize test paths for macOS tempdir symlink resolution * fix(secrets,worktrees): address CodeRabbit review findings Align RedactString token boundaries with secrets.Scan, strengthen the Anthropic bash redaction assertion, fix Clean fixtures so they reach the guards under test, and fail closed on ambiguous processAlive probes. * fix(secrets,worktrees): restore broad key redaction and legacy Clean safety Address human review on #855: keep sk- bodies with a digit filter instead of enumerated vendor prefixes, add a looser JWT form, restore the sk-test fixture, probe legacy ownership before dirty, treat non-INVALID Windows OpenProcess errors as alive, redact Abs/cwd release errors, and reclaim dead-owner leases on Prepare reuse. * fix(secrets,redaction): always redact known OpenAI key prefixes Alphabet-only sk-proj-/sk-svcacct-/sk-admin- tokens are still credentials; keep the digit filter only for unknown sk- vendor forms so kebab phrases like sk-learn-… stay un-redacted. * Preserve digit-free legacy keys during redaction. Refs #855 * fix(secrets,worktrees): address CodeRabbit findings on PR #855 Redact full compact JWE tokens, pin git locale for lock parsing, write the ownership marker atomically, surface missing-dir unlock failures, and fail closed when HEAD probes are indeterminate. Align Clean fixtures and digit-free known-prefix redaction tests with the shipped behavior. Refs #855 --------- Co-authored-by: Claude Sonnet 5 <[email protected]> Co-authored-by: cairn-code <[email protected]> Co-authored-by: euxaristia <[email protected]> |
||
|
|
0176ee49ff |
feat(agent): add PermissionModePlan for interactive read-only planning (#853)
* feat(agent): add PermissionModePlan for interactive read-only planning * fix(agent): drop redundant modeName branch, add plan mode regression tests string(permissionMode) already yields "plan" / "spec-draft", so the if/else recomputing modeName in the denial message was dead branching on the same values. Also add plan-mode coverage mirroring three of the four existing spec-draft regression tests: advertised tool set, and denied write_file/bash calls. The fourth (submit-and-stop review control) has no plan-mode analog, since plan mode has no submit tool. * fix(agent): deny request_permissions in plan/spec-draft even when the registry omits it request_permissions is dispatched by name in executeToolCall before the registry-based ToolAdvertised gate runs, so that gate only helps when the tool happens to be present in the caller's registry. A plan- or spec-draft-mode registry that simply omits the tool (rather than registering it as denied) let the call fall through to a real turn/session-scoped permission grant, defeating the read-only boundary. Deny it unconditionally at the top of executeRequestPermissions for both read-only modes instead. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(agent): suppress executable hooks while plan/spec-draft mode is active Plan mode promises a read-only turn, but sessionStart/sessionEnd fire on every run and beforeTool/afterTool fire around allowed read calls, and all four execute configured host commands outside the advertised-tool and sandbox gates — so a project hook could mutate the workspace or spawn a process from a session that advertises it cannot. Gate all four dispatch points on the run's permission mode, with a regression test asserting no hook command launches during a plan-mode run. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(agent): close plan-mode tool advertisement bypass toolAdvertisedInPlan whitelisted ask_user and update_plan by name alone, so a caller could register a mutating tool under either name and have it advertised and executed in plan mode. Validate every tool against its Safety() instead. Also exclude lsp_navigate, which is marked SideEffectRead but lazily spawns a real language-server process, contradicting plan mode's read-only guarantee. Add regression tests for both. * fix(agent): keep trust-gated hooks in spec-draft mode Plan mode still suppresses executable hooks so a read-only planning turn cannot spawn host processes via session or tool hooks. Spec-draft keeps the existing trust model: project hooks fire when the workspace (or its worktree trust root) is trusted. Unconditionally suppressing hooks in spec-draft broke TestExecSpecWorktreeInheritsTrustEndToEnd for trusted worktrees under --use-spec --worktree. * fix(agent): cover plan-mode spoof and lsp_navigate denials Expand the name-only spoof regression to both update_plan and ask_user, and add an execution-path denial for lsp_navigate so plan mode cannot spawn language servers even when a call still arrives. * fix(agent): filter tool_search deferred candidates by plan/spec-draft visibility tool_search resolved and ranked deferred tools by EnabledTools/DisabledTools only, never by the run's permission-mode visibility. tool_search itself is already denied at dispatch in plan/spec-draft (its Safety carries no side effect, so it fails the same SideEffect==Read advertisement gate direct calls use), so this was not reachable through the normal Run() path today. But it is a real landmine: if that outer gate is ever loosened independently (e.g. tool_search's no-side-effect Safety is judged advertisable), the loader had no gate of its own and would hand a deferred write/mutator tool's name, description, and full schema straight to a plan/spec-draft model. Mirror agent.ToolAdvertised's plan/spec-draft branches inside the tools package (toolAdvertisedForPermissionMode, next to the existing toolAllowedByFilters mirror that avoids the same import cycle) and apply it alongside the operator filters in visibleDeferredTools and visibleEagerToolNames. Added unit tests in tool_search_test.go for both modes, and an end-to-end agent test that force-calls tool_search in plan mode and asserts no schema leaks. Verified by reverting tool_search.go and confirming the new tests fail (one shows load_tools resolving to the mutator's name); restored and confirmed they pass. Also confirmed via a temporary probe that if plan mode's outer advertisement gate is loosened, this filter is what actually stops the leak. * fix(agent): require Safety for spec-draft ask_user/submit_spec Do not advertise or load re-registered control tools by name alone in spec-draft mode. ask_user must be SideEffectRead+Allow and submit_spec must be SideEffectWrite+Allow, matching the real tools. Apply the same filter in tool_search and add spoof regression tests. Refs Gitlawb/zero#642 * fix(agent): wire plan mode into TUI, CLI, and ACP entry points Address the P1 finding that PermissionModePlan was documented but never selected by /plan, zero exec, or ACP mode selectors. /plan on|off now toggles the session permission mode (restoring the prior mode on off), zero exec --plan selects plan for a run, and ACP advertises plan as a client-selectable mode. Integration coverage for each entry path. * fix(cli): reject --plan combined with --worktree Worktree preparation runs in runExec before the plan permission mode is assigned, so `zero exec --plan --worktree` could still trigger workspace mutation ahead of the read-only gate. Reject the combination during option validation, alongside the existing --use-spec/--skip-permissions- unsafe conflict checks, so no worktree prep can occur. Addresses a coderabbitai finding on PR #642. * test(tools): assert spoofed tool schema doesn't leak via tool_search The spoofed-control-tool regression only asserted on the description string; Parameters() exposed no distinctive schema marker, so a regression that leaked the schema without the description would still have passed. Add a spoofed_secret property to the test tool's schema and assert it's absent from result.Output alongside the description. Addresses a coderabbitai finding on PR #642. * fix(tui): gate local mutating commands behind plan mode /plan on only flips the agent permission mode, which gates agent tool calls. Local TUI commands that run entirely inside the TUI process bypass that gate: /rewind restores workspace files from a checkpoint, /export writes a transcript to disk, and /sandbox-setup spawns a native host process. Add a shared plan-mode guard at the start of dispatchCommand (mirroring the existing BTW-unavailable guard) that rejects these three commands while permissionMode is agent.PermissionModePlan, with regression coverage proving each is blocked with no mutation/process spawn in plan mode and unaffected outside it. Addresses a coderabbitai finding on PR #642. * fix(agent,specialist): layer plan mode system prompt and enforce read-only subagent mode * Fix jatmn review findings for PR 642 * fix(agent,cli,tui): resolve CodeRabbit review comments on active turn model field and --plan permission mode conflict * fix(agent): propagate permission mode to exec options and serialize ACP mode changes Parse permissionMode in resolveExecPermissionMode, acquire turnMu.Lock in ACP handleSetMode to serialize mode changes with active turns, and block MCP subcommands in TUI plan mode. Refs #642 * fix(agent,tui): update tests off removed tools.CoreTools/NewWriteFileTool/NewLSPNavigateTool wrappers Those were thin unscoped wrappers around the Scoped variants, deleted upstream in #706 since nothing else called them directly. Only these tests still did; switch to the Scoped calls main's own tests already use. * fix(agent): fail-closed beforeTool vetoes and permission-mode plan guards Keep beforeTool deny gates active under plan mode so hooksSuppressed no longer fails open, and stop propagating --permission-mode for auto/ask/member children so swarm members keep write tools. Apply --plan combination rejects to --permission-mode plan as well. Refs #853 * fix(agent): address CodeRabbit findings for plan-mode advertisement and entry paths Unify plan/spec-draft tool advertisement in tools.ToolAdvertisedForPermissionMode (with PermissionDeny short-circuit), cover ACP config and --permission-mode plan list-tools paths, and allow bare /mcp while blocking mutating MCP subcommands. Refs #853 --------- Co-authored-by: Claude Sonnet 5 <[email protected]> Co-authored-by: euxaristia <[email protected]> |
||
|
Anandan
|
96859c9bd1 |
feat(sandbox): unify command execution and enforcement (#781)
* feat(sandbox): unify command execution and enforcement * test(sandbox): make platform contracts portable * fix(sandbox): close execution lifecycle races Bound retained interactive output, preserve interruption state, and start workspace observation before process launch. Protect active runtime roots with cross-platform leases, capture transient protected-path creation events, and restore process attributes after PTY fallback. * fix(sandbox): restore strict macOS isolation Remove Seatbelt host-local network allowances after native validation showed localhost filters can reach host interfaces. Track process reap completion so retained sessions cannot signal a stale numeric PID. |
||
|
KRATOSandKRATOS
|
a21a052a4a |
feat(sandbox): disable the sandbox via config (#687) (#746)
* feat(sandbox): disable the sandbox via config (#687) Add a sandbox.enabled field (`"sandbox": {"enabled": false}`). ModeDisabled already short-circuits the engine but had no config surface. - SandboxConfig.Enabled *bool (pointer distinguishes an explicit false from an omitted key). - mergeConfig honors it (global config / CLI); applyConfiguredSandboxPolicy maps enabled:false to ModeDisabled. - mergeProjectConfig intentionally does NOT merge it: a cloned repo must not be able to disable the sandbox that constrains it — same posture as AdditionalWriteRoots and the Network tighten-only rule. * fix(config): honor sandbox.enabled from the CLI override path (#687 review) applyOverrides copied BlockUnixSockets/MonitorDenials but not the new Enabled tri-state pointer, so a CLI/programmatic Overrides{Sandbox:{Enabled:&false}} was dropped — contradicting the 'global config / CLI can disable' contract. Preserve the pointer in applyOverrides (runs after all config sources, so an explicit override wins) and add a resolver regression test: an explicit false override disables, and an explicit true override wins over a lower-precedence config false. * fix(config): keep sandbox.enabled out of the provider-command path (#687 review) mergeConfig now copies Sandbox.Enabled, and it is shared by two callers: the global-config merge (trusted) and the provider-command merge (not). A provider command is an arbitrary executable named in config, and LoadProviderCommand parses its stdout into a full FileConfig -- so one returning a valid provider plus {"sandbox":{"enabled":false}} could disable the sandbox meant to constrain it, despite the setting being documented as global-config/CLI only. Confirmed reproducible in both directions before the fix. Clear Sandbox.Enabled from the provider-command config before merging, mirroring the carve-out mergeProjectConfig already applies for the same reason. Done at the call site rather than inside mergeConfig because that helper must keep honouring the setting for the trusted global-config merge. Adds TestResolveSandboxEnabledIgnoredFromProviderCommand covering both directions (cannot disable, cannot enable) and asserting the provider itself is still applied, so the guard cannot be satisfied by dropping the command wholesale. --------- Co-authored-by: KRATOS <[email protected]> |
||
|
Vasanth T
|
378d538e24 |
perf(agent): execution profiles with one-shot posture escalation (PR10b+PR10c) (#740)
* perf(agent): execution profiles, --exec-profile, bench passthrough, TUI /profile (PR10b+10c) * fix(execprofile): harden profiles against review findings - fast escalates on critical-risk mutations, not high: the sandbox marks every shell command high before command analysis, so a high trigger spent the one-shot on the first go test of any task - escalation can now restore the provider-default effort a profile displaced (PostureEscalation.RestoreDefaultEffort; a plain empty target means leave untouched) - TUI /turns while a profile is active pins the budget: disarms the escalation turn target (mirroring exec's explicit --max-turns rule) and profile reverts use touched bits instead of value equality, so explicit choices survive even when they coincide with profile values - spec-draft runs get the same escalation policy as the main run instead of the displaced budget with no safety net - bench validates and canonicalizes --exec-profile before spawning: an unknown name previously produced an exit-0 report full of millisecond usage-error walls - turn summary prints exec-profile alongside mode; zero exec rejects an inline-empty --exec-profile=; balanced trace label documented and pinned as deliberate attribution * fix(execprofile): explicit effort choices disarm the escalation effort restore An explicit effort set AFTER profile selection could be cleared by a mid-run escalation (the effort analog of the /turns pin, introduced by RestoreDefaultEffort): TUI /effort and Ctrl+T now disarm the restore on a policy clone, and the spec-draft path only arms it when no explicit --spec-reasoning-effort governs the draft. * fix(execprofile): address review findings on the profile surfaces - --use-spec projects away a profile's self-correct knob instead of silently arming an option the draft runner ignores (the explicit --self-correct --use-spec combination is already a parse-time error) - RunTurnBench canonicalizes and validates ExecProfile at the library boundary, so a direct caller's casing/whitespace cannot fork report labels and an unknown name fails before anything runs - reverting a profile whose displaced budget was zero clears ZERO_MAX_TURNS (SetMaxTurnsEnv ignores zero, so sub-agents would have kept the removed profile's budget) - the TUI effort-disarm test fails instead of skipping when the catalog stops filling low effort * fix(tui): reconcile an active profile's effort fill on model switch The fill is per-model, not a session preference: fast selected on a model without low fills nothing, and switching to a model that supports it should behave like selecting the profile there; the reverse switch must not keep sending a level the destination does not support. Both switch paths (the /model command and the cross-provider picker) now re-derive the fill, leaving explicitly touched efforts alone and keeping the escalation's effort restore in step. Also: the revert-env test asserts key removal via LookupEnv, and applyProfileTurnBudget's pinned-budget parameter is named and documented as covering mode presets, not just the flag. * fix(tui): void profile effort bookkeeping when the model-switch drop erases it The /model path's pre-existing unsupported-effort drop ran before the profile reconciliation and left contradictory state behind: a dropped touched override kept blocking the profile from governing, and a dropped profile fill left a stale applied value with the effort restore still armed. The drop itself is load-bearing (the TUI run path forwards the effort to the provider unfiltered), so instead of preserving a known-unsupported level, the voided choice now resets the profile's effort bookkeeping and the reconcile re-derives from scratch. The whole sequence lives in one ring-parameterized helper driven by the switch target's authoritative ring, unit-tested against synthetic rings for both the void-then-refill arc and the supported-override survival. * fix(tui): only drop an explicit effort when the target ring is authoritative A nil ring meant both a catalog model with no effort controls and a live-discovered custom model the catalog cannot vouch for. The drop now requires an authoritative ring (a resolved catalog entry, whose ring may legitimately be empty); an unknown ring preserves an explicit preference, matching the cross-provider picker's behavior, while the profile's own fill still applies only where support is known. |
||
|
Vasanth T
|
60dc84e7a3 |
perf(openai): optimized turn session — background prewarm and prefix telemetry (PR8) (#723)
* trace(perf): add provider prewarm span and prefix-stability counters * feat(openai): optimized turn session with prewarm and prefix telemetry * feat(providers): gate optimized OpenAI turn sessions behind env flag * feat(cli): wire optimized turn sessions into headless exec * docs(readme): document the optimized OpenAI turn-session toggle * fix(openai): include tool descriptions in prefix digest, harden gate and test bounds * fix(openai): isolate prewarm tracing, skip non-reusable transports, fingerprint wire order and cache key * test(openai): pin keep-alive transports in probe-expecting tests * feat(providers): preserve resolved capabilities on switched-model fallback sessions |
||
|
KRATOSandKRATOS
|
e5670c427f |
perf(output): add token-aware semantic output budgeting (PR11) (#717)
* perf(output): add token-aware budget contract * perf(output): add semantic retention policies * perf(output): budget results after redaction * perf(output): propagate tool truncation metadata * perf(output): trace semantic budget decisions * perf(output): classify core tool output * docs(output): describe semantic output budgeting * test(output): make spill fallback portable on windows * fix(output): address semantic budgeting review findings * test(output): make hook budgeting tests portable * fix(output): retain oversized single-line file content * fix(output): apply semantic budgets to captured output * fix(output): preserve hook budgets and search paths * fix(output): preserve alias and truncation metadata * test(trace): validate output budget metadata --------- Co-authored-by: KRATOS <[email protected]> |
||
|
PierrunoYTandClaude Sonnet 5
|
9043baedcf |
fix(sandbox): scrub dynamic credential env vars (#682)
* fix(sandbox): scrub dynamic credential env vars * fix(sandbox): tolerate env assignments in configured sensitive keys A config value mistakenly given as a full assignment (e.g. "COMPANY_LLM_SECRET=...") would never match the real env key during scrubbing, silently re-exposing the credential to sandboxed commands. normalizeSensitiveEnvKeys now keeps only the name part before '=' and re-trims it, per PR review. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(sandbox): scrub credentials on direct/degraded command plans directCommandPlan (used when the native sandbox backend is unavailable, disabled, or not required, including the EnforcementDegraded fallback) left Env nil whenever spec.Env was nil, so exec.Cmd inherited the full caller environment unscrubbed. Configured apiKeyEnv credentials and dynamically named ZERO_OAUTH_<NAME>_CLIENT_SECRET variables leaked through that path even though the wrapped sandbox plans already scrubbed them. Route direct plans through scrubSensitiveEnv the same way wrapped plans do, snapshotting os.Environ() when spec.Env is nil so scrubbing still applies. Adds a regression test covering the degraded-fallback path with both a configured and a dynamically named secret. Co-Authored-By: Claude Sonnet 5 <[email protected]> --------- Co-authored-by: Claude Fable 5 <[email protected]> |
||
|
Vasanth T
|
abe24bcb10 |
Add per-turn tracing and a turn-benchmark harness (Phase 0 baseline) (#700)
* Add per-turn tracing and a turn-benchmark harness (Phase 0 baseline)
Introduce an opt-in `internal/trace` package that attributes a run's wall
time to named spans — prompt build, provider connect/queue, generation,
tool queue/execution, permission wait, verification, compaction,
persistence — plus counters for model requests, tool calls, retries,
reconnects, compactions, completion nudges, acceptance checks, model
switches, and input/cached/output tokens. The recorder is nil-safe (a nil
*Recorder is a no-op), so every stamp site can call it unguarded; an
untraced run is byte-identical to today.
The recorder threads into agent.Run via a new `Options.Trace` field and a
single `trace.WithContext` assignment, so the shared providerio seam
(connect around client.Do, queue around OAuth resolve, first token at the
first non-keepalive SSE payload) reaches it through `trace.FromContext`
without any provider-interface change. All instrumentation is opt-in: when
`Options.Trace` is nil the hot path is untouched.
Headless `zero exec` gains `--trace <path>` (and `ZERO_TRACE=<path>`),
writing an agenteval-contract-compatible NDJSON trace (`-` for stderr).
The trace is pure observation; enabling it does not change behavior.
A new per-turn benchmark harness extends `internal/perfbench` (reusing the
existing TaskSet/NumericStats/subprocess runner shapes): `RunTurnBench`
aggregates per-span median/P95 across iterations, ranks the top three
controllable latency sources by share of attributed time, rolls up per
task class, and totals tokens/counts. A checked-in 42-task manifest across
seven classes (nav, edit, fix, refactor, longproc, longctx, parallel) and
local offline fixtures back it, and `make baseline` runs it against the
built `zero`, writing `internal/perfbench/reports/baseline.json`. The
report is machine-specific and regenerated, not hand-edited.
The manifest is JSON rather than YAML to avoid adding a dependency;
per-task verification targets the specific test (`go test -run <name>`)
so fix tasks are independent. Each task is a fresh `zero exec` process, so
iterations are cold-start samples; a warm in-process path is left for
follow-up.
Verified: `go build ./...`, `go vet ./...`, `gofmt -l` clean on touched
files; `go test -race` green for internal/trace, internal/agent,
internal/perfbench, and cmd/zero-perf-bench.
* test(trace): make TestAttributionRatio clock-independent
The test recorded two synthetic 10ms spans with no real delay and asserted
the attribution ratio stayed <= 1.0. On a precise clock (Linux/macOS CI) the
wall is microseconds while attributed is 20ms, so attributed/wall blows past
1.0 and the test failed — even though the ratio legitimately exceeds 1.0
when spans overlap (parallel tool execution). On Windows the coarse timer
masked it, so the failure only surfaced in CI.
Assert the contract instead: attributed is the deterministic sum of span
durations, and the ratio equals attributed/wall (0 when wall is zero). Add a
zero-wall case for the divide-by-zero guard.
* Address review feedback on the per-turn tracer + benchmark harness
Benchmark runner (internal/perfbench):
- A nonzero agent exit code no longer counts as Passed. The runner set
VerifyErr but then fell through to outcome.Passed = true, so a crashed
run could be recorded as a pass. Return before the verification block
when VerifyErr is already set.
- A task now passes only when every iteration passes. The previous
"any iteration passes" rule let a flaky fail hide behind one good run.
- Each invocation copies its workspace fixture to a fresh temp dir and
runs the agent + verifier there, so mutating tasks (edit/fix/refactor)
can't dirty the checked-in fixtures or bleed into the next iteration.
- Trace load failures (missing/malformed trace file) are surfaced as
result warnings via a new TraceIssue field instead of being swallowed.
Tracer (internal/trace):
- Finish freezes the recorder: Counter, addSpan, and the first-event
stamps are dropped after Finish returns, so a snapshot captures exactly
one turn and late stamps can't mutate it.
- WriteText propagates the first write error instead of silently
swallowing sink failures.
Provider seam (internal/providers/providerio):
- FirstTokenAt is stamped only after a payload is accepted as real model
output, not before handle() decides to drop it as an error payload.
Agent loop (internal/agent):
- The compaction counter increments only when the history actually
shrinks, in both the proactive and reactive paths — paid no-ops no
longer inflate the compaction total.
- Document on Options.Trace that a fresh Recorder is required per Run.
CLI (internal/cli/exec):
- Finish the trace at the agent.Run boundary and defer only the
serialization, so the snapshot covers exactly the turn.
- Reject --trace / ZERO_TRACE for spec-draft runs with a clear error
instead of silently accepting and writing nothing.
Manifest + fixtures:
- Strengthen the edit verifiers: rename and value-bump tasks now assert
the old symbol is gone, and the getter task greps the method signature.
- Add the missing testdata/edit fixture (10 edit tasks referenced a
directory that was never committed, so the whole edit class errored
out at run time).
- The manifest test now stats each fixture so a missing dir fails at
load time instead of at every run.
- longproc benchmark keeps Process live across the loop; README code
fence gets a language tag and documents the per-invocation fixture copy.
- Add tests for the tracing wrapper (nil vs wired recorder, OnUsage
forwarding, token aggregation) and the frozen-recorder contract.
* Address review feedback: exclusive-time attribution + wired spans + fix-suite compile
Rewrite the trace attribution model so concurrent/nested spans no longer
double-count. Spans record wall intervals; the recorder derives a parent for
each by interval containment and computes exclusive time (duration minus the
union of its children). AttributedDuration sums only top-level spans; Coverage
is the union of intervals over wall, capped at 1, the honest ">=95%" metric.
Top-latency ranking uses exclusive time so shares sum to ~1 instead of being
inflated by overlapping provider_connect/generation and nested
permission_wait/tool_execution.
Wire the spans that were named but never stamped: verification around the
self-correct verify pass, and StampFirstVisibleEvent on the first forwarded
text event. Drop the tool_queue/process_wait/persistence constants that had no
stamps, and rename the misnamed near-noise prompt_build span to tool_partition.
Trim RequiredEventKeys to events a healthy short run actually emits; move
conditional phases (tools, permission, compaction, verification) to
OptionalEventKeys so a short trace does not false-fail.
Make ReadNDJSON fail loudly on a corrupt file: require a type:trace header,
error on span/counter lines that appear before it, and error when a header
yields no spans or counters.
Fix the blocking fix-suite compile error: Label's %d format verb on a string
tripped go test's vet before any fix task could verify. Make it a runtime
wrong-prefix bug ("admin-" vs "user-") that still type-checks, so each fix
task is independent again.
Manifest: correct refactor-04's stats.go to main.go, reword fix-07 to the
wrong-prefix bug, add a latency-only description (read-only tasks carry no
verification oracle by design), and frame the report README on exclusive time
and coverage.
Tests updated for the occurrence/exclusive/coverage model; trace, perfbench,
and agent green under -race.
* Fix exclusive-time round-trip, identical-interval nesting, and empty-trace parse
Three correctness bugs in the attribution model found by an adversarial review
of the prior commit, all undermining the no-double-count guarantee:
1. exclusive_ms:0 was overwritten with the inclusive Duration on re-parse. A
parent whose children tile its interval legitimately has exclusive time 0;
WriteNDJSON emits exclusive_ms:0, but ReadNDJSON's "<= 0 means missing"
fallback turned that back into the full duration, so every re-parsed trace
re-introduced the double-counting the model exists to prevent (the benchmark
harness, which ranks re-parsed traces, inherits it). Gate the fallback on
key ABSENCE, so a written 0 is preserved.
2. deriveNesting formed a 2-cycle for two spans with identical [start, end]
intervals: the symmetric containment check made each parent the other,
dropping both from top-level and zeroing AttributedDuration. Break the tie
deterministically — strict containment is an unambiguous parent; for
identical intervals, only the lower-indexed span may parent the other, so
one stays top-level.
3. ReadNDJSON returned a valid empty TurnTrace for empty/blank-only input, so a
run that crashed before emitting a trace (or where --trace was not honored)
masqueraded as a clean zero-attribution sample. Empty input now errors, so
the harness records a TraceIssue.
Also: decode counters via json.Number so int64 values above 2^53 round-trip
exactly instead of losing precision through float64; and drop the harness's
own "exclusive <= 0 -> Duration" ranking fallback, which was the same
double-count on the re-parsed path. Added regression tests covering the
zero-exclusive round-trip, the identical-interval nesting, empty-input
rejection, and counter precision.
* perf(turn-bench): split pass/fail into oracle tiers so the pass rate can't be misread as correctness
The turn benchmark counted any exit-0 task as passed, including the 24
read-only tasks (nav/longproc/longctx/parallel) that carry no verification
oracle — so the JSON tasksPassed / "N/M passed" headline could read as a
correctness verdict it never was. Split pass/fail into three oracle tiers:
- correctness (edit grep + fix go test): the only pass rate that can move
with model quality (tasksVerified / tasksPassed / correctnessPassRate)
- build-only (refactor go build): a non-positive oracle, reported as
buildPassRate and excluded from correctnessPassRate; the manifest declares
buildOnlyClasses: ["refactor"]
- latency-only (no oracle): counted in latencyOnlyTasks, never in any pass
rate
NewTurnExecRunner no longer sets Passed=true for tasks with no
verificationCommand, and the summary headline names each tier explicitly so
an exit-0 read-only run cannot inflate a correctness number. Bumps
TurnSchemaVersion to 2. Class breakdown and tier contract documented in
internal/perfbench/MANIFEST.md and reports/README.md.
Also install the trace first-token/visible-event forwarding handlers in
agent.Run whenever options.Trace is set, not only when a UI OnText /
OnReasoning callback is set, so a headless traced run (zero exec --trace)
still captures FirstTokenAt. forwardedVisibleText stays tied to the user
callback so stall-retry semantics are unchanged. Adds a regression
assertion to the existing traced-run test.
* Address CodeRabbit review on
|
||
|
euxaristia
|
b6117af86d |
fix(exec): stop false INCOMPLETE downgrades on conversational final messages (#608)
* fix(agent): scope self-reported incompletion to sentences about the current objective
The detector matched inability stems anywhere in the final message, so a
conversational recap of a past exchange ("You asked if I could work
autonomously ... so I couldn't actually do it at the time") downgraded a
completed run to INCOMPLETE. Match per sentence instead, drop quoted spans,
and skip sentences that retell a past exchange. The real transcript is now a
regression test, and genuine admissions still fire, both present tense and
past tense about the current task.
* feat(exec): add --no-completion-gate for conversational callers
A chat frontend driving zero exec has an operator present, so a final
message that hands the turn back ("the sandbox blocks network egress,
approve it and I'll continue") is a complete conversational answer, not an
unfinished task. The headless completion gate still stamped such runs
INCOMPLETE (exit 4) on the honest inability phrasing, and no phrase
heuristic can fix that case because the admission is true. The new flag
maps to RequireCompletionSignal=false so conversational callers can opt
out. Default behavior is unchanged.
* fix(exec): address review findings on quote stripping and --use-spec combination
stripQuoted now removes only balanced quoted spans: an opening delimiter
that never closes is restored verbatim, so a stray quote cannot swallow
the rest of the message and hide a genuine admission from the detector.
Regression tests cover dangling straight, curly, and backtick delimiters.
--no-completion-gate combined with --use-spec is now rejected at parse
time, mirroring the --self-correct check: the spec-draft path never
consults the completion gate, so the flag would be silently ignored.
|
||
|
beardthelionandKevin Codex
|
a880ce80a6 |
feat: gate project-scoped hooks, plugins, and MCP servers behind workspace trust (#529)
* feat(workspacetrust): add exact-match trust store for project config gating * feat(hooks): add ExcludeProject option to LoadConfig * feat(plugins): add ExcludeProject option to Load * feat(cli): gate project hooks and plugins behind workspace trust Trust check lives inside newHookDispatcherWithExtra and activatePlugins (fail-closed by construction); trustRoot is the original launch dir so a --worktree run inherits the source repo's trust. Callers OR the skip reports and emit one combined stderr notice. * feat(cli): add zero trust command (trust/list/remove) * fix(review): key spec-draft trust on the original launch dir; cover trust CLI edge branches Spec-draft runs (zero exec --use-spec --worktree) passed the reassigned worktree path as trustRoot, dropping a trusted repo's project hooks (fail-closed, but breaks worktree-inheritance). Thread the captured pre-worktree trustRoot into execSpecDraftRun. Add tests for the trust 'remove' too-many-args and --help branches, and a resolveTrust comment noting the per-chokepoint check must not be hoisted to callers. * test(cli): end-to-end trust-gate coverage through agent.Run and exec --worktree Drives the real agent loop with a fake tool-calling provider so a project beforeTool hook actually fires (or is gated away) through production code: gate blocks a tool call via agent.Run, and --worktree / --use-spec --worktree key trust on the original launch dir. All three are load-bearing (RED when the gate or the trustRoot threading regresses). * style(cli): gofmt struct alignment in exec_spec.go * fix(workspacetrust): compare stored trust entries literally, no symlink re-resolution External adversarial review (codex) found a false positive: IsTrusted re-normalized stored entries on every check (Abs+EvalSymlinks), so a trusted path later replaced by a symlink to attacker content re-resolved to the new target and matched. Stored entries are already canonical (normalized once at Trust time); compare them as literals and only normalize the incoming query. Add a regression test for the symlink-retarget case. * feat(mcp): gate project-scoped MCP servers behind workspace trust Closes the P0 an external adversarial review (codex) found: project ./.zero/config.json stdio MCP servers spawned before the hook/plugin gate, an ungated arbitrary-execution path that could also self-trust the workspace. Add ExcludeProject to config.ResolveMCP (skips the project layer) and thread a fail-closed trust check into every MCP spawn site (exec, TUI setup+refresh, mcp tools list, mcp check); reporting sites stay ungated. Verified the sole server-command exec (mcp/client.go) is reachable only through the now-gated RegisterTools paths. * test(cli): cover default-mode beforeTool firing through the trust gate Assert a project beforeTool hook fires in the default auto and ask permission modes (not only under unsafe), and is blocked when the workspace is untrusted, so the gate is verified in the mode a normal run actually uses. * docs(cli): note MCP servers in zero trust help text and doc comment The gate covers project hooks, plugins, and MCP servers, but the help string and runTrust doc comment listed only hooks and plugins. * test(cli): make workspace-trust tests portable on macOS and Windows setTrustConfigRoot set HOME on darwin, but config.UserConfigDir is XDG-first there (=$HOME/.config), so tests wrote trust.json and user hooks.json where the code never read them; set XDG_CONFIG_HOME on every non-Windows platform instead. writeMarkerHook shells out to /bin/sh, absent on Windows, so its trusted-case marker never appeared; skip it there like writeMarkerHookScript already does. TestRunTrustAddThenList compared `trust list` output (canonicalized via EvalSymlinks) against the raw cwd, which diverges on the Windows runner's 8.3 short names (RUNNER~1 -> runneradmin); compare the resolved path, and add TestRunTrustListShowsCanonicalPath to exercise that via a symlink on any platform. * feat(cli): surface the project MCP skip in the workspace-trust notice Project MCP servers were dropped silently in an untrusted workspace: the trust notice covered hooks and plugins but not MCP, and app.go discarded the resolveTrust error. registerMCPToolsForWorkspace now returns a trustSkip, gated on a new projectMCPConfigExists check plus the previously-dropped store-read error, and emitTrustNotice is variadic so the exec, spec-draft, interactive, and `mcp tools list` paths all fold in the MCP skip. The skip drives the notice only; the gate stays on resolveTrust/excludeProject, so a trusted workspace is unaffected and an untrusted one still fails closed. * fix(cli): print zero trust --help to stdout with a success exit zero trust -h/--help/help routed usage to stderr with the usage exit code, unlike every other subcommand (mcp, sandbox, skills, cron, ...) which prints help to stdout and returns success. Align it; the unknown-subcommand path still writes to stderr with the usage exit code. * fix(cli): gate oauth MCP login and surface mcp-check trust notice Two project-scoped MCP activation sites were left outside the workspace trust gate. zero mcp oauth login loaded a project-defined OAuth server from an untrusted workspace and ran the full discovery/registration/token-exchange flow against its configured URL. Gate resolveOAuthServer with resolveTrust so an untrusted clone's ./.zero/config.json OAuth server is dropped before login, and emit the same one-line trust notice the other spawn sites use. zero mcp check already gated the project layer but discarded the trust-check result and never emitted the notice, so a gated project server read as a bare "not configured". Emit the notice on the not-found branch. The three remaining ungated resolveMCPConfig(cwd, false) sites (backends doctor/snapshot and the extensions listing) are enumeration-only and stay ungated. * test(cli): adapt MCP startup-skip tests to two-arg resolveMCPConfig --------- Co-authored-by: Kevin Codex <[email protected]> |
||
|
PierrunoYTandCopilot App
|
062328b632 |
fix: warn about untracked scratch files left behind after a run (#571)
* fix: warn about untracked scratch files left behind after a run Fixes #551. Headless exec runs could leave behind debug/scratch files (e.g. _fix_test.py, _debug.py) created by write_file during iteration, which would then be silently swept up by a later git add -A. - FileTracker now records brand-new files (RecordCreated/CreatedFiles), distinct from the existing version-baseline tracking used for conflict detection. - write_file records a path as created only on first write, not on overwrite of an existing file. - After a headless run completes, exec.go checks git status for any created file still present and untracked, and surfaces a warning via the existing writer.warning() (works across text/-o json/stream-json). No files are deleted automatically — a wrong guess at 'scratch' could destroy real work, so this only surfaces what git already knows. Co-authored-by: Copilot App <[email protected]> * fix: address scratch warning review comments * test: normalize created file tracker paths * fix: track apply_patch-created scratch files * fix: address scratch warning review feedback * fix: tighten scratch warning lifecycle --------- Co-authored-by: Copilot App <[email protected]> |
||
|
Kevin Codex
|
95ccd5bc32 |
perf: universal tool-output ceiling with spill + async post-edit diagnostics (#518)
* feat(tools): universal token-based output ceiling with spill-to-disk
Cap every tool result at the registry boundary (16k estimated tokens,
ZERO_TOOL_OUTPUT_CEILING_TOKENS-tunable, 0 disables) unless the tool
manages its own deliberate budget. Head+tail truncation, full output
spilled to the existing redacted spill dir and re-readable via
grep/read_file, so nothing is lost — only deferred.
This closes the three unbounded paths a single call could flood the
context window through (and then re-bill every following turn):
- web_fetch: up to 256KiB default / 2MiB max of non-HTML body
- skill: full SKILL.md returned uncapped
- MCP-served tools: no budget at all
Self-budgeting tools opt out via an unexported marker interface (an MCP
server can never exempt itself): bash, exec_command (model-raisable),
read_file, read_minified_file, grep, glob, list_directory. Browser
snapshots deliberately stay under the net.
bash itself is tightened to match the unified token model: emit budget
96KiB -> 32KiB per stream (~24k -> ~8k tokens worst case per stream),
while capture retention stays at 96KiB head+tail so the new spill file
covers 6x more of the output than the transcript shows.
* perf(agent): collect post-edit LSP diagnostics off the tool-call path
Inline post-edit diagnostics blocked every edit_file/write_file result on
the language server: a >=300ms quiet-period debounce that reset on each
publish, seconds on the first edit per language, capped at 10s — the
largest recurring latency tax in an edit-heavy session.
Diagnostics are now collected in the background: a file is enqueued the
moment a mutating tool finishes, a single worker checks it while the rest
of the batch executes, and the loop drains completed results just before
building the NEXT provider request, appending errors as a user nudge.
The model always issues another request after an edit turn (to read its
tool results), so it still sees introduced errors at the same decision
point — no tool call ever stalls on the language server.
A file re-edited before its check runs is checked once in its final
state (the checker re-reads the file); re-edited after, it is re-queued
and the newer result wins. A drain that finds the worker still busy
gives up after 3s and delivers on a later turn — results stay accurate
because any further edit re-enqueues the file.
The tools-side inline mechanism (tools.RunOptions.Diagnostics) is kept
for direct API callers; the loop just no longer wires it.
* fix: address review — max-turns diagnostics drain, capture-gap marker, test hardening
- Drain background diagnostics before the max-turns final-answer request
so an error introduced by the LAST turn's edit is reported in the
summary instead of silently dropped (+ regression test).
- spillBashStreams marks the head/tail junction with a capture-gap
marker when boundedBuffer dropped the middle of a stream, so a spilled
log never reads as contiguous when it is not; junction position is
asserted in the test.
- Tests override TMP/TEMP alongside TMPDIR (os.TempDir reads TMP/TEMP on
Windows) via a shared setTestTempDir helper.
- TestBudgetBashCaptureReportsTrueTotal now asserts spill_path meta and
spill file content; exemption-list test gains read_minified_file.
* fix(test): use a real workspace root in async diagnostics tests
filepath.IsAbs("/ws") is false on Windows (no drive letter), so the
collector test's workspace-absolute assertion failed on the windows
smoke job. Root the fake workspace at t.TempDir() — absolute on every
platform — and assert against it.
* fix: pattern-scrub spill files and gate finalization on pending diagnostics
Review follow-ups (Vasanthdev2004):
- Spill files now pass the pattern-based secret scanner (AWS keys,
GitHub/Slack/OpenAI tokens, PEM blocks, JWTs) in addition to the
configured-key scrub. A bash spill runs before formatBashOutput's
scan, so without this a spilled file held in cleartext exactly the
credentials the transcript redacts. Applied centrally in
spillTruncatedOutput, covering bash, exec_command, and ceiling spills.
- Finalization diagnostics gate: a no-tool-call final answer now drains
pending post-edit diagnostics with the full inline-era budget (10s,
vs the 3s per-turn wait) before the run is accepted; introduced
errors append the nudge and give the model one more turn instead of
being silently dropped when no later turn exists. The max-turns
summary path uses the same finalization budget. Free for runs that
never edited — an idle collector returns immediately.
|
||
|
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. |
||
|
Ashwinhegde19andAshwin
|
f6c060631e |
fix(openai): handle Ollama reasoning stream deltas (#486)
* fix(openai): handle reasoning alias in stream deltas * test(cli): assert reasoning event order --------- Co-authored-by: Ashwin <[email protected]> |
||
|
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. |
||
|
Anandan
|
f401c66e81 |
polish: cut per-turn overhead, stabilize cache, add /new (#478)
* polish: cut per-turn overhead, stabilize cache, add /new Context economy - Compaction: preserveLast 8->6, triggerRatio 0.8->0.7 (both copies in sync). - System prompt: kill the "ELABORATE summary" directive; scale summary to the work. Comment discipline moved into the core prompt for all model families (was Anthropic-only); Minimax/DeepSeek/Qwen/etc. now get it. - Token-budget regression test ratchets the fixed per-turn overhead (base prompt + eager tool schemas) using the runtime's own estimator, so silent prompt/schema growth fails CI. Response quality - defaultResponseStyle balanced->concise. Reversible via /style. Tool discovery - <available_skills> block in the system prompt lists installed skills (name + one-line description) so the model loads the right one on the first call instead of guessing and reading the failure. Budget-capped at 640 bytes with an "...and N more" fallback; renders nothing when no skills are installed (byte-identical to prior prompt). Session lifecycle - /new starts a fresh session in place; previous session stays on disk and is named in the confirmation for /resume. Guards against mid-run reset. - /clear now notes that context persists and points to /new. Performance & caching - partitionTools no longer alpha-sorts the whole tools array every turn, which inserted a loaded deferred tool mid-array and shifted the cached prefix on every load. It now builds append-only regions [eager sorted] + [tool_search] + [loaded deferred sorted] so a mid-session load grows the tail instead of busting the eager prefix. Regression test asserts the eager block precedes any loaded tool. - /context now shows cache hit rate (flags low rate over >5 turns as a possible unstable prefix) and rolling average turn latency with time-to-first-token. Both reset on /new. Measured baselines (auto-mode, no workspace): base prompt 3162 tokens, eager tool schemas 2433 across 12 tools. Budget ceilings set ~10% above. Deliberately deferred (need a decision): inverted safety-policy rewrite (sandbox doesn't catch ~half the B-tier rules), core-tool/web_fetch deferral (activation-semantics blocker), aggressive prompt slim. * fix(tui): clear staged attachments + queued message on /new CodeRabbit: startNewSession reset conversation/usage/latency state but not the staged input (pendingImages, pendingImageLabels, pendingDocuments, queuedMessage), which is only consumed at prompt-submit. Without clearing it, the fresh session's first prompt silently inherited the previous session's images/documents/queued text. Test now stages all four and asserts they clear. |
||
|
KRATOS
|
6e0a665118 |
fix(auth): propagate credentials to every provider-build surface and pin children to the live provider (#366)
* fix(auth): propagate credentials to every provider-build surface and pin children to the live provider Sub-agents, swarm members, ACP sessions, and mid-run escalation switches could build providers without the credentials the parent was running with. This consolidates credential resolution so every surface gets the same auth: - Wrap deps.newProvider once in fillAppDeps to fold the stored API key in centrally; ACP, exec escalation, and buildProvider no longer patch it per call site (an apiKeyStored profile previously went out keyless on those paths and 401'd). - Export ZERO_PROVIDER at startup (buildProvider) and keep exporting on every runtime switch (wizard, onboarding, /model, /provider), so spawned children resolve the parent's live provider instead of whatever another process last persisted to config.json. Injected via appDeps so tests never mutate the process environment. - Share one OAuth login selection end to end: ProviderProfile. OAuthLoginCandidates (name, then catalog-ID fallback; nothing for a profile with its own configured credential) + oauth.FirstStored resolve the login ONCE, and the bound store key is threaded through providers.Options.OAuthLoginKey so the Codex account-id header always reads the same login that issued the bearer token. A renamed profile (name != catalog ID) now finds its login; an explicit API key is never silently overridden by a same-name or catalog-shared OAuth login. - Export ProviderProfile.HasConfiguredCredential as the single "profile is key-authed" predicate (cli/tui/config previously had three copies). - applyProviderWizard commits live state (provider, profile, name, env export) only after BOTH the build and the config persist succeed, so a persist failure no longer leaves the chat running on a provider the status line and children disagree with. Covered by red-green tests for each path: stored-key wrap, escalation switch, catalog-ID fallback (incl. case-variant names), explicit-key protection, same-login account selection, wizard persist-failure rollback, onboarding/startup env export. * test: isolate OAuth/config test storage and fix the flaky MCP response-id test Review + CI fixes, all test-only: - factory/cli OAuth tests: pin ZERO_OAUTH_STORAGE=file next to the temp ZERO_OAUTH_TOKENS_PATH — an inherited "keyring" value sends NewStore to the OS keychain and ignores the temp path entirely, so the tests could read (or clobber) the developer's real logins. newOAuthStoreForTest now owns both pins so a future call site can't forget them. - TestApplyProviderWizardExportsActiveProviderEnv: point m.userConfigPath at a temp config and stub newProvider, so the SUCCESS path exercises the full build+persist+commit sequence in isolation (the default empty path silently skipped persist; a changed default must never reach the real user config). - TestClientRequestWaitsForMatchingResponseID (ubuntu smoke flake): the responses were pre-loaded into a bytes.Buffer, so the client's reader goroutine could consume the matching response — and hit EOF — before request() registered its pending id, failing with "request() error = EOF" on fast runners. The fake server now sits on pipes and responds only after reading the outgoing request, matching production ordering; survives -race -count=50. |
||
|
Vasanth T
|
aa45edacc0 |
fix(modelregistry): one source of truth for a model's reasoning efforts (#335)
The /effort picker and the run-time effort resolver disagreed about which tiers a model supports. Registry.ReasoningEfforts falls back to name-based inference (reasoningEffortsForModelName) when the catalog lists no efforts, so the picker shows controls for proxy-served GPT-5 / Codex / o-series models. EffectiveReasoningEffort read model.ReasoningEfforts directly and never saw that fallback, so it coerced the same request to "none" — the picker advertised a tier the resolver silently dropped. Route both through a single effectiveReasoningEfforts helper so they can never diverge. A model the catalog enumerates with no efforts but whose name is a known reasoning family now has its requested effort honored end to end; non-reasoning models still resolve to "none". Also drop the orphaned, stale NOTE above forwardedReasoningEffort claiming effort "is not yet forwarded" (it is) and restore the reasoningEffortNotice doc comment to its own function. |
||
|
KRATOS
|
ca5e0b9bbb |
feat: context-aware compaction + usage gauge for all models (#332)
* fix(agent): enable auto-compaction for all models, not just registered ones Auto-compaction keys off the model's context window (enabled = ContextWindow > 0), resolved only from the curated registry. Unregistered models — GPT-5/Codex, xAI, Ollama-cloud, custom endpoints — resolved to 0, so both the proactive (~80%) and the reactive post-overflow compaction were silently disabled and long sessions overflowed. - TUI modelContextWindow now also consults live-discovered windows (accurate for proxy/custom models once /model has discovered them); unknown still returns 0 so display gauges show no guessed denominator. - New modelregistry.AgentContextWindow applies a positive FallbackContextWindow (200k) ONLY on the compaction-enable path (TUI run + CLI exec/spec), so compaction is on for every model while display stays accurate-or-empty. Exact windows always win. * feat(tui): show context usage (used / total + % gauge) for all models from launch The sidebar/status-line context gauge only shows a total + fill % when the active model's context window is known. For proxy/custom models (GPT-5/Codex, xAI, Ollama- cloud) the window isn't in the registry, so it showed only a used-token count. Warm model discovery for the active provider in Init() (background, non-blocking) so the active model's window is learned at launch and the gauge renders 'used / total tokens' + the fill % for every model — not just catalogued ones. Builds on the discovered-window resolution from the compaction fix. * fix(tui): address review — agent fallback on model-switch + active-provider window - Apply modelregistry.AgentContextWindow to /model and /mode pre-switch compaction requests (TargetContextWindow), so the 'compaction for all models' behavior also covers undiscovered/custom switch targets, not just the active run. - Scope the live-discovered context-window lookup to the active provider first (then others), so a model ID shared across providers resolves to the provider in use rather than an arbitrary match. - Tighten the resolution test to compare against the registry's actual gpt-4.1 window instead of != FallbackContextWindow. * fix(cli): give provider-discovered context windows precedence in headless exec Address the remaining review point: CLI exec/spec runs collapsed every uncatalogued model to the 200k fallback, ignoring its real window. resolveAgentContextWindow now, on a registry miss, queries the provider's live model list (resolving the credential from inline/stored/env) to get the model's actual context window before falling back. Discovery runs only on a registry miss (catalogued models pay no latency), is bounded by a 5s timeout, and degrades to the fallback on any error so a headless run is never blocked or failed. |
||
|
KRATOS
|
f8ac5eceb0 |
fix(agent): gate headless run completion on honesty (no false-success on no-tool-call / self-reported-incomplete turns) (#325)
* fix(agent): don't end a run as success on a no-tool-call turn mid-task
A turn that produced text but no tool call was always accepted as the final answer, so the loop reported success even when the model stopped mid-task (e.g. ended on "...Let me check the SSH configuration:" with plan steps still pending).
Add an opt-in completion gate (Options.RequireCompletionSignal): when a turn has no tool call and work clearly remains -- pending update_plan items, or the message ends on a continuation cue -- re-prompt the model to continue instead of finalizing. Bounded by maxContinueNudges and still by MaxTurns/the deadline; once the budget is spent the run finalizes as INCOMPLETE (Result.Incomplete) rather than success. Default off, so the interactive path is byte-identical.
Genuine single-turn completions (no pending plan, no cue) still finalize as success. Covered by internal/agent/completion_gate_test.go.
* feat(exec): report stalled headless runs as INCOMPLETE (exit 4)
Enable the agent completion gate for headless exec (RequireCompletionSignal) and map Result.Incomplete to run_end status "incomplete" with a new exit code 4, so a run that stalled mid-task (model stopped without a tool call while work remained, continue budget exhausted) is no longer reported as success. Interactive callers are unaffected.
* fix(agent): downgrade self-reported non-completion; advisory task-grounded acceptance
Reduce -- not eliminate -- false-success on headless runs. Two DETERMINISTIC, unit-tested gates (with the plan gate from the prior commit):
(a) self-report downgrade: if the final message admits the model guessed or could not meet the objective, finalize INCOMPLETE (exit 4), never success. Inability is matched by first-person STEMS generalized over verb/tense ("I cannot/can't/could not/am unable to/do not have/unable to ...") plus guess/fallback/uncertainty phrases, with a guard so success-y negations ("could not find any issues", "cannot reproduce") are not misread. (b7bc0b8's plan gate already forces INCOMPLETE on pending/in_progress update_plan items at termination.)
(b) task-grounded acceptance is ADVISORY, not a guarantee. When --self-correct is on it demands one bounded acceptance pass that re-derives the task's stated criterion and runs a concrete check, discouraging three false-success patterns (well-formed==correct, existing-tests-pass==objective-met, result==baseline-it-was-told-to-beat). But it is a prompt: a model that ignores it and confidently claims "PASS, all requirements met" still slips, because ZERO has no general oracle to verify correctness against a task's hidden criterion. Empirically (TB-2, qwen3-coder:480b) this reliably catches admissions and incomplete plans and REDUCES false-success, but a confident false PASS on a model-ceiling task is a residual, fundamental gap -- not a tuning miss.
Default off (RequireCompletionSignal); interactive callers are byte-identical. Covered by internal/agent/{acceptance_gate_test.go,completion_gate_test.go}.
* feat(exec): surface the INCOMPLETE reason in run_end and logs
When a headless run finalizes as INCOMPLETE, include Result.IncompleteReason in the session error event and a stderr warning so an honestly-incomplete run (e.g. "the final message admits the objective was not met") is debuggable rather than an opaque exit 4.
* fix(agent): harden completion gate per PR review
Addresses the correctness findings from the PR review.
self-report (#1): drop behavior-describing phrases ("fall back to",
"placeholder value", bare "best guess", "as a fallback", "without proper")
that also match legitimate final answers; keep first-person/uncertainty
admissions only, since these are matched without a context guard.
self-report (#5): scan every occurrence of each inability stem so an early
success-negation ("could not find any examples") no longer masks a later
genuine admission with the same stem ("could not implement it").
continuation cue (#6): require a trailing colon AND an action lead-in on the
final clause; stop flagging recommendations, plain summary colons, and
sign-offs. Still catches the mid-line "...Let me check the config:".
gate order (#8): check the self-report admission BEFORE pending-plan, so an
admitted-impossible task downgrades immediately with the accurate reason
instead of burning continue-nudges.
pending plan (#3): treat a pending/in_progress update_plan item as a
NUDGE-only weak signal -- it no longer forces INCOMPLETE on its own (a
completed run that left stale plan bookkeeping is trusted). Only a
continuation cue or a self-report admission finalizes INCOMPLETE.
max-turns (#4): a run cut off at the MaxTurns ceiling now finalizes
INCOMPLETE under the gate instead of being reported as success.
exec json/cron (#2, #7): for -o json, emit the terminal done with exit 4 on
an incomplete run (final() pre-emits a success done:0 for json that would
otherwise mask it); emit an error event -- not just a warning -- so the cron
failure extractor can recover the reason.
Deferred (noted on the PR): acceptance-only-when-mutated (#9, cost) and
reusing tools.normalizePlanStatus (#10, would widen scope to internal/tools).
Tests: add TestContinuationCueMatching and TestMaxTurnsCutoffIsIncompleteUnderGate,
extend TestSelfReportedIncompletionMatching with the #1/#5 cases, and replace
the in_progress=>incomplete test with TestPendingPlanAloneDoesNotForceIncomplete.
make build / go vet / make lint / go test ./... -race all green.
|
||
|
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
|
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
|
9047977895 |
fix: resolve product/UX audit findings for OSS launch (#282)
Resolve product/UX audit findings for the OSS launch: doctor now fails uncredentialed remote providers (no false green), Redact() no longer mangles the common first-run auth error, SSRF-safe loopback exemption unblocks keyless local models, new validated --theme flag, WCAG-AA contrast fixes, NO_COLOR compliance, clearer first-contact errors, and full OSS scaffolding (SECURITY/CoC/CHANGELOG/templates/release workflow). Audit ledger maps every Fixed claim to a real change with zero overclaims. |
||
|
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. |
||
|
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
|
eb608741b0 |
Align permission prompt rendering (#246)
* Align permission prompt rendering Remove normal permission prompt risk labels from focused cards, transcript rows, and permission detail text so risk metadata is not presented as the permission UI contract. Rename the recoverable deny option from a redirect-style prompt to a continue-without-running label that matches its current behavior. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tui Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tui ./internal/agent ./internal/cli Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Tighten permission prompt fallbacks Use conservative allow/deny choices when a permission request lacks backend-supplied decisions, avoiding impossible session or persistent grant options. Document that a failed session-grant recording attempt must not deny the current user-approved tool call; it only means a later matching call prompts again. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tui ./internal/agent Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Split permission deny and cancel decisions Add an explicit cancel permission decision that aborts the active run while keeping deny as a recoverable tool refusal. Expose cancel choices for command and apply-patch approval prompts, keep apply-patch off the recoverable deny path, and stop offering persistent always approvals for command/apply-patch prompts until scoped prefix approval exists. Classify cancel decisions as permission-decision events for TUI session logs and exec JSON output. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/tui ./internal/cli Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Stabilize OAuth secret creation Publish encrypted OAuth token secrets atomically under a short-lived creation lock so concurrent first-run callers never observe a half-written secret file. This fixes the Zero Review failure in TestLoadOrCreateSecretConcurrentConverges. Tested: go test ./internal/oauth -run TestLoadOrCreateSecretConcurrentConverges -count=100 Tested: go test ./internal/oauth Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/tui ./internal/cli Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check |
||
|
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
|
c38444157c |
Audit backlog: tui mouse API, notify webhook, swarm scheduling, daemon-remote bundles, MCP/oauth store unification, OS keyring, dead-code prune (#216)
* tui: migrate off the deprecated Bubble Tea mouse Type API (SA1019)
The mouse classification helpers checked both the current Button/Action pair and
the deprecated msg.Type / tea.Mouse* constants. Bubble Tea's parser always
populates Button+Action and only DERIVES Type from them, so the Type fallbacks
were pure redundancy (a left-button drag is Action==Motion, already covered by
mouseMotion). Drop them — clears all 11 SA1019 findings in internal/tui with no
behavior change. The left-drag test now asserts the Action==Motion path directly.
Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(SA1019 0)/
deadcode(no new) all pass.
* notify: wire the webhook sink behind an opt-in env var
The WebhookSink (Slack / generic JSON POST on turn completion) was fully
implemented, redaction-safe, and tested — but never attached to a Notifier, so
deadcode flagged its entire machinery (NewWebhookSink, Emit, text, redactLinks,
log, eventType, readSnippet) as unreachable.
Wire it via a small, testable helper, MaybeAddWebhookSink(n, env, logf):
- reads ZERO_NOTIFY_WEBHOOK_URL (+ optional ZERO_NOTIFY_WEBHOOK_SUMMARY)
- no-op when the URL is unset/blank, so it is safe to call unconditionally
- sourcing the URL from the environment keeps the secret token out of any
on-disk config file
Attached at both notifier construction sites:
- headless exec: failed deliveries log to stderr (never stdout); the sink
redacts before logging
- TUI: logf=nil so a delivery failure can't corrupt the alt-screen
The sink stays gated by the existing Mode/focus policy (verified by
TestNotifierOffSuppressesSinks), so a webhook only fires when notifications are
enabled (e.g. --notify both) — no change to default behavior. Opt-in and
fail-closed: with the env var unset nothing is attached.
deadcode: 100 -> 93 unreachable funcs (the 7 WebhookSink entries become
reachable); no new dead code. gofmt/vet/build(host+linux+windows)/test -race/
staticcheck(no new)/govulncheck(0) all pass.
* swarm: add opt-in recurring-spawn scheduler (wakeup + daily)
Adds a dependency-free Scheduler to internal/swarm so a member can be spawned on
a recurring schedule, plus a swarm_schedule tool to drive it.
- Schedule is interval-based ("wakeup", every >= 1s) with an optional first
delay and a MaxRuns cap; a daily "HH:MM" ("cron") time is expressed as a 24h
interval whose first delay lands on the next occurrence.
- Each fire spawns a fresh member through the existing Swarm.Spawn path, so
members inherit the recorded policy and are bounded by the team slot cap +
queue. Non-overlapping by default: a fire is skipped while the job's previous
spawn is still running, preventing queue pile-up.
- Scheduler is lazily created (Swarm.Scheduler()), opt-in (nothing runs until a
job is added), and tied to the Swarm's base context — Swarm.Close() stops the
scheduler first so no spawn fires during shutdown.
- swarm_schedule tool: action add|list|cancel; prompt-gated like swarm_spawn;
add validates schedule + agent type up front (fail fast).
The timing loop takes an injectable ticker, so the tests drive fires
deterministically with no real time (fire/skip/cap/cancel/close, plus pure
nextDailyDelay/parseClock/swarmInt). All swarm tests pass 3x under -race.
Gates: gofmt/vet/build(host+linux+windows)/test/staticcheck(clean)/govulncheck(0)/
deadcode(no new) all pass.
* daemon/remote: SessionLink + git-bundle upload over the bridge
Adds an opt-in way to ship a local repo's git history to a remote daemon and
link a remote working tree to it — without a shared filesystem or a network git
remote. The daemon core (protocol/server) is untouched; all changes live in the
remote package plus a CLI hook.
Wire:
- The auth handshake gains an optional Mode field ("" / "session" => a daemon
session as before; "bundle" => a one-shot git-bundle upload). Unknown modes,
or bundle mode when uploads are disabled, are denied (fail closed).
- Bundle transfer reuses the daemon frame codec: a header frame (link id +
exact size), then the bundle bytes as KindData frames, then a result frame.
Server (Bridge):
- BridgeOptions.BundleDir enables uploads (empty => refused). A received bundle
is size-capped (default 256 MiB), staged to a temp file, `git bundle verify`-d,
then `git clone`-d into a staging dir and atomically renamed over
<BundleDir>/<link-id>. Link ids are sanitized to a single path component and
containment is re-checked, so an upload can never escape the bundle dir.
Client:
- UploadRepoBundle creates `git bundle create --all` of a work tree, hashes it
(sha256), streams it over an authenticated bundle-mode TLS connection, and
returns a SessionLink {address, link id, remote path, bundle sha}. SessionLink
persists as a 0600 atomic JSON file (token never stored) with Save/Load/Validate.
CLI:
- `daemon serve-remote --bundle-dir <d>` enables uploads.
- `daemon link --remote <addr> --repo <dir> --id <name> [--out <file>]` uploads
and prints the remote path (+ a ready run command); `daemon link --show <file>`
prints a saved link.
Tests: git bundle create/verify/clone roundtrip; link-id sanitization + path
containment; SessionLink save/load/validate (0600); and a full bridge upload E2E
over real TLS (extract + content check + re-upload-replaces + disabled + bad
token). All remote tests pass 2x under -race; CLI smoke verified via the binary.
Gates: gofmt/vet/build(host+linux+windows)/test/staticcheck(no new)/
govulncheck(0)/deadcode(no new) all pass.
* mcp: unify OAuth token storage onto the shared oauth store
MCP server tokens lived in their own mcp-oauth-tokens.json with a bespoke
codec/lock, separate from the provider-login store (internal/oauth). The oauth
store already reserved the "mcp:" key namespace for exactly this. Unify them.
- mcp.TokenStore now delegates to internal/oauth.Store, keying each server's
token as "mcp:<server>". Its public API (StoredToken/TokenStatus/Save/Load/
Delete/Status/FilePath) is unchanged, so callers (network client, CLI) are
untouched. The bespoke read/write/lock code is removed in favor of the shared,
cross-process-locked, atomic store.
- Transparent, non-destructive migration on first construction: a legacy
mcp-oauth-tokens.json is imported into the unified store (only entries not
already present — a newer unified token is never overwritten), then renamed to
a ".migrated" backup so it is imported at most once and stays recoverable.
A missing / unreadable / unknown-schema legacy file is left untouched and
never blocks startup. An explicit FilePath suppresses the default-path
migration so tests/overrides can't touch the real user config.
- Server names that can't form a valid namespaced key (oauth.ValidateKey) are
rejected on save and skipped during migration. Added oauth.Store.FilePath().
The token format was already identical (access/refresh/type/scopes/expires), so
this is a key rename, not a format change. Existing TokenStore tests pass
unchanged (they exercise the API); added migration + namespacing + newer-wins
tests. Local run check: `zero mcp oauth status` migrates a seeded legacy file
into oauth-tokens.json under mcp:demo, renames the legacy to .migrated, and leaks
no token material.
Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(clean)/
govulncheck(0)/deadcode(no new) all pass.
* oauth: add an opt-in OS keyring storage backend
Introduces a pluggable storage backend for the unified oauth token store and a
dependency-free OS keyring implementation, selected with
ZERO_OAUTH_STORAGE=keyring (default stays the 0600 file).
- internal/keyring: a small, third-party-dep-free secret store backed by the OS
tooling — `security` on macOS, `secret-tool` (libsecret) on Linux; other
platforms report unsupported. On Linux the secret is passed via stdin (never
the argv); on macOS `security` takes it as an argument (briefly visible in the
process list) — documented, and a reason to prefer the file backend there.
A runner seam makes the per-OS command logic fully testable without a real
keychain.
- internal/oauth: Store now persists its JSON blob through a blobStore backend.
fileBlob preserves the exact prior behavior (0600, atomic write, cross-process
lock file). keyringBlob stores the blob as one base64 entry (keeps multi-line
JSON a single control-char-free value) via a KeyringClient. NewStore resolves
the backend from StoreOptions.Storage or ZERO_OAUTH_STORAGE; unknown storage
and an unavailable keyring fail closed with a clear error.
Because both `zero auth` and the (now unified) MCP token store construct their
store through oauth.NewStore, the keyring backend is honored by both with no
call-site changes. Added oauth.Store.FilePath() returns a "keyring:" identifier
for that backend.
No new module dependency (go.mod unchanged). Tests: keyring round-trip/not-found/
unsupported/missing-binary/stdin-not-argv via an injected runner; store keyring
round-trip + base64-at-rest + storage selection; all existing file-backend tests
pass unchanged. Local run check (macOS): validated the exact `security`
add/find/delete shapes and the not-found exit code (44) against the real binary,
and `mcp oauth status` with ZERO_OAUTH_STORAGE=keyring reads the real keychain
cleanly.
Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(no new)/
govulncheck(0)/deadcode(no new) all pass.
* all: prune unused unexported functions (dead-code sweep)
Removes 15 unexported functions/types/consts that are unreachable even from
tests (every one flagged by staticcheck U1000), clearing the U1000 noise the v16
audit surfaced. Conservative by design: only unexported, test-unused symbols are
removed — exported internal API (which may be an intentional surface or used only
by tests) is deliberately left in place.
Removed:
- agenteval: validateExpectedChangedFiles
- providers/gemini: const providerName
- tui: stripMarkdownInline, removeTrailingAtToken,
noopModelSwitchCompactionPolicy (+ BeforeModelSwitch), deleteComposerLineBefore,
deleteComposerLineAfter, deleteCompletedFileMentionBefore, mcpViewStatus,
mcpServerLines, fitCommandOutput, (*commandPicker).clearQuery,
tuiTheme.onPanel2, indentText
No behavior change (the symbols had no callers). staticcheck U1000 is now empty;
deadcode drops accordingly with no genuinely-new unreachable functions (verified
by a name-based diff vs origin/main). gofmt/vet/build(host+linux+windows)/full
test all pass.
* Address review feedback on the audit backlog
Resolve CodeRabbit review comments on this PR:
- keyring isNotFound: match the specific not-found exit codes (macOS `security`
44 = errSecItemNotFound; `secret-tool` 1) instead of treating any non-zero
exit as "no entry", so a real failure (locked keychain, permissions) surfaces
instead of being masked as absent.
- swarm scheduler daily_at: recompute each fire as the next local HH:MM rather
than adding a fixed 24h, so a wall-clock daily schedule does not drift across
DST. Adds Daily/Hour/Minute to Schedule and a clock to the Scheduler.
- swarm scheduler: reject Add when the scheduler context is already canceled
(not just when closed); re-check cancellation after a tick so a tick+cancel
race can't spawn one extra member after Cancel/Close.
- swarm schedule_tool swarmInt: reject non-integer / non-finite JSON numbers
(max_runs=1.9 errors instead of silently truncating to 1).
- mcp migrateLegacy: resolve a relative LegacyPath to absolute before the
same-file guard so it can't be bypassed by a relative spelling.
- oauth keyring store: serialize the keyring read-modify-write with a
cross-process lock file (best-effort, beside the config dir) so concurrent
processes don't clobber each other's tokens; previously withLock was a no-op.
Adds tests for the non-not-found keyring error, the daily delay recompute, the
Add-after-cancel guard, and non-integer swarmInt.
* swarm: make nextDailyDelay roll the day, not add 24h (DST)
CodeRabbit follow-up: the run loop already recomputes a daily job's delay each
cycle, but nextDailyDelay itself rolled "passed today" to the next occurrence
with Add(24*time.Hour). On a DST changeover day (23h/25h) that fires an hour
early/late. Use time.Date(..., day+1, hour, minute, ...) so Go normalizes the
date and applies the correct local offset, holding the wall-clock HH:MM across
DST. Adds an America/New_York spring-forward test (skips if tzdata is absent).
|
||
|
KRATOS
|
9ddef0e957 |
feat(swarm): multi-agent swarm over the specialist sub-agent (#207)
* feat(swarm): multi-agent swarm over the specialist sub-agent Add internal/swarm so an orchestrator can spawn and coordinate MULTIPLE specialist members concurrently under one team, communicate via per-agent mailboxes, hand work off, and adopt orphaned tasks. Additive and opt-in: the single Task tool is unchanged; swarm tools only act when invoked. - Registry/Definition: agent roster (built-in teammate/subagent + user-defined). - Mailbox: per-team/per-agent JSON inbox, owner-only 0600, file-locked with stale-break, size-capped, path-confined, and malformed-fails-closed. - Coordinator: in-memory task registry with terminal-state guards and stable per-agent colors. - Team: bounded concurrent members per team with a FIFO overflow queue that drains one-per-slot. - Lifecycle: spawn / handoff / orphan-adoption with bounded relaunch on temporary failures; members run under the Swarm's base context so they outlive the spawn call. - Tools: swarm_spawn, swarm_send, swarm_inbox, swarm_status, swarm_handoff, swarm_collect. - Wiring: each member is launched through specialist.Executor and inherits the orchestrator's model + permission mode (never widened) and runs under the same sandbox/policy. Mailbox state lives under the workspace so writes fall within the sandbox write rules. Swarm lifetime is paired with the specialist runtime. Scheduling (cron/wakeup) and daemon-pool-backed members are deferred follow-ups. * swarm: address CodeRabbit review (7 findings) - coordinator: SetStatus rejects unknown TaskStatus values (fail closed) so a task can never enter a state Summarize/lifecycle don't reason about. - permission inheritance ENFORCED end-to-end (was resolved but discarded): thread PermissionMode through specialist.TaskRunOptions -> BuildArgs/ BuildResumeArgs. A headless specialist child now runs "--auto high" (unsafe) ONLY when the parent is unsafe or no mode is given (preserves the Task tool's behavior); any other parent mode yields non-unsafe "--auto low", so a member never gains more authority than its orchestrator. Also fixed the swarm permission vocabulary to the real agent runtime modes (ask/spec-draft/auto/ unsafe) instead of TUI display names, and the launcher substitutes "auto" for an empty mode so a member can't fall into the empty-means-high path. - mailbox: symlink-aware confinement (EvalSymlinks) rejects an inbox dir that resolves outside BaseDir; dirs tightened to 0700 even if pre-existing with broad perms; lock release is ownership-token-aware so a stale-broken writer can't delete a newer writer's lock (split-brain). - member: recover panics inside a member goroutine -> fails that member only, never crashes the orchestrator. - tools: collapse() truncates on rune boundaries (no invalid UTF-8). Tests added for every fix. Existing specialist behavior is unchanged (empty PermissionMode keeps --auto high). Gates: gofmt/vet/build (host+linux)/test -race/staticcheck(no new)/govulncheck(0)/deadcode(no new) all pass. * swarm: address review round 2 (Windows lock + non-blocking notes) Blocking (Vasanth): mailbox acquireLock now treats Windows ERROR_ACCESS_DENIED / ERROR_SHARING_VIOLATION as lock-contended (retry/wait) instead of a fatal error, via a platform-aware isLockContended (lock_windows.go / lock_other.go). A contended lock under Windows file-sharing semantics no longer fails a send/read spuriously. TestMailboxConcurrentSends raised to 200 goroutines with a 10s lock timeout and now asserts zero send failures (regression guard). Non-blocking notes addressed: - Removed third-party reference-project names from doc comments (clean-room + provider-neutral); the Go code is original. - Documented the inbox-file size bound's ×2 (pretty-print overhead) constant. - Documented Send's O(N) rewrite cost + when to switch to append-only. - permissionRank: spec-draft now ranks below ask (it never widens authority). - Documented watch's per-member-id retry binding (Member.ID == MemberSpec.ID). - handoff Safety.Reason now also mentions the inbox write. Gates: gofmt · vet · build (host+linux+windows) · test -race · staticcheck (no new) · govulncheck (0) · deadcode (no new) all pass. |
||
|
KRATOS
|
73eb8d3376 |
Solidify sandbox: harden command analysis + wire/clean dormant policy surfaces (#196)
* Harden sandbox command risk by wiring the AST analyzer The AST-based command analyzer (sandbox/analyzer.go) was built and tested but never reached at runtime — the live risk classifier used only the regex detectors. Wire AnalyzeCommand into classifyWithScope as a second opinion. It walks the parsed shell tree, so it catches destructive/network programs the regexes miss (shred, fdisk, parted; telnet, ftp, sftp; and commands hidden behind sudo/env wrappers or a sh -c launcher) and flags an unparseable (obfuscated) script as elevated risk. Purely additive — it only ADDs risk categories, so a benign, parseable command classifies exactly as before, and a program name inside a quoted argument is not flagged (AST precision). Revives the dormant analyzer (sandbox unreachable funcs 13 -> 6). New tests cover the regex-missed destructive/network programs, nested launchers, the unparseable signal, the quoted-name no-false-positive case, and benign-stays-clean. Full suite + -race green. * Wire sandbox auto-allow-bash env opt-in; drop orphaned batch-autonomy Two dormant sandbox surfaces resolved: - WIRE: AutoAllowBashWhenSandboxed was enforced live (engine.go) but never set — no config field, and the ZERO_SANDBOX_AUTO_ALLOW_BASH env overlay (ApplyAutoAllowBashEnv) was unwired, so the documented opt-in did nothing. Wire it through the central policy builder (applyConfiguredSandboxPolicy, used by all engine constructions). Fail-safe: only turns ON when the env is truthy, only honored when the shell sandbox is actually active, default stays prompt, and an explicit config opt-in is preserved. Revives AutoAllowBashEnvEnabled + ApplyAutoAllowBashEnv; adds a policy-builder test. - DELETE: RequiredAutonomyForBatch/riskToAutonomy (batch.go + test) had no consumer — the agent loop enforces autonomy per tool call (loop.go), so a batch-ceiling helper was speculative and redundant. Wiring it would have meant fabricating a batch model. Sandbox now has no dormant funcs; the only remaining unreachable entries are the cross-platform seccomp pair (unixSocketBlockFilter is live on Linux; ApplyUnixSocketBlock is the intentional non-Linux no-op stub). * Assert risk level (and add wrapper cases) in AST analyzer tests Per review (Vasanth + CodeRabbit): the AST hardening tests checked only the risk category, not the escalation level, so a regression that downgraded destructive/network shell commands while preserving the category would slip through. Assert risk.Level too — Critical for destructive/network, High for unparseable — matching the pattern used throughout the file. Add sudo/env wrapper cases (sudo/env shred, sudo telnet) to cover effectiveProgram's wrapper unwrapping. |
||
|
KRATOS
|
e0dd67a4da |
Wire LSP diagnostics into self-correct (exec + TUI) (#191)
* Wire LSP diagnostics into headless --self-correct newExecSelfCorrector now backs the --self-correct loop with both halves: the workspace test plan AND an lsp.Manager-based diagnostics checker (IncludeLSP=true). The manager is lazy (Manager.Check degrades a missing language server to nil,nil), and a deferred cleanup shuts it down at run end, terminating any spawned sessions. This revives the internal/lsp client (44 -> 3 unreachable funcs); it was dead because the only consumer passed a nil checker with IncludeLSP=false. Add TestManagerCheckRealGopls: drives NewManager -> Check against a real gopls and asserts a real diagnostic; skips when gopls is absent, so it stays CI-safe. * Wire self-correct into the interactive TUI (on by default) runAgentWithOptions now builds a SelfCorrector for every turn, so post-edit verification (workspace test plan + LSP diagnostics) runs by default in the TUI. The spec-draft planning path is excluded, matching exec. A per-turn lsp.Manager is created and torn down when the run returns. Auto-fix vs report-only follows the active permission mode (unsafe->high, auto->medium, ask/other->low) via selfCorrectAutonomyForMode. * Keep TUI self-correct fast by default; gate the test half behind /selfcorrect The TUI default now runs only the LSP-diagnostics half of self-correct — cheap and scoped to the changed files — so it never adds the whole-repo test plan's latency to a turn (and a pre-existing failure can't hijack the agent). The project test plan is opt-in per session via a new /selfcorrect [on|off] command (alias /sc); IncludeTests reads m.selfCorrectTests, default false. exec keeps both halves. * Remove dead session.didClose from lsp session.didClose sent a textDocument/didClose notification but was never called (the manager only closes documents via Shutdown), so it was unreachable even from tests and flagged by staticcheck U1000. Removing it takes internal/lsp to 0 truly-dead funcs and 0 U1000 findings; the 2 remaining unreachable-from-main funcs (Available, URIToPath) are public, test-covered API. * Align /selfcorrect help with accepted arguments The usage/description advertised only [on|off] but the handler also accepts status, list, tests, full, and lsp. Surface the primary verbs (on|off|status) and clarify the on/off behavior so command discovery matches the handler (CodeRabbit). * Make /selfcorrect usage strings match the handler exactly Both the command definition and the invalid-argument error message now list the full accepted set [status|on|off|tests|full|lsp]; the redundant 'list' alias is dropped so docs and handler agree exactly. Addresses Vasanth's review (the error-message usage at session_controls.go was still [on|off]) and CodeRabbit's help-text finding. * Clarify /selfcorrect off semantics in help text Reword the description so 'off/lsp' reads as 'disable tests, LSP-only' rather than the ambiguous 'return to LSP-only', which could be misread as disabling the feature entirely (CodeRabbit). |
||
|
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
|
47c3cc8937 |
wedge stage 13: launch readiness (first-run + benchmark) (#186)
* stage 13: launch readiness (first-run + benchmark) First-run, model-agnostic-first: - provideronboarding: detect local OpenAI-compatible runtimes (Ollama/LM Studio) on their default ports and offer them with no API key required - provideronboarding: ClassifySetupProbe maps a live connectivity probe to a specific, fixable error class (bad key, wrong base URL, model not found, rate limit, config) instead of a stack trace; never panics - zero setup --verify: probe the provider after saving and report the fixable error on failure; print a concrete 'try this' headless example on success zero doctor hardening: - new sandbox.backend check (bubblewrap/sandbox-exec) with a platform-specific install remedy; warn (not fail) on policy-only fallback - new lsp.servers check listing missing language servers with per-binary install remedies; injectable GOOS/LookupExecutable for deterministic tests Self-correct surface + benchmark: - zero exec --self-correct wires the post-edit verify-and-correct loop via the project verifier; nil corrector when off keeps the agent loop byte-identical - perfbench task harness: reproducible Terminal-Bench-style runner over headless zero exec + stream-json; records model + commit + self-correct flag + date - zero-perf-bench tasks subcommand (with --dry-run) and a sample task set - docs/BENCHMARK.md: the number, the exact command, the model, and the with/without-self-correct methodology * stage 13: address review (verify-state, fixture paths, run_end exit, 404 classification) * stage 13: verifier inherits process environment (PATH/HOME/toolchain) |
||
|
KRATOS
|
6beb0b0908 |
wedge stage 09: finish plugin wiring (tools/hooks/skills) (#185)
* stage 09: finish plugin wiring (tools/hooks/skills)
Activate resolved plugin manifests so their declared tools, hooks, and
skills take effect at runtime instead of being parsed-but-unconsumed.
- internal/plugins/activate.go: Activate() registers each plugin tool into
the tools.Registry via a command-runner adapter (JSON args on stdin,
stdout/exit -> tools.Result, ${AGENT_PLUGIN_ROOT} expansion + env),
builds hooks.Definition values for plugin hooks, and exposes plugin
skill search roots. Permission maps allow/prompt/deny -> tools.Safety;
allow only survives load-time clamping when manifest auto-approval is
enabled, so mutating plugin tools are not auto-approved by default. A
plugin-aware skill tool merges the default skills dir with plugin roots
(recording duplicate names, never crashing). Malformed plugins/extensions
are skipped with a warning; activation order is deterministic.
- internal/cli: activatePlugins() invokes activation in both bootstrap
paths (interactive TUI in app.go and one-shot exec in exec.go), after
core/specialist/MCP registration. newHookDispatcherWithExtra folds plugin
hooks into the dispatcher's hook set. Fails open: a bad plugin warns and
is skipped, never wedging startup.
- internal/plugins/plugins.go: update the stale 'not yet registered' note.
* stage 09: normalize plugin command path separators for Windows
expandPluginRoot kept the manifest's forward slashes, so a ${AGENT_PLUGIN_ROOT}/bin/x command became a mixed-separator path on Windows (C:\...\001/bin/x), failing the path tests. Added expandPluginRootPath (filepath.FromSlash on the placeholder-expanded executable path only; args keep their slashes since they may be URLs/flags) and used it for the tool and hook command paths.
* stage 09: address plugin wiring review
- surface per-plugin load diagnostics on stderr (not just top-level errors)
- skip plugin tools whose name collides with an already-registered tool
- resolve manifest-relative plugin skill paths against the plugin dir
* stage 09: fall back to plugin path/id when manifest path unknown in load diagnostic
|
||
|
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]> |
||
|
gnanam1990
|
24e15e2138 |
providers/openai: forward reasoning effort to the chat completions request
zeroruntime.CompletionRequest had no effort field, so agent.Options.Reasoning Effort never reached the provider (cli/exec.go even noted it was 'not yet forwarded'). Add CompletionRequest.ReasoningEffort, thread it through every request the agent loop builds, and map it to OpenAI's reasoning_effort. The CLI gates the forwarded value against the resolved model: known non-reasoning models send nothing (matching the existing 'ignoring' advisory), known reasoning models send their effective level, and unknown custom endpoints forward the requested value as-is. Anthropic/Gemini mapping is handled separately. |
||
|
gnanam1990
|
32a6922916 |
agent: surface response truncation (finish_reason) to the user
Providers already normalized finish_reason into collected.FinishReason, but the agent loop never read it, so a final answer cut off at the output token cap (or by a content filter) was silently presented as complete. Carry the turn's finish reason into agent.Result, add Truncated()/TruncationNotice(), and emit a warning from the exec writer and a system row in the TUI when the response was truncated. |