mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 23:40:28 -10:00
feat/structural-code-query
214
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 KRATOS
|
cae0269e77 |
fix(tui): a copied NUL byte no longer panics the TUI on Windows (#876)
syscall.StringToUTF16 PANICS rather than returning an error when the string contains a NUL — documented standard-library behaviour — and github.com/atotto/clipboard's Windows path calls it directly. So one NUL anywhere in a copied transcript selection killed the whole program: "zero: tui error: program was killed: program experienced a panic", reported from a real Windows machine. The NUL reaches the clipboard because ansi.Strip removes escape SEQUENCES and leaves C0 bytes untouched, and the transcript carries them from two independent directions: tool output (a binary read, git's -z NUL-separated listings — which this repo itself parses in files_git_sweep.go) and Zero's own card protocol, whose row prefixes are literally "\x00command-card\x00" and "\x00plan-card\x00". Sanitised at the WRITE rather than at each source, because the sources are unbounded: any tool result a user can select from is one, and a fix per source is a fix the next tool re-breaks. The same pass protects the OSC52 fallback, where a stray control byte terminates the escape sequence early and silently corrupts the copy instead of crashing — which is likely why this went unnoticed off Windows, where the identical NUL produces a bad copy rather than a dead process. \n and \t survive: a multi-line selection is the normal case and both are legal clipboard content. Fixes #875 |
||
|
|
8e266797a8 |
perf: improve harness context efficiency (#838)
* Improve harness context efficiency
* Address harness efficiency review findings
* Make recaps idle and cancellable
Delay recap generation until the session has been inactive, orient it around the active goal and next task, cancel stale work on activity, and render the result ephemerally above the composer.
Also strengthen the formatter safety regression to exercise formatter-modified edit output.
* Track every replace-all edit span
Translate each replacement into its post-edit line range, including cumulative shifts from earlier replacements, so follow-up edits remain authorized for every model-supplied change.
* Make review regressions cross-platform
Verify replace-all tracking through the canonical edit tool path instead of a platform-sensitive temp path, and cover cancellation when recaps are disabled during generation.
* fix(tools): derive SeenWhole from the recorded ranges
SeenWhole was a flag set only by a SINGLE read covering the file, while
SeenRange judged coverage from the accumulated ranges. The two then
disagreed about identical reads: after reading lines 1-50 and 51-100 of a
100-line file, SeenRange(1,100) was true and SeenWhole was false.
That matters because write_file gates overwrites on SeenWhole. A file past
the read byte budget cannot be read whole in one call — the read is
truncated and RecordSeenRange is correctly skipped, since the model did not
see those lines — so no sequence of reads could ever set the flag, and the
overwrite was refused permanently with:
the intended change depends on content that has not been read exactly
in this session. Read the affected lines with read_file, then retry.
Advice that could not change the answer. edit_file was unaffected because it
gates on SeenRange, so the escape was to edit rather than rewrite, but a
model following the error text would loop.
SeenWhole is now computed from the ranges against the file's recorded line
count, so scoped reads that together cover a file make it writable and the
error text becomes actionable. The observation carries that line count, and
a read reporting a different one clears the accumulated ranges: they
describe a shape the file no longer has.
Coverage is judged by merging sorted ranges rather than walking every line
against every range, which the previous SeenRange did — same answer, and it
no longer scans line-by-line over a large file.
Co-Authored-By: Claude Opus 5 <[email protected]>
* test(tools): key the large-file test on the resolved path
read_file resolves the workspace root through EvalSymlinks before recording
anything, so the tracker is keyed on the resolved path. The test asserted
against the raw t.TempDir() path, which is the same string on Windows and a
different one on macOS, where the temp dir sits under /var — a symlink to
/private/var. The SeenWhole assertions would have looked up a key that never
existed: passing here, failing on macOS CI.
Also switches the line generator to fmt.Fprintf (staticcheck QF1012).
Both from CodeRabbit on #842.
Co-Authored-By: Claude Opus 5 <[email protected]>
* test(tools): actually switch the line generator to fmt.Fprintf
The previous commit's message claimed this was done. It was not: the edit
silently did not apply and I reported it fixed without re-reading the file,
so builder.WriteString(fmt.Sprintf(...)) survived at the head — caught by
@anandh8x, not by me.
Verified this time by running the linter rather than by reading the diff:
golangci-lint --enable-only unused,ineffassign,staticcheck ./internal/tools/
reports 0 issues.
* fix: keep exact-read workflows recoverable
* test: cover permission normalization wiring
* fix: commit file observations after output budgeting
* fix: defer file observations across retries
* test: normalize patch line endings
---------
Co-authored-by: Vasanthdev2004 <[email protected]>
Co-authored-by: Claude Opus 5 <[email protected]>
|
||
|
Anandan
|
588635487f |
feat(tui): replace /retitle with local /rename (#826)
* Improve session naming and resume picker * Address session rename review feedback |
||
|
Anandan
|
845df6c7d2 |
fix(tui): hide unavailable sidebar shortcut (#821)
* fix(tui): hide unavailable sidebar shortcut * test(tui): cover medium sidebar footer hint |
||
|
Anandan
|
15eb16f736 |
fix(tui): exclude permission waits from turn time (#820)
* fix(tui): exclude permission waits from turn time * fix(tui): exclude permission waits from TTFT |
||
|
Vasanth T
|
ec3b822505 |
fix(tui): paste clipboard images with Ctrl+V (#534) (#807)
* fix(tui): paste clipboard images with Ctrl+V Ctrl+V did nothing when the clipboard held a screenshot, while right-click paste attached it. The image plumbing was already there: routePaste probes the OS clipboard for an image whenever a paste arrives with empty text. The gap is that a clipboard holding only an image produces no bracketed paste at all, since the terminal has no text to send, so routePaste never runs and its probe never fires. Right-click reached the probe only because that path always delivers a clipboardReadMsg, empty or not. Bubble's own Ctrl+V binding stays disabled, so nothing else was listening for the key. Handle Ctrl+V by issuing the image probe directly. It reads image content only, never text, so bracketed paste remains the single text path and a Ctrl+V with text on the clipboard is unaffected. The probe yields no message when there is no image, keeping it a silent no-op, and it is gated on noBlockingModal so a permission prompt or picker still swallows the key. readClipboardImage becomes a var so the tests stub it instead of depending on the machine's real clipboard. Fixes #534 * fix(tui): also probe the clipboard on Cmd+V, and test the real key route Two things raised in review, both fair. The handler matched keyCtrl only, which is ModCtrl. macOS reports Command as ModSuper, as the rest of the TUI already models it, so Cmd+V never reached the image probe even though the change claimed to cover it. It now matches both, via a keySuper helper alongside the existing modifier helpers. Worth being precise about the limit: this only helps where the terminal delivers the key to the application. A terminal that handles Cmd+V itself pastes the clipboard TEXT and sends no key event at all, so an image-only clipboard still produces nothing for this to react to. That is a separate problem from the one being fixed here. The tests also did not exercise the route they were named for. One asserted that Update returned some non-nil command and then ran a freshly built probe; the other never called Update at all. Both now stub the reader BEFORE Update and run the command Update actually returned. That matters more than it sounds. Making the branch return a different non-nil command leaves the old assertion passing and fails the new one, which is exactly the hole: the old test proved a command was issued, not that it was the image probe. Removing the route entirely, or reverting to ModCtrl only, also fails now. * fix(imageinput): report an unreadable clipboard image instead of silence On macOS the clipboard reader shells to pngpaste or a PyObjC one-liner, and neither ships with the OS: pngpaste is Homebrew-only and Apple dropped the bundled PyObjC. A stock Mac therefore fails every time, and the failure was returned as (nil, nil), which the caller reads as "no image on the clipboard" and treats as a silent no-op. Pressing Ctrl+V produced nothing at all: no image, no error, no hint that anything was missing. The darwin path already knows the difference. It probes with `clipboard info` first and only reaches the extraction step once the clipboard is confirmed to hold an image, so a failure there is not absence, it is "there is an image and nothing here can read it". That now returns ErrClipboardImageUnreadable, which the existing error path turns into a transcript notice naming what to install. The defect predates this branch, but this branch is what makes the path reachable from Ctrl+V, so shipping without it would close #534 with a feature that is inert on one of the three platforms. Raised by gnanam, who measured it on a real Mac rather than inferring it. Deliberately narrow. The Linux arm cannot tell "no tool" from "no image" without probing separately, and Windows uses PowerShell, which is always present, so neither is touched. The test drives the whole path: stubbed reader returns the error, Ctrl+V issues the probe, and the assertion is that the failure reaches the transcript as an error row rather than stopping earlier. Swallowing the error again makes it fail. * test(imageinput): cover the darwin unreadable-clipboard decision itself The test added with the fix asserted around the change rather than on it. It substituted ReadClipboardImage with a stub returning the error, which proves the TUI surfaces an error handed to it, but never reaches readClipboardImageDarwin, where the decision lives. Reverting both production return sites to nil,nil left every package green, which is the definition of a test that is not testing the change. Caught by gnanam. readClipboardImageDarwin shells out twice and had no seam, so the two steps are now package-level vars, the same shape as the readClipboardImage var this change already relied on. That makes the case that matters reachable: the info probe succeeds and reports an image, and the extractor then fails. It cannot be arranged from outside the function, because from there both steps are one opaque call. Five cases, and the negative ones carry as much weight as the positive. An image with no working extractor, and an extractor that exits 0 with no bytes, must both be actionable. A clipboard offering no image, and a failing info probe, must both stay silent, because turning those into a notice would make every stray Ctrl+V nag the user. The extractor must not run at all in either silent case. Repeating gnanam's experiment now fails: reverting both return sites to nil,nil fails the two actionable cases and leaves the silent ones passing, which is exactly the discrimination the fix is supposed to add. |
||
|
Anandan
|
5d1869e7bc |
feat: add persistent session goals (#803)
* feat: add persistent session goals * Fix goal run isolation and guards * Strengthen loop goal isolation test * Cover goal edit state transitions * fix(goal): bound automatic continuations Persist a provider-independent continuation limit, require approval for model-created goals, and keep resume, BTW, and loop runs from starting or charging unrelated goal work. Also reject trailing lifecycle arguments and bound persisted objective length. * fix(goal): resume continuations after btw |
||
|
Anandan
|
518d2e53ba |
fix(windows): use PowerShell for command execution (#804)
* fix(windows): prefer PowerShell for command execution Detect PowerShell 7 or Windows PowerShell before falling back to cmd.exe, and share the selected host shell across agent tools and the interactive shell escape. Configure UTF-8 output, preserve restricted-sandbox MSYS safeguards, and add Windows coverage for pipelines, workspace writes, and interactive stdin sessions. * test(windows): align shell coverage with PowerShell Make Windows command fixtures and guidance assertions follow the selected host shell, preserve cmd fallback coverage, and use a genuine MSYS executable when PowerShell aliases are available. Remove the unsupported non-TTY PowerShell stdin assertion; Windows interactive stdin requires a separate PTY transport. * fix(sandbox): classify Windows executable suffixes Normalize .exe, .cmd, .bat, and .com command names before safety and network lookup so PowerShell invocations such as curl.cmd receive the same network permission handling as curl. Add analyzer and engine-level regressions for the Windows PowerShell command shape observed in CI. * fix(sandbox): normalize absolute Windows command paths Extract drive-letter and UNC basenames before removing shell escape characters so fully qualified Windows executables continue matching canonical safety and network classifications. Cover both drive and UNC curl paths with analyzer regressions. * fix(sandbox): normalize relative Windows paths Recognize drive-relative, explicit relative, and suffixed backslash executable paths before generic shell escape removal so Windows commands continue to match canonical sandbox risk entries. * fix(windows): harden PowerShell execution * fix(sandbox): classify PowerShell removal aliases |
||
|
|
583e6530bd |
fix(lint): resolve static analysis baseline (#769)
* fix(lint): resolve static analysis baseline * fix(lint): clear linux staticcheck findings Amp-Thread-ID: https://ampcode.com/threads/T-019fa018-b2ee-7442-8a31-80b2cf9dd270 Co-authored-by: Pierre Bruno <[email protected]> --------- Co-authored-by: Amp <[email protected]> |
||
|
Anandan
|
a4a805a6c8 |
fix(tui): reset quiet clock for new runs (#799)
New turns inherited lastStreamActivity from the prior turn, so a fresh request could immediately look stalled. Initialize stream activity from the same timestamp as turnStartedAt and cover the stale-clock case with a deterministic regression test. |
||
|
Anandan
|
9fe02ae52e | fix(sandbox): clarify scoped permission prompts (#798) | ||
|
euxaristiaandClaude Sonnet 5
|
97ef90d27a |
chore: remove code unreachable on every supported platform (#706)
* chore: remove code unreachable on every supported platform The Security and code health job's advisory deadcode step reports 152 unreachable functions on main. Running the same check under GOOS=linux, windows, and darwin shows 13 of those are false positives whose callers live in windows build-tagged files, so only functions unreachable on all three platforms are touched. Functions with no references anywhere are deleted, including the internal/reasoning package, which nothing imports. Functions that only tests reference move into per-package export_test.go files so tests keep their seams while the production binary sheds the code. Test call sites of deleted convenience wrappers are ported to the live APIs (scoped tool constructors, tools.DeferredSource, FinishReason checks). Kept despite the report: cron.Store.Runs (only reader of runs.jsonl, which production writes), and agenteval.TraceEventKeys and MissingTraceEvents (used across packages by the trace contract test). * docs(config): correct ValidateFile's errors.As claim Message is a flattened string (.Error() already called on the parse error), not a wrapped error, so callers can't recover the concrete json error type via errors.As. Fix the comment to match. Co-Authored-By: Claude Sonnet 5 <[email protected]> * chore(trace,skills): drop obsolete Sink adapters and fix stale doc references The dead-code pass removed the Emit methods but left Sink, NDJSONSink, and TextSink exported with a contract nothing implements; remove the types with their contract. Also remove the stray blank line at EOF of trace.go flagged by gofmt, and update skills package docs and HOW_ZERO_WORKS.md to stop directing callers to DiscoveryRoots/Duplicates, which are now test seams. Co-Authored-By: Claude Fable 5 <[email protected]> * chore: clean up remaining review nits on dead code removal Replace a pointless fmt.Errorf(...%w...).Error() with fmt.Sprintf using %v, since the wrapped error was discarded immediately anyway. Remove a comment block that was duplicated verbatim. * docs(skills): drop remaining references to test-only List/Get Load and MergedSkills comments still named List/Get as production entry points after they moved to export_test.go. Point callers at Load and ListFromRoots instead. * chore: remove now-unreferenced export_test.go seams execSessionManager.len() and likelySandboxDenied/sandboxDenialKind had no remaining callers after the sandbox unify-command-execution rebase; the production code they wrapped for tests no longer exists. --------- Co-authored-by: Claude Sonnet 5 <[email protected]> |
||
|
PierrunoYTandClaude Sonnet 5
|
5c58256abc |
fix(config): kill provider-command process tree via job object on Windows (#690)
* fix(config): kill provider-command process tree via job object on Windows taskkill /T walks the process tree by parent PID in user space and can miss descendants, leaving the sleeping child holding the inherited stdout/stderr pipes so cmd.Wait() blocked until natural completion (~10s instead of the 5s timeout). Assign the started command to a job object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE and terminate the job on timeout so the kernel kills the whole tree atomically; taskkill remains as a fallback. Also set cmd.WaitDelay so Wait can never hang on pipes held by an escaped orphan, and add a regression assertion that the sleeping child is actually gone after the timeout returns. Fixes #683 Co-Authored-By: Claude Fable 5 <[email protected]> * fix(config): harden provider-command tree termination Address code review on PR #690: terminate the process tree when cmd.Wait() returns exec.ErrWaitDelay (a background descendant kept inherited pipes open, previously left running); start the Windows process suspended and assign it to the job object before resuming its main thread, closing the window where a fast child could escape the job; drop JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE so a successful command's detached helpers survive Close() instead of being reaped; escape single quotes in generated PID-file paths; and correct a misnamed Windows exit-code constant in a comment. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(config): avoid gofmt smart-quote rewrite in doc comment gofmt's doc-comment formatter rewrites a trailing '' into a curly closing quote, which CI's gofmt -l check flags as unformatted. Reword to avoid the pattern instead of embedding a raw quote pair. * fix(config): terminate leaked descendants on nonzero exit, avoid PID reuse Terminate the process tree on any error returned from cmd.Wait, not only exec.ErrWaitDelay. Go only returns ErrWaitDelay when the shell itself exited successfully; a nonzero exit (e.g. sleep 600 & exit 7) yields the bare *ExitError instead, so a detached background child that held the inherited pipes open was never terminated on that path. Also stop falling through to the taskkill/PID fallback once a job handle successfully contained the tree: by the time Terminate runs on the ErrWaitDelay path, cmd.Wait has already reaped the root process, so its PID may have been reused by an unrelated process, and taskkill /T /PID against a reused PID would force-kill the wrong tree. Addresses PR #690 review feedback. * fix(config): surface suspended-thread resume failures instead of hanging resumeMainThread now reports whether it actually resumed at least one thread. If Toolhelp/OpenThread/ResumeThread all fail, the launched process would otherwise sit suspended forever, never exit on its own, and get misreported as a plain 5s provider-command timeout instead of the real resume failure. attachCommandProcess now terminates the process immediately in that case. Also documents the known, non-regressive limitation that the taskkill fallback (used only when job creation/assignment itself fails) can still miss a descendant that has reparented away from the tree it walks by PID. Addresses remaining PR #690 review feedback; PR description corrected separately to drop the outdated JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE claim. * fix(config): guard taskkill with retained process identity * fix(config): retain POSIX process group identity * test(tui): match provider wizard shortcut * test(tui): pin credential-step env for deterministic footer assertion The credential step's footer only shows the /→ shortcut once a key is present, whether typed or inherited from the provider's AuthEnvVars. The previous commit changed the assertion to "Enter/→ continue" without accounting for that, so the test only passed by coincidence when ANTHROPIC_API_KEY happened to be set in the environment — failing deterministically on a clean CI runner (as smoke reported on all three OSes). Pin the env var empty and assert the no-key footer text so the test is correct regardless of ambient environment. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(config): preserve provider process attributes * test(config): keep SysProcAttr coverage portable --------- Co-authored-by: Claude Fable 5 <[email protected]> |
||
|
euxaristiaandClaude Fable 5
|
5bb1bc11c3 |
fix(tui): fix rendering corruption over multipass + Windows Terminal (#709)
* fix(tui): disable bubbletea hard scroll optimization unconditionally Vendors a patched charm.land/bubbletea/v2 (via a local go.mod replace) with hard scroll optimization (using terminal scroll-region sequences to shift existing lines instead of repainting) disabled unconditionally, instead of only on GOOS=="windows". That GOOS check only catches a Windows terminal driving the program directly. It misses a Linux process running inside a multipass VM whose output is ultimately displayed by Windows Terminal over a remote shell, which corrupts rendering badly (garbled/overlapping text, worse on resize) the same way the check exists to prevent. There's no reliable way to detect that remote hop from inside the VM, so scroll optimization is now off in all cases. * fix(tui): force redraw on streamed newlines to clear stale caret The streaming caret (appendStreamingCursor) is appended to whichever visual line is currently last. Over a remote terminal chain (multipass VM output displayed by Windows Terminal), the renderer's incremental cell diff sometimes fails to clear the caret's old cell when a newline moves it to a new line, leaving ghost carets behind. Fire tea.ClearScreen when a streamed delta contains a newline: that's exactly the moment the caret moves and the risk exists, so it forces a full redraw right then instead of leaving it to the incremental diff, without doing so on every animation tick. * chore(tui): trim upstream repo metadata from vendored bubbletea fork The vendored bubbletea-v2-patched module is ~99% byte-identical to upstream charm.land/bubbletea/v2 v2.0.8; only cursed_renderer.go differs (one line disabling hard scroll optimization). The .github workflows, README.md, UPGRADE_GUIDE_V2.md, Taskfile.yaml and CI config files are upstream repo metadata that serve no purpose inside this repo, so remove them to slim the PR. The Go source and testdata golden files are kept because Go's replace directive requires the full module to be present and buildable. Refs #709 * fix(tui): address review round on the rendering workaround and vendored fork Fork correctness fixes (all flagged by review): - key.go: KeyMediaRecord repeated the previous const expression and decoded record presses as previous-track; map it to uv.KeyMediaRecord. - mouse.go: MouseButton11 likewise collapsed onto MouseButton10. - termios_unix.go: BSDLY/BS0 are output-delay bits; read them from Oflag instead of Lflag so useBackspace is set correctly. - cursed_renderer.go: count wrapped rows with (lineWidth-1)/w so an exact terminal-width multiple no longer inserts a blank line; compare colors by normalized RGBA via a nil-safe helper instead of interface equality (which panics on non-comparable implementations); and make clearScreen invalidate lastView so the queued erase flushes with the next frame instead of deferring until an unrelated view change — the reordering that could leave the ghost caret in place. - exec_test.go: implement reset() on the spy renderer and inject it before Run; assert exec preserves renderer state (releaseTerminal with reset=false). - logging_test.go: restore the default logger's output/prefix/flags and fail fast on setup errors. - tea_test.go: tie the Quit-pumping goroutine to a channel closed when Run returns instead of leaking it until process exit. TUI scoping (review P2): the forced redraw on streamed newlines is now rate-limited to ~10/s so heavy streaming output no longer turns every newline into a full-screen repaint, and ZERO_NO_STREAM_CLEAR=1 opts a correctly-rendering terminal back into the fast incremental path. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(tui): guarantee deferred stream-clear redraw and other review fixes A throttled stream-clear could drop the redraw entirely when a newline arrived in the last delta of a turn, leaving a ghost caret on screen. Add a one-shot deferred flush timer plus a flush on stream end. Fix a stale comment and a test that leaked global logger state in the vendored bubbletea fork. * fix(tui): use m.now for stream-clear throttle and harden tests Drive stream-clear rate limiting through the mockable clock and a shared streamClearThrottle constant so both the agentTextMsg and flush paths stay in sync. Add an explicit two-newline-within-window regression and detect ClearScreen by function pointer so tests do not evaluate Tick side effects. * fix(tui): relocate bubbletea fork out of third_party Move the maintained bubbletea/v2 hard-scroll patch from third_party/bubbletea-v2-patched to patches/bubbletea-v2 and update the root go.mod replace. third_party/ is immutable vendored content per AGENTS.md; a maintained fork belongs outside that tree. * fix(ci,docs): run vendored bubbletea fork tests and document unsupported go install path patches/bubbletea-v2 is its own Go module so `go test ./...` from the repo root never touched its 7 test files. Add a CI step (and a make test-patches target for local use) that runs the fork's own suite across the smoke matrix. The root go.mod's local `replace charm.land/bubbletea/v2 => ./patches/bubbletea-v2` also breaks `go install github.com/Gitlawb/zero/cmd/zero@<version>`, since a versioned install can't resolve a replace pointing at a filesystem path. Document that path as unsupported in README.md and docs/INSTALL.md and point at git clone + go build instead, per jatmn's review findings on PR #709. * ci: run vendored bubbletea fork tests with -race CodeRabbit review on this PR. The fork exists specifically to patch a concurrency bug (charmbracelet/bubbletea#1690), so its regression test needs the race detector to actually catch a reintroduced race instead of just passing on a benign-looking run. * fix(tui): drop vendored bubbletea fork; keep stream-clear workaround Maintainers asked not to ship an in-tree bubbletea replace: it breaks versioned go install and is a large maintenance surface for a narrow multipass/Windows Terminal scroll bug. Remove patches/bubbletea-v2 and the replace directive; keep the model.go full-redraw-on-streamed-newline workaround (ZERO_NO_STREAM_CLEAR to opt out). Restore normal install docs. Refs Gitlawb/zero#709 --------- Co-authored-by: Claude Fable 5 <[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
|
3967d49d64 |
feat(tui): permission prompt takes free-text feedback inline (#780)
* feat(tui): let the permission prompt take free-text feedback inline "No, and tell Zero what to do differently" now opens an inline input on the permission card instead of silently cancelling: type an instruction, Enter sends it, Esc returns to the option list. The text is delivered as a Deny decision whose Reason is the typed instruction, so the agent surfaces it via deniedPermissionResult as the tool result and the model adjusts in the SAME turn — where the old cancel returned an error and aborted the run. Empty input + Enter falls back to a plain cancel (the prior meaning). No agent-side change: PermissionDecision.Reason already carries free text to the model. The UI mirrors the ask_user "type your own answer" pattern — the feedback field reuses the shared composer input and the card renders it in the transcript body. Also: a focused permission prompt now suppresses the composer (and idle hints/plan panel), keeping only the status line, exactly like the ask_user modal. Its keys are inert anyway, so a "describe a task for zero…" box below the card was misleading — and in feedback mode the shared input would otherwise echo in two places. Tests cover open / submit-as-Deny-with-text / empty-cancels / Esc-returns, the composer suppression in both modes, and that the feedback text renders exactly once. * fix(tui): preserve the composer draft across permission feedback (#780 review) anand + CodeRabbit (Major): entering feedback mode cleared the shared composer input via SetValue(""), and neither submit nor cancel restored it — so a user with a half-typed or queued next-turn message who chose "tell Zero what to do differently" lost that draft. Snapshot the composer value into pendingPermissionPrompt.savedDraft when feedback mode opens, and restore it on BOTH exits (submit and Esc-cancel). Only the feedback text is consumed, delivered via the decision Reason. Adds regression tests for both paths; mutation-verified against the SetValue("") regression. * fix(tui): guard permission-feedback backspace and clicks (#780 review) Vasanthdev2004 review, all on the permission path: 1. Real bug: Backspace on an empty feedback field dropped a staged attachment. The keyBackspace case runs before the typing branch and, on an empty composer (feedback mode clears it), fell to removeLastAttachment — silently dropping a staged image/doc that savedDraft does not restore. Route Backspace to the shared input while typing so it is a plain edit of the feedback text. 2. Defence-in-depth: the mouse click resolver in handleTranscriptSelectionMouse resolved a clicked permission option (Allow included) with no typing check. It is safe today only because renderFocusedPermissionPrompt returns nil clickable offsets in feedback mode — a single early-return in a function other PRs also edit. Add an explicit !typing guard on the click path so the safety is stated, not emergent. 3. Test hardening (also raised by CodeRabbit): assert len(got) == 1 before indexing got[0] in the draft-preservation test, so a regression fails cleanly instead of panicking. Tests: TestPermissionFeedbackBackspaceKeepsStagedAttachment (mutation-verified against the missing guard) and TestPermissionFeedbackRendersNoClickableOptionsWhileTyping. --------- Co-authored-by: KRATOS <[email protected]> |
||
|
KRATOSandKRATOS
|
722bb31216 |
fix(tui): stop the permission card clashing on cool themes (#778)
The focused permission prompt was the only prompt card in the TUI that used
brand/warning colours for its interior, and both read badly on cool themes such
as dracula:
1. The selected option ("Yes, proceed") was filled with zeroTheme.badge — the
brand chip meant for short labels (" 0 ", " ASK ", " SPEC REVIEW "), a
full-brightness accent fill. Every other selectable list in the TUI (model
picker, provider list, / palette) highlights its selected row with onSel/selBg
instead. The permission card was the outlier, painting a saturated accent slab
across the row and skipping the card tint every other line composed onto.
2. The card BODY was tinted with permBg, an amber-family wash intended to signal
a warning surface. It works on the warm default palette but renders as a
brown-yellow slab over cool themes, and it made the permission card the only
prompt whose body carries a fill — ask_user, spec review and plan all use a
transparent body (lipgloss.NewStyle()).
Fix both to match the rest of the TUI: the selected row uses onSel(ink) like
every other picker, and the body drops its permBg fill for a transparent one.
Warning identity is unchanged — the amber PERMISSION badge and the amber-mixed
border still mark it as a permission gate; only the clashing interior fills go.
TestFocusedPermissionSelectedRowUsesSelectionTintNotBrandChip asserts against
palette entries (not hardcoded hex, so it survives a theme retune): the selected
row carries selBg and not the brand accent, the body carries no permBg wash, and
the PERMISSION badge keeps its amber fill so the card still reads as a warning.
Co-authored-by: KRATOS <[email protected]>
|
||
|
KRATOSandKRATOS
|
b30c3971b4 |
fix(tui): model rows labelled by id when the description is prose; keep the sidebar under the / palette (#775)
* fix(tui): label model rows by id when the description is prose
The model picker (provider wizard AND onboarding, which share displayLabel)
labelled each row with the provider's Description, falling back to the ID only
when the description was "generic". That is right when Description holds a short
display name -- "Grok 4.3" for x-ai/grok-4.3, which is what the field is for and
why the row prefers it over an ugly raw ID.
Providers do not all honour that. Aggregators commonly return a prose blurb
instead ("Fast DeepSeek model for efficient chat, coding help, and agent
loops"), which fails as a label twice: it overflows the row, and because such
blurbs are written per model FAMILY rather than per model, several DIFFERENT
models collapse to byte-identical rows. Observed in a live picker with two
identical "Open-weight GPT model for self-hosted reasoning..." rows and two
identical "Kimi multimodal agent model..." rows -- the distinguishing ID was
visible only in the detail line of whichever row happened to be highlighted, so
the list could not be used to pick between them.
providerWizardGenericModelDescription cannot catch these: it rejects vague
descriptions ("live model", "catalog default"), whereas these are specific, just
shared and sentence-shaped. So gate on shape instead -- a display name is short
and few-worded; anything longer is prose and the ID (unique by construction)
makes the better label. Friendly names are still preferred where providers
supply them, so the existing behaviour is unchanged for well-behaved catalogs.
* fix(tui): keep the sidebar open under the / command palette
sidebarAvailable grouped suggestionsActive() with the full-screen overlays
(setup, wizards, pickers), whose stated rationale is that they "take over the
chat column and render at full width". That is not true of the `/` palette:
suggestionOverlay draws it through centerRenderedBlock at
minInt(width, suggestionPaletteMaxWidth), a centred box capped at 76 cells
floating over the chat column.
Treating it as full-screen collapsed the whole layout on every `/`: the sidebar
disappeared, the plan panel dropped out of it and re-rendered inline above the
composer, and the transcript re-wrapped to full width -- a jarring reflow for a
popup, and worst mid-run with a live plan on screen.
It cannot overlap the sidebar either: twoColumnTranscriptView renders the
overlay with width = chatColumnWidth, so the palette is capped at
min(chatColumn, 76) and centred inside the chat column. Mouse hit-testing is
unaffected because sidebarLineAtMouse carries its own suggestionsActive() guard,
so clicks still reach the palette rather than the sidebar rows beneath it.
The genuinely full-width overlays keep suppressing the sidebar, pinned by the
new test alongside the palette case.
---------
Co-authored-by: KRATOS <[email protected]>
|
||
|
KRATOSandKRATOS
|
8c6d3022bd |
feat(tui): add /undo as an alias for /rewind (#698) (#747)
* feat(tui): add /undo as an alias for /rewind (#698) /undo was unregistered and fell through to commandUnknown. Add it to the /rewind command's aliases (the same mechanism /quit, /find, /mcp-status use), so it dispatches to the existing rewind handler with args intact. * test(tui): cover the /undo numeric-sequence form (#698 review) Add a parseCommand("/undo 123") assertion so both the latest and numeric <sequence> forms of the /rewind contract are exercised through the alias. --------- Co-authored-by: KRATOS <[email protected]> |
||
|
PierrunoYT
|
4cbd144d11 |
fix(providers): stop "provider not found" for env-derived profiles (#716)
* fix(providers): stop "provider not found" for env-derived profiles * fix(providers): address env profile review feedback * fix(providers): keep env JSON response schemas stable |
||
|
PierrunoYT
|
d74ceb1127 |
fix(tui): cache settled alt-screen transcript (#647)
* fix(tui): cache settled alt-screen transcript * fix(tui): invalidate settled alt-screen cache on in-place status row updates setDoctorStatusRow, setSandboxSetupStatusRow, and setCompactStatusRow all mutate an already-flushed transcript row in place (m.transcript[i] = row) without invalidating the alt-screen settled cache. Since none of frontier, width, or row count change on such an update, settleTranscript's rebuild guard never fires and the View fast path serves the stale cached snapshot - e.g. the doctor connectivity spinner freezes on its first frame. Zero altScreenSettledWidth on the in-place path, mirroring the existing pattern in toggleTranscriptRow, so the next settle rebuilds the cache. Addresses PR #647 review feedback from Vasanthdev2004. * fix(tui): invalidate settled transcript cache * fix(tui): invalidate settled file selection cards |
||
|
Anandan
|
2e267bdbee |
feat(tui): add isolated /btw conversations (#748)
* feat(tui): add isolated btw conversations Add /btw with inline-question support, isolated non-resumable session forks, background main-run routing, and safe return behavior. Refs #637 * test(tui): cover btw return controls Preserve drafts when Ctrl+C is pressed during an active BTW run and cover returning to the parent through the documented /btw toggle. * Address BTW isolation review feedback * Reject explicit resumes of side sessions * Harden BTW isolation lifecycle |
||
|
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. |
||
|
|
6b9c2f0c08 |
feat(aimlapi): AI/ML API provider with guided onboarding (top-up + key issuance) (#655)
* feat(aimlapi): AI/ML API guided onboarding (top-up + key issuance) Add the AI/ML API provider integration to the zero CLI: a guided TUI onboarding sub-flow that takes an existing key or an email top-up and writes the issued key into the provider profile, attributed to the Gitlawb rebate partner. - internal/aimlapi: OpenAI-compatible client, partner-checkout top-up, key validation, streaming, and endpoint config (Paths A/B/C). - providercatalog / providermodelcatalog: register the aimlapi provider with default headers and models. - tui: aimlapi onboarding screen wired into the provider wizard and onboarding flow. Default endpoints point at production (auth/app/api/pay/aimlapi.com); each is overridable via AIMLAPI_*_URL env vars. Co-Authored-By: Claude Opus 4.8 <[email protected]> * fix(aimlapi): address review findings in onboarding flow - topup: use errors.As for the 5xx retry check so a wrapped APIError still retries instead of aborting a paid checkout. - onboarding: a Path-A top-up on an email with no account now lands on a single coherent "key saved, balance not topped up" screen instead of a green "ready" plus a red error. - onboarding: an empty amount field is rejected instead of silently falling through to the $25 default. - collapse the two identical top-up prompt constants into MsgTopUpPrompt and drop the dead amountPrompt() branch. - reuse stdlib maps.Clone instead of a third bespoke header-copy helper. Co-Authored-By: Claude Opus 4.8 <[email protected]> * fix(aimlapi): retry transient poll errors and sharpen amount validation * fix(aimlapi): retry transient poll errors and surface real amount errors * docs(aimlapi): document exported client API; drop out-of-scope gitignore entry Addresses the two CodeRabbit PR checks on #655: - Add doc comments to the exported aimlapi client API (Client, APIError, the session/checkout/onboarding types and their methods) and ParseAmountUSD, to satisfy the docstring-coverage check. - Remove the unrelated /zero.exe~ .gitignore entry that an earlier review-fix commit added; it is out of scope for the provider work, so the PR no longer touches .gitignore. Co-Authored-By: Claude Opus 4.8 <[email protected]> * fix(aimlapi): render aimlapi row like other recommended providers * fix(aimlapi): resolve onboarding review findings (Path A funding, base URL, request cancellation) * fix(aimlapi): validate pasted API keys * test(aimlapi): clarify pasted key validation * test(aimlapi): cover key-balance low-balance transition * fix(aimlapi): reject non-finite top-up amounts * fix(cli): scope and canonicalize provider headers * fix(tui): distinguish invalid verification codes * feat(aimlapi): resume account-bound top-ups safely * feat(tui): offer top-up for existing AIMLAPI keys * feat(tui): reuse AIMLAPI env key on first run * feat(tui): reuse configured AIMLAPI profiles * test(aimlapi): trim redundant flow coverage * fix(tui): preserve existing AIMLAPI profile metadata * fix(tui): clone AIMLAPI headers in env fallback * fix(aimlapi): address remaining review findings * fix(aimlapi): harden onboarding lifecycle * fix(aimlapi): preserve validated checkout context * fix(aimlapi): harden checkout retry state * fix(tui): clarify AIMLAPI configured choices * fix(aimlapi): make checkout retries idempotent * fix(aimlapi): validate checkout responses * fix(providers): reconcile aimlapi catalog after rebase --------- Co-authored-by: Lookoff123 <[email protected]> Co-authored-by: Claude Opus 4.8 <[email protected]> |
||
|
Leonardo Faoro
|
7f669f4550 |
feat(tui): Ctrl+X leader chords and emacs menu navigation (#699)
* feat(tui): Ctrl+X leader chords and emacs menu navigation Add Ctrl+X leader shortcuts for common slash commands (with Ctrl+X ? chord map), and Ctrl+P/N previous/next selection in modals and pickers. * fix(tui): drop Ctrl+X e /edit leader chord /edit replaces the composer and would discard an in-progress draft; leave it unbound so leader shortcuts stay draft-preserving. * fix(tui): consume idle Ctrl+N as a no-op Reserve Ctrl+N for emacs menu navigation so it never falls through to remapped global bindings when no selection surface is open. * docs: drop manual CHANGELOG entries for TUI shortcuts |
||
|
euxaristia
|
2b42cd567b |
fix(tui): stop the composer cursor blinking while typing or unfocused (#672)
* fix(tui): stop the composer cursor blinking while typing or unfocused The composer's cursor blinked on a fixed timer unconditionally, including mid-keystroke, which made typing feel janky since the cursor's job is to mark the current position. Gate the existing blink tick on two states instead: force solid while a keystroke landed within the last 500ms (reusing the existing lastCharTime tracker), and hide it entirely while the terminal window is unfocused (new terminalFocused field, driven by tea.FocusMsg/ BlurMsg). Idle and focused keeps the prior blink behavior unchanged. Also decouples view.ReportFocus from the notifier (it was previously gated on m.notifier != nil, which is effectively always true in production but left cursor-focus behavior fragile if that ever changes) so focus reporting is requested unconditionally. * fix(tui): apply caret visibility synchronously and in the argument-hint line - A keypress, focus, or blur now updates composerCursorVisible in its own message handler instead of waiting for the next 530ms composerBlinkMsg tick, which could render a typed character caret-less, leave a visible caret in an unfocused terminal, or keep it hidden after a refocus. - commandArgumentHintComposerLine takes the caret visibility state instead of painting its cursor cell unconditionally, so the required-argument hint path follows the same focus and blink contract as the main composer renderer. - New tests assert the state immediately after each message, with no blink tick in between, and cover the argument-hint render in both states. * fix(tui): apply the caret state on composer paste A paste is the same immediate-input transition as a keypress: routePaste now refreshes lastCharTime and sets the caret visible for both the bracketed tea.PasteMsg and right-click clipboardReadMsg paths, so a paste right after the blink phase hid the caret renders solid immediately instead of waiting for a tick that would toggle off a stale idle state. |
||
|
euxaristia
|
665bd4b3d0 |
WIP: feat(tui): TUI theme presets (Dune and Neon) (#634)
* draft(tui): add theme interface and Claude/Codex presets proposal Introduces the Theme interface alongside the initial specifications and styling models for the Claude (card-based, warm) and Codex (high-density, split-screen cyberpunk) layout presets. Detailed design plans are documented in docs/THEMES.md. * fix(tui): implement Claude/Codex themes on the existing palette system theme.go had replaced the tuiTheme/buildTheme/palette architecture every renderer depends on with a standalone Theme interface using the wrong lipgloss import, breaking the build and orphaning the existing theme registry, /theme command, and picker. Revert theme.go to the working implementation and add claude and codex as real palette entries in theme_palettes.go, following the file's own extension pattern. Both pass the WCAG AA contrast and gray-ramp invariants asserted across the whole registry, and are picked up automatically by --theme, ZERO_THEME, /theme, and the picker. Rewrite docs/THEMES.md to describe the actual palette-registry architecture instead of the removed interface. * fix(tui): rename Claude/Codex to neutral Dune/Neon, ensure WCAG AA contrast compliance, and add tests * style(tui): gofmt theme_select_test.go (drop trailing whitespace) The ubuntu smoke run fails on gofmt -l; this clears the only unformatted file in the draft so CI goes green. * test(tui): cover the CLI/env resolution path for the dune and neon presets TestNewThemePresetsWired only checked lookupTheme and the IsDark grouping. Add coverage for the actual --theme/ZERO_THEME precedence path (resolveThemeMode) and confirm applyTheme swaps zeroTheme to each preset's own palette rather than a built-in fallback. * fix(tui): raise Dune/Neon palette contrast and fix theme docs Darken dune's accent, blue, and red tokens so the selected-row caret, local-model dot, and diff deletion text clear WCAG AA against selBg and delBg, including after ANSI-256 downsampling. Also correct THEMES.md's description of bare /theme (it opens a live-preview picker, not a list), and add the dune/neon names to the ZERO_THEME/--theme tables in README.md and README_ZH.md. * fix(tui): keep Dune and Neon readable after ANSI-256 downsampling - Dune's accent becomes #724028: still AA on selBg and panel in truecolor (5.46:1 and 7.03:1), and its xterm-256 quantization (#444444) now clears AA on quantized selBg at 6.47:1 where the previous #7c4712 quantized to #875f00 at 3.81:1. Blue already survived quantization at 4.67:1. - Neon's diff bands move to hexes that quantize to distinct green and red xterm entries (addBg #083c10 -> #005f00, delBg #3c0810 -> #5f0000, addBgWord #147828 -> #008700, delBgWord #74202e -> #870000) instead of collapsing to the same grays, while keeping every truecolor invariant. - New TestExtendedThemeANSI256Contrast quantizes the palette tokens the way a 256-color terminal does and asserts the Dune selected-row pairs stay AA and the Neon bands keep their green/red identity and mutual separation. - docs/THEMES.md is now linked from the Documentation sections of both READMEs, and the Unreleased changelog inventory says twelve themes and names dune and neon. * fix(tui): keep Neon diff content AA after quantization, fix error border - faintest becomes #74c468: line numbers now quantize to #87d75f, which clears AA on the xterm-green addBg (#005f00) where the old #58af69 quantized to 2.95:1, while staying dimmer than faint so the gray ramp stays monotonic. - addInk becomes #ecffdc, quantizing to #ffffd7 for 4.60:1 on the word band's #008700 (was 4.29:1). Both rendered diff-content pairs are now asserted in the ANSI-256 test instead of documented as omissions. - cardErr becomes #9a4042: 3.01:1 against the panel (2.43:1 before) and 3.65:1 quantized, meeting the 3:1 non-text component threshold, with the border surface asserted in both renderings. - docs/THEMES.md no longer claims the extended invariants run against the whole registry: it now says new palettes must be added to the per-palette contrast tests or given equivalent coverage. |
||
|
euxaristia
|
9acb4113cc |
feat(cli): show ZERO wordmark on --version (#673)
* feat(cli): show ZERO wordmark on --version The TUI's empty-state ZERO ascii art gets exposed via a new tui.Wordmark helper and printed above the version line for `zero -v`/`--version`. Coloring only applies when stdout is a real TTY and NO_COLOR isn't set, so redirected or piped output (scripts parsing the version) stays plain. * fix(release): match --version output by suffix, not exact string The release smoke check compared zero --version output to an exact "zero <version>" string, which broke once --version started printing the wordmark above the version line. Check for a "version: <version>" suffix instead so the smoke check doesn't couple to the wordmark art. * fix(cli,tui): keep --version machine-readable off-TTY, drop wordmark color - When stdout is not a terminal, --version prints exactly "zero <version>" again: pipes, redirects, command substitutions, and the release smoke contract all parse that single record, so the banner is a TTY-only affordance now. - The TTY path keeps the banner but restores "zero <version>" as the version record instead of "version: <version>". - Wordmark renders uncolored: only newModel resolves --theme/ZERO_THEME, so painting the global dark palette here was unreadable on light terminals. The terminal's default foreground works on any background. - smokeVersion goes back to requiring the exact "zero <version>" output instead of the suffix match that papered over the banner. |
||
|
|
d66a9dda69 |
feat(providers): add AI/ML API preset (rebased onto main) (#621)
* feat(providers): add AI/ML API preset * fix(providers): relocate aimlapi preset and drop referral header Addresses the two review asks on #402: - Move the AI/ML API descriptor out of catalog slot #2 (it sat above the first-party OpenAI/Anthropic/Google entries) down next to openrouter in the aggregator cluster. Re-sync expectedCatalogIDs and the TransportOpenAICompat order-list to match. - Drop the always-on X-AIMLAPI-Partner-ID / Integration-Repo / Integration-Version attribution headers. There's no partner arrangement behind the preset, so a non-overridable referral tag on every user's request isn't something to ship on a community preset. aimlapi now rides through plain CopyHeaders like every other provider. Removes HeadersForCatalog (it existed only for this special-case), reverts the factory and discovery call sites to CopyHeaders, and drops the header test. The factory/discovery tests now assert the endpoint, auth, and the user's own custom headers survive while no attribution header is sent. * fix(providers): address CodeRabbit review on #621 - Document AIMLAPIID for docstring coverage. - Apply providerio.CopyHeaders for Anthropic and Google provider paths, matching the OpenAI path introduced for aimlapi header isolation. Co-authored-by: Cursor <[email protected]> * fix(providers): gofmt factory.go to unblock CI smoke Remove mixed tab/space indentation in the openai.New options block so ubuntu-latest fmt-check passes. --------- Co-authored-by: Dmitry Tumanov <[email protected]> Co-authored-by: Vasanthdev2004 <[email protected]> Co-authored-by: Cursor <[email protected]> Co-authored-by: KRATOS <[email protected]> |
||
|
PierrunoYT
|
8502449430 | test(tui): isolate provider wizard credentials (#688) | ||
|
euxaristia
|
4c986d327b |
feat(tui): press up to edit queued messages (#656)
* feat(tui): press up to edit queued messages Pressing up with a message queued pops it back into the composer for editing, taking priority over history recall. While a message is queued the empty composer shows a hint placeholder. A second prompt queued during the same run now stacks under the first instead of replacing it. * refactor(tui): remove openclaude references from queued_message comments |
||
|
euxaristia
|
aa73a76f1b |
fix(tui): resolve pending askUser callbacks to prevent runner hangs (#620)
When a run is completed or canceled (agentResponseMsg) while an askUser prompt is pending, or when a stale/superseded askUserRequestMsg is received, the answer callback is now invoked with nil. This unblocks the waiting agent loop goroutine, preventing goroutine leaks and runner hangs. |
||
|
euxaristia
|
0184581ec2 |
fix(agent,tui): resolve git branch detection when starting Zero in subdirectories (#613)
Currently, both gitBranchForPrompt (agent) and gitBranch (tui) check for a .git entry directly inside the current working directory. When starting Zero in nested folders/subdirectories of a repository, the check fails and the branch segment is omitted. Export findProjectGitRoot as FindProjectGitRoot in agent package, and use it in both functions to resolve the correct repository root before checking HEAD and resolving the branch name. Also resolve relative worktree gitdirs relative to the repository root. |
||
|
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]> |
||
|
PatrickNoFilterandPatrickNoFilter
|
8e9149f1a2 |
fix(tui): paste protection for Termux char-by-char paste (#573)
* fix(tui): paste protection for Termux char-by-char paste Termux context menu paste injects characters one at a time, including newlines as raw KeyEnter events. This caused every pasted multiline text to be submitted prematurely before the user could review it. The fix adds a timing-based paste detector that works alongside the existing bracketed-paste handler (tea.PasteMsg): - Three new model fields: lastCharTime, lastKeyTime, burstCount - Timing trackers run before any early return in case tea.KeyPressMsg - Enter handler checks burstCount > 1 (2+ chars within 100ms = paste) - Composer check gated with burstCount > 0 to avoid infinite composer lock - burstCount reset at every intentional action (permission, picker, submit, wizard, spec review, etc.) so normal typing is unaffected The 100ms window catches all observed Termux paste speeds while remaining under typical human inter-key intervals. The burstCount resets at modal boundaries prevent false triggers in tests and real usage. * fix(tui): reset burstCount in Enter handler wizard paths CodeRabbit review found 3 Enter-handler paths (providerWizard, mcpAddWizard, mcpManager) that delegated without resetting burstCount. The equivalent post-switch paths already had the reset but the Enter-specific branches were missing it, creating a window where stale burst state could cause false paste detection. * fix(tui): raise paste burst threshold to >2, use injectable clock * fix(tui): gate paste detection to Termux only, add bidirectional tests - Paste burst detection (insert newline on rapid Enter) now only activates under Termux (TERMUX_VERSION env var), so fast desktop typing never false-triggers. - Switched burst tracker from time.Now() to m.now() for deterministic testing. - Added 3 tests: • TestTermuxBurstInsertsNewline — Termux + 3 chars + Enter → newline • TestTermuxFastTypingSubmits — Termux + 2 chars + Enter → submit • TestDesktopBurstNotAffected — Desktop + 3 chars + Enter → submit * test: extract burst test helper, fix assertions, add reset-after-submit test - Extracted shared burstTestModel helper + typeKeys helper to eliminate duplication across the 3 tests (CodeRabbit nitpick). - Added m.pending assertion to submit tests (cmd != nil alone is weak). - Added TestBurstResetAfterSubmit verifying burstCount is properly reset after submission — 2 chars + Enter still submits. * test: add explicit pending check and lastKeyTime reset in burst reset test * fix(tui): align composer-multiline guard with same burstCount > 2 threshold * test: add TestMultilineBurstSubmits for multiline composer + 2-char burst --------- Co-authored-by: PatrickNoFilter <[email protected]> |
||
|
Vasanth T
|
8f156506dc |
fix(tui): update picker_test for switchProviderModel 4-value return (#589)
#568 added a bool return to switchProviderModel (model, string, bool, tea.Cmd) and updated every call site except picker_test.go:771, which still assigned three values. That broke the internal/tui test build and turned Smoke red on main. Add the missing receiver here. |
||
|
euxaristia
|
c7346fbcf0 |
fix(tui): bypass toggleSidebar and toggleMouse global shortcuts when composer is non-empty (#576)
* fix(tui): bypass toggleSidebar and toggleMouse global shortcuts when composer is non-empty Allows standard readline/Emacs cursor movement keybindings (Ctrl+B and Ctrl+E) to work inside the prompt composer when typing a prompt. This prevents these keys from being globally intercepted to toggle the sidebar or mouse mode when the composer is populated. * fix(tui): remove trailing whitespace, scope composer guard to default chords Addresses jatmn's review: - Removed a trailing-whitespace-only blank line in keybinding_help_test.go that was failing the diff-hygiene CI check. - The composerValue() == "" guard on toggleMouse/toggleSidebar applied to the whole keyMatch(...) result, so a user who remaps either binding to a chord that doesn't conflict with readline navigation (Ctrl+E/Ctrl+B) lost that shortcut entirely while typing. The guard now only applies when the match came through the default chord; a remapped binding fires regardless of composer state. Added TestRemappedToggleBindingsIgnoreComposerGuard covering both bindings. * fix(tui): treat an explicit default-chord config the same as unset Addresses jatmn's follow-up review: the prior fix checked !b.isZero() to decide whether a binding is a genuine, non-conflicting remap, but a user can explicitly configure toggleMouse: "ctrl+e" or toggleSidebar: "ctrl+b" (the same chord as the built-in default). parseBinding gives that a non-zero parsedBinding, so isZero() alone wrongly treated it as safe to bypass the composer guard, letting Ctrl+E/Ctrl+B hijack keystrokes mid-sentence again. requiresEmptyComposer now compares the resolved binding against the canonical default chord directly (parsedBinding is a plain comparable struct), so both "unset" and "explicitly re-affirmed to the same chord" require an empty composer; only a binding that resolves to a genuinely different chord bypasses it. Added TestExplicitDefaultChordConfigStillRequiresEmptyComposer. * refactor(tui): dedupe the composer-gated toggle check CodeRabbit nitpick: the toggleMouse/toggleSidebar dispatch cases both repeated !requiresEmptyComposer(...) || m.composerValue() == "" inline. Extracted canFireComposerGatedToggle so both cases share one expression. |
||
|
PierrunoYTandCursor
|
d0c4e62cd4 |
feat(tui): remember recent provider+model selections in /model picker (#568)
* feat(tui): remember recent provider+model selections in /model picker The /model picker's "Recent" section only ever showed the currently active model, making it hard to switch back to a previously used provider+model pair once you've moved on to something else (especially when the same model name exists across multiple providers). Track a short automatic history of provider-qualified switches instead: newest first, capped at 5, deduped by provider+model pair (not model name alone), and selectable across providers like any other picker row. Closes Gitlawb/zero#562 Co-authored-by: Cursor <[email protected]> * chore: gofmt fixes * fix(tui): restore favorites one-row-per-model semantics in /model picker Favorites de-dup had switched to the provider+model key, letting the same favorited model ID surface once per provider. Revert to per-Value de-dup for Favorites, and skip any model ID already shown under Favorites from Recent and Catalog so favorited models don't reappear in a second group. Recent and Catalog still de-dup among themselves by provider+model so cross-provider rows remain distinct. Also correct the recordRecentModel doc comment: in-memory history is updated even without a user config path (only persistence is skipped), not a no-op. Addresses Copilot review comments on #568. * fix(tui): set OwnerProvider on registry catalog rows; dedupe recent-model normalization - registryModelPickerItem now tags rows with OwnerProvider (the registry entry's canonical provider), fixing the plain-registry fallback path (no saved providers resolved any models) where two providers offering the same model id would collide in pickerItemDedupKey and silently drop a row. - Export config.NormalizeRecentModels and have internal/tui/picker.go's normalizeRecentModelEntries delegate to it instead of maintaining an independent copy of the same trim/dedupe/cap logic, so the TUI and persisted-config normalization can never drift apart. - Add regression tests for both. Addresses remaining CodeRabbit review comments on #568. * fix(tui): batch recent-model persistence into one write per switch; tighten tests - Add recordRecentModels(pairs...), a batched form of recordRecentModel that normalizes and persists exactly once per switch instead of twice (outgoing + incoming pair each triggered their own read-modify-write to userConfigPath). handleModelCommand and switchProviderModel now call it once with both pairs. - picker_test.go: capture and assert switchProviderModel's status return in TestSwitchProviderModelRecordsRecentHistory instead of discarding it, so a failed switch fails at the actual point of failure. - picker_test.go: also assert the persisted config (not just in-memory recentModels) after the reorder/dedupe case in TestModelCommandRecordsAndPersistsRecentHistory. - picker_test.go: pin down the surviving Favorites row's OwnerProvider in TestAssembleModelPickerItemsFavoritesDedupByModelIDAndHideFromOtherGroups, and drop the tautological otherRecent assertion (other-model was never added to recent, so it could never fail). - resolver_test.go: add TestResolveIgnoresProjectOnlyRecentModels, covering the case where only project config sets preferences.recentModels and user config omits it — resolved value must stay empty. Addresses remaining CodeRabbit nitpick comments on #568. * fix(tui): route unresolvable OwnerProvider rows in-place; align recent-history keys - choosePicker: only treat a picker row as cross-provider when its OwnerProvider resolves to an actual saved provider. Registry-fallback catalog rows (tagged with a provider *kind* string) and stale "Recent" rows referencing a since-removed provider now fall back to an in-place model switch instead of erroring "unknown provider". - handleModelCommand/switchProviderModel: capture and record recent-history entries using m.providerName (the resolved value recentModelPairsForPicker pins the active row with) instead of the raw, possibly-empty provider profile Name, so a no-name provider profile can no longer produce a duplicate "Recent" row for the same switch. - newModelPicker: scope the registry-fallback's active-model exclusion by provider too, consistent with the provider-aware de-dup used elsewhere. - recentModelPairsForPicker: delegate to config.NormalizeRecentModels instead of reimplementing the same trim/dedup/cap logic. - Remove the unused recordRecentModel wrapper. - assembleModelPickerItems: build Favorites from recent/catalog directly instead of allocating a concatenated copy of both slices. --------- Co-authored-by: Cursor <[email protected]> |
||
|
|
ba99fa8d48 |
fix(provider): stop dropping custom no-auth providers on restart (#558)
* fix(provider): stop dropping custom no-auth providers on restart Custom OpenAI/Anthropic-compatible catalog entries hardcode RequiresAuth as a template default for the setup wizard, not a fact about the user's actual endpoint. Leaving the credential step blank (e.g. a local llama.cpp server) still stamped the saved profile with a guessed OPENAI_API_KEY/ANTHROPIC_API_KEY env var, and that same static RequiresAuth flag then made the profile look like it was missing a required credential on the next run, silently filtering it out of /model. Add a Custom flag to the catalog descriptor so both the wizard (write side) and setupMissingCredentialEnv (read side) judge these profiles by their own explicit APIKeyEnv instead of the catalog's static default. Fixes #555 Co-Authored-By: Claude Sonnet 5 <[email protected]> Co-authored-by: Pierre Bruno <[email protected]> * fix(provider): self-heal legacy custom profiles stamped by the old wizard bug CodeRabbit flagged that profiles saved before this fix could already carry the phantom APIKeyEnv the old wizard stamped on every custom provider (e.g. OPENAI_API_KEY), and the new custom-branch logic would keep treating any non-empty APIKeyEnv as an explicit requirement — leaving already-affected users' providers filtered out of /model even after upgrading. Treat an APIKeyEnv that exactly matches the catalog's own guessed default as indistinguishable from unset, so already-broken saved profiles self-heal without a config.json migration. A deliberately different env var name (e.g. via --api-key-env) still means auth is required. Co-Authored-By: Claude Sonnet 5 <[email protected]> Co-authored-by: Pierre Bruno <[email protected]> * fix(provider): centralize custom-endpoint credential logic, fix two more surfaces Copilot's review of the earlier fix found the same RequiresAuth-as- authoritative bug in two more places that decide whether a saved provider needs a credential: - validateProviderRuntimeReady (zero providers check) would hard-error a no-auth custom endpoint with "requires API key". - providerWizardCredentialLabel and providerCredentialRequired (/providers status, wizard summary) would still claim/warn that one was needed. Added config.ProviderProfile.MissingCredentialEnv as the single source of truth for this decision (custom-endpoint self-heal included), and had setupMissingCredentialEnv and validateProviderRuntimeReady delegate to it. providerWizardCredentialLabel and providerCredentialRequired take the same Custom-aware fix directly, since their non-catalog fallback path uses runtime-resolved provider-kind metadata that shouldn't be collapsed into the canonical helper. Co-Authored-By: Claude Sonnet 5 <[email protected]> Co-authored-by: Pierre Bruno <[email protected]> * fix(ci): request pinned Go toolchain directly Co-authored-by: Pierre Bruno <[email protected]> --------- Co-authored-by: Amp <[email protected]> |
||
|
Anandan
|
87158a1c90 |
feat: voice dictation (speech-to-text) (#557)
* ## Speech-to-text dictation Adds voice dictation to Zero. Run `/voice` to enter hold-to-record mode, then hold **Space** to dictate and release to transcribe (run `/voice` again to turn it off so Space types normally). Pick a backend with `/stt-model`. ### Backends - **Local (sherpa-onnx)** — offline, no API key. Auto-downloads a model via `/stt-model`; the engine binary is fetched once and shared across models. Family auto-detection covers the mainstream offline families (Whisper, transducer/Zipformer, Moonshine, Paraformer, SenseVoice, and the CTC families FireRedASR, NeMo, TeleSpeech, WeNet, Zipformer-CTC, Dolphin, Omnilingual — plus the encoder/decoder families Canary and FireRedASR-AED). - **Cloud batch** — Groq and OpenAI Whisper. Keys resolve from a configured provider, the encrypted credential store, or the env var; a missing/invalid key is entered inline. - **Streaming** — a warm local sherpa-onnx websocket server (live transcript as you speak) on desktop, or Deepgram / OpenAI Realtime. ### Highlights - Live audio-reactive waveform while recording (real mic levels when streaming; a synthesized wave for batch). - `/stt-model` download chooser: recommended models first, Live/Batch tag per row, size + download state, and a search filter. - Streaming transcript is inserted live into the composer and committed on stop; batch inserts on stop. Insert-for-review by default (`stt.autoSubmit` to fire straight at the agent). - The warm streaming server is released when `/voice` is turned off and on exit. ### Implementation notes - No CGO and no bundled runtimes — every OS integration shells out via `exec.Command` with discrete argv (never a shell string). - The streaming server manager mirrors the LSP manager's warm-subprocess lifecycle. - OS-boundary code (recorders, transcribers, server) is unit-tested via injected fakes; real microphone / websocket behavior is covered by env-gated smoke tests. ### Testing - `go build ./...`, `go vet ./...`, and the config / dictation / cli / tui suites pass. - Verified end-to-end against real downloaded binaries + models (offline batch, streaming websocket server, and the model auto-download), including the FireRedASR-CTC and NeMo streaming transducer families. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added speech-to-text dictation with both batch and streaming modes, supporting local (sherpa-onnx) and cloud providers, including WAV/M4A detection. * Introduced voice dictation mode with STT model selection, local engine/model auto-download, and an inline prompt to save cloud API keys. * Added new TUI commands for voice/STT model selection and enabled F1–F12 key bindings. * **Bug Fixes** * Improved streaming dictation to preserve accumulated transcript partials and keep the editor region consistent. * Refined TUI status/picker rendering, including clearer loading/search behavior and more reliable selection alignment. <!-- end of auto-generated comment: release notes by coderabbit.ai --> * fix(dictation): address CodeRabbit review feedback - validateSTTConfig: add NumThreads bounds check - downloadVerifyExtract: stage extraction into a sibling temp dir, atomic-rename into destDir on success so a partial extract cannot leave a truncated engine binary that passes fileExists() on next run - startProcess: close pipe fds when cmd.Start() fails - ServerManager.Shutdown: honor the caller's context deadline - audioSpillDir: tighten any pre-existing directory to 0700 - startVariantDownload: refuse re-entry while a download is in flight - maybeOpenSTTDownloadPicker: defer Provider=local persistence to applyEngineComponents (commit-on-success), matching the cloud flow - fetchSTTModelsCmd: bound with a 30s timeout client+context - newSTTDownloadPickerFrom.isCurrent: compare path segments via filepath.Base to avoid substring false-positives * fix(dictation): harden recorder.Stop, Esc-cancel priority, streaming region Three correctness issues from PR review: 1) recorder.Stop() panicked (nil-deref) when called on a StartStreaming session because StartStreaming intentionally leaves r.proc nil. Reject that misuse with a clear error. Termux's batch path also leaves r.proc nil by design — keep that path working. 2) Esc was consumed by an active dictation session before the confirming second Esc could cancel a pending run, so a user mid-recording who double-Esc's to kill the run found the first Esc swallowed by dictation. Gate dictation-cancel on the Esc not being a pending-run confirm. 3) applyStreamingText hard-reset the cursor to regionStart on every partial, yanking focus away from users editing elsewhere in the composer, and stored regionStart/End as absolute indices that became stale the moment the user typed or pasted before the live region. Preserve the cursor when it's outside the region, and snapshot the prefix BEFORE the region so the next partial can detect external edits and shift [start,end) to stay aligned. |
||
|
Kevin Codex
|
16550569c1 |
feat(providers): OAuth login profiles and list-first /provider manager (#560)
* feat(providers): OAuth login profiles and list-first /provider manager Make multiple providers a first-class workflow: every OAuth login is now a visible, switchable provider, and /provider manages the whole lifecycle. OAuth logins become provider profiles: - zero auth chatgpt / zero auth login <preset> and all four TUI login paths (wizard + first-run, browser + device-code) now upsert a catalog-backed profile via config.EnsureCatalogProvider after storing the token — without stealing the active provider. Previously the token was stored invisibly: the provider showed up nowhere and 'providers use' failed not-found. - Switching to an OAuth provider no longer inlines the bearer as an API key, which suppressed the runtime resolver (no token refresh, no Codex chatgpt-account-id header → guaranteed 401s). The credential gate now recognizes stored OAuth logins on keyless profiles. - providers list / current report truthful credential state: stored keys count as configured (APIKeySet honors apiKeyStored) and token logins show 'oauth login'; providers check accepts an OAuth login as a credential. List-first /provider manager (TUI): - /provider opens the saved-provider list: Enter activates via the shared switch path, 'a' adds (existing wizard), 'e' opens a field-level editor (endpoint, model, key, description, and safe rename — the stored key, activeProvider, and live ZERO_PROVIDER follow the new name), 'd' deletes with an inline confirm and stored-key cleanup. Esc walks back one level. - Credential states resolve asynchronously so keychain reads never block the render loop; 'stored key missing' surfaces broken keychain entries. - Implemented as manager steps on providerWizardState so every existing overlay gate, key route, and mouse hook covers it unchanged. CLI + config parity: - zero providers remove <name> (deletes the stored key, hands the active pointer off) and zero providers rename <old> <new>. - config.RemoveProvider / RenameProvider: read-modify-atomic-write; rename migrates the credential-store entry write-new-then-delete-old so an interruption can never lose the key. * fix(providers): address PR #560 review findings All six review findings validated as real; fixes with regression tests: - Persist the apiKeyStored marker when a key is captured onto an existing profile: mergeProfile deliberately ignores the marker (a project config must not claim the user's stored keys at resolve time), so the user-config WRITE path (UpsertProvider) now re-applies it — previously editing a key onto an env/no-key profile stranded the secret in the credential store while every ApplyStoredAPIKey gate skipped it. [jatmn P2] - Let active-pointer recovery select OAuth-login providers: firstUsableProvider now applies the same providerHasOAuthLogin check as setupRequired/usableSavedProviders, so a stale activeProvider plus a saved ChatGPT/xAI login recovers instead of forcing onboarding. [jatmn P2] - Delete stored keys from the store BESIDE the config being edited: providers remove and the TUI manager delete now use ProviderKeyStoreAt (matching setup/rename) instead of the default-path store. [jatmn P2] - Roll back the key migration when RenameProvider's config write fails, so the store and config.json can't desync (key under the new name, config still naming the old one). [CodeRabbit] - Clear provider descriptions for real: new SetProviderDescription writes the field verbatim (the upsert merge treats empty as unchanged); the manager's editor routes description changes through it. [jatmn P3] - Surface stored-key deletion failures in providers remove (stderr warning + keyError in the JSON payload) and the TUI delete status, instead of letting a lingering secret read as a clean removal. [CodeRabbit] * fix(providers): second review round — case-only rename, logout hint, test hermeticity - Skip the credential-store migration on case-only renames (groq -> Groq): the store normalizes names case-insensitively, so Set(new)+Delete(old) target ONE entry and the delete removed the key that was just "moved". The existing entry already serves the new name. [CodeRabbit] - Point the delete flow's cleanup hint at the stored login that actually exists: after a rename ({name:"codex", catalogID:"chatgpt"}) the token lives under the catalog id, and `zero auth logout codex` would delete nothing. oauthLoginName resolves the same FirstStored candidate the runtime uses. [jatmn P3] - Lock in APIKeyEnv semantics after a key edit: the env reference survives as an explicit override (a SET env var wins at resolve time; the stored key applies when it is absent). [CodeRabbit nitpick] - Make main's new firstUsableProvider tests hermetic: they read the REAL OAuth token store, so any developer with an actual xai/chatgpt login gets spurious failures (found because this machine has one). Also rebased onto main; the only conflict was comment wording in firstUsableProvider, where the audit commit (#481) independently landed the same OAuth-login fallback check this branch added. * fix(tui): read stored keys from the active config's credential store The TUI's write paths (SecureProviderProfile capture, rename migration, manager delete) all target the store beside m.userConfigPath, but the read paths still opened the DEFAULT-path store: profileWithCredential (feeding the /model and manager switch gate), the model picker's discovery credential lookup, and the manager's async credential-status probe. With a non-default config path, a provider saved with only apiKeyStored had its key in the config-adjacent store, so the gate rejected it as having no usable credential and the manager showed 'stored key missing' for a healthy profile. All TUI stored-key reads now resolve through providerKeyStoreForPath (config-adjacent, default-path fallback for ephemeral sessions), so the UI reads from the same store it writes to. Regression test covers both the credential probe and the switch path with a key seeded only beside the config. Also consolidates the OAuth test-store seeding on withAuthStore (now pins ZERO_OAUTH_STORAGE=file so an inherited keyring backend can't bypass the temp path) per review nitpick. * fix(providers): resolve self-review findings — atomic edits, key normalization, threading Findings from a multi-agent code review of this branch (8 finder angles, independently verified); all confirmed against the code, several with empirical repros. Correctness: - config.EditProvider: one atomic read-modify-write for the manager's Save (rename + fields + verbatim description + stored-key marker + key migration with write-failure rollback). Replaces the rename→upsert→ describe sequence that (a) turned a case-only rename (groq → Groq) into a DUPLICATE profile via UpsertProvider's case-sensitive merge, and (b) could fail halfway, leaving config.json renamed while savedProviders and ZERO_PROVIDER still held the old name (retry then failed 'not found'). A rename now also migrates a freshly captured replacement key (captured under the current name; the migration moves it). - savedProviders is mutated surgically on manager delete/edit instead of being replaced with the raw user-config list — the session list is seeded from the RESOLVED (project-config layered) and usability-FILTERED set, so wholesale replacement dropped project-contributed providers and resurrected filtered stubs for the rest of the session. - oauth.ProviderKey normalizes provider names to lower case at the single write/lookup choke point: `zero auth login xAI` stored provider:xAI while every candidate lookup was case-sensitive — a successful login invisible to the runtime. Map-based presence checks lowercase to match. - OAuth login persistence moved onto the Update goroutine (message handlers) — a background tea.Cmd (device-code polls land minutes later) doing an unlocked config read-modify-write raced UI-thread writes (lost updates). The write error is now surfaced and the in-memory mirror is skipped on failure, so a profile that never reached disk can't show as a working provider that vanishes on restart. UX / consistency: - Esc inside a manager-launched add flow walks back to the provider list from EVERY step (was: first step only; deeper steps destroyed the overlay and the user's place). In-flight OAuth logins are invalidated on the way out; footers say 'Esc back'. - switchProviderModel returns a structured outcome; the manager branches on it instead of strings.Contains(text, "Switched to") — UI copy is no longer a control-flow contract (a provider named 'Switched to prod' could close the manager on a refusal). - Manager delete's keychain subprocess + OAuth lookup run in a follow-up tea.Cmd (the confirm keypress no longer blocks the render loop); the credential probe takes ONE token-store snapshot instead of 2 reads per row, and resolved states carry over across reloads (no 'checking…' flash). - Credential-state reporting converges: /provider status recognizes OAuth logins (was 'api key: not set' + warning for a WORKING provider), doctor uses the shared HasConfiguredCredential definition plus OAuth awareness (its 'matching ProviderSnapshot.APIKeySet' claim is true again), and the manager's classifier falls through a stale stored-key marker to env/OAuth the way the runtime does. * Fix provider manager test formatting |
||
|
PierrunoYTandCopilot Autofix powered by AI
|
008bc9b3f3 |
fix: address bugs found in a multi-agent codebase audit (#481)
* fix: address bugs found in a multi-agent codebase audit Five parallel review agents each swept a different area (self-update, TUI keybindings, agent/session handling, sandbox/shell safety, and auth/credential storage) and surfaced nine concrete defects, all fixed here with regression tests: - sandbox: grant_scope path comparisons were case-sensitive, letting a persisted deny grant be bypassed by spelling a Windows path with different case - securefile/oauth: createSecretFile's Windows ERROR_ACCESS_DENIED handling was only applied to the oauth token store, not the shared credstore backend; both also masked the real lock-contention error behind a misleading "file does not exist" timeout - cli: firstUsableProvider ignored OAuth logins when picking a fallback provider, forcing unnecessary re-onboarding for an already-authenticated user - sessions: exec prompt summarization truncated on a raw byte offset, which could split a multi-byte UTF-8 rune and embed invalid UTF-8 into a resumed session's prompt - tui: configurable keybindings had no collision detection, so a remapped chord could silently make a hardcoded shortcut (or another configured binding) permanently unreachable - tools: the Windows shell-syntax pre-flight check wasn't quote-aware and could block harmless commands whose quoted arguments merely contained operator-like text - update: checksum verification didn't cross-check the archive filename it verified against the one being extracted; extractZip didn't reject symlink-mode entries like extractTarGz already did; replaceBinary's restore-on-failure rename had no retry, risking a permanently missing binary on a transient Windows file lock * fix: close remaining PR 481 review findings * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * fix(tools): drop stale ; test case after rebasing onto main's MSYS check Rebasing fix/bug-hunt-findings onto main pulled in #476, which replaced this file's regex-based windows_shell_syntax detection with a segment/word-anchored check and deliberately stopped treating ; as a command separator on Windows (cmd.exe doesn't split on ; the way bash does). detectShellCommandIssue itself came out of the rebase unchanged from main (our quote-masking version was fully superseded by #476's more thorough segment-based approach), but the quoted-content test this PR added still asserted the old bash-style semantics for `echo "foo" ; ls -la`, which now correctly returns no issue since it's a single echo invocation with literal arguments under cmd.exe, not a real ls invocation. --------- Co-authored-by: Copilot Autofix powered by AI <[email protected]> |
||
|
Kevin Codex
|
fd69233e33 |
feat(tui): show CLI version on the startup home screen (#538)
The home screen orientation line now leads with the build version (v0.2.0 · cwd · branch · model). Release builds already inject internal/cli.version via ldflags; dev builds show "dev". |
||
|
Vasanth T
|
387fe67ee7 |
feat(tui): /loop — repeat a prompt or command on an interval or self-paced (#502)
* feat(tui): /loop foundation — command parsing, loop state, safety bounds First slice of the /loop feature (#501): the self-contained core with no TUI wiring yet. parseLoopCommand handles the leading-interval, trailing-"every", bare-self-paced, and list/stop forms; loopState models one session-scoped loop with doom-loop + iteration-cap fields; interval/self-pace clamps and duration formatting round it out, with unit tests. Because Zero drives loops off a Go timer rather than a cron backend, arbitrary intervals (90s, 7m) are supported with no whitelist limitation. * feat(tui): wire /loop into the turn lifecycle — dispatch, idle tick, continuation Second slice of #501. Adds the controller and wires it in: - /loop command registered (commands.go) and dispatched to handleLoopCommand. - Loop state on the model (loops, activeLoopID, loopSeq/loopTicking poll guards, loopNextWake/loopDone self-pace channel). - An idle poll tick (loopTickMsg, ~1s) fires the earliest due loop only when the session is idle — mirrors the launchQueuedMessageIfReady guard so loops and a user's queued prompt never collide; a self-perpetuating tea.Tick that stops when no loops remain. - The agentResponseMsg completion seam advances a loop iteration (schedule next wake for fixed, model delay for self-paced) with doom-loop, iteration-cap, and done stop conditions, restarting the ticker if other loops remain. - /loop list|stop|stop all, footer summary + list rendering, and a guard that only a prompt or custom /command may loop (not a built-in). Controller tests cover start/advance/self-paced-delay/done/doom/cap/stop/busy. * feat(tui): /loop self-paced adaptive cadence + footer summary + /clear note Third slice of #501. - Self-paced loops now use an adaptive cadence (2m early -> 30m heartbeat as iterations accrue), with the loopNextWake channel kept as an override for a future explicit model signal. - Persistent footer segment "↻ N loops · next 3:05pm" while loops are active. - /clear notes that loops keep running (it wipes the transcript, not the session) so a loop never silently fires into a cleared screen. - Tests for adaptive cadence + footer summary. * test(tui): drive /loop end-to-end through Update (submit + stale tick) * docs(readme): add /loop to the slash-command table * fix(tui): stop looping on repeated failure + clear loop tag on cancel Addresses CodeRabbit review on #502: - advanceLoop now counts consecutive failed iterations (loopMaxFailures=3) and stops the loop, so a loop whose turns keep erroring no longer churns to the iteration cap (the doom guard only sees identical non-empty answers). - cancelRun clears activeLoopID and re-arms the interrupted loop for its next cadence, so a cancelled iteration is not left running forever and the next unrelated turn is not misattributed as that loop's completion. * fix(tui): finish /loop v1 — real self-paced completion, session scope, leave gate Response to the review. The self-paced path was the real gap: it advertised model-chosen completion but nothing ever wrote the done/next-wake fields, so it was just adaptive polling dressed up. Replaced that with an actual control channel — each self-paced iteration runs a short maintenance playbook and ends its reply with a control line (LOOP: DONE / LOOP: CONTINUE [interval]) that advanceLoop parses to stop the loop or pick the next wake. The two dead model fields are gone; done and next-wake now come from the model's output. Rest of the review: - loops stop on a session switch (/new, /resume) instead of firing the previous conversation's prompt into the new one - validateLoopTarget resolves custom /commands up front and rejects unknowns, so a mistyped `/loop 5m /nope` isn't scheduled and then re-sent as literal text - a one-shot confirm gates /clear and /quit while loops are active - age-based auto-expire (24h) next to the iteration cap - the two CodeRabbit test nits (HasPrefix footer check, bare `/loop stop` on a single loop) Hardening from a self-review pass over the above: - normalize CRLF and allow a trailing note on the control line, so `LOOP: DONE — all clean` and \r\n replies still stop the loop - disarm the leave-confirm before the queue/exit early returns, so an interposed prompt can't leave it falsely armed - a loop no longer fires behind an open picker/modal; a run launched behind the /resume picker would otherwise complete into whatever session you switch to * fix(tui): address /loop review round 2 — ticker re-arm, self-paced command, resume guard - stopLoop re-arms the poll ticker after removing one loop, so stopping one of several no longer strands the rest with no tick scheduled (jatmn). - a bare self-paced /command is rejected with a pointer to the interval form: self-paced drives a free-form goal via the playbook + LOOP: control line, which a custom command's expanded template never carries, so it would silently degrade to adaptive polling (jatmn). - /resume only tears loops down on an actual session-id change; /resume latest or /resume <currentID> can resolve to the already-active session, whose loops are its own, not a "previous" session's (CodeRabbit). - tests: skipped-behind-modal loop stays scheduled, footer carries the next-wake time, the confirmed /clear actually wipes the transcript, plus multi-loop stop keeping the ticker alive and self-paced-/command rejection. * docs(readme): clarify /loop runs a prompt or custom command, not built-ins |
||
|
Gautam ManchandaniandGautam Manchandani
|
809489a6e3 |
Batch session event persistence (#526)
Co-authored-by: Gautam Manchandani <[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.
|
||
|
KRATOS
|
284a6c86cb |
Skills: deterministic invocation — /<skill-name>, searchable /skills picker, discovery-budget fix (#520)
* feat(skills): make skill invocation deterministic and discoverable
Skills were model-pulled only: the model had to notice a skill in a
640-byte system-prompt list (~6 skills visible; the rest collapsed into
'…and N more') and call the skill tool on its own. Users could not run a
skill at all — typing /<skill-name> reported 'unknown command'. The two
halves of the fix:
User invocation
- /<skill-name> [args] now runs an installed skill directly: resolved
after builtins and user commands, the skill body is inlined as the
prompt with the typed args appended, and submitted through the same
launch path user commands use.
- /skills lists installed skills with their slash form, live from disk.
Labels are honest: a skill shadowed by a builtin, alias, user command,
or a case-colliding sibling is listed bare with a '(via skill tool)'
note instead of advertising a dead /name.
- The slash palette suggests skills (labeled '(skill)') read through the
same loader dispatch uses, so mid-session installs appear and shadowed
or case-colliding names are never advertised.
- Skill and user-command invocations now apply the plain-prompt run-state
guards: mid-run submissions queue the EXPANDED prompt (the queue flush
submits literal text), compaction-in-flight warns, exiting drops.
- Skills resolve through a lazy plugin-aware loader (default dir merged
with plugin skill roots — the same set the skill tool and the system
prompt see), TTL-cached so palette keystrokes don't re-read SKILL.md
bodies.
Model triggering
- Raise the skills-list budget 640 -> 4096 bytes: the list is the model's
only discovery surface, and the old cap made skills past ~6 invisible —
invocation reliability collapsed exactly for users invested in skills.
- Raise description truncation 100 -> 200 runes: the description carries
the trigger conditions ('use when…'); cutting them is what stopped the
model from firing the skill.
- Strengthen the instruction line: scan the list before acting, call the
matching skill FIRST, don't substitute your own approach.
* feat(tui): /skills opens a searchable skill picker
Replace the static /skills card with the same picker UX /model uses:
type-to-filter (ranked search), arrow-key selection, Enter runs the
chosen skill immediately. Selection is by the skill's exact name, so
skills the slash path cannot reach — names shadowed by a builtin or
user command, or names that don't fit a slash token — are still
runnable from the picker, where the choice is unambiguous. The text
card remains only as the no-skills install hint, and the picker path
applies the same run-state guards as slash invocation.
* feat(tui): ask-first guard for bare skill invocations
A skill invoked with no request submits only its body — instructions
with no target ("review the PR" — which PR?) — and the model improvises
a target instead of asking, which is especially wrong in a fresh
session. Bare invocations (slash and picker) now append a conditional
note: if the instructions need a target or details not already clear
from the conversation, ask first; self-contained skills still just run.
Invocations that carry a request are unchanged.
* fix(tui): review follow-ups — restore typed invocation on compaction guard, cap merged suggestions
- The compaction-in-flight guard for skill/user-command invocations fired
after the dispatch path had already cleared the composer, so the typed
"/name args" was lost and had to be retyped; thread the raw invocation
through and restore it into the composer (the plain-prompt guard keeps
the composer intact by firing before the clear). The picker path has no
typed form and restores nothing.
- Cap the merged autocomplete list at the shared bound: each source
self-caps, but three sources could stack to ~3x the palette limit.
- Tighten the shadowed-skill palette test to assert on the advertised
Name rather than the fallback Desc wording.
* chore: ignore zero-skills local build artifact
A locally built test binary under this name was accidentally committed
(and has been scrubbed from the branch history); ignore the name so it
cannot recur.
|
||
|
PatrickNoFilter
|
0f69d995e9 |
fix: Termux/Android support — PRoot scroll, SIGSYS sandbox, build docs (#509)
* fix(tui): TracerPid PRoot detection, Ctrl+U/D direction, composer guard - Replace Termux env var detection with isRunningUnderPRoot() via /proc/self/status TracerPid, cached with sync.OnceValue - Mouse mode falls back to CellMotion (not AllMotion) under PRoot to fix broken touch scrolling - Add Shift+Up/Shift+Down before plain KeyDown/KeyUp for transcript scroll with permission/askuser cursor + composer guard - Add Ctrl+U (scroll up) and Ctrl+D (scroll down) with proper direction: Ctrl+U=move up, Ctrl+D=move down - Add guards for active modals (wizard, picker, spec review, etc.) to avoid intercepting their Ctrl+U/D keybindings - Tests: TestPermissionCursorCtrlU/D (direction regression), TestShiftUp/DownComposerGuard (composer guard) - References issue #507 * fix(sandbox): avoid faccessat2 for Termux/Android portability Replace exec.LookPath with lookupExecutable that uses os.Stat instead of the faccessat2 syscall. The faccessat2 syscall (439) is blocked by the Samsung seccomp filter on Android, causing SIGSYS termination. os.Stat uses the newfstatat/statx syscalls which are universally permitted. This is part of broader Termux/Android support for Zero. * docs(install): add Termux/Android build and configuration guide Document how to build Zero for native Android/Termux: GOOS=android to avoid the SIGSYS crash, proot bind-mount for DNS resolution, PRoot scroll fix, and free-tier provider setup. * fix(sandbox): skip empty PATH segments, use platform-aware executable check |
||
|
Alaa Elsamouly
|
30bff28efc |
Fix/detailed view scroll select (#498)
* fix(tui): strip all ANSI escapes from copied transcript text renderAssistantMarkdownText embeds bold markers for prose and ANSI color sequences from chroma for fenced code blocks. stripMarkdownRenderControls only stripped the bold markers, leaking ANSI codes into clipboard text. Switch to ansi.Strip() to remove all escape sequences. * refactor(tui): extract rendering dispatch and add detailed bool to transcriptBodyItems Extract rowRenderFn and transcriptRowDispatchFn types and refactor rendering methods into *Fn variants parameterized by the render function. Add detailed bool parameter to transcriptBodyItems to prepare for the detailed transcript viewport. All callers pass false so there is no behavior change yet. Extract transcriptRowBodyHeightCacheKeyOpts from transcriptRowBodyHeightCacheKey so the body cap can be configured per caller. * feat(tui): scrollable detailed transcript viewport with sidebar suppression Enable keyboard scrolling (PgUp/PgDown/Up/Down) in detailed transcript mode by routing through scrollChat instead of swallowing the keys. Rewrite detailedTranscriptView to use the transcriptBodyItems pipeline so the body renders through the same viewport/scroll engine as the live view. Implement detailedTranscriptFooter with copy-status and jump-to-bottom hint integration. Suppress the sidebar in detailed mode so the full chat width is available for reading uncapped tool output. * test(tui): detailed transcript scroll with PgUp/Down and Up/Down arrows TestPgUpDownScrollsDetailedTranscript verifies that PgUp scrolls toward older content and PgDown returns to the bottom in detailed mode. TestUpDownArrowsScrollDetailedTranscript verifies that arrow keys scroll one line at a time and bottom is clamped at offset 0. * test(tui): ANSI strip verification for markdown rendering controls and selectable text TestStripMarkdownRenderControlsStripsAllANSI verifies bold markers, color sequences, and combined sequences are stripped. TestSelectedTranscriptTextStripsANSIFromHighlightedCode verifies that chroma-highlighted code blocks produce clean plain text in the selectable layer. * fix for coderabit, and better fewer code. * fix(tui): address PR review feedback dead code, hover clear, and rebindable key label * format * addressing the review issue 1. the ctrl + b in detailed mode, added the guard to not be able to do it in detailed mode so it affect its state in the normal mode. 2. added a test coverage for keyDown after `keDown then keyUp` so it covers that. * fix(tui): close file drill-in when entering detailed transcript toggleDetailedTranscript now calls exitFileView before returning, so an active file drill-in is automatically closed when the user enters detailed transcript mode. * test(tui): verify file drill-in closes on detailed toggle Adds TestDetailedTranscriptClosesFileView and TestDetailedTranscriptStaysClosedOnSecondToggle to guard against the file-view bypass bug. * fix(tui): block composer hit-test while detailed transcript is active composerMouseSelectionBlocked and mouseOverComposer now check m.transcriptDetailed so mouse clicks and wheel events in the composer area reach the transcript body instead of being silently consumed. * test(tui): verify composer mouse is ignored in detailed mode Adds TestComposerMouseSelectionBlockedInDetailedMode to guard against composer hit-test swallowing mouse events during detailed transcript viewing. |
||
|
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. |