mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-10 13:11:31 -10:00
fix/opencode-session-header
19
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
aadb4a27e9 |
fix(specialist): keep the pinned model when a specialist is resumed (#1009)
* fix(specialist): keep the pinned model when a specialist is resumed Metadata.Model exists so a bounded, delegated task can run on a cheaper model than its parent, and BuildArgs appends it to the child argv through appendModelArgs. BuildResumeArgs never did. So a specialist pinned to a cheap model ran on that model exactly once. The moment the orchestrator resumed it, the child fell back to whatever the parent's configured model resolved to. Nothing surfaced it: the resumed child starts normally and does the work, so the only symptom is the bill. Cost-motivated delegation quietly stopped saving anything. Resuming does not restore the recorded model on its own. sessions.PrepareExec records the model a run used but never feeds it back into provider construction, so the flag has to be passed again rather than relied upon. BuildResumeArgsInput now carries ParentModel and ParentReasoningEffort, the same fallbacks the fresh path takes, and runResume passes what TaskRunOptions already held. The reasoning-effort rule travels with the model unchanged: the parent's effort is inherited only when the manifest pins no model of its own, because a manifest that chose a different model has not agreed to the parent's effort for it. Regressions drive both builders and compare them, so the two paths cannot drift again: a pinned model survives resume, an unpinned one still inherits the parent's, both halves of the effort rule hold, and the flag keeps its position relative to --auto in both. Refs #554 * test(specialist): cover the resume call site, not just its builder The builder tests all call BuildResumeArgs directly, so dropping the ParentModel field from the runResume call site still compiled and still passed every one of them. The defect this fixes lived at the call site, so it needs a test that goes through Run. Driven through the real dispatch with the RunChild seam capturing argv. * test(specialist): guard the fresh call site as well runFresh and runResume each construct their builder input by hand and carry a byte-identical ParentModel line. Deleting either compiles and, until now, deleting the fresh one was silent. The resume half is what this branch repairs. This covers the other half so the pair cannot drift again in the direction nobody was looking. |
||
|
|
0176ee49ff |
feat(agent): add PermissionModePlan for interactive read-only planning (#853)
* feat(agent): add PermissionModePlan for interactive read-only planning * fix(agent): drop redundant modeName branch, add plan mode regression tests string(permissionMode) already yields "plan" / "spec-draft", so the if/else recomputing modeName in the denial message was dead branching on the same values. Also add plan-mode coverage mirroring three of the four existing spec-draft regression tests: advertised tool set, and denied write_file/bash calls. The fourth (submit-and-stop review control) has no plan-mode analog, since plan mode has no submit tool. * fix(agent): deny request_permissions in plan/spec-draft even when the registry omits it request_permissions is dispatched by name in executeToolCall before the registry-based ToolAdvertised gate runs, so that gate only helps when the tool happens to be present in the caller's registry. A plan- or spec-draft-mode registry that simply omits the tool (rather than registering it as denied) let the call fall through to a real turn/session-scoped permission grant, defeating the read-only boundary. Deny it unconditionally at the top of executeRequestPermissions for both read-only modes instead. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(agent): suppress executable hooks while plan/spec-draft mode is active Plan mode promises a read-only turn, but sessionStart/sessionEnd fire on every run and beforeTool/afterTool fire around allowed read calls, and all four execute configured host commands outside the advertised-tool and sandbox gates — so a project hook could mutate the workspace or spawn a process from a session that advertises it cannot. Gate all four dispatch points on the run's permission mode, with a regression test asserting no hook command launches during a plan-mode run. Co-Authored-By: Claude Fable 5 <[email protected]> * fix(agent): close plan-mode tool advertisement bypass toolAdvertisedInPlan whitelisted ask_user and update_plan by name alone, so a caller could register a mutating tool under either name and have it advertised and executed in plan mode. Validate every tool against its Safety() instead. Also exclude lsp_navigate, which is marked SideEffectRead but lazily spawns a real language-server process, contradicting plan mode's read-only guarantee. Add regression tests for both. * fix(agent): keep trust-gated hooks in spec-draft mode Plan mode still suppresses executable hooks so a read-only planning turn cannot spawn host processes via session or tool hooks. Spec-draft keeps the existing trust model: project hooks fire when the workspace (or its worktree trust root) is trusted. Unconditionally suppressing hooks in spec-draft broke TestExecSpecWorktreeInheritsTrustEndToEnd for trusted worktrees under --use-spec --worktree. * fix(agent): cover plan-mode spoof and lsp_navigate denials Expand the name-only spoof regression to both update_plan and ask_user, and add an execution-path denial for lsp_navigate so plan mode cannot spawn language servers even when a call still arrives. * fix(agent): filter tool_search deferred candidates by plan/spec-draft visibility tool_search resolved and ranked deferred tools by EnabledTools/DisabledTools only, never by the run's permission-mode visibility. tool_search itself is already denied at dispatch in plan/spec-draft (its Safety carries no side effect, so it fails the same SideEffect==Read advertisement gate direct calls use), so this was not reachable through the normal Run() path today. But it is a real landmine: if that outer gate is ever loosened independently (e.g. tool_search's no-side-effect Safety is judged advertisable), the loader had no gate of its own and would hand a deferred write/mutator tool's name, description, and full schema straight to a plan/spec-draft model. Mirror agent.ToolAdvertised's plan/spec-draft branches inside the tools package (toolAdvertisedForPermissionMode, next to the existing toolAllowedByFilters mirror that avoids the same import cycle) and apply it alongside the operator filters in visibleDeferredTools and visibleEagerToolNames. Added unit tests in tool_search_test.go for both modes, and an end-to-end agent test that force-calls tool_search in plan mode and asserts no schema leaks. Verified by reverting tool_search.go and confirming the new tests fail (one shows load_tools resolving to the mutator's name); restored and confirmed they pass. Also confirmed via a temporary probe that if plan mode's outer advertisement gate is loosened, this filter is what actually stops the leak. * fix(agent): require Safety for spec-draft ask_user/submit_spec Do not advertise or load re-registered control tools by name alone in spec-draft mode. ask_user must be SideEffectRead+Allow and submit_spec must be SideEffectWrite+Allow, matching the real tools. Apply the same filter in tool_search and add spoof regression tests. Refs Gitlawb/zero#642 * fix(agent): wire plan mode into TUI, CLI, and ACP entry points Address the P1 finding that PermissionModePlan was documented but never selected by /plan, zero exec, or ACP mode selectors. /plan on|off now toggles the session permission mode (restoring the prior mode on off), zero exec --plan selects plan for a run, and ACP advertises plan as a client-selectable mode. Integration coverage for each entry path. * fix(cli): reject --plan combined with --worktree Worktree preparation runs in runExec before the plan permission mode is assigned, so `zero exec --plan --worktree` could still trigger workspace mutation ahead of the read-only gate. Reject the combination during option validation, alongside the existing --use-spec/--skip-permissions- unsafe conflict checks, so no worktree prep can occur. Addresses a coderabbitai finding on PR #642. * test(tools): assert spoofed tool schema doesn't leak via tool_search The spoofed-control-tool regression only asserted on the description string; Parameters() exposed no distinctive schema marker, so a regression that leaked the schema without the description would still have passed. Add a spoofed_secret property to the test tool's schema and assert it's absent from result.Output alongside the description. Addresses a coderabbitai finding on PR #642. * fix(tui): gate local mutating commands behind plan mode /plan on only flips the agent permission mode, which gates agent tool calls. Local TUI commands that run entirely inside the TUI process bypass that gate: /rewind restores workspace files from a checkpoint, /export writes a transcript to disk, and /sandbox-setup spawns a native host process. Add a shared plan-mode guard at the start of dispatchCommand (mirroring the existing BTW-unavailable guard) that rejects these three commands while permissionMode is agent.PermissionModePlan, with regression coverage proving each is blocked with no mutation/process spawn in plan mode and unaffected outside it. Addresses a coderabbitai finding on PR #642. * fix(agent,specialist): layer plan mode system prompt and enforce read-only subagent mode * Fix jatmn review findings for PR 642 * fix(agent,cli,tui): resolve CodeRabbit review comments on active turn model field and --plan permission mode conflict * fix(agent): propagate permission mode to exec options and serialize ACP mode changes Parse permissionMode in resolveExecPermissionMode, acquire turnMu.Lock in ACP handleSetMode to serialize mode changes with active turns, and block MCP subcommands in TUI plan mode. Refs #642 * fix(agent,tui): update tests off removed tools.CoreTools/NewWriteFileTool/NewLSPNavigateTool wrappers Those were thin unscoped wrappers around the Scoped variants, deleted upstream in #706 since nothing else called them directly. Only these tests still did; switch to the Scoped calls main's own tests already use. * fix(agent): fail-closed beforeTool vetoes and permission-mode plan guards Keep beforeTool deny gates active under plan mode so hooksSuppressed no longer fails open, and stop propagating --permission-mode for auto/ask/member children so swarm members keep write tools. Apply --plan combination rejects to --permission-mode plan as well. Refs #853 * fix(agent): address CodeRabbit findings for plan-mode advertisement and entry paths Unify plan/spec-draft tool advertisement in tools.ToolAdvertisedForPermissionMode (with PermissionDeny short-circuit), cover ACP config and --permission-mode plan list-tools paths, and allow bare /mcp while blocking mutating MCP subcommands. Refs #853 --------- Co-authored-by: Claude Sonnet 5 <[email protected]> Co-authored-by: euxaristia <[email protected]> |
||
|
|
cd0eb194e4 |
feat(tools): give tool results an image channel and add view_image (#843)
* feat(tools): give tool results an image channel and add view_image Closes #824. tools.Result carried only text, so no tool could hand the model a picture. The sharpest edge was our own capture tooling: browser_screenshot, desktop_screenshot and browser_pdf wrote a PNG, set Meta["artifact_path"] and returned "Artifact captured: <path>". Nothing read that path afterwards, so the model asked for a screenshot, was told one existed, and could not look at it. Zero was never image-blind on the way in — zeroruntime.ImageBlock, Message.Images and internal/imageinput are mature. What was missing is the model-initiated leg. Result.Images is delivered as a following USER message, not on the tool result itself. Every provider drops images on a tool-role message: Anthropic maps a tool result to a tool_result block whose content is a string, Gemini to a functionResponse, and OpenAI guards its image content-parts to the user role with an explicit comment saying so. Attaching them to the tool result would have plumbed the whole path and delivered nothing. A separate message also keeps one tool result per tool call, which the providers validate. Blocks with no data, or a media type outside the provider allow-list, are dropped rather than sent — a rejected image would fail the whole turn, and the tool's text still stands on its own. The message names the tool, since the images arrive detached from their result and nothing else says which call produced them when several ran in one turn. view_image resolves through resolveScopedReadPath before imageinput.LoadFile. That scoping is load-bearing: LoadFile joins a relative path to the workspace root and otherwise takes the path as given, so without it the tool would read anywhere on disk. It is registered with the other scoped readers and declared read-only and thread-safe, matching read_file. The capture tools now attach what they wrote. A PDF or an unreadable file yields no image and the text stands alone, so a capture is never failed over its attachment. Co-Authored-By: Claude Opus 5 <[email protected]> * fix(agent): keep tool results contiguous when a tool returns an image The image messages are user-role, and the first version appended each one immediately after its own tool result. With two tool calls in a turn that produces tool_result, user, tool_result — and a user message between two tool results breaks strict provider replay. Anthropic makes it concrete. appendUserBlocks coalesces consecutive user content into one block list, and tool_result blocks must come first in that message, so the interleaved shape maps to: [tool_result, text, image, tool_result] which the API rejects. loop.go already documents this exact rule three lines below the change, for the self-correction feedback it defers for the same reason. Images are now collected during the tool loop and appended once every tool_result is recorded — including the aborted placeholders on the abort, stop-reason and repeated-failure paths, so an early exit cannot strand them mid-sequence either. The original test used a single tool call, which cannot exhibit the bug. The new one runs two calls where the first returns an image and asserts the image message does not fall between the first and last tool result. Also from review: view_image joins the "execute" tool category, which grants read_file but not view_image, so a specialist declared with tools: [execute] could not look at a screenshot it had just captured. Covered by a test that holds for every category granting read_file rather than naming one. And the core read-only set is now asserted by tool name, not only by count — a count still passes when one reader is swapped for another. Co-Authored-By: Claude Opus 5 <[email protected]> * test(agent): pin the tool-result/image ordering contract The assertions only tracked the first and last tool-result index, so four distinct broken orderings passed: a message wedged between the two tool results, a duplicated image flush, a duplicated tool result, and both results answering the same tool_call_id. Mutation-testing each of those against the real loop confirmed the old assertions were green on all four. Now it collects every tool-result index and id and requires exactly two, in c1-then-c2 order, at adjacent indexes, with the image following the last one and delivered exactly once. Adjacency is the assertion that carries the contract: Anthropic needs every tool_result block ahead of any other block in the user message they coalesce into, so anything between the two results is the 400 this change exists to avoid. The image is deliberately NOT pinned to exactly last+1. appendUserBlocks merges consecutive user-role content, so [tool_result, tool_result, text, image] maps to tool_result blocks first and replays fine; pinning would fail that valid shape while killing nothing the adjacency and count assertions do not already catch. messageShape reports the actual role ordering on failure, and the loop uses two ifs rather than a switch so an image left on a tool-role message is reported as the misplacement it is instead of "the image never reached the model". --------- Co-authored-by: Claude Opus 5 <[email protected]> |
||
|
|
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]> |
||
|
|
177442cfe4 |
fix(specialist): cap max specialist nesting depth (#491)
* fix(specialist): cap max specialist nesting depth A Task specialist can recursively spawn further specialists via --depth. Without a ceiling, a runaway or malicious chain of Task calls can recurse indefinitely, exhausting process and memory resources. Reject any run whose depth exceeds a hard cap of 8. Related to #470, not closing it: today's normal specialist/swarm path cannot reach this recursively, since child sessions are launched with --tag specialist and shouldRegisterExecSpecialistTools suppresses the Task tool for that tag (see PR description for details). This is a defense-in-depth guard, not a fix for an active exploit. Split out of #468 at reviewer request. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(specialist): reject depth cap at the boundary, not past it Addresses jatmn's review on PR #491: - Executor.Run checked `CurrentDepth > maxSpecialistDepth`, but this call is what launches the child at CurrentDepth+1. A parent already AT the cap passed the check and started a child one level past it, only getting rejected on that child's own next call. Change to `>=` so the guard fires before the over-cap child is launched. - Add TestRunRejectsDepthAtMax to cover the boundary explicitly. * test(specialist): cover resume boundary in depth-cap guard The pre-branch depth guard in Run sits before the fresh/resume split, but the boundary test only exercised the fresh path, leaving a future refactor free to move the guard without any test catching a resume regression. --------- Co-authored-by: Claude Sonnet 5 <[email protected]> |
||
|
|
7fb0092de7 |
fix: agent runtime resilience — stream stalls, weak-model tool args, and sub-agent reliability (#349)
* fix(local): defeat the macOS stale-pooled-connection hang (transport + stall-retry)
Root cause (you confirmed it: both chatgpt AND ollama hung, only on macOS): providers
fell back to http.DefaultClient, whose pooled keep-alive connections get reused on a
later turn after the server/NAT silently dropped them. The model call is a POST
(non-idempotent), so Go won't auto-retry it on a fresh conn — it blocks forever.
macOS keeps dead pooled conns around longer than Linux, so it reproduced on macOS only.
1. providerio.HTTPClient now returns a shared, tuned transport (ResponseHeaderTimeout
60s bounds a hung POST that gets no response; IdleConnTimeout 30s stops dead conns
from lingering to be reused). Routed openai/codex through it too (anthropic/gemini
already used HTTPClient). Slow first tokens / long reasoning are unaffected — the
header timeout only bounds time-to-first-response-header, not the stream body.
2. Agent loop now retries a stream idle/stall timeout that produced NO output yet, on
a fresh connection (capped + backoff). A partial-output turn is NOT retried (would
duplicate). Recovers any stall that slips past the transport timeout.
Local only — not pushed.
* fix(local): tolerate concatenated tool-arg JSON + widen response-header timeout
1) Weak models (minimax-m3, glm) pack MULTIPLE concatenated top-level JSON objects
into one tool-call arguments slot ({A}{B}), which json.Unmarshal rejects with
'invalid character {' after top-level value' (~9 failed tool calls/session).
New recoverableToolArguments returns the first JSON value ONLY when the payload is
one value optionally followed by more WHOLE JSON values; it rejects a bad/truncated
first value OR trailing NON-JSON garbage ({"x":1}xyz) so genuine corruption still
errors (adversarial-review fix). decodeToolArguments uses it at the central dispatch
(executeToolCall) so {A}{B} runs the first object; historySafeToolCalls uses the
SAME helper so the replayed transcript records the recovered first object (not {}),
keeping dispatch and history consistent. Truly-malformed JSON still hits the
existing sanitize-to-{} + retry path. Trailing objects are dropped (the model
re-issues on a later turn).
2) Raise the shared transport ResponseHeaderTimeout 60s -> 120s so a slow cloud proxy
(ollama *:cloud) that withholds its 200 header until first token isn't wrongly
aborted; still bounds a truly dead reused connection.
Local only — not pushed.
* feat(local): add /turns command + raise default maxTurns 30->50
Agentic multi-step runs (build + delegate + swarm + MCP + self-verify) routinely hit
the 30-turn ceiling mid-task and never reached the later steps; removing /mode earlier
also left NO interactive way to raise the budget.
- /turns [n]: per-session tool-turn budget setter (commandGroupSession), mirroring
/selfcorrect. Empty/status shows the current budget; a positive int sets
agentOptions.MaxTurns (read by the run at model.go); clamped to maxTurnsCeiling=500
to guard against typos; invalid/non-positive shows usage without changing it.
- defaultMaxTurns 30 -> 50 (matches the old 'deep' preset). Raise per-run with /turns
for long tasks (e.g. /turns 150 for a full acceptance run).
Local only — not pushed.
* fix(local): swarm reports failed members as failed + sub-agents inherit /turns budget
Issue B — a swarm member whose child exited non-zero (e.g. exit 4 / max-turns) was
recorded [done] (swarm_status '0 failed'), so the orchestrator — which can't see the
AGENTS panel — trusted incomplete work. The specialist launcher ignored
res.Result.Status; it now surfaces a StatusError result as a member FAILURE
(errors.New(report)) so the swarm marks it [failed]. Added Coordinator.FailWithSession
and switched the member watch path to it so a failed member keeps its session id
(still drillable) and its report rides along as the failure message.
Issue A — a member hit its OWN turn limit because /turns raised only the parent's
budget; children re-resolved config.json's default. New config.SetMaxTurnsEnv exports
ZERO_MAX_TURNS, read in applyEnv -> cfg.MaxTurns; /turns now exports it so spawned
sub-agents / swarm members (which inherit the env) run with the same budget. A
non-numeric/zero env value never clobbers the configured budget.
Tests: launcher non-zero-exit -> failed (+ session kept); SetMaxTurnsEnv round-trip
+ garbage-env guard; /turns exports + clamps the env. Full gate green (73 pkg).
Local only — not pushed.
* fix(local): corrective error for unknown specialist name + clamp ZERO_MAX_TURNS
1) A model that invents a specialist name (validator-runner, file-creator,
file-writer — none registered) got an opaque 'specialist not found' and kept
spawning doomed sub-agents. loadManifest now lists the available specialists and
notes that 'worker' has shell/edit while explorer/code-review are read-only, so the
model self-corrects to a capable specialist. New availableSpecialistList helper.
2) Defense-in-depth (review nit): applyEnv now clamps an inherited ZERO_MAX_TURNS to
MaxTurnsCeiling, so a raw shell 'export ZERO_MAX_TURNS=999999' can't bypass the
/turns ceiling. Ceiling promoted to config.MaxTurnsCeiling (single source of truth,
referenced by the TUI /turns clamp).
Tests: unknown-name error lists worker/explorer/code-review; applyEnv clamps an
over-ceiling env value. Full gate green (73 pkg).
Local only — not pushed.
* fix(local): surface sub-agent kill signal instead of opaque exit -1
When a sub-agent child is terminated by a signal, command.Wait returns an ExitError
whose ExitCode() is -1, losing the cause. Capture ProcessState.String() (portable,
e.g. "signal: killed") into ChildRunResult.Signal and have BuildFinalResult render
it — "Subagent terminated by a signal (signal: killed) — killed before it finished.
Likely causes: the OS out-of-memory killer ... a timeout, or cancellation." — instead
of an opaque "Subagent failed (exit -1)". The message lists causes rather than
asserting OOM, since timeout/cancellation also SIGKILL the child. A captured signal
always surfaces even if a late run_end reported a clean exit. The background task
path (which can't carry the signal name) emits the same hint for a negative exit code.
* address CodeRabbit review on #349
- loop.go: gate stall-retry on a turn-level forwardedAnything flag (wrapped
callbacks), so a retry never re-streams on top of output already forwarded earlier
in the turn (e.g. across the reactive-compaction retry). New test covers a
reasoning-then-stall (collected.Text empty but output forwarded).
- loop.go: route a reissued stream's non-stall error through the SAME recovery as
the initial stream (image-rejection wrapping / context-limit compaction) via a
shared recoverStreamError closure, instead of returning it raw.
- exec.go: drop the hardcoded worker/explorer/code-review guidance from the
unknown-specialist error; rely on the dynamically-rendered available list (a
custom registry may not have those names).
- streamer.go: kill-signal message lists OOM/timeout/cancellation evenhandedly
instead of asserting OOM (the branch also covers timeouts/cancellations).
- launcher_specialist.go + exec.go: preserve the child SessionID on a post-start
failure too, so FailWithSession keeps those failed members drillable.
- model.go: block /turns while a run is active (mid-run change would make the
inherited ZERO_MAX_TURNS budget inconsistent for later-spawned sub-agents).
|
||
|
|
46c44a8753 |
feat(swarm): let swarm members build (write + sandboxed shell) (#313)
* feat(swarm): let swarm members build (write + sandboxed shell) Swarm subagents were effectively read-only: a member runs headless at "--auto low" (PermissionModeAuto), and ToolAdvertised strips every prompt-requiring tool from the model's list — so write_file/edit_file/ apply_patch/bash were never handed to the member. It only saw read/grep/ glob/plan tools, so it could not build and the orchestrator did all the writes itself. Add PermissionModeMemberAuto: a headless mode that ADVERTISES the in-workspace mutators (SideEffectWrite + SideEffectShell) on top of the Auto set, while behaving exactly like Auto everywhere else. The sandbox engine still decides at call time — in-workspace writes and sandbox-backed shell auto-allow; out-of-workspace writes, network, and destructive commands prompt and so are denied headless. member-auto normalizes to Auto in the sandbox layer, so no sandbox authority is widened beyond what an interactive auto agent already has. Scope: swarm members only. The launcher sets MemberAutonomy; the executor emits "--auto member" for a non-unsafe member (unsafe still -> high, plain specialists still -> low). Task-tool specialists are unchanged. Windows note: shell only auto-runs when the OS sandbox is active; without `zero sandbox setup` a member writes files but cannot run builds. * style: gofmt member_auto_test.go |
||
|
|
9e800b4ffc |
fix five concurrency/leak bugs (swarm, specialist, oauth, mcp, cron) (#265)
Surfaced by a full-tree audit; each ships a regression test where the behavior is deterministically testable. - swarm: deliver a handoff's mailbox note BEFORE registering the new task, so a mailbox failure can't leave a phantom pending task in the coordinator. - specialist: put a foreground sub-agent child in its own process group and group-kill on cancel/timeout (+ WaitDelay backstop), so a build/server it forked dies with it instead of orphaning. - oauth: serialize on-demand token refresh per key (keyed mutex) and re-load inside the lock, so parallel callers don't each spend the single-use refresh token (all but one would otherwise fail at the token endpoint). - mcp: treat a "source already gone" rename during legacy-token migration as success, so a concurrent migrator doesn't wrongly drop an OAuth MCP server. - cron: claim a due job atomically (advance NextRunAt under the per-job lock) before running it, so two schedulers can't double-fire the same job. |
||
|
|
b237e9849a |
feat(tui): sticky plan panel + specialist task cards + task table overlay (#255)
* feat(tui): sticky plan panel + specialist task cards + task table overlay Three new TUI surfaces that give users live visibility into agent progress and specialist delegation, so long turns and subagent work never look frozen. Sticky plan panel (plan_panel.go): - Pinned between the title bar and transcript body, shows the agent's step-by-step plan from the update_plan tool as a live checklist. - Header with spinner + progress count + elapsed; text progress bar; per-step list with status icons (✓ completed, ⠹ in_progress, ○ pending, ✗ failed) and individual step timings. - Auto-collapses to a one-line green summary 30s after completion. Ctrl+P toggles expand/collapse. - Plan state synced from update_plan tool results via CurrentPlan(); timestamps preserved across the tool's full-replacement updates by matching on content. Specialist cards (specialist_card.go): - New rowSpecialist transcript row kind; EventSpecialistStart/Stop now handled in session.go hydration (previously fell through to default and were silently dropped on resume). - Animated card with spinner + name + task + elapsed + token count + tool call count. Blue while running, green when done, red on error. - specialistTracker tracks live state; Task tool calls/returns are intercepted in the OnToolCall/OnToolResult callbacks to start/complete tracking and emit card rows. - Height cache marks running specialist rows unstable so the spinner animates. Task table overlay (task_table.go): - Ctrl+G toggles a full-width overlay showing all specialists in the session as a sortable table: #, Specialist, Task, Status, Time, Tokens, Tools. - Summary bar with running/completed/error counts, total tokens, aggregate elapsed. - Arrow keys navigate rows; Esc closes the overlay. Tests: plan_panel_test.go (7 tests), specialist_card_test.go (11 tests), task_table_test.go (8 tests). All existing TUI tests pass. * fix(update_plan): expose structured nested item schema so models stop sending bad arguments The plan property's Items schema was omitted because of an outdated comment claiming the serializer drops nested Properties/Required. The serializer (propertyToRuntimeMap) already handles them recursively. Without the nested schema, weaker models guessed the item structure from a text description and sent malformed arguments, hitting the 4-strike guardrail halt. Now exposes Items as an object with content (string, required), status (enum), and notes (string). * fix(tui): route plan + specialist updates through message sink, not stale closures The OnToolCall/OnToolResult callbacks captured model by value, so m.plan.updateFromItems() and m.specialists.start/complete() mutated stale copies that never reached the live model. Introduced planUpdateMsg, specialistStartMsg, and specialistCompleteMsg tea.Msg types sent through runtimeMessageSink, handled in updateModel() where they mutate the live model. * feat(tui): subchat drill-in for specialist sessions Enter on a task table row loads the child session's events and swaps the transcript view to show the subagent's full conversation (user messages, agent text, tool calls, results). A nav bar at the top shows the specialist name/task and a back hint. ArrowUp or Esc pops back to the main chat at the saved scroll position. - subchat.go: subchatState with enter/exit, renderSubchatNavBar - transcript_selection.go: transcriptBodyItemsFromRows renders arbitrary rows (reuses the same rendering pipeline as the main transcript) - model.go: Enter on task table row triggers drill-in; ArrowUp/Esc exits; transcriptView() short-circuits to child rows when subchat is active * fix(ci): strip CRLF line endings and BOM from plan_tool_test.go and update_plan.go gofmt and git diff --check failed on ubuntu CI because PowerShell's Set-Content wrote UTF-8 with BOM and CRLF. Stripped both to plain LF without BOM. * feat(tui): add renderLeftRuleCard helper for borderless specialist cards * feat(tui): switch specialist cards to left-rule rendering, drop hint line * feat(tui): add renderSpecialistSummary one-line rollup for specialist cards * fix(tui): correct slice offset in renderSpecialistSummary and add pluralization test The muted tail was sliced at summary[len(spinnerView)] but the spinner sits at byte offset 2 (after the 2-space indent), so the slice cut into the spinner's UTF-8 continuation bytes and dropped the indent. Slice at 2+len(spinnerView) instead. Also add a test asserting '2 errors' is present and '2 error' (singular) is absent when two specialists errored. * refactor(tui): remove task table overlay — cards carry everything now * feat(tui): render specialist summary line above cards in transcript * feat(tui): mark specialist card lines as clickable selectable rows * feat(tui): click specialist card to drill into subchat, Enter as keyboard fallback * tui: clear selectedSpecialistID after Enter drill-in Fixes Task 7 review issue: selectedSpecialistID persisted after Enter drill-in, causing a subsequent Enter to re-drill into the specialist's subchat instead of submitting the composer. Clear it immediately after subchat.enter succeeds so the keyboard fallback is one-shot per click; click re-sets it, so click->Enter still works. * chore: gofmt model.go + test files * chore: gitignore .superpowers/ and docs/superpowers/ — local skill artifacts * fix(tui): remove Enter keyboard fallback for subchat drill-in — mouse click only * fix(tui): reconcile specialist childSessionID from tool call ID to real session ID on completion * feat(specialist): add Progress callback field to RunOptions and TaskRunOptions * feat(specialist): stream child stdout line-by-line via Progress callback * feat(agent): wire OnToolProgress callback for specialist child events * feat(tui): show live tool-call progress in specialist cards * fix(swarm): update RunChild mock signature for new progress callback param * fix(tui): render plan panel inline in transcript, not pinned at top * chore: gofmt model.go, specialist_card.go, specialist_click_test.go * fix(tui): address PR review - plan panel without specialists, stream reader safety, resume dedup + payload keys |
||
|
|
03784268ac |
token efficiency: lossless cost accounting + minified file reads (read_minified_file) (#256)
* token efficiency: lossless cost accounting + minified file reads Two complementary improvements to how Zero handles tokens. ACCOUNTING — make cost measurement lossless and correct: - Carry all five token dimensions end-to-end (input, cache-read, cache-write, output, reasoning) in one Usage; add CacheWriteTokens (cache creation), which the Anthropic adapter previously parsed then discarded. - Price cache-writes at their premium rate (Anthropic 1.25x input, derived per model). Regression-safe: a model without the rate bills cache-write at the input rate exactly as before. - Persist cache-read/cache-write/reasoning in EventUsage so `zero usage report` reconstructs the SAME cost as the live tracker instead of over-pricing cache-heavy sessions. A single EventUsagePayload writer is paired with the reader so the JSON keys can't drift. - Add a live context-window gauge to the status bar (used/window + percent, graded green/amber/red) using the last request's input tokens. REDUCTION — read_minified_file, a dense token-cheap file view: - New internal/minify package + read_minified_file tool returning a comment-free, line-number-free view: Go via the real go/parser AST; C/Java/CSS/SCSS/LESS/ Python via a string-aware comment lexer (handles strings, char literals, triple-quotes, Java text blocks, Python f-strings incl. 3.12 same-quote nesting). Every other language falls back to whitespace-only, so the stripper can never corrupt content. - ~32% fewer tokens on a realistic multi-file read; +~102 fixed tokens for the tool schema, recovered many times over by a single subsystem read. Tests: cache-write pricing + fallback, lossless persistence round-trip, the context gauge, comment-stripping golden cases per language (comment-chars in strings, Java text blocks, Python f-string nesting, CSS has no //), and the unsupported-language safe fallback. gofmt / vet / build / -race all green. * specialist: register read_minified_file in the tool registry + read-only sets Smoke caught TestKnownToolNamesMatchCoreRegistry drifting once read_minified_file joined the core read-only tools: the specialist tool allowlist (knownToolNames), the read-only/edit/execute categories, and the read-only auto-approve set did not list it. It is a read-only companion to read_file, so it belongs alongside read_file in all of them; updated the default-allowlist test accordingly. |
||
|
|
995b9086c7 |
feat(subagent): proactive specialist delegation + swarm execution fixes (#243)
* feat(subagent): proactive specialist delegation + swarm execution fixes Fixes: - Task tool dropped the parent's permission mode, so every delegated sub-agent ran at --auto high regardless of the parent. Forward PermissionMode so a child never exceeds its parent's authority. - Swarm members never executed: swarm_spawn accepts agent types teammate/subagent, but the launcher looked those up as specialist names (only worker/explorer/code-review exist), so every member failed with "specialist not found". Run each member from its swarm definition via an inline manifest on the executor. Enhancements: - Auto-delegation: the orchestrator system prompt now lists the available specialists and when to use each, nudging it to offload read-heavy work to a sub-agent so large tool output stays out of the main context. - @-mention: a leading "@<specialist> <task>" in the TUI composer delegates to that specialist (transcript keeps the verbatim mention); the @ palette suggests specialists on the first token, file references mid-message. - swarm.maxTeamSize config caps concurrent members per team (0 => default 8; negative rejected), honored in user and project config. - Proactive spawning: read-only specialists (explorer/code-review) auto- approve via a new args-aware tool permission (tools.ArgsPermissioner), so the agent delegates exploration off a normal prompt without an approval prompt; write-capable specialists, resume, and unknown names still prompt. The exec gate registers specialist tools at medium/high autonomy so a non-trivial headless run can auto-delegate read-only too. * fix(specialist): fail-safe child autonomy when PermissionMode is unset specialistAutonomy mapped an empty PermissionMode to "--auto high" (unsafe), silently re-introducing the privilege escalation the rest of this PR fixes: an orchestrator that forgets to wire PermissionMode would get an unsafe child. The Task tool and swarm both now propagate a resolved mode, so "" only occurs for a caller that omitted it -- default that case to "low". Only an explicit unsafe parent now yields --auto high; everything else (incl. unset/unknown) is fail-safe low. Updates the stale "back-compat" comments and the tests that documented the old behavior. Addresses the non-blocking follow-up from PR review. |
||
|
|
9ddef0e957 |
feat(swarm): multi-agent swarm over the specialist sub-agent (#207)
* feat(swarm): multi-agent swarm over the specialist sub-agent Add internal/swarm so an orchestrator can spawn and coordinate MULTIPLE specialist members concurrently under one team, communicate via per-agent mailboxes, hand work off, and adopt orphaned tasks. Additive and opt-in: the single Task tool is unchanged; swarm tools only act when invoked. - Registry/Definition: agent roster (built-in teammate/subagent + user-defined). - Mailbox: per-team/per-agent JSON inbox, owner-only 0600, file-locked with stale-break, size-capped, path-confined, and malformed-fails-closed. - Coordinator: in-memory task registry with terminal-state guards and stable per-agent colors. - Team: bounded concurrent members per team with a FIFO overflow queue that drains one-per-slot. - Lifecycle: spawn / handoff / orphan-adoption with bounded relaunch on temporary failures; members run under the Swarm's base context so they outlive the spawn call. - Tools: swarm_spawn, swarm_send, swarm_inbox, swarm_status, swarm_handoff, swarm_collect. - Wiring: each member is launched through specialist.Executor and inherits the orchestrator's model + permission mode (never widened) and runs under the same sandbox/policy. Mailbox state lives under the workspace so writes fall within the sandbox write rules. Swarm lifetime is paired with the specialist runtime. Scheduling (cron/wakeup) and daemon-pool-backed members are deferred follow-ups. * swarm: address CodeRabbit review (7 findings) - coordinator: SetStatus rejects unknown TaskStatus values (fail closed) so a task can never enter a state Summarize/lifecycle don't reason about. - permission inheritance ENFORCED end-to-end (was resolved but discarded): thread PermissionMode through specialist.TaskRunOptions -> BuildArgs/ BuildResumeArgs. A headless specialist child now runs "--auto high" (unsafe) ONLY when the parent is unsafe or no mode is given (preserves the Task tool's behavior); any other parent mode yields non-unsafe "--auto low", so a member never gains more authority than its orchestrator. Also fixed the swarm permission vocabulary to the real agent runtime modes (ask/spec-draft/auto/ unsafe) instead of TUI display names, and the launcher substitutes "auto" for an empty mode so a member can't fall into the empty-means-high path. - mailbox: symlink-aware confinement (EvalSymlinks) rejects an inbox dir that resolves outside BaseDir; dirs tightened to 0700 even if pre-existing with broad perms; lock release is ownership-token-aware so a stale-broken writer can't delete a newer writer's lock (split-brain). - member: recover panics inside a member goroutine -> fails that member only, never crashes the orchestrator. - tools: collapse() truncates on rune boundaries (no invalid UTF-8). Tests added for every fix. Existing specialist behavior is unchanged (empty PermissionMode keeps --auto high). Gates: gofmt/vet/build (host+linux)/test -race/staticcheck(no new)/govulncheck(0)/deadcode(no new) all pass. * swarm: address review round 2 (Windows lock + non-blocking notes) Blocking (Vasanth): mailbox acquireLock now treats Windows ERROR_ACCESS_DENIED / ERROR_SHARING_VIOLATION as lock-contended (retry/wait) instead of a fatal error, via a platform-aware isLockContended (lock_windows.go / lock_other.go). A contended lock under Windows file-sharing semantics no longer fails a send/read spuriously. TestMailboxConcurrentSends raised to 200 goroutines with a 10s lock timeout and now asserts zero send failures (regression guard). Non-blocking notes addressed: - Removed third-party reference-project names from doc comments (clean-room + provider-neutral); the Go code is original. - Documented the inbox-file size bound's ×2 (pretty-print overhead) constant. - Documented Send's O(N) rewrite cost + when to switch to append-only. - permissionRank: spec-draft now ranks below ask (it never widens authority). - Documented watch's per-member-id retry binding (Member.ID == MemberSpec.ID). - handoff Safety.Reason now also mentions the inbox write. Gates: gofmt · vet · build (host+linux+windows) · test -race · staticcheck (no new) · govulncheck (0) · deadcode (no new) all pass. |
||
|
|
474f494b93 |
Deep-audit remediation + sandbox hardening (batches 1–35) (#189)
* audit batch 1: redact nested secrets fully; count image tokens in compaction estimate
- secrets.Redact: replace longest matches first so a containing PRIVATE KEY
block is redacted before a nested shorter match corrupts its exact string
(audit medium internal/secrets/scanner.go:81) + regression test.
- agent compaction estimateTokens: add a flat per-image token cost so an
image-heavy context trends toward compaction instead of reading as ~0
(audit medium internal/agent/compaction.go:58) + test.
- ledger: re-verify against current main; mark scanner.go:35 and
redaction.go:362 already-fixed-on-main, record this batch's fixes.
* audit batch 2: worktrees defaultRunGit separates stdout/stderr
Capture git stdout and stderr into separate buffers instead of CombinedOutput,
so parsed rev-parse output is not polluted by git warnings and CommandResult.Stderr
carries the real error text (audit medium internal/worktrees/worktrees.go:247) + test.
* audit: re-verify all 87 pending findings against current main
Independent agent per file re-checked each open/partial finding against the
actual current code (the 06-11 ledger was verified on a since-diverged branch),
with an adversarial second pass on every already_fixed verdict. Authoritative
result: 78 genuinely pending (66 still_open + 12 partial); 9 rows were already
fixed on main and are reclassified. Full table in 2026-06-13-reverification.md.
* audit batch 3 (agent): cancellation identity, always-allow w/o sandbox, compaction usage, dedup
- loop.go: check ctx.Err() before collected.Error so a cancelled run returns
context.Canceled (errors.Is holds) instead of a plain stringified error.
- loop.go: 'Always allow' with no sandbox engine now allows THIS call instead of
denying it (persistence is skipped when there's nowhere to persist).
- loop.go: remove a duplicate schema["items"] assignment in propertyToRuntimeMap.
- compaction.go: forward OnUsage (token accounting) through the summarizer while
still omitting OnText, so compaction's token cost is counted in usage/budgets.
- tests for all three behavioral fixes (each fails pre-fix).
* audit batch 4 (cli): list-tools -o json, interrupted -o json terminal, surface session-record failures
- exec --list-tools -o json now emits a JSON tool listing instead of falling
through to the human-readable text (audit medium exec.go:182) + test.
- interrupted run with -o json now emits a terminal error+done on stdout instead
of only printing to stderr, so a JSON consumer sees a clean end (medium exec.go:500) + test.
- execSessionRecorder: surface a latched session-append failure to stderr once at
run end (deferred) instead of silently dropping it (medium exec_sessions.go:114/115) + test.
- left app.go trailing-arg ignore as-is: documented-intentional, TUI has no argv prompt.
* audit batch 5 (zerogit): status -z porcelain (renames + non-ASCII paths); document env-drop on plain Runner
- parseStatus: switch `git status` to `--porcelain -z` and parse the
NUL-delimited form (audit medium zerogit.go:212). Previously the default
--short output was split on newlines, so a rename `R old -> new` was kept as
one bogus path and a non-ASCII/whitespace filename arrived C-quoted+escaped
(e.g. `"caf\303\251.txt"`). The -z form never quotes paths and emits a
rename/copy as `XY <dest>\0<src>`, so we now record the destination verbatim
and consume the source field. + TestParseStatusZHandlesRenamesAndSpecialPaths.
- resolveRunners: document that the plain-Runner EnvRunner adapter intentionally
drops env (audit low zerogit.go:417). The branch is reached only when a caller
supplies a custom Runner without a RunGitEnv — in practice fake-Runner tests.
Production leaves both nil and gets defaultRunGit/defaultRunGitEnv, which honor
GIT_INDEX_FILE, so snapshot-diff isolation holds. Routing env calls to real git
instead would break the fake-Runner test pattern and fix nothing in production.
- update Inspect/Commit mock tests to the -z status format and the new command.
* audit batch 6 (config): reject negative maxTurns; drop dead contract-gap helpers
- FileConfig.UnmarshalJSON now rejects a negative maxTurns instead of letting the
`MaxTurns > 0` merge gates silently drop it and fall back to the default,
hiding the misconfiguration (audit medium resolver.go:136). 0 is left as the
"unset" sentinel (omitempty) and still falls back to the default; the CLI
--max-turns flag continues to reject 0 too since there it is distinguishable
from absent. + TestResolveRejectsNegativeMaxTurns / ...ZeroFallsBackToDefault.
- remove dead config/contracts.go (ContractGap, DefaultContractGaps,
FindContractGapsByMilestone, ContractOwnerRuntime) and its test — referenced
only by that test, never by production (audit low contracts.go:12).
- NOT changed: ResolveMCP intentionally does not run the provider command
(TestResolveMCPDoesNotRunProviderCommand guards this), so the "ZERO_PROVIDER_COMMAND
MCP servers dropped" row is by-design, not a defect.
* audit batch 7 (mcp): bound captured MCP server stderr
connectStdio attached a plain bytes.Buffer as cmd.Stderr that is only ever read
on initialize failure, yet it kept growing for the entire process lifetime as a
long-lived server logged to stderr (audit medium client.go:98). Replace it with
a concurrency-safe boundedBuffer capped at 64 KiB that keeps the head (enough for
init-error diagnostics) and discards the rest, reporting full write lengths so
os/exec never sees a short write. + TestBoundedBufferCapsRetainedBytes.
Left as-is (not defects worth the risk in the MCP client):
- client.go writer.write under client.mu: already designed to release the lock
before the unbounded response wait; the residual concern is a server that stops
draining stdin, an involved async rewrite for a theoretical deadlock.
- readLoop not answering server-initiated requests (ping): responding safely
needs re-locking around the shared writer; no configured server relies on it.
* audit batch 8 (tools/providers): read_file truncation marker; drop dead provider branch
- read_file: when a max_lines cut occurs, append an explicit "[truncated: N more
line(s)... set start_line=X to continue]" marker to the output. Previously only
the Result.Truncated flag was set, which is invisible in the rendered text, so
the model could not distinguish a cut read from a complete one (audit low
read_file.go:89). + TestReadFileToolMarksTruncation.
- providers/factory: simplify `if !explicitProvider || isImplicitOpenAI(...)` to
`if !explicitProvider` and delete the now-unused isImplicitOpenAI. The clause
was dead: explicitProvider==true implies ProviderKind or Provider is set, but
isImplicitOpenAI required both empty, so it could never contribute a case
(audit low factory.go:118). Behavior-preserving; providers tests still green.
* audit batch 9 (sessions): Tree builds from a single List() snapshot (no N+1 disk scans)
Store.Tree recursed via ListChildren per node, and each ListChildren ran a full
store.List() that re-read every metadata.json from disk — O(nodes * total-sessions)
reads for one tree (audit medium lineage.go:140). Snapshot all sessions once, index
children by parent in memory, and recurse over that. Child ordering is factored into
sortChildSessions so ListChildren and Tree stay identical; path-scoped cycle
detection (delete(seen,...)) is preserved. Existing tree/lineage tests still pass.
Left as-is: store.go timestamp() second precision (RFC3339) — switching to
RFC3339Nano would break lexical UpdatedAt ordering against existing second-precision
records ("...00.5Z" sorts before "...00Z"), so it is not a safe one-line change.
* audit batch 10 (specialist): serialize accounting dedup-then-append
recordSpecialistStop and appendSpecialistUsageRollup checked for an existing event
(specialistEventExists → ReadEvents) and then appended under SEPARATE store locks,
so two concurrent finishers — a foreground onExit racing a TaskOutput poll, or a
background reaper — could both pass the dedup check and write duplicate stop/usage
events (audit medium accounting.go:72). Guard each check-then-append pair with a
package-level accountingMu (Executor is used by value, so a struct field would not
be shared). + TestRecordSpecialistStopDedupesUnderConcurrency (passes under -race).
* audit batch 11 (zeroruntime/gemini): O(1) text accumulation; gemini cached-token counts
- zeroruntime CollectStreamWithOptions accumulated assistant text with
`collected.Text += event.Content`, reallocating the whole string on every chunk
(O(n^2) over a long streamed response). Accumulate in a strings.Builder on the
collector and materialize it once in flush, the single finalization point hit
before every return (audit low helpers.go:104). Existing collect tests still pass.
- gemini decoded usageMetadata without cachedContentTokenCount, so a cached-prompt
response reported zero cached input tokens while the other providers report them
(audit medium gemini/types.go:88). Decode the field and thread it through
streamState into NormalizeUsage as CachedInputTokens. + extended
TestStreamCompletionEmitsTextUsageAndReasoningTokens.
* audit batch 12 (specialist): parse child stream without a line-length cap; drop dead helper
- ParseStream used a bufio.Scanner capped at 1 MiB, so one large stream-json line
from a child (a big tool result or final answer) hit bufio.ErrTooLong and aborted
the entire specialist run (audit medium streamer.go:43). Switch to a bufio.Reader
with ReadString, which has no per-line limit; the child is our own trusted
subprocess, so its output length is the legitimate bound. Line numbering and
blank-line/EOF handling are preserved. + TestParseStreamHandlesLineLargerThan1MiB.
- remove dead formatTaskOutput (output_tool.go:244): zero callers and zero tests;
it only forwarded to formatTaskOutputSummary, which is the one actually used.
* audit batch 13 (background/specialist): kill background tasks as a process group
Background children were launched in the parent's process group and terminated by
signalling only the single PID, so any process the task forked was orphaned and
outlived a cancel; the signal also risked hitting a recycled PID after the child
was reaped (audit medium process_posix.go:38/50).
- ConfigureChildProcessGroup (Setpgid on POSIX, no-op on Windows where taskkill /T
already tree-kills) is applied at launch so the child leads its own group.
- terminateProcess now SIGTERM/SIGKILLs the negative PID (the whole group), with a
pid>1 guard so a bogus 0/1 can never expand to "our own group" or "everything",
and treats ESRCH as already-gone. Liveness polls the group. Targeting a group
(rarely recycled vs. a bare PID) also shrinks the reaped-then-recycled window.
- specialist launchBackgroundProcess opts every child into its own group; on a
SetPID failure it now kills the child + group, marks the task errored, and records
the stop (deduped) instead of leaking an untracked orphan.
- tests launch as group leaders; new TestTerminateProcessKillsForkedChildren proves
a forked grandchild dies with the group.
* audit batch 14 (sandbox): quote-aware shell segment splitting
splitShellSegments used a quote-unaware strings.Replacer, so a shell operator
inside quotes was treated as a separator: `git commit -m "use top | less"` split
into a `less` segment and `echo "a; vim b"` into a `vim` segment, both falsely
flagged as interactive (audit medium safe_command.go:138).
Replace it with a quote-aware scanner that mirrors the shell: single quotes make
everything literal; double quotes keep $(...)/`...` substitution active but treat
|, ;, && as literal; unquoted text splits on the same operator set as before
(;, |, ||, &&) plus substitution boundaries. Real unquoted operators still split,
so an interactive program behind a separator — or inside a live `"$(...)"`
substitution — is still caught (no new false negatives, the security-relevant
direction). + TestDetectInteractiveQuoteAwareSeparators covering both directions.
* audit batch 15 (cron): collapse DST fall-back repeated hour to one fire
On a fall-back day the clock repeats an hour (e.g. 01:30 EDT then 01:30 EST), so
Next("30 1 * * *", <01:30 EDT>) returned the same-day 01:30 EST — firing a daily
job twice (audit medium schedule.go:171, which previously guarded only the
spring-forward gap).
Next now skips a match whose local wall-clock minute equals `after`'s but whose
absolute time is later (the repeated-hour duplicate) and keeps searching. Normal
forward search always advances the wall-clock, so this only triggers on the
fall-back repeat; when the scheduler passes the last fire time as `after`, the
repeated hour collapses to a single fire. + TestNextDSTFallBackDoesNotRepeatHour
(spring-forward termination test still passes).
Note: Next is stateless, so it dedupes relative to `after`; a caller that passes
an arbitrary mid-hour "now" instead of the last fire time is out of scope here.
* audit batch 16 (mcp): skip SSE notifications before the response; larger SSE buffer
- decodeSSERPCMessage returned the first non-empty `message` event unconditionally,
so a server-initiated notification/request arriving before the response made the
caller see an id mismatch and fail (audit medium network_client.go:621). It now
skips any message carrying a method (notifications/requests have one; a response
does not) and returns the first real response; the caller's rpcIDMatches still
validates the id. + TestDecodeSSERPCMessageSkipsNotifications.
- raise the per-event SSE scanner cap from 1 MiB to a bounded 8 MiB so a large but
legitimate MCP message no longer hits bufio.ErrTooLong and fails the request
(audit medium network_client.go:637); still bounded against an unbounded server.
Out of scope (kept): full reconnect of a persistent SSE stream after a fatal read
error — a larger change than this remediation.
* audit batch 17 (sessions): fsync session metadata before the atomic rename
writeMetadata wrote the temp file with os.WriteFile (no sync) and renamed it into
place, so a crash right after the rename could expose a metadata file whose bytes
were never flushed — a torn or empty file that corrupts the whole session (audit
medium store.go:632). Write+fsync the temp via a new writeFileSync helper before
renaming, closing that window. + TestWriteFileSyncRoundTrips.
Deliberately scoped:
- Event-log appends (appendEventLocked) are NOT fsynced per append: that would add
a disk flush to every event (every tool call), and a hard-crash loss of the log's
unsynced tail is acceptable, whereas torn METADATA is not. Metadata is the
integrity-critical part and is what this syncs.
- Directory fsync is omitted for cross-platform simplicity (it errors on Windows);
the file fsync already prevents the torn-contents failure, the serious case.
Left for follow-up (in this bucket but not safe/clear wins here):
- hooks append sequence still re-reads the file per append; the in-memory cache is
cross-process-unsafe and the safe last-line read is fiddly for a low-volume log.
- Fork event re-append/de-dup is nuanced usage-accounting and needs its own change.
* audit batch 18 (agent): count tool-defs in compaction estimate; don't deny an always-allow on persist failure
- maybeCompact's estimate omitted the tool definitions, which ride on every
request, so the real input could exceed the model limit while the message-only
estimate stayed under threshold and compaction never fired (audit medium
compaction.go:58). partitionTools is now computed before maybeCompact (it does
not depend on the messages) and the exposed tools' estimated tokens are added to
both the threshold check and the post-compaction shrink check. + estimateToolDefTokens
and TestEstimateToolDefTokensCountsDefinitions.
- PermissionDecisionAlwaysAllow denied the call when persistPermissionGrant failed,
overriding what the user explicitly allowed (audit medium loop.go:468). Persisting
is now best-effort: a failure (or no sandbox engine) just means the grant is not
remembered and the user is re-prompted next time, not denied now. This also drops
the dead requestEvent.GrantMatched/Grant writes (audit low loop.go:473) — the
emitted event derives them from the sandbox decision via buildPermissionEvent.
Left intentional (documented, not defects): the reactive-retry path omits OnText to
avoid duplicating mid-stream text (loop.go:183); OnContext/MeasureContext is a
complete, tested context-breakdown scaffold awaiting a consumer (types.go:183).
* audit batch 19 (usage): price escalated usage from the event's own model
exec persists payload["model"] on --allow-escalation runs (the model can change
mid-run), but BuildReport priced every event from the session's Metadata.ModelID
and never read the payload, so usage from an escalated model was mis-priced at the
base model (audit medium report.go:17). usageEventPayload now carries Model and
BuildReport prefers it, falling back to the session model when absent (older
events stay byte-identical and behave as before). + TestBuildReportPricesFromEventModelWhenPresent.
* audit batch 20 (sessions): don't copy usage events into a fork (no double-count)
Fork re-appended every parent event, including provider_usage, into the child, so
a usage report aggregating the parent and the fork double-counted the parent's
usage (audit medium store.go:387). Skip EventUsage when copying — it already
counted against the parent and is not part of the conversation history a fork
replays — and record the actual copied count in the fork marker. + TestStoreForkSkipsUsageEvents.
* audit batch 21 (cli/skills): warn on duplicate skill names instead of silently dropping
skills.Load deduplicates same-named skills (first directory wins) and records the
dropped collisions via skills.Duplicates, but nothing surfaced them, so a shadowed
skill just vanished with no signal — and Duplicates had no production caller (audit
low skills.go:91). `skills list` now reports each collision to stderr (keeping
stdout/--json clean), using Duplicates. + TestRunSkillsListWarnsOnDuplicateNames.
Note: skills.Get is kept — it is a by-name accessor used throughout the package's
tests, not dead code.
* audit batch 22 (cli/cron): capture the run error from stream-json stdout
Cron jobs run exec with --output-format stream-json, so a failure is reported as an
`error` event on STDOUT, but the run record's Error was set only from stderr — so a
failed job often recorded an empty/uninformative error while the real message sat
unread in outBuf (audit low cron_run.go:150). On a non-zero exit, scan the stdout
stream-json for the error event's message and prefer it, falling back to stderr.
+ extractStreamJSONError and TestExtractStreamJSONError.
* audit batch 23 (sandbox): richer macOS seatbelt profile (signal + mach-lookup)
The generated sandbox-exec profile was default-deny with only process/sysctl/file
allowances, so two real gaps remained on macOS (sandbox quality report H1):
- A sandboxed script could not signal the children it spawned — `kill` returned
"Operation not permitted" under seatbelt, breaking common `cmd & ... kill`,
test-runner, and timeout patterns. Add `(allow signal (target self) (target pgrp))`,
which stays scoped to the command's own process group.
- XPC to system daemons was denied, so keychain (securityd/trustd), user/group
lookup (opendirectoryd), preferences (cfprefsd), network config
(SystemConfiguration), launch services, and the pasteboard all failed. Add a
curated `(allow mach-lookup ...)` allowlist (`sandboxMachServices`).
Neither touches the file-write or network rules, so the workspace boundary is
unchanged — verified on a real macOS host: `security`/`scutil`/`pbpaste`/self-kill
now succeed while writes to $HOME and /etc are still denied. iokit-open was tried
and dropped as unnecessary. + unit assertions (TestSandboxExecProfileGrantsSignalAndMachLookup)
and extended the darwin integration test with a self-kill case and an
outside-workspace deny check.
* audit batch 24 (sandbox/tools): remove dead OnSandboxDecision hook
RunOptions.OnSandboxDecision had no setter anywhere, yet registry.RunWithOptions
spawned a goroutine (with panic recovery) for it on every sandboxed tool call —
pure per-call overhead for an unreachable callback (sandbox report M1). The sandbox
decision is already surfaced to observers via the permission event
(buildPermissionEvent reads the decision), so this hook is redundant; delete the
field and its call site.
* audit batch 25 (sandbox/cli): exact-path grants — create and revoke by path
The engine already matched ScopeFile grants and DeriveScope already produced them
from path/file tool args, but the CLI could neither create one explicitly nor
revoke a single one (sandbox report L3 / prompt 7). Add:
- GrantStore.RevokePath(tool, path): revokes only the grant whose scope matches the
given file/dir, leaving tool-wide and other-path grants intact (path canonicalized
to absolute like stored scopes). + TestGrantStoreRevokePathRemovesOnlyMatchingScope.
- `zero sandbox grants allow <tool> --path <p>`: persists an exact-file (ScopeFile)
grant, path resolved to absolute so it matches lookups and revokes.
- `zero sandbox grants revoke <tool> --path <p>`: revokes just that grant (default
still revokes all grants for the tool). + TestRunSandboxGrantsCreateAndRevokeByPath.
The TUI permission-card "Allow always (exact path)" option is intentionally left for
the TUI feature branches (it overlaps #187/#188); the engine + CLI now cover the
full create/match/revoke round-trip for file-scoped grants.
* audit batch 26 (sandbox): Engine.Precheck — report violations before execution
Add Engine.Precheck(ctx, request) []Violation, which reports the sandbox
violations that would block a tool request before it runs (sandbox report / prompt 6).
It reuses Evaluate so policy is not duplicated: an allowed or merely-prompted
request yields no violations; a denied request yields its violation (via the shared
violationsFromDecision helper, with a ViolationPolicyDenied fallback for a deny that
carries no structured violation). A nil engine yields nothing.
+ TestEnginePrecheckReportsViolationsBeforeExecution.
Wiring Precheck into the batch-confirmation prompt is intentionally left to the TUI
branch: it touches prompt rendering, and short-circuiting the agent loop's deny path
would skip the before/after-tool hooks that currently fire around a policy-denied
call. The engine API is in place for that consumer.
* audit batch 27 (sandbox): scoped-egress domain prompt with timeout + caching
The scoped-egress proxy failed closed on any host outside the allowlist. Add an
opt-in DomainPrompt callback (prompt 8): an UNKNOWN host (neither allowed nor
explicitly denied) is authorized at request time via the callback instead of being
refused outright. An explicitly denied host is still refused without prompting; the
callback is bounded by PromptTimeout (default 60s, timeout/error => deny); decisions
are cached for the proxy's lifetime so a host is prompted at most once. With no
callback set the strict fail-closed behavior is unchanged. Wired through both the
CONNECT and HTTP handlers via authorizeTarget.
+ TestEgressProxyAuthorizeDomainPrompt / ...TimeoutDenies / ...NilPromptFailsClosed.
The user-facing prompt that supplies the callback is left to the permission UI (TUI)
branch; the egress mechanism (callback + timeout + cache) is in place and tested.
* audit batch 28 (sandbox): AST-based shell analysis (AnalyzeCommand)
Add AnalyzeCommand(script) AnalysisResult, a static shell analyzer built on the
mvdan.cc/sh/v3 parser (prompt 12). Walking the parsed command tree makes it a more
precise second opinion than the regex detector: a program name only counts when it
is an actual command, so `echo "vim ..."`, `printf 'open with vim'`, and
`git commit -m "rm -rf /"` are clean, while real commands are flagged —
interactive (editors/pagers + REPLs, with the same -e/script suppression as
nonInteractiveREPLFlags), destructive (rm -rf/-fr, dd, mkfs, shred, ...), and
network (curl/wget/ssh/nc/...). An unparseable script yields TooComplex so callers
can treat it as higher-risk. + TestAnalyzeCommand (16 cases) / ...EmptyIsClean.
Dependency: mvdan.cc/sh/v3 pinned to v3.11.0 — compatible with the existing go 1.24
directive (the latest v3.13.1 would have bumped the module to go 1.25). It is kept
as a standalone analyzer for callers to consult; feeding it into Engine.Evaluate is
deliberately not done here, since that would change allow/deny classification.
* audit batch 29 (sandbox): unify proxy-env generation (ProxyEnv)
Export ProxyEnv as the single source of truth for proxy-env injection (prompt 9
req 1), replacing the unexported scopedProxyEnv used by the sandboxed-shell path.
+ TestProxyEnv.
Investigation that bounds the rest of prompt 9 (documented in the ProxyEnv doc):
- web_fetch already honors HTTP_PROXY/HTTPS_PROXY — its transport clones
http.DefaultTransport, whose Proxy is http.ProxyFromEnvironment.
- MCP children already inherit the parent env (mergeProcessEnv appends os.Environ),
so they pick up any proxy vars the agent has.
So the only missing link to scope FetchUrl/MCP is a SESSION-level egress proxy whose
address reaches the agent process. That is deliberately NOT wired here: doing it
safely requires allowlisting the active LLM provider's own domain first, or the
agent's provider calls would route through the scoped proxy and be denied — a
correctness pitfall that also can't be verified end-to-end on this host. Flagged
rather than shipped blind.
* audit batch 30 (sandbox): Linux seccomp Unix-socket block — UNVERIFIED, needs Linux CI
Adds a pure-Go (no cgo, no vendored binary) seccomp BPF filter that denies
socket(AF_UNIX) with EPERM on x86-64/arm64, closing the Unix-socket gap bubblewrap's
filesystem/network isolation leaves open (prompt 11):
- internal/sandbox/seccomp.go: the classic-BPF program as a platform-neutral
[]sockFilter, built and UNIT-TESTED (structure + every jump offset in range — the
classic way a hand-written BPF filter goes silently wrong).
- seccomp_linux.go: ApplyUnixSocketBlock installs it via prctl(NO_NEW_PRIVS) +
prctl(SET_SECCOMP, MODE_FILTER). seccomp_other.go is a no-op everywhere else.
- cmd/zero-seccomp: a tiny linux-only exec wrapper that applies the filter then
execs the real command (the standard way to install seccomp in a child with
os/exec); degrades gracefully (warns + runs) if the kernel lacks seccomp.
⚠️ HONEST CAVEAT: I developed this on macOS and CANNOT run the actual AF_UNIX block
here. darwin build, `GOOS=linux` cross-compile, vet, and the structure test all pass,
but the unit test verifies the program's SHAPE, not that the kernel actually blocks
AF_UNIX — that MUST be verified on Linux CI (create a Unix socket under the wrapper
and assert EPERM). Safe to ship as-is: it is linux-only, no-op on every other
platform, and not wired into the bwrap command yet (opt-in prefixing the sandboxed
command with zero-seccomp is the remaining integration), so it cannot regress any
current path.
* audit batch 31 (sandbox): macOS denial log monitor (opt-in, default off)
Re-adds the macOS seatbelt-denial monitor as an opt-in feature (prompt 10). When
Policy.MonitorDenials is set, the sandbox-exec profile tags its default-deny with a
marker and bash tails `log stream` for that tag during the command, appending any
captured denials as a <sandbox_violations> block to the command's stderr so the
model can see what was blocked.
- DenialMonitor (darwin) tails/parses tagged denials with a capped, deduped ring
buffer and noise filtering; no-op stub on other platforms. parseSandboxDenyLine is
unit-tested. + profile-tag test + appendSandboxViolations test.
- Default OFF: with MonitorTag empty, StartDenialMonitor is a no-op and the command
path is byte-for-byte unchanged, so there is no regression or per-command overhead
unless enabled.
CAVEAT: on at least one macOS host (verified live) seatbelt denials are NOT delivered
to the `log stream`-queryable unified log, so the monitor captures nothing there. It
is the standard mechanism and works on macOS versions that do log denials; shipping
it default-off means it never affects whether a command runs.
* audit batch 32 (sandbox): wire seccomp Unix-socket block into bubblewrap (opt-in)
Completes prompt 11: the zero-seccomp helper (which installs the AF_UNIX seccomp
filter) is now actually invoked by the sandbox instead of only existing as a
standalone binary.
- bubblewrapCommandPlan, when Policy.BlockUnixSockets is set, ro-binds the
discovered zero-seccomp helper into the sandbox at its host path and prefixes the
command with it so the filter is installed before the real argv runs.
- findSeccompHelper discovers the helper next to the running executable first, then
on PATH; it returns "" when absent, so the sandbox degrades gracefully (runs
without the extra filter — bubblewrap's fs/net isolation still applies) rather
than failing the command. Exposed as a package var so the wiring is unit-tested
on any OS.
- Reachable from config: SandboxConfig.BlockUnixSockets and .MonitorDenials now map
onto the policy in applyConfiguredSandboxPolicy (opt-in only — config can turn a
hardening flag on, never off). Both default OFF.
- Tests: helper-prefix present when enabled / absent by default; config flags apply
independently and stay off when omitted.
Default OFF and Linux-only effect; the runtime seccomp enforcement still needs a
Linux host/CI to exercise end-to-end (the wiring and BPF program are unit-tested).
* audit batch 33 (sandbox): enforce scoped/deny network policy in web_fetch & web_search
Completes prompt 9: the built-in network tools now honor the SAME allow/deny/scoped
policy the sandboxed shell egress proxy enforces, instead of bypassing it. Before
this, web_fetch/web_search would reach any public host even when the policy was
NetworkDeny or NetworkScoped — the egress proxy only constrained shell commands.
- New shared gate Engine.NetworkHostAllowed(host) reuses the egress domain matcher
(domainAllowed/normalizeDomains/effectiveNetwork) as the single source of truth:
allow → all hosts; deny → none; scoped → only AllowedDomains minus DeniedDomains
(empty allowlist collapses to deny). Disabled policy / nil engine impose nothing.
- web_fetch is now sandbox-aware: the host is checked before the request AND on every
redirect, so a denied/unlisted host fails closed before any dial. Plumbed via a
networkPolicyGate so the no-sandbox path is byte-for-byte unchanged.
- web_search is sandbox-aware: the configured backend's endpoint host is checked
before the query leaves the machine; blocked under deny, and under scoped unless the
backend host is allowlisted.
- The registry already routes sandbox-aware tools through RunWithSandbox when an
engine is present, so this activates automatically wherever the sandbox is wired.
- Tests: engine gate matrix (deny/allow/scoped/denylist/empty/disabled/nil + host:port);
web_fetch & web_search block denied/unlisted hosts without dialing and allow listed
ones; endpoint-host parsing.
* audit batch 34 (sandbox): prove network-policy wiring end-to-end + document opt-in flags
Verification follow-up to batches 31-33; no behaviour change.
- Adds a registry-level test that exercises the FULL chain for the scoped-network
enforcement: registry.RunWithOptions -> engine.Evaluate (passes a scoped,
egress-capable sandbox-exec backend) -> routes web_fetch through RunWithSandbox ->
per-host allowlist blocks an unlisted host before any dial. This proves the
Tool-interface value actually satisfies sandboxAwareTool at the registry boundary,
not just in a direct method call.
- Documents the two opt-in hardening flags (sandbox.blockUnixSockets,
sandbox.monitorDenials) and the scoped web_fetch/web_search behaviour in the README
so the config keys are discoverable. Both remain off by default.
Confirmed wiring: agent loop forwards options.Sandbox to the registry; the registry
routes sandbox-aware tools (web_fetch, web_search) through RunWithSandbox when an
engine is present; MonitorTag is set only on the macOS sandbox-exec plan; the
seccomp helper prefix is added only on the bubblewrap plan when BlockUnixSockets is
set; both config flags map onto the policy via applyConfiguredSandboxPolicy.
* audit batch 35 (sessions): fix Windows CI failure in TestWriteFileSyncRoundTrips
The exact-permission assertion (perm == 0o600) added in batch 17 fails on
windows-latest because Windows does not honor Unix permission bits: Go reports
0o666 for a writable file regardless of the mode passed to OpenFile. This was the
sole cause of the failing Smoke (windows-latest) CI job; macOS and Ubuntu passed.
Guard the perm check with runtime.GOOS != "windows" so it still verifies the mode
on the platforms that enforce it, while the round-trip (write + read-back) is still
asserted on every platform.
* audit batch 36: address CodeRabbit review (5 findings)
- background/process_posix: after SIGKILL, poll until the process group is
actually gone (SIGKILL is async) and return an error if it survives past a
second deadline, instead of returning nil while descendants still race.
- sandbox/safe_command: only treat ')' as a substitution boundary when it closes
an active $(...). A literal ')' (e.g. echo "a) less") no longer splits into a
fake `less` segment and is no longer misflagged as interactive. Depth-tracked so
nested $(...) still isolate their inner command. Direct splitShellSegments test
(fails pre-fix: yields ["echo \"a", "less\""]).
- cron/schedule: the DST fall-back collapse now only applies when `after` sits
exactly on the minute boundary (its "last fire time" form). With sub-minute
precision (e.g. 01:30:30 EDT) the first 01:30 fire already passed, so the repeated
01:30 EST is the legitimate next fire and is returned — restoring strictly-after
semantics. New test fails pre-fix (skipped to next day).
- config/types: maxTurns error message now reads "must be >= 0" to match the check
(0 is accepted; only < 0 is rejected). Negative-maxTurns test asserts the message.
- docs/audit: header date/branch metadata now matches the body (2026-06-13,
fix/audit-batch) instead of the stale 2026-06-11/fix/audit-critical-high snapshot.
Verified: gofmt clean; go vet clean; host + linux + windows builds; affected suites
green. Each behavioural fix ships with a test confirmed to fail without the change.
* audit batch 37: address CodeRabbit re-review (12 findings)
Findings on the audit code from batches 21-36; each behavioural fix ships with a
regression test (several confirmed to fail without the change).
sandbox/analyzer (AnalyzeCommand):
- Unwrap launcher prefixes (sudo/env/nice/bash -c/...) and recurse into `sh -c`
payloads so `sudo rm -rf`, `env curl …`, `bash -c 'vim x'` classify on the real
command, not the launcher. Mirrors DetectInteractiveCommand.
- Honor rm's `--` end-of-options marker so `rm -- -rf` (delete a file named -rf) is
not flagged destructive.
sandbox/engine: NetworkHostAllowed now shares one backend-aware effective-mode
helper with Evaluate, so scoped collapses to deny when the backend can't enforce
scoped egress — the per-tool gate can no longer permit traffic the engine-level
decision would fail closed.
sandbox/egress: cache domain-prompt decisions by host:port, not host, so one
"allow" can't authorize other ports on the same host.
sandbox/safe_command: the segment splitter is now escape-aware (\| , \" ) so an
escaped operator/quote can't manufacture a fake interactive segment.
sandbox/log_monitor (darwin): Wait() the `log stream` child after draining stdout
so the cancelled process isn't leaked as a zombie.
tools/web_search: confine the search backend to same-host redirects (with a limit)
so a redirect chain can't egress to a host the scoped/deny policy never authorized.
cli/sandbox: an explicit empty --path (`--path=` / `--path ""`) now fails closed
instead of silently widening an allow to tool-wide or a revoke to all-grants.
cli/skills: warn on stderr when duplicate-skill detection fails instead of
swallowing the error.
specialist/exec: don't mask a failed orphan-kill — join the TerminateProcess error
with the SetPID error so a surviving process is visible.
background/process_posix: resolve the PGID and only group-signal when pid leads its
own group; otherwise signal the individual PID. Avoids both a false-success on a
non-leader ESRCH and the hazard of signalling an unrelated (possibly our own) group.
Verified: gofmt + go vet clean; host + linux + windows builds; full go test ./...
green.
* audit batch 38: address CodeRabbit re-review round 2 (3 findings)
Follow-ups to batch 37, each with a regression test:
- safe_command (splitter): a ')' is now only a substitution boundary when it
closes an active, UNQUOTED $(...). A quote-state stack saved at each '$(' lets a
substitution run in a fresh quoting context and restores the outer quoting on
its matching ')'. Fixes `echo $(printf "a) less")` splitting into a fake `less`
segment, while still isolating the inner command of `echo "$(vim x)"`.
- egress: deduplicate in-flight domain prompts per host:port. Concurrent requests
for the same unknown target now wait on the first prompt and reuse its cached
decision instead of each firing a prompt (a prompt storm) and racing to write
the final decision. Race-tested with 25 concurrent authorizers → one prompt.
- analyzer + safe_command: wrapper option-value consumption is now wrapper-specific
(wrapperValueOptionsByProg) instead of a global option set. A valueless flag like
`sudo -n` no longer swallows the real payload, so `sudo -n rm -rf` /
`sudo -n curl …` are correctly classified destructive/network; `sudo -u root vim`
still consumes its value. Shared by the AST analyzer and the regex detector so
they can't diverge.
Verified: gofmt + go vet clean; host + linux + windows builds; go test ./... and
-race on sandbox/background all green.
* audit batch 39: address CodeRabbit re-review round 3 (12 findings)
All verified against current code; each behavioural fix has a regression test.
Security / correctness:
- tools/web_search: redirect policy now also refuses a scheme change, blocking a
same-host https→http downgrade that would send the query + bearer token over
plaintext.
- sandbox/safe_command: a ')' is only a substitution boundary when it closes an
active UNQUOTED $(...), and backticks now get the same quote-state save/restore
as $( — so `echo "`a | less`"` and `echo "$(true | less)"` no longer hide the
inner pager from interactive detection.
- sandbox/analyzer + safe_command: wrapper option-value consumption stays
wrapper-specific, and unwrapping no longer aborts on a dynamic ($x) wrapper arg
once a wrapper is active, so `env "$opts" curl` / `sudo "$x" rm -rf` are still
classified.
- sandbox/egress: in-flight prompts deduped per host:port (carried from batch 38);
the prompt callback is now cancellable (context cancelled on timeout) so a stuck
prompt can abort instead of leaking; signature threaded through.
- sandbox/runner: per-plan unique denial tag (pid+counter) so concurrent monitored
sandbox-exec runs can't ingest each other's denials; helper selection now
requires an executable regular file so a non-exec sibling degrades gracefully.
- mcp/network_client: aggregate SSE event-size cap (not just per-line) so a server
can't grow memory with many unterminated data: lines.
- specialist/accounting: a previously-recorded stop/usage with an empty runId is a
catch-all and now suppresses a later runId-bearing duplicate.
- sessions/lineage: Tree fetches the root via Get() first so a corrupt root
surfaces its real error instead of degrading to "not found".
Docs/tests:
- background/terminate: docstring now states the leader-only group-kill (PID-only
fallback for non-leaders); test child-liveness probe uses ESRCH specifically.
- cli/sandbox: grant usage strings include --path.
Verified: gofmt + go vet clean; host + linux + windows builds; go test ./... and
-race on sandbox/background all green.
* audit batch 40: address CodeRabbit re-review round 4 (8 fixed, 1 skipped)
Verified each against current code.
Fixed:
- sessions/lineage: PANIC FIX — store.Get returns (nil, nil) for a missing
session, so the batch-39 root prefetch could nil-deref. Guard root == nil and
return a not-found error. Regression test added (was a panic).
- agent/compaction: the reactive recover() now records lowWaterMark in the SAME
combined (messages + tool-defs) token domain maybeCompact uses, so the proactive
shrink-guard compares like with like. recover() takes tools; callers pass
request.Tools.
- mcp/network_client: SSE aggregate counter no longer counts a joining newline for
the first data line (only when prior content exists), avoiding premature rejects.
- sessions/store: fsync the parent directory after the metadata rename so the
rename is durable across a crash (no-op on Windows, which can't fsync a dir).
- sandbox/egress: parse the target port with strconv.Atoi instead of
net.LookupPort (no context-unaware resolver use).
- sandbox/runner: escape the per-plan denial tag through sandboxProfileString when
embedding it in the seatbelt profile.
- sandbox/seccomp_linux: guard against an empty filter program before taking
&kernelFilters[0] (defensive; the program is fixed non-empty today).
- cli/cron_run_test: assert last-error-wins with two error events.
- background/terminate, cli/sandbox usage, process_posix_test ESRCH (carried).
Skipped (with reason):
- zerogit worktree-rename (Y='R') test: git porcelain v1 -z only emits the
rename-source field when the INDEX column is R/C; the worktree column never
carries 'R' with a source field in v1, so the fixture would test output git
doesn't produce, and making parseStatus consume on code[1]=='R' would risk
mis-consuming the following entry for a plain ` R` status. Left as-is.
Verified: gofmt + go vet clean; host + linux + windows builds; go test ./... and
-race on sandbox/background all green.
* audit batch 41: address CodeRabbit re-review round 5 (4 findings)
All verified against current code; code fixes have regression tests.
- agent/compaction: summarizeWithFallback's reduce pass no longer swallows ALL
errors. A context-limit error still falls back to the joined partial summaries
(documented "extreme" case), but a non-context failure (auth/network/provider)
now surfaces unchanged, per the function's contract. Two tests (propagate vs.
joined-fallback); the propagate test fails without the fix.
- sandbox/safe_command + analyzer: wrapper value-consuming options now include the
long spellings (sudo --user root, env --unset NAME, timeout --signal …), so a
long flag's value is no longer mis-read as the program (interactive/destructive/
network detection bypass). A `--flag=value` token carries its own value and never
consumes the next token. Tests for space and = forms in both the AST analyzer and
the regex detector.
- docs/audit: the status-ledger summary table's open/partial cells (64/14) were the
lone outlier — the ledger's own narrative and 2026-06-13-reverification.md both
state 66/12. Corrected to 66/12 (66+12+97 = 175); derivation: body 69 open + 18
partial minus the 9 already-fixed-on-main (3 open + 6 partial) = 66/12 = 78
pending, fixed 88+9 = 97. Added a comment documenting the summary-vs-body
relationship. The reverification doc already states 66/12, so both now agree.
Verified: gofmt + go vet clean; host + linux + windows builds; full go test ./...
green.
* audit batch 42: address CodeRabbit re-review round 6 (1 fixed, 1 kept w/ proof)
CodeRabbit re-surfaced two "duplicate" refinements; CI was already green (its 6th
pass was a COMMENT with no new findings).
- cli/sandbox_test (TestRunSandboxGrantsRejectsEmptyPath): now seeds a tool-wide
grant and asserts grant-store immutability after EACH rejected empty-`--path`
call (reflect.DeepEqual before/after), so a mutation in one call can't be masked
by a later one — and a buggy "revoke all" is actually observable (it's a no-op on
an empty store). Stronger than the suggested patch.
- zerogit.parseStatus: KEPT consuming the rename source on the index column only
(code[0]), with an explanatory comment. Verified empirically that git porcelain
v1 -z reports a rename/copy (and emits the extra source field) only in the index
column: `git mv` → "R new\0old\0"; a worktree-only rename → " D old\0?? new\0"
(delete + untracked, never "R" in code[1]). Consuming on code[1]=='R'/'C' would
match no real git output and would only risk mis-consuming the next entry on
malformed input, so the suggestion is declined as incorrect for v1.
Verified: gofmt + go vet clean; host + linux + windows builds; affected suites green.
|
||
|
|
754700aeea |
specialist: keep name in session title so description-less runs resume
A specialist run with no description set --session-title to just the description (empty), leaving AgentName empty on resume and making the session unresumable. Always prefix the title with the specialist name and fall back to the bare name when no description is given. |
||
|
|
be8100e526 |
Add specialist lifecycle accounting (#120)
* Add specialist lifecycle accounting Record specialist start and stop events in parent sessions and roll child stream-json usage into one parent usage event. Background TaskOutput performs the same roll-up idempotently for persisted completed tasks. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/specialist Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Harden npm wrapper smoke test on Windows Clear inherited NODE_OPTIONS when executing the Node wrapper fixture and allow a longer Windows timeout for hosted-runner cold starts. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/npmwrapper ./internal/specialist Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Address specialist accounting review feedback Normalize mixed usage event totals before roll-up, preserve started child exit codes on execution errors, and make accounting payload assertions fail loudly on missing or mistyped fields. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/specialist ./internal/npmwrapper Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check |
||
|
|
27f0fe8f30 |
Harden specialist background lifecycle (#116)
* Harden specialist background lifecycle * Preserve killed background task state |
||
|
|
936bbea59d |
Add specialist background task tools (#115)
* Add specialist background task tools * Address specialist background review feedback |
||
|
|
a60154b70a |
Add specialist Task runtime (#113)
* Add specialist Task runtime * Harden specialist resume identity |
||
|
|
2f50158bca |
Add specialist execution foundation (#111)
* Add specialist execution foundation * Normalize specialist session ids |