mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-12 16:23:18 -10:00
fix-exec-output-buffer-oom
9
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Gnanam
|
a766163401 |
fix: pre-push review findings on the exec_command buffer-cap fix
Ran an adversarial review before opening the PR. It found the truncation notice added alongside the buffer cap didn't actually work: prefixing execOutputTruncatedMessage into drainString()'s text meant the notice always sat ~2MiB before the end of the combined output, but the tool's own downstream truncateExecOutput() keeps at most a 400KB tail (even at the maximum allowed max_output_tokens) — so the notice was reliably swallowed, or chopped into an unreadable fragment at smaller budgets. Verified with direct reproductions against the real functions. Fixed by moving the signal out of the text stream entirely: - execOutputBuffer gains consumeTruncated() (read+reset, called once per collect()) and peekTruncated() (read-only, for status views) instead of injecting a marker into the drained bytes. - collect() now returns (string, bool); both call sites thread the bool through to execToolResult, which sets meta["output_buffer_truncated"] and appends the notice to the response body *after* truncateExecOutput runs, so it can never be re-truncated or land in a discarded region. - Also flagged: a session torn down without ever being polled again after truncation loses the signal entirely, since nothing drains/logs it on teardown. Full audit-trail logging felt like overkill for a soft transparency signal on top of a hard memory-safety fix, but exposed peekTruncated() on ExecSessionSnapshot and surfaced it in the /ps row (background_terminals.go) so it's visible any time a still-running session is checked — covers the common case (the review's own scenario: a long-lived dev server nobody's polling but that's still alive) even though a session reaped before ever being looked at is still a gap. New tests: reworked TestExecOutputBufferCapsUndrainedData for the consume/peek API, and TestExecToolResultSurfacesBufferTruncationOutsideByteBudget reproducing the review's exact failure case (max_output_tokens=1) and proving the notice now survives it. |
||
|
Gnanam
|
16ce70b7a9 |
fix: cap unbounded exec_command output buffer to prevent OOM kill
execOutputBuffer.data accumulated every byte a backgrounded exec_command
session wrote, with no size cap, unlike buffer.recent which is correctly
bounded to recentExecOutputBytes for the /ps preview. drainString() only
clears data when the agent actually polls that session again — so a
long-lived background session nobody polls (e.g. a dev server left running
after the run that started it was cancelled) grows this buffer forever as
long as the process keeps writing, with no ceiling.
Traced this directly to a real crash: a session that had been running for
~5.5 hours (an earlier turn invoking swarm agents got cancelled mid-flight,
per that session's own event log) accumulated to ~42.7GB of compressed
memory, at which point macOS's memorystatus killed the process outright
("killing zero due to low-swap").
Fixed by capping data at maxExecOutputBufferBytes (2MiB), trimming from the
front like recent already does, and surfacing a one-time truncation notice
on the next drain so data loss is visible rather than silent.
|
||
|
Anandan
|
d29ab98a79 |
Improve sandbox permission handling (#323)
Retry shell commands outside the sandbox when sandboxed output only reflects the sandbox namespace, and keep network retries sandboxed when a network grant is enough. Show user-facing permission reasons for escalated shell commands instead of exposing internal sandbox policy text. Tested: env GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/tui -run 'TestRunUsesEscalatedJustificationForPermissionPrompt|TestRunUsesUserFacingEscalatedFallbackForPermissionPrompt|TestPermissionPromptMapsEscalatedSandboxReason|TestRunRetriesShellUnsandboxedAfterSandboxNamespaceLimitedOutput' Tested: env GOCACHE=/tmp/zero-go-cache go test ./internal/tools Tested: env HOME=/tmp/zero-test-home-clean XDG_CONFIG_HOME=/tmp/zero-test-config-clean GOCACHE=/tmp/zero-go-cache go test ./... |
||
|
Anandan
|
6be50cd3a7 |
fix(sandbox): retry approved network shell commands (#301)
* fix(sandbox): retry approved network shell commands * test(sandbox): fix portable smoke expectations * fix(sandbox): restore Windows offline egress block |
||
|
KRATOSandgnanam1990
|
f7ee189ba5 |
fix: resolve 15 findings from the 2026-06-20 deep audit (#281)
* docs(audit): 2026-06-20 deep adversarial audit report
Audit-only deliverable (no source changed). Fresh origin/main (
|
||
|
Vasanth T
|
bfbdbb10f6 |
test(tools): make write_stdin "\x03" the trigger in the interrupt test (#273)
The de-flake infra (resilientTempDir + deterministic <-session.done wait) is already on main, but the test still called session.terminate() directly before write_stdin "\x03" — so the session was already reaped when the code under test (exec_command.go's Ctrl-C branch) ran, and the assertion no longer guarded it: the test would pass even if that branch were deleted (caught in review). Capture the session handle before the interrupt (write_stdin removes a finished session from the manager), then make write_stdin "\x03" the operation that terminates, then wait on session.done. Keeps the de-flake; restores the coverage. Verified by mutation: removing session.terminate() from the Ctrl-C branch makes this test fail. Runs 15x locally without flaking. |
||
|
Anandan
|
f5038151c8 |
Add managed background terminals (#270)
* Add managed background terminal sessions * Stop background terminals on TUI exit * Cover approved local network sandbox mode * Polish background terminal rendering * Tighten exec session cleanup guidance * Address background terminal review feedback * Kill exec session trees on Windows * Trigger CI for Windows cleanup |
||
|
Vasanth TandClaude Opus 4.8
|
c572791edf |
feat: clipboard image paste + provider 400 handler for image rejection (#268)
* fix(agent): abort on image-rejection 400s instead of retrying in a loop
* feat(imageinput): add ReadClipboardImage for OS clipboard image extraction
* feat(tui): clipboard image paste — detect and attach screenshots from OS clipboard
* fix(imageinput): use temp file for Windows clipboard image (PowerShell stdout can't emit raw binary)
* test: handle both image-present and image-absent clipboard states
* fix(vision): add kimi-k2 and qwen-vl to vision-capable name heuristic
* fix(tui): render image errors as red error rows instead of grey system notes
* feat(vision): check models.dev InputModalities for vision support instead of only name heuristics
The vision gate now checks three sources:
1. Curated model registry (catalog authority + name heuristic)
2. Discovered model list from models.dev (InputModalities contains 'image')
3. Name heuristic fallback for unknown models
This means custom/ollama/cloud models (kimi-k2, qwen-vl, etc.) are correctly
detected as vision-capable when models.dev reports them with image input
modality — no more manual name matching needed.
* chore: gofmt clipboard_test.go
* fix(imageinput): remove shell injection in the Linux clipboard reader
The Linux clipboard reader built shell commands by concatenating a
clipboard-derived MIME type into `sh -c`:
exec.Command("sh", "-c", "wl-paste --type "+t+" 2>/dev/null")
exec.Command("sh", "-c", "xclip ... -t "+t+" -o 2>/dev/null")
`t` comes from `wl-paste --list-types` / `xclip TARGETS` and is only filtered by
`HasPrefix(t, "image/")`, so a hostile clipboard offerer could register a target
like `image/png; rm -rf ~` — it passes the prefix check and then executes
through the shell. A real (if low-likelihood) local code-exec vector.
Drop the shell entirely: every helper now runs via exec.Command(prog, args...)
with the MIME type passed as a discrete argument (matching the existing no-shell
pattern in pdf.go). Stderr is discarded by not assigning it. Factored into
runClipboardStdout + imageMIMETypes helpers.
Test: imageMIMETypes filters to image/* lines and returns even an
injection-shaped type verbatim (safe now that it never reaches a shell).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
* test(tools): de-flake TestWriteStdinInterruptTerminatesSession
The test had two Windows-CI flakes that reddened unrelated PRs:
1. It relied on write_stdin's 1000ms yield window being long enough for the
async SIGKILL + reap to land before asserting the session was no longer
running. On a loaded runner the reap exceeded that window, so write_stdin
reported "still running" and the assertion failed. Fix: after terminating,
wait on session.done deterministically (30s safety timeout that fails loudly
if the kill genuinely hangs); the common case returns the instant the
process exits.
2. The SIGKILL'd child had the test's t.TempDir() as its cwd, and Windows may
not release the directory handle the instant the process is reaped — so
t.TempDir()'s immediate RemoveAll on cleanup failed with "being used by
another process". Fix: resilientTempDir retries RemoveAll for up to 5s
(best-effort; a leaked temp dir never fails the test).
Verified 20x locally; full internal/tools suite green.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
|
||
|
Anandan
|
cf54e2cb0e |
Add persistent exec command sessions (#267)
* Add persistent exec command sessions Introduce exec_command and write_stdin tools backed by a shared session manager so long-running shell commands can remain alive and be polled or interrupted across tool calls. Wire exec_command through the existing shell permission, network prompt, command-prefix approval, sandbox grant scope, specialist manifest, and prompt/documentation paths. Keep web_fetch limited to public remote HTTP(S) URLs so localhost/private URLs go through shell commands where sandbox network permission applies. Tested: git diff --cached --check * Fix exec session lifecycle review issues |