mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-10 17:57:54 -10:00
fix/opencode-session-header
22
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
dcf229fecd |
refactor(tests): replace duplicated wait-for helpers with testutil.WaitFor (#1039)
Fixes #1023 |
||
|
|
0021475d4c |
fix(swarm): stop original member before handoff (#957)
* fix(swarm): stop source before handoff * fix(swarm): make handoff recovery fail closed * test(swarm): remove handoff timing assumptions |
||
|
|
6fe0d1edcd |
fix(lockutil): use kernel-held advisory locks (#950)
* fix(lockutil): use kernel-held advisory locks * fix(lockutil): harden cross-platform lock files * test(lockutil): require one concurrent holder |
||
|
|
bdbda89f3e |
fix(swarm): close lifecycle admission on shutdown (#776)
* fix(swarm): close lifecycle admission on shutdown * fix(swarm): admit lifecycle work without holding the lock across Launch Addresses the P1 on #776 plus the two documentation P3s. [P1] beginLifecycleAdmission returned with lifecycleMu.RLock() held for the caller's entire duration — including the call into the injected MemberLauncher.Launch. Close needs the write lock before it can call s.cancel(), so a launcher that blocks in Launch waiting on its context (the ordinary shape of a launcher that waits for a slot, a daemon connection, or a sandbox handshake) deadlocked every Close caller: the launch held the read lock while Close waited for the write lock to reach the very cancellation the launch needed to unblock. beginLifecycleAdmission is now a counted ticket: it holds lifecycleMu only across the closed check and a lifecycleWork.Add(1), then returns a release func (lifecycleWork.Done) the caller defers. Close flips closed and releases the lock BEFORE calling cancel, then waits out lifecycleWork (admitted-but-unfinished work) followed by watchers (goroutines admitted work may still be starting) — so it remains a true completion barrier without holding any lock across external launcher code. Every call site (Spawn, Handoff, AdoptOrphans, the retry path in watch, and afterExit) moved to the ticket pattern. [P3] beginLifecycleAdmission's contract and the Scheduler/Close handoff are now both documented at the point that matters: the func-returning signature makes the release contract explicit in the type itself, and Scheduler's comment spells out why the read/write lock ordering guarantees exactly one of Scheduler or Close closes whatever scheduler ends up installed. Tests: TestCloseCancelsLauncherBlockedInLaunch reproduces the exact deadlock (spawn parked in Launch, Close in a second goroutine, both must complete once cancellation reaches the launcher) — verified it hangs under the old held-lock pattern by temporarily restoring it. TestCloseWaitsForAdmittedSpawn is the converse: Close must not return while an admitted spawn is still running. Validated with `go test -race ./internal/swarm/... -count=1` under WSL/Linux (this session's Windows host has no C toolchain for -race directly), plus `go vet ./...` under WSL and go build/vet/test on Windows. * fix(swarm): fail late queue admissions and dequeues after shutdown Addresses jatmn's two #776 review findings: - Close swept (clearQueue) every team's queue before waiting for admitted-but-in-flight lifecycle operations to finish. A Spawn, Handoff, or AdoptOrphans call that got its ticket right before closed flipped could still append to a team's queue after the sweep ran, stranding that spec forever (never launched, since shutdown began; never failed, since the sweep won't run again). Reorder Close to wait for lifecycleWork before sweeping queues, so every queue append from already-admitted work is visible to the sweep. - afterExitAdmitted dequeued a queued spec (t.onExit) and launched it unconditionally once its own ticket was granted, even though Close can flip closed in the gap between that dequeue (which makes the spec invisible to the clearQueue sweep) and the launch. Re-check closed after dequeuing so a spec pulled off the queue after shutdown began fails with ErrSwarmClosed instead of starting a new worker. Both regression tests are verified to fail against the pre-fix code (confirmed by temporarily reverting each fix and rerunning). Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(swarm): snapshot teams after lifecycle work stops Amp-Thread-ID: https://ampcode.com/threads/T-019fa018-a4d5-7078-a80b-802bab23607e Co-authored-by: Pierre Bruno <[email protected]> * fix(swarm): reject late admitted launches during shutdown Re-check the closed state at the final launcher boundary so lifecycle work admitted before Close cannot start or retry a member after shutdown has already won the race. Add deterministic late-dispatch and scheduler-handoff coverage, stress concurrent Spawn and Close under the race detector, and keep the late-team snapshot regression isolated from launcher behavior. Amp-Thread-ID: https://ampcode.com/threads/T-019fa55a-4f7f-766e-a312-a7db232b9987 Co-authored-by: Pierre Bruno <[email protected]> * fix(swarm): reject post-close launch commits Amp-Thread-ID: https://ampcode.com/threads/T-019fa7a0-1223-701d-9529-48ba5d7cf8c8 Co-authored-by: Pierre Bruno <[email protected]> * fix(swarm): gate retry relaunch on the post-close commit The post-Launch shutdown gate added for spawns and queue drains was missing from the bounded-retry path, which called the launcher and swapped in the new handle unguarded. Since FuncLauncher returns a live handle immediately without consulting its context, a Close landing between the pre-check and Launch's return left the watcher supervising a member relaunched after shutdown began — and recording that member's outcome on the task. Extract the commit gate as commitLaunch and route the relaunch through it via relaunchAdmitted, so an initial launch and a retry pass the same three gates: admission, the pre-Launch re-check, and the post-Launch commit. When shutdown wins, the handle is reaped and the watcher records the terminal failure instead of adopting it. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --------- Co-authored-by: Claude Sonnet 5 <[email protected]> Co-authored-by: Amp <[email protected]> |
||
|
|
1bb6b5745a |
fix(swarm): wait for job.Runs directly in scheduler skip test (#667)
The launcher recording a spawn and the job's Runs counter incrementing are sequential but distinct steps in the scheduler goroutine (fireIfIdle's Spawn call, then run's job.incRuns()). The test polled only the launcher's recorded count, then asserted Runs immediately with no wait, so a scheduler goroutine preempted between those two steps could be observed with a stale Runs value under CI load. Wait for Runs itself instead of assuming the launcher signal implies it. |
||
|
|
6fc1220f6a |
fix(sandbox): scrub sensitive credentials from sandbox environment (#660)
* fix(windows): retry atomic file renames on Access is denied / Sharing violation * test(swarm): add unit tests for rename retry and non-retryable errors * test(swarm): skip TestMailboxRenameRetry on non-Windows platforms * fix(sandbox): scrub sensitive provider credentials from sandbox env * refactor(sandbox): unify environment creation and reduce lines of code * fix(sandbox): forward workspaceRoot through sandboxEnvironment, gate Windows env on backend * style(sandbox,swarm): remove stray blank lines flagged by gofmt * refactor(fsutil): extract shared rename retry helper, derive scrub list from provider catalog Address CodeRabbit review feedback: - Add internal/fsutil.RenameWithRetry with an injectable rename override and move the four duplicated renameWithRetry/isWindowsSharingViolation copies (cron, sessions, sandbox, swarm) onto it. Mailbox keeps its rename test seam by passing its field through. Add fsutil tests for the Windows retry and non-retryable paths. - Remove the orphaned metadata.json.tmp in cron writeJob when the rename permanently fails, matching the sibling writers. - Derive scrubSensitiveEnv keys from providercatalog AuthEnvVars plus a static list of non-catalog secrets, so new providers are scrubbed automatically. AWS_PROFILE and GOOGLE_APPLICATION_CREDENTIALS stay because they are pointers, not secrets. * fix(sandbox): deny sandboxed reads of cloud credential stores by default Address CodeRabbit finding that scrubbed env pointers still left the underlying credential files readable under the read-all workspace posture: - Add credentialDenyReadPaths: default profile-level deny-read entries for ~/.aws, ~/.config/gcloud, ~/.azure, and the GOOGLE_APPLICATION_CREDENTIALS target file, wired into both permission profile builders so the seatbelt and Linux helper backends enforce them. Entries are stat-filtered and a store nested under a user AllowRead entry is dropped, keeping an explicit opt-out. - Keep the defaults out of Policy.DenyRead, whose emptiness gates escalated execution, and skip Windows so the profile deny list does not flip the runner off the WRITE_RESTRICTED token path. - Scrub GOOGLE_APPLICATION_CREDENTIALS from the sandbox env; keep AWS_PROFILE since the deny-read rule on ~/.aws is the real protection and the SDKs fall back to the default profile regardless. - Add credential deny path tests, extend the scrub test, relax the profile test's exact deny count to containment. * test(cli): pin HOME in sandbox policy golden test The default credential-store deny-read entries depend on what exists in the real home directory (the macOS CI image ships ~/.aws), which leaked host paths into the policy JSON golden comparison. Point HOME at an empty temp directory so the golden stays hermetic. * fix(sandbox): protect GOOGLE_APPLICATION_CREDENTIALS without a home, retry lock violations Address CodeRabbit review feedback: - credentialDenyReadPaths no longer returns early when the home lookup fails; home-based candidates become conditional and the GOOGLE_APPLICATION_CREDENTIALS target is always considered. - fsutil.RenameWithRetry also retries ERROR_LOCK_VIOLATION (errno 33), matching the mcp lock handling; helper renamed to isWindowsSharingOrLockViolation. - Extend tests: homeless GOOGLE_APPLICATION_CREDENTIALS case and a lock violation step in the retry sequence. * fix(sandbox): tighten env-scrub test assertion and add missing secret keys TestScrubSensitiveEnv only checked an allowlist plus a length count, so a duplicate allowed key silently replacing a required one would still pass. Compare the exact expected key/value map instead. Also add ZERO_WEBSEARCH_API_KEY and ZERO_DAEMON_REMOTE_TOKEN to scrubSensitiveEnv's sensitiveKeys list: both are real bearer secrets read from the environment but were missing from the scrub list, leaving them readable by sandboxed commands when set in the parent shell. |
||
|
|
da41c3a75b |
fix(lock): prevent POSIX lock file overwrite and leak on Windows/Unix (#628)
* fix(lock): prevent POSIX lock file overwrite and leak on Windows/Unix During lock reclamation, reclaiming a suspected stale lock renames the file aside and renames it back if not stale. On POSIX systems, the rename back silently overwrites any new lock file created in the gap. On Windows, it fails to rename back and leaks the stale file. Introduce a cross-platform restoreLockFile helper: - On POSIX: Use os.Link to link the file back (which atomic-fails if the target already exists), then remove the temporary link. - On Windows: Use os.Rename (which safely fails if the target exists). If restoring fails, delete the temporary file immediately to prevent leaks. * fix(lockutil): fix Windows build by using golang.org/x/sys/windows Addresses jatmn's follow-up review: the required Windows smoke check was failing because syscall.MoveFileEx is not exported by the standard library's syscall package on this platform, so internal/lockutil (and everything importing it: cron, daemon, hooks, oauth) failed to compile on Windows. Switched to golang.org/x/sys/windows, which does export MoveFileEx with the same signature, already a direct module dependency. Verified on a real Windows machine that the no-flags call still fails with ERROR_ALREADY_EXISTS when the destination exists, and that this satisfies errors.Is(err, os.ErrExist), which cron/daemon/hooks/oauth's reclaim paths all depend on to decide whether the sidelined lock was genuinely superseded (safe to discard) versus some other, unrelated failure (must be preserved). TestRestoreLockFile already covered both the successful-restore and fail-when-target-exists cases; it now passes on Windows. * fix(lockutil): route lock file deletes through RemoveLockFile with Windows retry loop * fix(lockutil): close restore split-brain and unify RemoveLockFile RestoreLockFile could fail with errors other than os.ErrExist (hardlink- incapable filesystems such as FAT or some FUSE/network mounts, ENOSPC, Windows sharing violations). Callers only clean up on os.ErrExist, so any other failure left the lock path missing with the live holder's lock stranded in the sidelined file; the next O_EXCL create then succeeded and two processes held the lock. RestoreLockFile now falls back to an O_EXCL copy in that case, which preserves the no-overwrite guarantee, and a successful restore with a failed sidelined-file cleanup reports nil instead of a failed restore. RemoveLockFile now has one cross-platform contract: removing an already missing file reports nil on POSIX too, matching Windows, and daemon release() drops its caller-side ErrNotExist filter. Tests: TestRestoreByCopy and TestRemoveLockFile run on all platforms; Windows-tagged tests pin the ERROR_ALREADY_EXISTS mapping to os.ErrExist and exercise the RemoveLockFile retry loop to success and to bounded exhaustion. The package doc comment documents the shared contract. * refactor(swarm): extract reclaimStaleLock to mirror other lock callers The stale-lock reclaim in acquireLock was inline, so its restore path (a holder rotated a fresh lock in the gap between the stale check and the rename) could not be regression-tested the way cron, daemon, hooks, and oauth test theirs. Extract it into reclaimStaleLock with the same shape as the other callers, behavior unchanged, and add the mirrored regression test. * test(hooks,oauth): add stale lock reclaim regression tests Mirror the cron reclaim test in the two packages that lacked coverage for reclaimStaleLock: a genuinely stale lock is reclaimed, a fresh lock (reacquired in the gap) is restored intact rather than stolen, and a missing lock reports no reclaim. daemon already had equivalent tests in lock_test.go. * fix(lock): fail closed and drop sidelined file when restore fails A non-os.ErrExist error from RestoreLockFile means both the no-replace restore and its O_EXCL copy fallback failed, so the lock path may be missing while the sidelined holder is live. Callers previously swallowed that error and their next O_EXCL create would succeed, breaking mutual exclusion. reclaimStaleLock in cron, daemon, hooks, oauth, and swarm now returns an error for that case and the acquire loops fail closed instead of re-acquiring. The sidelined .stale.* file is now removed on every restore failure, not only os.ErrExist: once the restore has failed it has no protocol function (release only consults the lock path), so keeping it only leaked files. Also replace the numeric errno literals in the Windows RemoveLockFile retry loop with windows.ERROR_SHARING_VIOLATION and windows.ERROR_ACCESS_DENIED. * refactor(lockutil): extract shared ReclaimStaleLock helper Consolidate the duplicated reclaimStaleLock functions from cron, hooks, oauth, and swarm into lockutil.ReclaimStaleLock. Each caller now passes an mtime-based isLive predicate to the shared helper, which owns the rename-aside, restore-if-live, cleanup, ErrExist handling, and fail-closed error propagation. Daemon keeps its own reclaimStaleLock because it uses PID-based liveness (different predicate shape and no staleAfter duration). The per-package reclaim regression tests are removed since the shared helper is thoroughly tested in lockutil/reclaim_test.go (dead lock reclaimed, live lock restored, missing lock, fail-closed on restore error, sidelined file dropped when new holder wins). * fix(lockutil): propagate non-contention rename failures from ReclaimStaleLock A rename-aside failure that is not a lost race previously mapped to the benign lost-race result, so acquirers spun until their deadline and reported a timeout instead of the real failure. Keep only a missing source and the Windows sharing-violation/access-denied contention errnos benign (neither Windows errno maps to os.ErrExist, and ERROR_SHARING_VIOLATION does not map to os.ErrPermission either, so the classification compares raw errnos) and return everything else so callers fail closed. Also convert daemon's private reclaimStaleLock to the shared helper so it picks up the same contract, and reword the caller error messages from "restore reclaimed" to "reclaim stale" since the error can now come from the rename aside as well. * fix(lockutil): publish restore-by-copy atomically and honor lost mailbox reclaims restoreByCopy created the destination lock file empty before copying into it, so a crash mid-copy could leave a live holder's lock permanently unrepresented at the lock path. Stage the copy under a private name and publish it with the platform's no-replace primitive instead. swarm's mailbox lock discarded the reclaimed boolean and always retried immediately, so a lost reclaim race (or Windows sharing-violation contention) could hot-spin past the lock timeout instead of falling through to the bounded wait like the other lock callers. * fix(lockutil): shrink the reclaim/restore window with a fast replacing rename Restoring a lock that turned out live went through the no-replace path (plus its copy fallback on some failures) first, which is slow enough that a fourth process's unrelated O_EXCL create could win lockPath while it was absent for the whole call. RestoreLockFile's no-replace contract would then report os.ErrExist and the caller silently dropped the sidelined lock, orphaning the still-running original holder. restoreLiveLock now tries a single replacing rename straight back first, falling back to the no-replace path only if that fails. It shrinks the window from a whole restore call down to roughly one syscall; it does not close it, since the fast path can silently overwrite a competing lock rather than detect it. A fully race-free reclaim would need an OS-level advisory lock checked non-destructively instead of by moving the file, which is a larger change than this. |
||
|
|
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).
|
||
|
|
84823d7be1 |
fix(swarm): constrain agent_type to the registered roster via a dynamic enum (#344)
* fix(swarm): constrain spawn/handoff agent_type to the registered roster via enum swarm_spawn and swarm_handoff described agent_type / to_agent_type as a free-form string with only an 'e.g. teammate, subagent' hint, so a model could pass an unregistered type (e.g. 'worker') and only discover it was invalid from the runtime 'unknown agent type' error — wasting tool calls, especially on weaker models. Set the parameter's enum dynamically from the live roster (Registry.AgentTypes()), so the schema advertises the valid set (built-ins plus any user-registered agents) up front and an invalid type can't be sent. Falls back to a free-form string when the roster is unavailable, so the tool never advertises an empty, un-satisfiable enum. The runtime 'available agent types: …' error is kept as a backstop. * test(swarm): also assert handoff to_agent_type enum picks up a custom roster type Address review: the custom-type recheck only re-read spawnTool, so a regression leaving handoffTool on the built-in list would have passed. |
||
|
|
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 |
||
|
|
11926cc5d2 |
feat(tui): click a swarm member in the sidebar to open its subchat (#311)
* feat(tui): click a swarm member in the sidebar to open its subchat Specialists were drillable (click the card → subchat shows the child session), but swarm members shown in the AGENTS sidebar were not — even though each member already runs as a `zero exec --init-session-id <sid>` child that persists a durable session in the same store the TUI reads. This wires that session through so a member row is clickable. - swarm: Coordinator records the member's child session id on completion (CompleteWithSession / Task.SessionID); swarm_collect emits a task_id → session_id map in its Result.Meta. - tui: the OnToolResult closure forwards that map via swarmSessionsMsg into m.swarmSessionMap; swarmAgent carries the session id; the AGENTS sidebar records a clickable hit per member whose session is known. - tui: sidebarLineAtMouse maps a left-click in the sidebar column to a member row (screen-Y == sidebar line index; sidebar starts at chatColumnWidth + 3), and the existing specialist-card drill-in path (subchat.enter) is reused verbatim. Members are clickable only once their session id is known (after swarm_collect), so in-flight members simply aren't clickable — no dead clicks. The subchat machinery, Result.Meta transport, and the click handler are all reused; no new persistence or agent-loop changes. * test(tui): assert sidebar member click drills into the member session Address review: the routing test only checked the click was handled, so it could pass even if the drill-in didn't enter the expected subchat. Seed a real session in the store and assert subchat.active + childSessionID == the member's session after the click. |
||
|
|
258f1a31c9 |
fix(swarm): make swarm_collect wait for members instead of polling (#310)
* fix(swarm): make swarm_collect wait for members instead of polling Swarm spawn was fire-and-forget and the only way to learn a member had finished was to call swarm_status/swarm_collect, which returned an instant snapshot. So an orchestrator had to poll in a loop, flooding the chat with repeated status cards and never getting a clean "done". swarm_collect now BLOCKS until every member of the team reaches a terminal state, then returns their results in one call: - Coordinator gains a change-broadcast channel and WaitSettled(ctx, team), which blocks until the scope has no pending/running task (or ctx is done). Every state transition (register/setstatus/finish/reassign) wakes waiters. - Swarm.CollectWait wraps it; the collect tool calls it bounded by the run context (user cancel) and a timeout (default 600s, cap 1800s, overridable via timeout_seconds), so a stuck member returns partial state instead of hanging forever. - Tool descriptions now steer the model: spawn then collect once; status is a one-shot snapshot, not something to poll. TUI: collapseRepeatedStatusCard drops the previous identical swarm status/collect card when a check repeats verbatim back-to-back, so any residual re-checks don't stack duplicate blocks in the chat. * fix(swarm): address review on blocking collect (race + timeout overflow) - WaitSettled: take the settled decision and the returned snapshot in the SAME locked pass. Previously the snapshot was re-read after releasing the RLock, so a concurrent Register/Reassign could make the call return a non-terminal task it never waited on — breaking the blocking contract. Removed the now-dead scopedList; shared sortTasksByCreation with List. - collectWaitTimeout: clamp timeout_seconds in float space before converting to time.Duration. A very large value overflowed the int64-ns Duration and wrapped negative, making context.WithTimeout fire immediately instead of honoring the cap. - test: release the gated member via defer in TestCollectBlocksUntilMembersFinish so a failing assertion can't leak the blocked goroutine; add a collectWaitTimeout clamp/overflow unit test. |
||
|
|
63c0e278ff |
feat(tui): chat + context sidebar redesign, AGENTS panel, animations (#300)
* feat(tui): chat + context sidebar redesign, AGENTS panel, animations, plan-timer fix
Two-column managed-mode layout: the existing chat (scroll engine, reduced width)
on the left + a new right context sidebar — AGENTS (spawned subagents: status
glyph, name, and a live working detail for running ones, with a running/total
count), the live PLAN, and a token readout. The sidebar is suppressed on the
home screen and under overlays/subchat; alt-screen managed mode, wide terminals.
Also: freeze the plan clock while the agent is idle (an in_progress step left
mid-plan when the agent yields stops ticking instead of counting forever), and a
clean-room animation port (phase scroll indicator + a cosine ripple working
line; the spinner was already animated, frozen under ZERO_REDUCED_MOTION).
Reuses the scroll/viewport engine; inline mode unchanged. New unit tests for the
sidebar, plan-clock freeze, and the pure animation helpers; go build/vet/test green.
* feat(swarm): live AGENTS sidebar + un-defer coordination tools
Sidebar AGENTS section now reflects the swarm/team accurately:
- name each member by a short 1-2 word task (not subagent-N)
- a mild, slow cool pulse on live member names (no per-letter flicker)
- members disappear when swarm_status reports them [done]/[failed]
- hide bogus "specialist not found" tool-misroute attempts
- keep the animation tick alive while sidebar agents exist
Plus the deeper fix so a coordinating model stops misrouting swarm calls
to the specialist tool ("specialist swarm_send not found"): the swarm
COORDINATION tools (swarm_send/status/inbox/collect) un-defer once a
swarm is active, while spawn/schedule/handoff stay discoverable via
tool_search. A new DeferralEligible() interface keeps all swarm tools
counting toward DeferThreshold even when exposed, so un-deferring can
never deactivate deferral / force-expose MCP tools in any config.
* feat(tui): polish pass — hierarchy, status line, motion, sidebar toggle
A grounded polish pass across the TUI (17 items):
- color: spread the gray tiers (AA-safe), lime working-ripple (no semantic
green/amber), calm steady ask-label, dim completed plan-step bodies,
bold-muted sidebar headers + stateful PLAN count
- status line: run-state chip (permission mode + effort) replaces the
duplicated provider; composer rule is model-only; context-fill gauge down to
the narrow tier + a sidebar context-% chip
- footer: persistent idle key-hint (managed mode) + a jump-to-bottom cue
- motion: pulsing streaming caret; running specialist spins; swarm/specialist
agents linger with a fading ✓ before removal (no abrupt pop)
- layout: Ctrl+B toggles the sidebar; padded ' │ ' divider; sidebar gated to
the medium tier so it never starves the chat
- home screen: cwd · branch · model orientation + example prompts
- cleanup: drop dead autoTag, fix 'six swarm tools' miscount
go build/vet/test green; -race clean; no hex outside theme.go.
* feat(tui): reading column for the chat transcript
Cap transcript body rows at a readable ~90-col measure and indent them by a
small left gutter, so on a wide terminal the chat text no longer runs
edge-to-edge. Only true transcript rows use the reading column; the frame,
composer, status line, sidebar, title bar, empty state, and prompts keep the
full chatColumnWidth. Streaming text shares the column so it doesn't snap on
finalize. Plus extra vertical spacing before headings and between paragraphs.
Implementation keeps the height cache valid (rows render at contentWidth, which
the width-keyed cache already captures) and the gutter is a horizontal post-pad;
each selectable line's textStart is shifted by the gutter so click-to-select and
the selection highlight stay aligned. Mouse card/toggle hit-testing is Y-based,
so it's unaffected. go build/vet/test green; -race clean.
* fix(tui): plain cancellation marker + copyable card/toggle rows
- 'Run cancelled.' renders as a plain amber '⊘ Run cancelled.' line instead of a
heavy box (other system notices keep their box).
- Copy root cause: the app's text-selection skipped 'button' rows (collapsible
toggles, specialist cards) — they carried no selectable text, so a drag-select
omitted exactly that (dull) content. Fix: those rows now carry their plain text
+ start, so a selection dragged THROUGH them copies their content; a direct
click still toggles/drills (resolved on press, before selection). The selection
highlight no longer excludes them. Permission buttons stay excluded.
- Surface Ctrl+E in the idle hint — it releases the mouse for native terminal
selection, which copies everything (the zero-risk full-copy path).
go build/vet/test green; -race clean.
* fix(tui): render all system notices as plain lines, not boxes
Every system notice (mouse-mode changes, mode set, run cancelled, etc.) now
renders as a plain marked line instead of a bordered box: a faint '·' marker +
muted text for info, amber '⊘' for a run cancellation. Multi-line notices keep
the marker on the first line and indent the rest. Command cards and error rows
keep their boxes. go build/vet/test green.
* fix(tui): separate think→act tool groups with a blank line
Reasoning headers (Thought for X) interleave the tool cards (tool → thought →
tool …), which broke the existing tool-card→tool-card gap rule, so consecutive
list_directory/read_file/update_plan groups packed into a dense wall. Now a
reasoning header that follows a tool card gets a blank line above it, so each
think→act group reads as a distinct block. go build/vet/test green.
* feat(tui): colour markdown emphasis + headings for readability
Assistant prose now reads as three tiers instead of one flat ink colour:
- body text stays ink
- bold / important words render in the brand accent (lime) so they pop
- markdown headings render accent + bold + underline as a clear top tier
go build/vet/test green.
* fix(tui): calmer prose colours + a working-phase label
- Revert the accent colour on bold/important words: emphasis is weight-only now
(no colour), dark-mode-friendly.
- Headings drop the bright lime — distinguished by ink + bold + underline instead
of a saturated accent colour.
- Working status line gains a phase label ('writing' while the answer streams,
'thinking' otherwise) so a long, output-less step reads as live progress
('Working · thinking · 55s') rather than a frozen screen.
go build/vet/test green.
* fix(tui): paint selection highlight in the gutter-shifted coordinate
Mouse text-selection looked like it started in the wrong place: the highlight
landed a few cells right of where you clicked, so selecting/copying felt
impossible to aim.
Cause: the row renderers painted the selection highlight on the UNshifted,
unpadded line, then the body item applied the reading-column gutter (pad lines +
shift selectable textStart) afterward. But the stored selection points are in the
shifted/displayed coordinate (that's how the mouse maps them), so the highlight
was computed against unshifted textStart and landed gutter-cells off. (The copied
text was actually correct — extraction uses the shifted selectable — but the
visual selection was wrong, which is what made it un-aimable.)
Fix: stop highlighting inside the row renderers; add finalizeTranscriptBodyRow,
which pads + shifts THEN paints the highlight, so it's computed in the same
coordinate the mouse uses and lands exactly on the click. Also applies the
highlight uniformly to all rows + the interim block. Regression test added.
|
||
|
|
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 (
|
||
|
|
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. |
||
|
|
b0a280eaf6 |
audit: fix four high-severity concurrency/correctness bugs (#261)
Surfaced by a full-tree audit; all are latent (the suite passed) and live in race windows / error edges the tests don't exercise. Each ships a regression test. - swarm: liveAgents() now counts queued specs, so AdoptOrphans can no longer re-dispatch (double-execute) a task whose member is merely waiting for a free slot over the concurrency cap. - cron/hooks/oauth: stale-lock reclaim is now atomic (rename aside, then verify-and-restore if it turns out fresh) instead of a blind Remove that let two racers both reclaim and hold the lock; also falls through to the bounded wait instead of hot-spinning when a reclaim never wins. - lsp: a dead language-server session is evicted and restarted instead of being returned forever, so one server crash no longer permanently breaks diagnostics (and spuriously fails self-correct every iteration). - config: an API-key env var no longer overwrites a same-named compatible-transport provider's kind when no base URL is supplied. |
||
|
|
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 |
||
|
|
eed61cf8ec |
tools: defer swarm tool schemas + keyless SearXNG web_search backend (#258)
Two fixes for the raised issues — the idle tool-schema prefix and native web search.
Idle prefix — make the swarm tools deferred-eligible:
- The 7 swarm_* tools are an advanced, rarely-first-move feature the base system
prompt does not name, so they now implement Deferred() and load on demand via
tool_search instead of shipping full schemas in the eager per-request prefix.
Core tools and specialist Task stay eager (no friction on common operations).
- partitionTools keys off the shared tools.IsDeferred check exactly as it does for
MCP tools, so this reuses the existing, proven mechanism. Measured ~470 fewer
prompt tokens per request in a swarm-enabled (--auto high) config, with no
behavioral loss — swarm still works, just loaded when needed.
Web search — keyless SearXNG backend:
- web_search gains a "searxng" provider: ZERO_WEBSEARCH_PROVIDER=searxng with a
SearXNG URL queries GET /search?format=json with NO API key, parsing the
{results:[{title,url,content,score}]} shape. Self-host or point at any SearXNG
instance for native search without baking a shared key or a flaky default.
Tests: swarm tools are deferred-eligible; the SearXNG backend issues the GET, sends
no auth header, maps fields, and respects the limit.
|
||
|
|
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. |
||
|
|
c38444157c |
Audit backlog: tui mouse API, notify webhook, swarm scheduling, daemon-remote bundles, MCP/oauth store unification, OS keyring, dead-code prune (#216)
* tui: migrate off the deprecated Bubble Tea mouse Type API (SA1019)
The mouse classification helpers checked both the current Button/Action pair and
the deprecated msg.Type / tea.Mouse* constants. Bubble Tea's parser always
populates Button+Action and only DERIVES Type from them, so the Type fallbacks
were pure redundancy (a left-button drag is Action==Motion, already covered by
mouseMotion). Drop them — clears all 11 SA1019 findings in internal/tui with no
behavior change. The left-drag test now asserts the Action==Motion path directly.
Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(SA1019 0)/
deadcode(no new) all pass.
* notify: wire the webhook sink behind an opt-in env var
The WebhookSink (Slack / generic JSON POST on turn completion) was fully
implemented, redaction-safe, and tested — but never attached to a Notifier, so
deadcode flagged its entire machinery (NewWebhookSink, Emit, text, redactLinks,
log, eventType, readSnippet) as unreachable.
Wire it via a small, testable helper, MaybeAddWebhookSink(n, env, logf):
- reads ZERO_NOTIFY_WEBHOOK_URL (+ optional ZERO_NOTIFY_WEBHOOK_SUMMARY)
- no-op when the URL is unset/blank, so it is safe to call unconditionally
- sourcing the URL from the environment keeps the secret token out of any
on-disk config file
Attached at both notifier construction sites:
- headless exec: failed deliveries log to stderr (never stdout); the sink
redacts before logging
- TUI: logf=nil so a delivery failure can't corrupt the alt-screen
The sink stays gated by the existing Mode/focus policy (verified by
TestNotifierOffSuppressesSinks), so a webhook only fires when notifications are
enabled (e.g. --notify both) — no change to default behavior. Opt-in and
fail-closed: with the env var unset nothing is attached.
deadcode: 100 -> 93 unreachable funcs (the 7 WebhookSink entries become
reachable); no new dead code. gofmt/vet/build(host+linux+windows)/test -race/
staticcheck(no new)/govulncheck(0) all pass.
* swarm: add opt-in recurring-spawn scheduler (wakeup + daily)
Adds a dependency-free Scheduler to internal/swarm so a member can be spawned on
a recurring schedule, plus a swarm_schedule tool to drive it.
- Schedule is interval-based ("wakeup", every >= 1s) with an optional first
delay and a MaxRuns cap; a daily "HH:MM" ("cron") time is expressed as a 24h
interval whose first delay lands on the next occurrence.
- Each fire spawns a fresh member through the existing Swarm.Spawn path, so
members inherit the recorded policy and are bounded by the team slot cap +
queue. Non-overlapping by default: a fire is skipped while the job's previous
spawn is still running, preventing queue pile-up.
- Scheduler is lazily created (Swarm.Scheduler()), opt-in (nothing runs until a
job is added), and tied to the Swarm's base context — Swarm.Close() stops the
scheduler first so no spawn fires during shutdown.
- swarm_schedule tool: action add|list|cancel; prompt-gated like swarm_spawn;
add validates schedule + agent type up front (fail fast).
The timing loop takes an injectable ticker, so the tests drive fires
deterministically with no real time (fire/skip/cap/cancel/close, plus pure
nextDailyDelay/parseClock/swarmInt). All swarm tests pass 3x under -race.
Gates: gofmt/vet/build(host+linux+windows)/test/staticcheck(clean)/govulncheck(0)/
deadcode(no new) all pass.
* daemon/remote: SessionLink + git-bundle upload over the bridge
Adds an opt-in way to ship a local repo's git history to a remote daemon and
link a remote working tree to it — without a shared filesystem or a network git
remote. The daemon core (protocol/server) is untouched; all changes live in the
remote package plus a CLI hook.
Wire:
- The auth handshake gains an optional Mode field ("" / "session" => a daemon
session as before; "bundle" => a one-shot git-bundle upload). Unknown modes,
or bundle mode when uploads are disabled, are denied (fail closed).
- Bundle transfer reuses the daemon frame codec: a header frame (link id +
exact size), then the bundle bytes as KindData frames, then a result frame.
Server (Bridge):
- BridgeOptions.BundleDir enables uploads (empty => refused). A received bundle
is size-capped (default 256 MiB), staged to a temp file, `git bundle verify`-d,
then `git clone`-d into a staging dir and atomically renamed over
<BundleDir>/<link-id>. Link ids are sanitized to a single path component and
containment is re-checked, so an upload can never escape the bundle dir.
Client:
- UploadRepoBundle creates `git bundle create --all` of a work tree, hashes it
(sha256), streams it over an authenticated bundle-mode TLS connection, and
returns a SessionLink {address, link id, remote path, bundle sha}. SessionLink
persists as a 0600 atomic JSON file (token never stored) with Save/Load/Validate.
CLI:
- `daemon serve-remote --bundle-dir <d>` enables uploads.
- `daemon link --remote <addr> --repo <dir> --id <name> [--out <file>]` uploads
and prints the remote path (+ a ready run command); `daemon link --show <file>`
prints a saved link.
Tests: git bundle create/verify/clone roundtrip; link-id sanitization + path
containment; SessionLink save/load/validate (0600); and a full bridge upload E2E
over real TLS (extract + content check + re-upload-replaces + disabled + bad
token). All remote tests pass 2x under -race; CLI smoke verified via the binary.
Gates: gofmt/vet/build(host+linux+windows)/test/staticcheck(no new)/
govulncheck(0)/deadcode(no new) all pass.
* mcp: unify OAuth token storage onto the shared oauth store
MCP server tokens lived in their own mcp-oauth-tokens.json with a bespoke
codec/lock, separate from the provider-login store (internal/oauth). The oauth
store already reserved the "mcp:" key namespace for exactly this. Unify them.
- mcp.TokenStore now delegates to internal/oauth.Store, keying each server's
token as "mcp:<server>". Its public API (StoredToken/TokenStatus/Save/Load/
Delete/Status/FilePath) is unchanged, so callers (network client, CLI) are
untouched. The bespoke read/write/lock code is removed in favor of the shared,
cross-process-locked, atomic store.
- Transparent, non-destructive migration on first construction: a legacy
mcp-oauth-tokens.json is imported into the unified store (only entries not
already present — a newer unified token is never overwritten), then renamed to
a ".migrated" backup so it is imported at most once and stays recoverable.
A missing / unreadable / unknown-schema legacy file is left untouched and
never blocks startup. An explicit FilePath suppresses the default-path
migration so tests/overrides can't touch the real user config.
- Server names that can't form a valid namespaced key (oauth.ValidateKey) are
rejected on save and skipped during migration. Added oauth.Store.FilePath().
The token format was already identical (access/refresh/type/scopes/expires), so
this is a key rename, not a format change. Existing TokenStore tests pass
unchanged (they exercise the API); added migration + namespacing + newer-wins
tests. Local run check: `zero mcp oauth status` migrates a seeded legacy file
into oauth-tokens.json under mcp:demo, renames the legacy to .migrated, and leaks
no token material.
Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(clean)/
govulncheck(0)/deadcode(no new) all pass.
* oauth: add an opt-in OS keyring storage backend
Introduces a pluggable storage backend for the unified oauth token store and a
dependency-free OS keyring implementation, selected with
ZERO_OAUTH_STORAGE=keyring (default stays the 0600 file).
- internal/keyring: a small, third-party-dep-free secret store backed by the OS
tooling — `security` on macOS, `secret-tool` (libsecret) on Linux; other
platforms report unsupported. On Linux the secret is passed via stdin (never
the argv); on macOS `security` takes it as an argument (briefly visible in the
process list) — documented, and a reason to prefer the file backend there.
A runner seam makes the per-OS command logic fully testable without a real
keychain.
- internal/oauth: Store now persists its JSON blob through a blobStore backend.
fileBlob preserves the exact prior behavior (0600, atomic write, cross-process
lock file). keyringBlob stores the blob as one base64 entry (keeps multi-line
JSON a single control-char-free value) via a KeyringClient. NewStore resolves
the backend from StoreOptions.Storage or ZERO_OAUTH_STORAGE; unknown storage
and an unavailable keyring fail closed with a clear error.
Because both `zero auth` and the (now unified) MCP token store construct their
store through oauth.NewStore, the keyring backend is honored by both with no
call-site changes. Added oauth.Store.FilePath() returns a "keyring:" identifier
for that backend.
No new module dependency (go.mod unchanged). Tests: keyring round-trip/not-found/
unsupported/missing-binary/stdin-not-argv via an injected runner; store keyring
round-trip + base64-at-rest + storage selection; all existing file-backend tests
pass unchanged. Local run check (macOS): validated the exact `security`
add/find/delete shapes and the not-found exit code (44) against the real binary,
and `mcp oauth status` with ZERO_OAUTH_STORAGE=keyring reads the real keychain
cleanly.
Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(no new)/
govulncheck(0)/deadcode(no new) all pass.
* all: prune unused unexported functions (dead-code sweep)
Removes 15 unexported functions/types/consts that are unreachable even from
tests (every one flagged by staticcheck U1000), clearing the U1000 noise the v16
audit surfaced. Conservative by design: only unexported, test-unused symbols are
removed — exported internal API (which may be an intentional surface or used only
by tests) is deliberately left in place.
Removed:
- agenteval: validateExpectedChangedFiles
- providers/gemini: const providerName
- tui: stripMarkdownInline, removeTrailingAtToken,
noopModelSwitchCompactionPolicy (+ BeforeModelSwitch), deleteComposerLineBefore,
deleteComposerLineAfter, deleteCompletedFileMentionBefore, mcpViewStatus,
mcpServerLines, fitCommandOutput, (*commandPicker).clearQuery,
tuiTheme.onPanel2, indentText
No behavior change (the symbols had no callers). staticcheck U1000 is now empty;
deadcode drops accordingly with no genuinely-new unreachable functions (verified
by a name-based diff vs origin/main). gofmt/vet/build(host+linux+windows)/full
test all pass.
* Address review feedback on the audit backlog
Resolve CodeRabbit review comments on this PR:
- keyring isNotFound: match the specific not-found exit codes (macOS `security`
44 = errSecItemNotFound; `secret-tool` 1) instead of treating any non-zero
exit as "no entry", so a real failure (locked keychain, permissions) surfaces
instead of being masked as absent.
- swarm scheduler daily_at: recompute each fire as the next local HH:MM rather
than adding a fixed 24h, so a wall-clock daily schedule does not drift across
DST. Adds Daily/Hour/Minute to Schedule and a clock to the Scheduler.
- swarm scheduler: reject Add when the scheduler context is already canceled
(not just when closed); re-check cancellation after a tick so a tick+cancel
race can't spawn one extra member after Cancel/Close.
- swarm schedule_tool swarmInt: reject non-integer / non-finite JSON numbers
(max_runs=1.9 errors instead of silently truncating to 1).
- mcp migrateLegacy: resolve a relative LegacyPath to absolute before the
same-file guard so it can't be bypassed by a relative spelling.
- oauth keyring store: serialize the keyring read-modify-write with a
cross-process lock file (best-effort, beside the config dir) so concurrent
processes don't clobber each other's tokens; previously withLock was a no-op.
Adds tests for the non-not-found keyring error, the daily delay recompute, the
Add-after-cancel guard, and non-integer swarmInt.
* swarm: make nextDailyDelay roll the day, not add 24h (DST)
CodeRabbit follow-up: the run loop already recomputes a daily job's delay each
cycle, but nextDailyDelay itself rolled "passed today" to the next occurrence
with Add(24*time.Hour). On a DST changeover day (23h/25h) that fires an hour
early/late. Use time.Date(..., day+1, hour, minute, ...) so Go normalizes the
date and applies the correct local offset, holding the wall-clock HH:MM across
DST. Adds an America/New_York spring-forward test (skips if tzdata is absent).
|
||
|
|
bb7de29c80 |
feat(oauth): reusable provider OAuth engine + zero auth CLI (#210)
* feat(oauth): reusable provider OAuth engine + `zero auth` CLI
Add internal/oauth: a transport/identity-agnostic OAuth 2.0 engine that powers
provider login alongside the MCP-server OAuth zero already has. It is additive
and provider-neutral — NO providers, endpoints, or client identities are baked
into the binary; every provider is configured entirely via ZERO_OAUTH_<NAME>_*
env vars, so the engine works with any OAuth 2.0 / OIDC server.
Engine (internal/oauth):
- PKCE (S256 mandatory; "plain" refused) + per-flow CSRF state.
- Authorization-code flow: BuildAuthorizationURL / ExchangeCode / Refresh, with
an https-only token-endpoint guard (loopback exempt) and error bodies that
carry only error/error_description (never token material).
- Loopback callback server: 127.0.0.1-only, OS-assigned port, single use, state
verified, then closed.
- Device-authorization grant (RFC 8628): RequestDeviceCode + PollDeviceToken
honoring authorization_pending / slow_down / interval / expiry (headless/SSH).
- RFC 8414 + OIDC discovery (issuer-driven endpoint resolution).
- Namespaced token store ("provider:<name>" / "mcp:<name>"), 0600, file-locked
(ownership-aware), atomic writes, malformed-fails-closed; GetFresh (on-demand
refresh) + Handle401 (forced refresh).
- Opt-in proactive RefreshScheduler.
CLI (zero auth):
- login <provider> [--device] [--scope ...], logout, status [provider],
refresh <provider> [--watch]. Status/output never print token material.
- Registered in app.go dispatch + top-level help.
MCP OAuth (internal/mcp) is left byte-for-byte unchanged (zero regression);
sharing the engine with MCP and encrypted-at-rest/keyring storage are documented
follow-ups. Tests cover every path (PKCE, auth URL, https-guard + redaction,
exchange/refresh, loopback capture + state-mismatch + timeout, device pending/
slow_down/expired, store namespacing/0600/locking, provider env resolution,
manager login (loopback+device)/GetFresh/Handle401, scheduler). Gates: gofmt /
vet / build (host+linux+windows) / test -race / staticcheck (no new) /
govulncheck (0) / deadcode (no new) all pass.
* oauth: fix Windows smoke — use OS-appropriate path in store-path test
TestResolveStorePathHonorsOverride hardcoded a unix "/tmp/custom/tok.json"
literal, which is not absolute on Windows (no drive), so ResolveStorePath
correctly resolves it against the current drive and the verbatim comparison
failed (D:\tmp\custom\tok.json). The production code is correct; the test now
builds the override with filepath.Join(t.TempDir(), ...) so it is absolute and
clean on every OS.
* oauth: address CodeRabbit review (11 findings)
Security / correctness:
- flow: caller-supplied ExtraAuthParams/extraParams can no longer override the
reserved OAuth/PKCE fields (state, redirect_uri, code_challenge[_method],
response_type, client_id) — e.g. forcing method=plain.
- device: a device-authorization response without expires_in now gets a bounded
default lifetime (fail closed), and the poll loop re-checks expiry AFTER the
interval sleep so it never polls past the deadline (e.g. after slow_down).
- lock: handle WriteString/Close errors on the token lock file — a partial write
no longer strands an undeletable lock.
- store: a non-nil env map is now authoritative (hermetic) — a controlled map
never falls back to ambient ZERO_OAUTH_* / HOME / XDG_CONFIG_HOME.
- scheduler: a transient loadForKey error backs off and retries (bounded) instead
of permanently stopping proactive refresh.
- cli auth: reject flags a subcommand does not accept (e.g. login --watch,
status --device) and reject empty --scope values — fail fast.
Lint blockers:
- manager: restructured the discovery fallback so the non-nil-err branch no longer
returns nil (nilerr).
- providers: dropped the ineffectual flow initializer (ineffassign).
Tests: assert Save/Load errors in the scheduler test; the slow_down device test
no longer depends on post-expiry polling. Added tests for reserved-param
stripping, hermetic env, device default-expiry, and CLI flag validation. The
store-path test was already made OS-portable in the Windows-smoke fix.
Gates: gofmt/vet/build(host+linux+windows)/test -race/staticcheck(no new)/
govulncheck(0)/deadcode(no new) all pass.
* swarm: de-flake TestMailboxConcurrentSends on Windows CI
The 200-way concurrent-send stress test inherited from the swarm package timed
out on Windows CI (103/200 sends hit the 10s lock timeout): the lock's 20ms
retry sleep starves waiters when Windows file ops are slow under heavy
contention. Shorten the retry to 2ms (a freed lock is re-acquired promptly) and
raise the test's lock timeout to 60s so a legitimately-slow CI never times a send
out. Passes repeatedly under -race. (CI de-flake of a test that surfaced on
#210's Windows smoke; functionally unrelated to the OAuth changes.)
* oauth: address CodeRabbit re-review (4 findings)
- loopback (security): NewLoopbackListener refuses an empty CSRF state — an empty
state would match a callback carrying no state at all, defeating the check.
- device (correctness): RequestDeviceCode now sends client_secret when set, so a
confidential client authenticates on the device-authorization endpoint too
(consistent with the token poll).
- cli auth: document --watch in the help Flags section.
- scheduler test: assert the seed Save error instead of ignoring it.
Also scrubbed a stray third-party reference from a loopback.go comment.
Tests added: empty-state rejection, device client_secret is sent. Gates:
gofmt/vet/build/test -race/staticcheck(no new)/govulncheck(0)/deadcode(no new)
all pass.
* mcp(oauth): delegate the OAuth engine to internal/oauth (dedup) (#212)
OAuth Phase 2: the MCP OAuth code now reuses the shared internal/oauth engine
instead of carrying its own copy of the transport/identity-agnostic primitives.
internal/mcp/oauth.go drops ~110 lines (526 -> 418): newPKCE, newState,
discoverAuthorizationServer, the authorize-URL builder, token exchange/refresh,
the token-response parser, and joinWellKnown now delegate to internal/oauth.
Behavior-preserving: MCP keeps its own OAuthConfig/StoredToken types,
LoginOptions/Login orchestration, loopback handling, registerClient (dynamic
client registration — not part of the shared engine), CLI output, and on-disk
token format (mcp-oauth-tokens.json, raw server-name keys). The existing mcp
oauth tests pass UNCHANGED. The only behavioral delta is a hardening inherited
from the shared engine: the token endpoint must be https (loopback exempt) —
a credential is no longer sent to a plaintext non-loopback endpoint.
Stacks on #210 (the internal/oauth package). Gates: gofmt/vet/build(host+
linux+windows)/test -race/staticcheck(no new)/govulncheck(0)/deadcode(no new
vs base) all pass; `zero mcp oauth status` output is identical.
|
||
|
|
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. |