Audit backlog: tui mouse API, notify webhook, swarm scheduling, daemon-remote bundles, MCP/oauth store unification, OS keyring, dead-code prune#216
Merged
Conversation
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.
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.
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.
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 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.
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.
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.
Contributor
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds a recurring swarm spawn scheduler with a ChangesSwarm Recurring Scheduler
Daemon Remote Git-Bundle Upload
OS Keyring Backend for OAuth Tokens
Environment-driven Webhook Notification Wiring
Dead-Code Removal and TUI Cleanup
Sequence Diagram(s)sequenceDiagram
participant CLI as zero daemon link
participant UploadRepoBundle
participant Bridge
participant BundleStore as BundleDir (server FS)
CLI->>UploadRepoBundle: repoDir, linkID, cfg
UploadRepoBundle->>UploadRepoBundle: git bundle create (temp file)
UploadRepoBundle->>UploadRepoBundle: SHA-256 hash bundle
UploadRepoBundle->>Bridge: dialAuthenticated(ModeBundle) + TLS handshake
Bridge->>Bridge: verify token + validate ModeBundle
Bridge->>UploadRepoBundle: authResponse OK
UploadRepoBundle->>Bridge: bundleHeader{LinkID, Size}
UploadRepoBundle->>Bridge: KindData frames (bundle bytes)
Bridge->>Bridge: streamFramesToFile → temp file
Bridge->>Bridge: git bundle verify
Bridge->>BundleStore: extractBundle → atomic clone + rename
Bridge->>UploadRepoBundle: bundleResult{OK, RemotePath}
UploadRepoBundle->>CLI: SessionLink{RemotePath, BundleSHA256}
CLI->>CLI: print hint / Save SessionLink --out
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
internal/daemon/remote/bundle.go (1)
302-306: 💤 Low valueConsider adding a timeout to
isGitWorktree.Unlike the other git helpers (
runGit), this function runs without a context/timeout. Whilegit rev-parse --is-inside-work-treeis typically instant, a hung git process could block the caller indefinitely.♻️ Optional: add timeout for consistency
-func isGitWorktree(dir string) bool { - cmd := exec.Command("git", "-C", dir, "rev-parse", "--is-inside-work-tree") - out, err := cmd.CombinedOutput() - return err == nil && strings.TrimSpace(string(out)) == "true" +func isGitWorktree(dir string) bool { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "git", "-C", dir, "rev-parse", "--is-inside-work-tree") + out, err := cmd.CombinedOutput() + return err == nil && strings.TrimSpace(string(out)) == "true" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/daemon/remote/bundle.go` around lines 302 - 306, The isGitWorktree function executes a git command without a context or timeout, which could block indefinitely if the git process hangs, unlike other git helper functions like runGit that use timeouts. Update the isGitWorktree function to accept a context parameter and use it when creating the exec.Command, similar to how other git helpers in the codebase handle timeouts. This ensures consistent timeout protection across all git operations.internal/cli/daemon.go (1)
577-584: 💤 Low valueWhen
--showis combined with upload flags, the upload flags are silently ignored.If a user accidentally runs
zero daemon link --show saved.json --remote host:9000 --repo . --id proj, the command will just print the saved link and ignore--remote,--repo,--id. This could be surprising.Consider either warning when both sets of flags are present, or documenting that
--showtakes precedence.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/daemon.go` around lines 577 - 584, When the --show flag is used with upload flags (--remote, --repo, --id), the upload flags are silently ignored because the function returns early after processing the --show flag. Add validation logic within or before the `if strings.TrimSpace(show) != ""` block to detect when both --show and any upload flags are provided simultaneously, and issue a warning to the user (via stderr or logging) that the upload flags will be ignored and --show takes precedence. This should happen before the LoadSessionLink call so users are aware of the behavior rather than silently ignoring their input.internal/mcp/oauth_store.go (1)
214-220: 💤 Low valueReturning
nilafter a non-niljson.Unmarshalerror triggersnilerrlint.The intent is correct (leave unreadable legacy files alone), but the pattern of checking
err != nilthen returningnilis a code smell. Consider making the intent explicit:Suggested fix
var legacy tokenFile - if err := json.Unmarshal(data, &legacy); err != nil { - return nil // unreadable legacy file: leave it in place, don't block startup + if jsonErr := json.Unmarshal(data, &legacy); jsonErr != nil { + return nil // unreadable legacy file: leave it in place, don't block startup (intentional) }Using a different variable name avoids shadowing and makes the intentional nil-return clearer. Alternatively, add a
//nolint:nilerrdirective if the project uses golangci-lint.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/mcp/oauth_store.go` around lines 214 - 220, The json.Unmarshal error check triggers the nilerr lint rule when returning nil after detecting a non-nil error. To fix this while maintaining the intent of leaving unreadable legacy files in place, either rename the error variable from err to something more descriptive (like unmarshalErr) to make the intentional nil-return explicit and avoid shadowing, or alternatively add a //nolint:nilerr directive above the json.Unmarshal call if your project uses golangci-lint. The goal is to clarify that returning nil here is intentional for this legacy file handling scenario.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/keyring/keyring.go`:
- Around line 179-189: The isNotFound function currently returns true for any
non-zero exit code, but it should only return true for platform-specific "not
found" exit codes. Refactor the isNotFound function to check for the specific
exit code that indicates a missing entry: on macOS the security tool returns 44
for item-not-found, and on Linux the secret-tool lookup returns 1 for not found.
Update the function to check coder.ExitCode() against these platform-specific
values using build tags or runtime checks, returning true only when the exit
code matches the platform's "not found" indicator, rather than treating all
non-zero exits as not found.
In `@internal/swarm/schedule_tool.go`:
- Around line 107-109: The daily_at schedule mode sets sch.Every to a fixed 24 *
time.Hour interval which causes drift across DST transitions since a fixed
24-hour duration doesn't preserve wall-clock "HH:MM local time" scheduling.
Instead of using the fixed 24-hour interval, compute the Every field dynamically
using nextDailyDelay function on each cycle to recalculate the next fire time
based on the current local time and desired hour/minute, ensuring the schedule
remains accurate across DST transitions.
- Around line 211-217: The float64 case in the type conversion logic silently
truncates decimal values like 1.9 to 1 without validation, which can lead to
unexpected schedule limits. In the float64 case block, add validation to check
that the value is not NaN, not infinite, and has no fractional part (i.e.,
math.IsNaN(n), math.IsInf(n, 0), or checking if n != float64(int(n))). If any of
these conditions are true, return false to reject the value instead of silently
truncating it with int(n).
In `@internal/swarm/scheduler.go`:
- Around line 227-236: The select statement in the scheduler tick loop can race
between job cancellation and ticker firing. When both job.ctx.Done() and ch are
ready simultaneously, select may choose the ch case and proceed to call
fireIfIdle, spawning a job after cancellation. After the ch case completes in
the select block, add a re-check of job.ctx.Done() before calling
fireIfIdle(job) to ensure the job wasn't cancelled while waiting for the tick.
- Around line 149-153: The Add method in scheduler.go currently only checks the
s.closed flag when deciding whether to reject new jobs, but does not account for
the scheduler context being already canceled. Add a check for s.ctx.Done() in
addition to the existing s.closed check within the same lock section, so that if
the context is already canceled (indicating the scheduler has been closed or its
parent context canceled), the Add method returns an error message like "swarm:
scheduler context is done" to prevent accepting jobs that would immediately
exit.
---
Nitpick comments:
In `@internal/cli/daemon.go`:
- Around line 577-584: When the --show flag is used with upload flags (--remote,
--repo, --id), the upload flags are silently ignored because the function
returns early after processing the --show flag. Add validation logic within or
before the `if strings.TrimSpace(show) != ""` block to detect when both --show
and any upload flags are provided simultaneously, and issue a warning to the
user (via stderr or logging) that the upload flags will be ignored and --show
takes precedence. This should happen before the LoadSessionLink call so users
are aware of the behavior rather than silently ignoring their input.
In `@internal/daemon/remote/bundle.go`:
- Around line 302-306: The isGitWorktree function executes a git command without
a context or timeout, which could block indefinitely if the git process hangs,
unlike other git helper functions like runGit that use timeouts. Update the
isGitWorktree function to accept a context parameter and use it when creating
the exec.Command, similar to how other git helpers in the codebase handle
timeouts. This ensures consistent timeout protection across all git operations.
In `@internal/mcp/oauth_store.go`:
- Around line 214-220: The json.Unmarshal error check triggers the nilerr lint
rule when returning nil after detecting a non-nil error. To fix this while
maintaining the intent of leaving unreadable legacy files in place, either
rename the error variable from err to something more descriptive (like
unmarshalErr) to make the intentional nil-return explicit and avoid shadowing,
or alternatively add a //nolint:nilerr directive above the json.Unmarshal call
if your project uses golangci-lint. The goal is to clarify that returning nil
here is intentional for this legacy file handling scenario.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1be8280f-4cfc-40d8-9c80-33eb3fa9ac96
📒 Files selected for processing (37)
internal/agenteval/suite.gointernal/cli/auth.gointernal/cli/daemon.gointernal/cli/exec.gointernal/daemon/remote/auth.gointernal/daemon/remote/bridge.gointernal/daemon/remote/bundle.gointernal/daemon/remote/bundle_test.gointernal/daemon/remote/client.gointernal/daemon/remote/sessionlink.gointernal/keyring/keyring.gointernal/keyring/keyring_test.gointernal/mcp/oauth_store.gointernal/mcp/oauth_store_test.gointernal/notify/webhook_wire.gointernal/notify/webhook_wire_test.gointernal/oauth/store.gointernal/oauth/store_keyring_test.gointernal/providers/gemini/provider.gointernal/swarm/schedule_tool.gointernal/swarm/schedule_tool_test.gointernal/swarm/scheduler.gointernal/swarm/scheduler_test.gointernal/swarm/team.gointernal/swarm/tools.gointernal/swarm/tools_test.gointernal/tui/assistant_markdown.gointernal/tui/autocomplete.gointernal/tui/command_center.gointernal/tui/composer.gointernal/tui/mcp_view.gointernal/tui/model.gointernal/tui/mouse.gointernal/tui/mouse_test.gointernal/tui/picker.gointernal/tui/theme.gointernal/tui/view.go
💤 Files with no reviewable changes (10)
- internal/tui/command_center.go
- internal/tui/theme.go
- internal/tui/autocomplete.go
- internal/tui/picker.go
- internal/providers/gemini/provider.go
- internal/tui/composer.go
- internal/agenteval/suite.go
- internal/tui/mcp_view.go
- internal/tui/view.go
- internal/tui/assistant_markdown.go
Vasanthdev2004
approved these changes
Jun 16, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Verdict: APPROVED ✅
Reviewed locally in worktree review-pr216 at e49234f. The PR is a sweep — 12 commits, 90 files, +12,485 / −255 net (most of the diff is already-merged code from prior PRs in the branch; the actual net additions are smaller). gofmt/vet/build clean; all relevant test packages pass — internal/oauth (8.7s), internal/mcp (7.5s), internal/daemon (1.6s), internal/daemon/remote (8.9s), internal/notify (2.4s), internal/swarm (27.5s), internal/keyring (0.8s).
Headline changes (all merged on top of prior work):
-
internal/keyring/— new 211-line package:security(macOS) andsecret-tool(Linux) wrapper, with 10s per-invocation timeout,errors.Asfor*exec.ExitError(so the "not found" path is testable without a real keychain), and anErrUnsupportedfor Windows/other. Package docstring explicitly flags the macOS argv-visible-secret caveat and points users to the file backend if that matters. The whole thing shells out to OS tools, no third-party dependency. -
internal/oauth/store.go+internal/mcp/oauth_store.go— the unification #212 was designed to enable. MCP tokens now live in the same shared store under themcp:namespace, inheriting every security invariant #210 introduced: 0600, atomic write, file-locked, schema-versioned, hermetic env, redact-safe Status. Migration is one-shot and non-destructive: onNewTokenStore, a legacymcp-oauth-tokens.jsonis read, imported, then the file is renamed<legacy>.migratedso it never re-imports. Newer unified entries are never overwritten. IfFilePathis set explicitly, the migration is skipped (so a custom-configured test path can't accidentally pull from the real user config). Excellent design. -
internal/notify/webhook_wire.go— 40-line opt-in wiring helper. Strictly no-op whenZERO_NOTIFY_WEBHOOK_URLis unset; otherwise adds aWebhookSinkto the notifier. URL is passed throughenv(so a test can inject withoutos.Setenv). The existing Mode/focus policy still gates delivery, so a webhook fires only when notifications are enabled (--notify bothetc.) — same posture as the rest of the notify surface. -
internal/swarm/scheduler.go+schedule_tool.go— recurring spawns, opt-in (nothing runs unlessScheduler.Addis called), one goroutine per job,WaitGroup-joined,Closeis idempotent and cancels via the parent context. Validation up front (interval ≥ 1s, agent type registered, non-empty task). Non-overlap: a fire is skipped when the previous spawn is still running, with aSkippedcounter exposed viaList().MaxRunsbounds the loop. -
internal/daemon/remote/{bundle.go, sessionlink.go, bridge.go, client.go, auth.go}— completes the deferred follow-ups from #211: authenticated git-bundle transfer for repo sync andSessionLinkevent POST with bounded retry. Both go through the same auth + framing path; the bridge'sBridge.handleenforces auth before any control frame, then hands the connection to the appropriate handler. Bundle size is 512 KiB per frame (under the 1 MiB daemon cap), 2-minute git timeout. -
TUI mouse API migration — the deprecated Bubble Tea
tea.MouseTypeis gone in favor oftea.MouseClick. Small, additive, scoped tointernal/tui/mouse.go. Tests updated to match. -
Dead-code sweep — removed unused unexported functions across the agent eval and providers packages. Compile-clean.
One nitpick (already raised on #210): validateTokenEndpoint is still applied to cfg.IssuerURL and cfg.AuthorizationEndpoint (the function name implies "token endpoint" only). Cosmetic — the check is correct (https or loopback), but the name is misleading. Worth a follow-up rename to validateHttpsOrLoopbackURL across all three PRs (#210, #216, #217). Not a blocker.
Overall: this is the kind of audit-backlog sweep that compounds: a keyring package, a real store unification with safe migration, a notification sink, a scheduling layer, a remote-bundle transfer, and a dead-code prune — all in one PR, all building on top of #210/#211. The migration logic alone is a small piece of work done right (non-destructive, idempotent, opts out for explicit FilePath). Merging.
🤖 Generated with Claude Code
Collaborator
Author
|
@CodeRabbit rereview |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
internal/keyring/keyring.go (1)
183-187:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTighten not-found detection; Line 186 currently swallows real backend failures.
isNotFoundtreats any non-zero exit as “entry missing”, so real tool/runtime failures can be silently converted intofound=false, err=nilbyGet/Delete. This masks permission/locked-keychain/service-availability failures.Proposed fix direction
- func isNotFound(err error) bool { + func isNotFound(goos string, err error) bool { var coder interface{ ExitCode() int } if errors.As(err, &coder) { - return coder.ExitCode() != 0 + code := coder.ExitCode() + switch goos { + case "darwin": + return code == 44 + case "linux": + // Avoid classifying all non-zero exits as not-found. + // Use stricter disambiguation (stderr/content) or fallback to error. + var rerr *runError + return code == 1 && errors.As(err, &rerr) && strings.TrimSpace(rerr.stderr) == "" + } } return false }- if isNotFound(err) { + if isNotFound(k.goos, err) {What is the documented not-found behavior for macOS `security find-generic-password` and Linux `secret-tool lookup` (exit code and stderr), and which failures share the same exit code as not-found on Linux?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/keyring/keyring.go` around lines 183 - 187, The isNotFound function incorrectly treats all non-zero exit codes as indicating a not-found condition, which masks real backend failures like permission errors or locked keychains. Instead of checking if coder.ExitCode() is not equal to zero, you need to check for the specific exit code that indicates an entry was not found. Research the documented exit codes for macOS security find-generic-password and Linux secret-tool lookup commands to determine which specific exit code(s) actually mean "entry not found", then update the condition in isNotFound to check for those specific values rather than any non-zero exit code. This ensures that other exit codes representing real failures are not silently converted to found=false, err=nil.
🧹 Nitpick comments (3)
internal/swarm/schedule_tool_test.go (1)
85-107: ⚡ Quick winAdd validation coverage for non-integer
max_runspayloads.Include cases like
1.5(and optionally NaN/Inf payload handling) so parser behavior is locked down and cannot silently truncate in future changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/swarm/schedule_tool_test.go` around lines 85 - 107, Add validation test cases for non-integer max_runs payloads to the cases slice in the schedule_tool_test.go test. Include test cases with max_runs set to non-integer values like 1.5 (float), and optionally NaN and Inf to ensure parser behavior is properly validated and locked down. Each case should include appropriate map[string]any arguments with agent_type and task (or other valid required fields) along with the non-integer max_runs value, and should expect a StatusError response like the other validation test cases in the loop.internal/swarm/scheduler_test.go (1)
136-156: ⚡ Quick winAdd a regression case for
Swarm.Close()before firstSwarm.Scheduler()access.Current coverage asserts Add fails after closing an already-created scheduler. Please also assert failure when
Closehappens first, then scheduler is created lazily andAddis attempted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/swarm/scheduler_test.go` around lines 136 - 156, Add a new regression test case that covers the scenario where Swarm.Close() is called before Swarm.Scheduler() is accessed for the first time. Create a new test function that: creates a Swarm using New(), immediately calls sw.Close(), then lazily accesses the scheduler via sched := sw.Scheduler(), and finally verifies that attempting to call sched.Add() with any valid arguments fails, ensuring the scheduler properly rejects operations after the swarm has been closed regardless of access order.internal/mcp/oauth_store_test.go (1)
236-253: ⚡ Quick win
TestTokenStoreNamespacedFromProviderdoesn’t currently exercise provider/MCP coexistence.The test description says both namespaces coexist, but it only seeds MCP data. Add a provider token in the same unified store before asserting MCP
Status()filtering.Suggested test extension
import ( "os" "path/filepath" "runtime" "testing" "time" + + "github.com/Gitlawb/zero/internal/oauth" ) @@ func TestTokenStoreNamespacedFromProvider(t *testing.T) { @@ if err := mcpStore.Save("shared", StoredToken{AccessToken: "mcp-token"}); err != nil { t.Fatal(err) } + providerStore, err := oauth.NewStore(oauth.StoreOptions{FilePath: unified}) + if err != nil { + t.Fatal(err) + } + if err := providerStore.Save(oauth.ProviderKey("shared"), oauth.Token{AccessToken: "provider-token"}); err != nil { + t.Fatal(err) + } statuses, err := mcpStore.Status()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/mcp/oauth_store_test.go` around lines 236 - 253, The TestTokenStoreNamespacedFromProvider test claims to verify coexistence of MCP tokens and provider login tokens but only seeds MCP data. Extend the test to also create and save a provider token to the same unified store file before calling mcpStore.Status(). This requires creating a separate token store instance configured for provider access (likely through a different TokenStoreOptions configuration or by directly manipulating the file), saving a provider token (for example with name "provider-token" or reusing "shared"), and then verifying that mcpStore.Status() returns only the MCP namespace data without including the provider token data. This demonstrates proper namespace filtering and coexistence in the unified store.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/mcp/oauth_store.go`:
- Around line 203-205: The comparison at line 204 in the same-file guard
compares a cleaned but potentially relative legacyPath with the result of
store.store.FilePath() which returns an absolute path. This means a relative
LegacyPath pointing to the same file will not match and the guard will not
prevent self-migration. Convert legacyPath to an absolute path (in addition to
the existing filepath.Clean call) before comparing it with
store.store.FilePath() to ensure that relative paths to the same file are
properly detected and the guard prevents self-migration.
In `@internal/oauth/store.go`:
- Around line 390-392: The keyringBlob.withLock method is a no-op that doesn't
perform any actual locking, allowing concurrent processes to overwrite each
other's changes when Save and Delete perform their read-modify-write operations
on the shared blob. Implement a cross-process lock mechanism in the
keyringBlob.withLock method to protect against concurrent writes across
different processes. This can be achieved by using a shared lockfile that is
acquired before calling the provided function and released afterward, ensuring
that only one process can modify the keyring blob at a time.
---
Duplicate comments:
In `@internal/keyring/keyring.go`:
- Around line 183-187: The isNotFound function incorrectly treats all non-zero
exit codes as indicating a not-found condition, which masks real backend
failures like permission errors or locked keychains. Instead of checking if
coder.ExitCode() is not equal to zero, you need to check for the specific exit
code that indicates an entry was not found. Research the documented exit codes
for macOS security find-generic-password and Linux secret-tool lookup commands
to determine which specific exit code(s) actually mean "entry not found", then
update the condition in isNotFound to check for those specific values rather
than any non-zero exit code. This ensures that other exit codes representing
real failures are not silently converted to found=false, err=nil.
---
Nitpick comments:
In `@internal/mcp/oauth_store_test.go`:
- Around line 236-253: The TestTokenStoreNamespacedFromProvider test claims to
verify coexistence of MCP tokens and provider login tokens but only seeds MCP
data. Extend the test to also create and save a provider token to the same
unified store file before calling mcpStore.Status(). This requires creating a
separate token store instance configured for provider access (likely through a
different TokenStoreOptions configuration or by directly manipulating the file),
saving a provider token (for example with name "provider-token" or reusing
"shared"), and then verifying that mcpStore.Status() returns only the MCP
namespace data without including the provider token data. This demonstrates
proper namespace filtering and coexistence in the unified store.
In `@internal/swarm/schedule_tool_test.go`:
- Around line 85-107: Add validation test cases for non-integer max_runs
payloads to the cases slice in the schedule_tool_test.go test. Include test
cases with max_runs set to non-integer values like 1.5 (float), and optionally
NaN and Inf to ensure parser behavior is properly validated and locked down.
Each case should include appropriate map[string]any arguments with agent_type
and task (or other valid required fields) along with the non-integer max_runs
value, and should expect a StatusError response like the other validation test
cases in the loop.
In `@internal/swarm/scheduler_test.go`:
- Around line 136-156: Add a new regression test case that covers the scenario
where Swarm.Close() is called before Swarm.Scheduler() is accessed for the first
time. Create a new test function that: creates a Swarm using New(), immediately
calls sw.Close(), then lazily accesses the scheduler via sched :=
sw.Scheduler(), and finally verifies that attempting to call sched.Add() with
any valid arguments fails, ensuring the scheduler properly rejects operations
after the swarm has been closed regardless of access order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 69ed5360-ac75-4fe6-a625-869d2549e7ae
📒 Files selected for processing (37)
internal/agenteval/suite.gointernal/cli/auth.gointernal/cli/daemon.gointernal/cli/exec.gointernal/daemon/remote/auth.gointernal/daemon/remote/bridge.gointernal/daemon/remote/bundle.gointernal/daemon/remote/bundle_test.gointernal/daemon/remote/client.gointernal/daemon/remote/sessionlink.gointernal/keyring/keyring.gointernal/keyring/keyring_test.gointernal/mcp/oauth_store.gointernal/mcp/oauth_store_test.gointernal/notify/webhook_wire.gointernal/notify/webhook_wire_test.gointernal/oauth/store.gointernal/oauth/store_keyring_test.gointernal/providers/gemini/provider.gointernal/swarm/schedule_tool.gointernal/swarm/schedule_tool_test.gointernal/swarm/scheduler.gointernal/swarm/scheduler_test.gointernal/swarm/team.gointernal/swarm/tools.gointernal/swarm/tools_test.gointernal/tui/assistant_markdown.gointernal/tui/autocomplete.gointernal/tui/command_center.gointernal/tui/composer.gointernal/tui/mcp_view.gointernal/tui/model.gointernal/tui/mouse.gointernal/tui/mouse_test.gointernal/tui/picker.gointernal/tui/theme.gointernal/tui/view.go
💤 Files with no reviewable changes (10)
- internal/tui/theme.go
- internal/providers/gemini/provider.go
- internal/tui/assistant_markdown.go
- internal/tui/autocomplete.go
- internal/tui/composer.go
- internal/tui/view.go
- internal/agenteval/suite.go
- internal/tui/command_center.go
- internal/tui/picker.go
- internal/tui/mcp_view.go
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.
Collaborator
Author
|
Pushed
Added tests for the non-not-found keyring error, the daily delay recompute, the Add-after-cancel guard, and non-integer @coderabbitai review |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/swarm/scheduler.go (1)
335-341:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDST transition can shift the fire time by ±1 hour on changeover days.
Using
next.Add(24 * time.Hour)doesn't preserve wall-clock time across DST transitions. On spring-forward day, a 09:00 target would fire at 10:00; on fall-back, it could fire at 08:00. The next cycle self-corrects (recomputing from the new wall time), so the drift is transient—but the code claims to be DST-stable when it isn't fully.Creating the next-day time directly via
time.Datewithday+1ensures the exact wall-clock target regardless of DST:Proposed fix
func nextDailyDelay(now time.Time, hour, minute int) time.Duration { next := time.Date(now.Year(), now.Month(), now.Day(), hour, minute, 0, 0, now.Location()) if !next.After(now) { - next = next.Add(24 * time.Hour) + next = time.Date(now.Year(), now.Month(), now.Day()+1, hour, minute, 0, 0, now.Location()) } return next.Sub(now) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/swarm/scheduler.go` around lines 335 - 341, The nextDailyDelay function uses next.Add(24 * time.Hour) which doesn't preserve wall-clock time across DST transitions, causing fires to shift by ±1 hour on DST changeover days. Replace the next.Add(24 * time.Hour) call with a direct time.Date call that constructs the next day's time using day+1 while preserving the same hour, minute, second, and nanosecond values, ensuring consistent wall-clock scheduling regardless of DST transitions.
🧹 Nitpick comments (1)
internal/swarm/scheduler.go (1)
40-48: 💤 Low valueConsider validating Hour and Minute when Daily is true.
Schedule.validate()doesn't check thatHouris 0–23 andMinuteis 0–59 whenDailyis set. WhileparseClockin the tool layer validates these,ScheduleandAddare exported—direct callers could pass invalid values thattime.Datesilently normalizes (e.g.,Hour: 25becomes 01:00 next day), leading to surprising behavior rather than a clear error.Proposed validation
func (sch Schedule) validate() error { if sch.Every < minScheduleInterval { return fmt.Errorf("swarm: schedule interval must be >= %s", minScheduleInterval) } if sch.MaxRuns < 0 { return errors.New("swarm: schedule max_runs must be >= 0") } + if sch.Daily { + if sch.Hour < 0 || sch.Hour > 23 { + return errors.New("swarm: schedule hour must be 0-23") + } + if sch.Minute < 0 || sch.Minute > 59 { + return errors.New("swarm: schedule minute must be 0-59") + } + } return nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/swarm/scheduler.go` around lines 40 - 48, The Schedule.validate() method is missing validation for Hour and Minute fields when Daily is true. Add validation logic to the validate() method to check that Hour is within the range 0–23 and Minute is within the range 0–59 when the Daily field is set to true. Return an appropriate error message for each invalid field (similar to the existing error messages in the method) to prevent silent normalization of invalid time values by time.Date.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/swarm/scheduler.go`:
- Around line 335-341: The nextDailyDelay function uses next.Add(24 * time.Hour)
which doesn't preserve wall-clock time across DST transitions, causing fires to
shift by ±1 hour on DST changeover days. Replace the next.Add(24 * time.Hour)
call with a direct time.Date call that constructs the next day's time using
day+1 while preserving the same hour, minute, second, and nanosecond values,
ensuring consistent wall-clock scheduling regardless of DST transitions.
---
Nitpick comments:
In `@internal/swarm/scheduler.go`:
- Around line 40-48: The Schedule.validate() method is missing validation for
Hour and Minute fields when Daily is true. Add validation logic to the
validate() method to check that Hour is within the range 0–23 and Minute is
within the range 0–59 when the Daily field is set to true. Return an appropriate
error message for each invalid field (similar to the existing error messages in
the method) to prevent silent normalization of invalid time values by time.Date.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e05bc24-46bf-4dff-8dc5-94e1064b991b
📒 Files selected for processing (8)
internal/keyring/keyring.gointernal/keyring/keyring_test.gointernal/mcp/oauth_store.gointernal/oauth/store.gointernal/oauth/store_keyring_test.gointernal/swarm/schedule_tool.gointernal/swarm/scheduler.gointernal/swarm/scheduler_test.go
🚧 Files skipped from review as they are similar to previous changes (6)
- internal/oauth/store_keyring_test.go
- internal/keyring/keyring_test.go
- internal/keyring/keyring.go
- internal/mcp/oauth_store.go
- internal/swarm/schedule_tool.go
- internal/oauth/store.go
|
Triggering a full review now. ✅ Action performedReview finished.
|
Collaborator
Author
|
@CodeRabbit approvable ? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# List all review comments on PR `#216` from coderabbitai
gh pr view 216 --repo Gitlawb/zero --json reviews,reviewDecision,state
# Also check for unresolved comments/threads
gh api repos/Gitlawb/zero/pulls/216/comments \
--jq '[.[] | select(.user.login == "coderabbitai[bot]") | {id: .id, body: .body[0:120], path: .path, line: .line}]' \
2>/dev/null | head -100 || true
# Check PR review threads
gh api repos/Gitlawb/zero/pulls/216/reviews \
--jq '[.[] | {id: .id, user: .user.login, state: .state, submitted_at: .submitted_at}]' \
2>/dev/null | tail -50 || trueLength of output: 42723
Remaining blocker:
// Current (still drifts on DST changeover day):
next = next.Add(24 * time.Hour)
// Fix: use time.Date with day+1 to get exact wall-clock target:
next = time.Date(now.Year(), now.Month(), now.Day()+1, hour, minute, 0, 0, now.Location())The commit Status of everything else:
Fix |
Resolve the conflicts from #213 (AES-256-GCM encrypted token store) landing on main, which overlapped #216's keyring/store-unification work in the same files: - internal/oauth/store.go: unify the two backends. Keep #216's blobStore architecture (file vs keyring) and graft #213's optional crypter onto the file path, so the three backends compose: "" / "file" (plaintext), "encrypted-file" (AES-256-GCM, crypter set), and "keyring". StoreOptions keeps Encrypted as a legacy alias for Storage=="encrypted-file"; readState decrypts and writeState seals when crypter != nil. - internal/cli/auth.go: newAuthManager validates ZERO_OAUTH_STORAGE against {file, encrypted-file, keyring} and passes it through as Storage (the auto-merge had kept #213's encrypted-only validation, which rejected keyring); merge the help text to document both encrypted-file and keyring. Both feature suites pass (encrypt + keyring store, the invalid-storage CLI guard, mcp/swarm/keyring); adds a test for the explicit Storage="encrypted-file" selector. The only remaining cli failures are the pre-existing env-dependent doctor/exec tests that pass in CI.
Collaborator
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
Collaborator
Author
|
@CodeRabbit approvable ? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "nextDailyDelay" --type go -A 20Length of output: 6937
// scheduler.go:338 — still drifts on DST changeover day:
next = next.Add(24 * time.Hour)
// Fix:
next = time.Date(now.Year(), now.Month(), now.Day()+1, hour, minute, 0, 0, now.Location())The goroutine correctly calls Also note: the existing |
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).
Collaborator
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
Collaborator
Author
|
@CodeRabbit approvable ? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "nextDailyDelay" --type go -A 20Length of output: 8593
All blockers resolved — approving now. [approve] |
Collaborator
Author
|
@CodeRabbit approve |
✅ Action performedComments resolved and changes approved. |
This was referenced Jul 7, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One PR, seven focused commits — the remaining audit-backlog items. All additive, fail-closed, Go-stdlib-only (no new module deps;
go.mod/go.sumunchanged), provider-neutral, with tests and (where applicable) a local run check. Each commit is independently green.Commits
TypeAPI (SA1019) — drops the redundantmsg.Typefallbacks in the 5 mouse helpers (the parser always setsButton+Actionand only derivesType). Clears all 11SA1019ininternal/tui; no behavior change.WebhookSinkwas never attached. NewMaybeAddWebhookSinkreadsZERO_NOTIFY_WEBHOOK_URL(no-op when unset; secret stays out of config) and is wired into headless exec + TUI. Gated by the existing Mode/focus policy. deadcode −7 (the sink machinery becomes reachable).Scheduler(interval ≥1s, optional first delay, MaxRuns; dailyHH:MMas a 24h interval) + aswarm_scheduletool (add/list/cancel, prompt-gated). Non-overlapping by default (skips a fire while the prior spawn runs). Deterministic tests via an injected ticker.bundleconnection mode (daemon core untouched), size-capped +git bundle verify-d + extracted into a per-link work tree (link ids sanitized + containment-checked).SessionLinkpersists 0600 (token never stored). CLI:daemon serve-remote --bundle-dir,daemon link. Full TLS E2E test.internal/oauthstore under themcp:namespace (same file as provider logins). Transparent, non-destructive migration: imports a legacymcp-oauth-tokens.json(never overwriting a newer entry), then renames it to.migrated. Public API unchanged. Local run check: real binary migrates a seeded legacy file, no secret leak.security(macOS) /secret-tool(Linux), blob stored base64 at rest, selected byZERO_OAUTH_STORAGE=keyring. File backend behavior preserved exactly. Honored by bothzero authand the unified MCP store with no call-site changes. Local run check (macOS): validated the realsecuritycommand shapes + not-found exit (44) andmcp oauth statusagainst the real keychain.U1000-confirmed (unexported, test-unused) symbols. Conservative: exported internal API left intact.staticcheck U1000now empty.Verification (integrated branch)
gofmt ·
go vet· build host + linux + windows · fullgo test ./... -race(0 failures) ·staticcheck(no new findings — remaining are pre-existing nits unrelated to these changes) ·govulncheck(0) ·deadcode(100 → 80, no genuinely-new unreachable funcs, verified by a name-based diff).go.mod/go.sumunchanged.Supersedes the individually-opened #214 and #215, folded in here as commits 1–2.
Summary by CodeRabbit
Release Notes
New Features
zero daemon linkto upload repos as bundles to a remote bridge (--bundle-dir) and optionally write a session link (--out).swarm_schedulefor recurring swarm member spawns.ZERO_NOTIFY_WEBHOOK_URLis set (optionalZERO_NOTIFY_WEBHOOK_SUMMARY).Improvements
ZERO_OAUTH_STORAGEstartup validation and support forencrypted-fileandkeyring, plus updatedzero auth --helpstorage documentation.Chores