mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-12 08:41:32 -10:00
chore/remove-dead-code
82
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 KRATOS
|
b1ab01f53f |
Hooks management command + build & supply-chain hardening (#193)
* Bump toolchain to go1.26.4 to clear reachable stdlib CVEs go.mod pinned toolchain go1.24.13, whose bundled standard library has 9 govulncheck-reachable CVEs (crypto/tls KeyUpdate DoS, net/http2 loop, crypto/x509 x3, net, net/url, os, net/textproto). CI and release build via go-version-file: go.mod, so shipped binaries inherited them. Bumping the toolchain directive to go1.26.4 clears all nine — govulncheck ./... now reports 0 reachable (minimum that clears them is go1.25.11; go1.26.4 matches the team's local Go). The go directive stays at 1.24.2 (minimum language version). Verified: go build/vet/gofmt clean, full go test ./... green. * Add govulncheck CI gate and SHA-pin GitHub Actions - ci.yml: new 'security' job runs govulncheck as a hard gate (fails on a reachable vulnerability; passes now that the toolchain is bumped) plus an advisory deadcode step that surfaces dormant code without blocking. - Pin every GitHub Action to a full commit SHA (checkout, upload-artifact, github-script) with a version comment, across all four workflows — previously only setup-go was pinned, leaving the mutable v4/v7 tags as a supply-chain risk. * Add hooks management command (add/remove/enable/disable) Wires the previously-dormant hooks.ConfigStore (10 unreachable funcs -> 0) into the CLI: zero hooks add <id> --event <event> --command <cmd> [--name --description --matcher --arg --user --json] zero hooks remove|enable|disable <id> [--user --json] Writes the project hook config by default (<cwd>/.zero/hooks.json) or the user config with --user, reusing the store's locking + atomic writes + validation (normalizeDefinition). New hooks are enabled; state is managed via enable/disable. JSON output is secret-scrubbed via redaction. Mirrors the existing 'zero mcp add/remove/enable/disable' shape. Covered by add/remove/toggle round-trip, validation, unknown-event, and JSON-redaction tests. * Fix govulncheck CI job: pin GOTOOLCHAIN to the go.mod toolchain go run govulncheck@latest selected the toolchain from govulncheck's own go.mod (downgrading to go1.25.11), which then could not load our go1.26-requiring packages (fips140only_go1.26.go), failing the security job. Resolve GOTOOLCHAIN from go.mod's toolchain line so govulncheck and deadcode both run under go1.26.4. * Address review: harden CI checkout, pin tool versions, validate hook event CodeRabbit findings on #193: - ci.yml: persist-credentials: false on all checkout steps (smoke/performance/security) — these jobs run repo code and never push, so don't keep the token in git config. - ci.yml: pin [email protected] and [email protected].0 instead of @latest, so the security gate is deterministic and not subject to supply-chain drift. - hooks: add exported IsValidEvent/KnownEvents (single source); parseEvent reuses it, and 'zero hooks add' now rejects an invalid --event with a usage error at parse time instead of falling through to the Upsert app-error crash path. |
||
|
Vasanth T
|
6895cbb4aa | feat: add backend lifecycle status command (#192) | ||
|
KRATOS
|
e0dd67a4da |
Wire LSP diagnostics into self-correct (exec + TUI) (#191)
* Wire LSP diagnostics into headless --self-correct newExecSelfCorrector now backs the --self-correct loop with both halves: the workspace test plan AND an lsp.Manager-based diagnostics checker (IncludeLSP=true). The manager is lazy (Manager.Check degrades a missing language server to nil,nil), and a deferred cleanup shuts it down at run end, terminating any spawned sessions. This revives the internal/lsp client (44 -> 3 unreachable funcs); it was dead because the only consumer passed a nil checker with IncludeLSP=false. Add TestManagerCheckRealGopls: drives NewManager -> Check against a real gopls and asserts a real diagnostic; skips when gopls is absent, so it stays CI-safe. * Wire self-correct into the interactive TUI (on by default) runAgentWithOptions now builds a SelfCorrector for every turn, so post-edit verification (workspace test plan + LSP diagnostics) runs by default in the TUI. The spec-draft planning path is excluded, matching exec. A per-turn lsp.Manager is created and torn down when the run returns. Auto-fix vs report-only follows the active permission mode (unsafe->high, auto->medium, ask/other->low) via selfCorrectAutonomyForMode. * Keep TUI self-correct fast by default; gate the test half behind /selfcorrect The TUI default now runs only the LSP-diagnostics half of self-correct — cheap and scoped to the changed files — so it never adds the whole-repo test plan's latency to a turn (and a pre-existing failure can't hijack the agent). The project test plan is opt-in per session via a new /selfcorrect [on|off] command (alias /sc); IncludeTests reads m.selfCorrectTests, default false. exec keeps both halves. * Remove dead session.didClose from lsp session.didClose sent a textDocument/didClose notification but was never called (the manager only closes documents via Shutdown), so it was unreachable even from tests and flagged by staticcheck U1000. Removing it takes internal/lsp to 0 truly-dead funcs and 0 U1000 findings; the 2 remaining unreachable-from-main funcs (Available, URIToPath) are public, test-covered API. * Align /selfcorrect help with accepted arguments The usage/description advertised only [on|off] but the handler also accepts status, list, tests, full, and lsp. Surface the primary verbs (on|off|status) and clarify the on/off behavior so command discovery matches the handler (CodeRabbit). * Make /selfcorrect usage strings match the handler exactly Both the command definition and the invalid-argument error message now list the full accepted set [status|on|off|tests|full|lsp]; the redundant 'list' alias is dropped so docs and handler agree exactly. Addresses Vasanth's review (the error-message usage at session_controls.go was still [on|off]) and CodeRabbit's help-text finding. * Clarify /selfcorrect off semantics in help text Reword the description so 'off/lsp' reads as 'disable tests, LSP-only' rather than the ambiguous 'return to LSP-only', which could be misread as disabling the feature entirely (CodeRabbit). |
||
|
Anandan
|
d1c5af7392 |
Polish model status and reasoning streams (#190)
* Polish model status and reasoning streams
* Polish custom provider setup flow
* Clean up pending composer hints
* Address custom provider setup review
* Polish composer overflow handling
* Fix composer paste and wheel edge cases
* Use line counts for paste previews
* Revert "Use line counts for paste previews"
This reverts commit
|
||
|
Vasanth T
|
250bf24c9d |
feat: add MCP manager UX (#188)
* feat: add MCP manager UX * fix: harden MCP manager UX * test: stabilize agent eval timeout on Windows * fix: preserve MCP server metadata * fix: clarify empty MCP view * feat: make MCP command a manager view * fix: make MCP manager usable in TUI * fix: make MCP manager selectable in TUI * fix: simplify MCP manager popup * feat: add searchable MCP marketplace * fix: address MCP review issues * fix: address MCP follow-up review * feat: add MCP setup wizard * feat: detect MCP setup requests in chat * fix: address MCP manager review feedback * fix: avoid hijacking MCP chat prompts * fix: persist MCP view cache fallback |
||
|
KRATOS
|
474f494b93 |
Deep-audit remediation + sandbox hardening (batches 1–35) (#189)
* audit batch 1: redact nested secrets fully; count image tokens in compaction estimate
- secrets.Redact: replace longest matches first so a containing PRIVATE KEY
block is redacted before a nested shorter match corrupts its exact string
(audit medium internal/secrets/scanner.go:81) + regression test.
- agent compaction estimateTokens: add a flat per-image token cost so an
image-heavy context trends toward compaction instead of reading as ~0
(audit medium internal/agent/compaction.go:58) + test.
- ledger: re-verify against current main; mark scanner.go:35 and
redaction.go:362 already-fixed-on-main, record this batch's fixes.
* audit batch 2: worktrees defaultRunGit separates stdout/stderr
Capture git stdout and stderr into separate buffers instead of CombinedOutput,
so parsed rev-parse output is not polluted by git warnings and CommandResult.Stderr
carries the real error text (audit medium internal/worktrees/worktrees.go:247) + test.
* audit: re-verify all 87 pending findings against current main
Independent agent per file re-checked each open/partial finding against the
actual current code (the 06-11 ledger was verified on a since-diverged branch),
with an adversarial second pass on every already_fixed verdict. Authoritative
result: 78 genuinely pending (66 still_open + 12 partial); 9 rows were already
fixed on main and are reclassified. Full table in 2026-06-13-reverification.md.
* audit batch 3 (agent): cancellation identity, always-allow w/o sandbox, compaction usage, dedup
- loop.go: check ctx.Err() before collected.Error so a cancelled run returns
context.Canceled (errors.Is holds) instead of a plain stringified error.
- loop.go: 'Always allow' with no sandbox engine now allows THIS call instead of
denying it (persistence is skipped when there's nowhere to persist).
- loop.go: remove a duplicate schema["items"] assignment in propertyToRuntimeMap.
- compaction.go: forward OnUsage (token accounting) through the summarizer while
still omitting OnText, so compaction's token cost is counted in usage/budgets.
- tests for all three behavioral fixes (each fails pre-fix).
* audit batch 4 (cli): list-tools -o json, interrupted -o json terminal, surface session-record failures
- exec --list-tools -o json now emits a JSON tool listing instead of falling
through to the human-readable text (audit medium exec.go:182) + test.
- interrupted run with -o json now emits a terminal error+done on stdout instead
of only printing to stderr, so a JSON consumer sees a clean end (medium exec.go:500) + test.
- execSessionRecorder: surface a latched session-append failure to stderr once at
run end (deferred) instead of silently dropping it (medium exec_sessions.go:114/115) + test.
- left app.go trailing-arg ignore as-is: documented-intentional, TUI has no argv prompt.
* audit batch 5 (zerogit): status -z porcelain (renames + non-ASCII paths); document env-drop on plain Runner
- parseStatus: switch `git status` to `--porcelain -z` and parse the
NUL-delimited form (audit medium zerogit.go:212). Previously the default
--short output was split on newlines, so a rename `R old -> new` was kept as
one bogus path and a non-ASCII/whitespace filename arrived C-quoted+escaped
(e.g. `"caf\303\251.txt"`). The -z form never quotes paths and emits a
rename/copy as `XY <dest>\0<src>`, so we now record the destination verbatim
and consume the source field. + TestParseStatusZHandlesRenamesAndSpecialPaths.
- resolveRunners: document that the plain-Runner EnvRunner adapter intentionally
drops env (audit low zerogit.go:417). The branch is reached only when a caller
supplies a custom Runner without a RunGitEnv — in practice fake-Runner tests.
Production leaves both nil and gets defaultRunGit/defaultRunGitEnv, which honor
GIT_INDEX_FILE, so snapshot-diff isolation holds. Routing env calls to real git
instead would break the fake-Runner test pattern and fix nothing in production.
- update Inspect/Commit mock tests to the -z status format and the new command.
* audit batch 6 (config): reject negative maxTurns; drop dead contract-gap helpers
- FileConfig.UnmarshalJSON now rejects a negative maxTurns instead of letting the
`MaxTurns > 0` merge gates silently drop it and fall back to the default,
hiding the misconfiguration (audit medium resolver.go:136). 0 is left as the
"unset" sentinel (omitempty) and still falls back to the default; the CLI
--max-turns flag continues to reject 0 too since there it is distinguishable
from absent. + TestResolveRejectsNegativeMaxTurns / ...ZeroFallsBackToDefault.
- remove dead config/contracts.go (ContractGap, DefaultContractGaps,
FindContractGapsByMilestone, ContractOwnerRuntime) and its test — referenced
only by that test, never by production (audit low contracts.go:12).
- NOT changed: ResolveMCP intentionally does not run the provider command
(TestResolveMCPDoesNotRunProviderCommand guards this), so the "ZERO_PROVIDER_COMMAND
MCP servers dropped" row is by-design, not a defect.
* audit batch 7 (mcp): bound captured MCP server stderr
connectStdio attached a plain bytes.Buffer as cmd.Stderr that is only ever read
on initialize failure, yet it kept growing for the entire process lifetime as a
long-lived server logged to stderr (audit medium client.go:98). Replace it with
a concurrency-safe boundedBuffer capped at 64 KiB that keeps the head (enough for
init-error diagnostics) and discards the rest, reporting full write lengths so
os/exec never sees a short write. + TestBoundedBufferCapsRetainedBytes.
Left as-is (not defects worth the risk in the MCP client):
- client.go writer.write under client.mu: already designed to release the lock
before the unbounded response wait; the residual concern is a server that stops
draining stdin, an involved async rewrite for a theoretical deadlock.
- readLoop not answering server-initiated requests (ping): responding safely
needs re-locking around the shared writer; no configured server relies on it.
* audit batch 8 (tools/providers): read_file truncation marker; drop dead provider branch
- read_file: when a max_lines cut occurs, append an explicit "[truncated: N more
line(s)... set start_line=X to continue]" marker to the output. Previously only
the Result.Truncated flag was set, which is invisible in the rendered text, so
the model could not distinguish a cut read from a complete one (audit low
read_file.go:89). + TestReadFileToolMarksTruncation.
- providers/factory: simplify `if !explicitProvider || isImplicitOpenAI(...)` to
`if !explicitProvider` and delete the now-unused isImplicitOpenAI. The clause
was dead: explicitProvider==true implies ProviderKind or Provider is set, but
isImplicitOpenAI required both empty, so it could never contribute a case
(audit low factory.go:118). Behavior-preserving; providers tests still green.
* audit batch 9 (sessions): Tree builds from a single List() snapshot (no N+1 disk scans)
Store.Tree recursed via ListChildren per node, and each ListChildren ran a full
store.List() that re-read every metadata.json from disk — O(nodes * total-sessions)
reads for one tree (audit medium lineage.go:140). Snapshot all sessions once, index
children by parent in memory, and recurse over that. Child ordering is factored into
sortChildSessions so ListChildren and Tree stay identical; path-scoped cycle
detection (delete(seen,...)) is preserved. Existing tree/lineage tests still pass.
Left as-is: store.go timestamp() second precision (RFC3339) — switching to
RFC3339Nano would break lexical UpdatedAt ordering against existing second-precision
records ("...00.5Z" sorts before "...00Z"), so it is not a safe one-line change.
* audit batch 10 (specialist): serialize accounting dedup-then-append
recordSpecialistStop and appendSpecialistUsageRollup checked for an existing event
(specialistEventExists → ReadEvents) and then appended under SEPARATE store locks,
so two concurrent finishers — a foreground onExit racing a TaskOutput poll, or a
background reaper — could both pass the dedup check and write duplicate stop/usage
events (audit medium accounting.go:72). Guard each check-then-append pair with a
package-level accountingMu (Executor is used by value, so a struct field would not
be shared). + TestRecordSpecialistStopDedupesUnderConcurrency (passes under -race).
* audit batch 11 (zeroruntime/gemini): O(1) text accumulation; gemini cached-token counts
- zeroruntime CollectStreamWithOptions accumulated assistant text with
`collected.Text += event.Content`, reallocating the whole string on every chunk
(O(n^2) over a long streamed response). Accumulate in a strings.Builder on the
collector and materialize it once in flush, the single finalization point hit
before every return (audit low helpers.go:104). Existing collect tests still pass.
- gemini decoded usageMetadata without cachedContentTokenCount, so a cached-prompt
response reported zero cached input tokens while the other providers report them
(audit medium gemini/types.go:88). Decode the field and thread it through
streamState into NormalizeUsage as CachedInputTokens. + extended
TestStreamCompletionEmitsTextUsageAndReasoningTokens.
* audit batch 12 (specialist): parse child stream without a line-length cap; drop dead helper
- ParseStream used a bufio.Scanner capped at 1 MiB, so one large stream-json line
from a child (a big tool result or final answer) hit bufio.ErrTooLong and aborted
the entire specialist run (audit medium streamer.go:43). Switch to a bufio.Reader
with ReadString, which has no per-line limit; the child is our own trusted
subprocess, so its output length is the legitimate bound. Line numbering and
blank-line/EOF handling are preserved. + TestParseStreamHandlesLineLargerThan1MiB.
- remove dead formatTaskOutput (output_tool.go:244): zero callers and zero tests;
it only forwarded to formatTaskOutputSummary, which is the one actually used.
* audit batch 13 (background/specialist): kill background tasks as a process group
Background children were launched in the parent's process group and terminated by
signalling only the single PID, so any process the task forked was orphaned and
outlived a cancel; the signal also risked hitting a recycled PID after the child
was reaped (audit medium process_posix.go:38/50).
- ConfigureChildProcessGroup (Setpgid on POSIX, no-op on Windows where taskkill /T
already tree-kills) is applied at launch so the child leads its own group.
- terminateProcess now SIGTERM/SIGKILLs the negative PID (the whole group), with a
pid>1 guard so a bogus 0/1 can never expand to "our own group" or "everything",
and treats ESRCH as already-gone. Liveness polls the group. Targeting a group
(rarely recycled vs. a bare PID) also shrinks the reaped-then-recycled window.
- specialist launchBackgroundProcess opts every child into its own group; on a
SetPID failure it now kills the child + group, marks the task errored, and records
the stop (deduped) instead of leaking an untracked orphan.
- tests launch as group leaders; new TestTerminateProcessKillsForkedChildren proves
a forked grandchild dies with the group.
* audit batch 14 (sandbox): quote-aware shell segment splitting
splitShellSegments used a quote-unaware strings.Replacer, so a shell operator
inside quotes was treated as a separator: `git commit -m "use top | less"` split
into a `less` segment and `echo "a; vim b"` into a `vim` segment, both falsely
flagged as interactive (audit medium safe_command.go:138).
Replace it with a quote-aware scanner that mirrors the shell: single quotes make
everything literal; double quotes keep $(...)/`...` substitution active but treat
|, ;, && as literal; unquoted text splits on the same operator set as before
(;, |, ||, &&) plus substitution boundaries. Real unquoted operators still split,
so an interactive program behind a separator — or inside a live `"$(...)"`
substitution — is still caught (no new false negatives, the security-relevant
direction). + TestDetectInteractiveQuoteAwareSeparators covering both directions.
* audit batch 15 (cron): collapse DST fall-back repeated hour to one fire
On a fall-back day the clock repeats an hour (e.g. 01:30 EDT then 01:30 EST), so
Next("30 1 * * *", <01:30 EDT>) returned the same-day 01:30 EST — firing a daily
job twice (audit medium schedule.go:171, which previously guarded only the
spring-forward gap).
Next now skips a match whose local wall-clock minute equals `after`'s but whose
absolute time is later (the repeated-hour duplicate) and keeps searching. Normal
forward search always advances the wall-clock, so this only triggers on the
fall-back repeat; when the scheduler passes the last fire time as `after`, the
repeated hour collapses to a single fire. + TestNextDSTFallBackDoesNotRepeatHour
(spring-forward termination test still passes).
Note: Next is stateless, so it dedupes relative to `after`; a caller that passes
an arbitrary mid-hour "now" instead of the last fire time is out of scope here.
* audit batch 16 (mcp): skip SSE notifications before the response; larger SSE buffer
- decodeSSERPCMessage returned the first non-empty `message` event unconditionally,
so a server-initiated notification/request arriving before the response made the
caller see an id mismatch and fail (audit medium network_client.go:621). It now
skips any message carrying a method (notifications/requests have one; a response
does not) and returns the first real response; the caller's rpcIDMatches still
validates the id. + TestDecodeSSERPCMessageSkipsNotifications.
- raise the per-event SSE scanner cap from 1 MiB to a bounded 8 MiB so a large but
legitimate MCP message no longer hits bufio.ErrTooLong and fails the request
(audit medium network_client.go:637); still bounded against an unbounded server.
Out of scope (kept): full reconnect of a persistent SSE stream after a fatal read
error — a larger change than this remediation.
* audit batch 17 (sessions): fsync session metadata before the atomic rename
writeMetadata wrote the temp file with os.WriteFile (no sync) and renamed it into
place, so a crash right after the rename could expose a metadata file whose bytes
were never flushed — a torn or empty file that corrupts the whole session (audit
medium store.go:632). Write+fsync the temp via a new writeFileSync helper before
renaming, closing that window. + TestWriteFileSyncRoundTrips.
Deliberately scoped:
- Event-log appends (appendEventLocked) are NOT fsynced per append: that would add
a disk flush to every event (every tool call), and a hard-crash loss of the log's
unsynced tail is acceptable, whereas torn METADATA is not. Metadata is the
integrity-critical part and is what this syncs.
- Directory fsync is omitted for cross-platform simplicity (it errors on Windows);
the file fsync already prevents the torn-contents failure, the serious case.
Left for follow-up (in this bucket but not safe/clear wins here):
- hooks append sequence still re-reads the file per append; the in-memory cache is
cross-process-unsafe and the safe last-line read is fiddly for a low-volume log.
- Fork event re-append/de-dup is nuanced usage-accounting and needs its own change.
* audit batch 18 (agent): count tool-defs in compaction estimate; don't deny an always-allow on persist failure
- maybeCompact's estimate omitted the tool definitions, which ride on every
request, so the real input could exceed the model limit while the message-only
estimate stayed under threshold and compaction never fired (audit medium
compaction.go:58). partitionTools is now computed before maybeCompact (it does
not depend on the messages) and the exposed tools' estimated tokens are added to
both the threshold check and the post-compaction shrink check. + estimateToolDefTokens
and TestEstimateToolDefTokensCountsDefinitions.
- PermissionDecisionAlwaysAllow denied the call when persistPermissionGrant failed,
overriding what the user explicitly allowed (audit medium loop.go:468). Persisting
is now best-effort: a failure (or no sandbox engine) just means the grant is not
remembered and the user is re-prompted next time, not denied now. This also drops
the dead requestEvent.GrantMatched/Grant writes (audit low loop.go:473) — the
emitted event derives them from the sandbox decision via buildPermissionEvent.
Left intentional (documented, not defects): the reactive-retry path omits OnText to
avoid duplicating mid-stream text (loop.go:183); OnContext/MeasureContext is a
complete, tested context-breakdown scaffold awaiting a consumer (types.go:183).
* audit batch 19 (usage): price escalated usage from the event's own model
exec persists payload["model"] on --allow-escalation runs (the model can change
mid-run), but BuildReport priced every event from the session's Metadata.ModelID
and never read the payload, so usage from an escalated model was mis-priced at the
base model (audit medium report.go:17). usageEventPayload now carries Model and
BuildReport prefers it, falling back to the session model when absent (older
events stay byte-identical and behave as before). + TestBuildReportPricesFromEventModelWhenPresent.
* audit batch 20 (sessions): don't copy usage events into a fork (no double-count)
Fork re-appended every parent event, including provider_usage, into the child, so
a usage report aggregating the parent and the fork double-counted the parent's
usage (audit medium store.go:387). Skip EventUsage when copying — it already
counted against the parent and is not part of the conversation history a fork
replays — and record the actual copied count in the fork marker. + TestStoreForkSkipsUsageEvents.
* audit batch 21 (cli/skills): warn on duplicate skill names instead of silently dropping
skills.Load deduplicates same-named skills (first directory wins) and records the
dropped collisions via skills.Duplicates, but nothing surfaced them, so a shadowed
skill just vanished with no signal — and Duplicates had no production caller (audit
low skills.go:91). `skills list` now reports each collision to stderr (keeping
stdout/--json clean), using Duplicates. + TestRunSkillsListWarnsOnDuplicateNames.
Note: skills.Get is kept — it is a by-name accessor used throughout the package's
tests, not dead code.
* audit batch 22 (cli/cron): capture the run error from stream-json stdout
Cron jobs run exec with --output-format stream-json, so a failure is reported as an
`error` event on STDOUT, but the run record's Error was set only from stderr — so a
failed job often recorded an empty/uninformative error while the real message sat
unread in outBuf (audit low cron_run.go:150). On a non-zero exit, scan the stdout
stream-json for the error event's message and prefer it, falling back to stderr.
+ extractStreamJSONError and TestExtractStreamJSONError.
* audit batch 23 (sandbox): richer macOS seatbelt profile (signal + mach-lookup)
The generated sandbox-exec profile was default-deny with only process/sysctl/file
allowances, so two real gaps remained on macOS (sandbox quality report H1):
- A sandboxed script could not signal the children it spawned — `kill` returned
"Operation not permitted" under seatbelt, breaking common `cmd & ... kill`,
test-runner, and timeout patterns. Add `(allow signal (target self) (target pgrp))`,
which stays scoped to the command's own process group.
- XPC to system daemons was denied, so keychain (securityd/trustd), user/group
lookup (opendirectoryd), preferences (cfprefsd), network config
(SystemConfiguration), launch services, and the pasteboard all failed. Add a
curated `(allow mach-lookup ...)` allowlist (`sandboxMachServices`).
Neither touches the file-write or network rules, so the workspace boundary is
unchanged — verified on a real macOS host: `security`/`scutil`/`pbpaste`/self-kill
now succeed while writes to $HOME and /etc are still denied. iokit-open was tried
and dropped as unnecessary. + unit assertions (TestSandboxExecProfileGrantsSignalAndMachLookup)
and extended the darwin integration test with a self-kill case and an
outside-workspace deny check.
* audit batch 24 (sandbox/tools): remove dead OnSandboxDecision hook
RunOptions.OnSandboxDecision had no setter anywhere, yet registry.RunWithOptions
spawned a goroutine (with panic recovery) for it on every sandboxed tool call —
pure per-call overhead for an unreachable callback (sandbox report M1). The sandbox
decision is already surfaced to observers via the permission event
(buildPermissionEvent reads the decision), so this hook is redundant; delete the
field and its call site.
* audit batch 25 (sandbox/cli): exact-path grants — create and revoke by path
The engine already matched ScopeFile grants and DeriveScope already produced them
from path/file tool args, but the CLI could neither create one explicitly nor
revoke a single one (sandbox report L3 / prompt 7). Add:
- GrantStore.RevokePath(tool, path): revokes only the grant whose scope matches the
given file/dir, leaving tool-wide and other-path grants intact (path canonicalized
to absolute like stored scopes). + TestGrantStoreRevokePathRemovesOnlyMatchingScope.
- `zero sandbox grants allow <tool> --path <p>`: persists an exact-file (ScopeFile)
grant, path resolved to absolute so it matches lookups and revokes.
- `zero sandbox grants revoke <tool> --path <p>`: revokes just that grant (default
still revokes all grants for the tool). + TestRunSandboxGrantsCreateAndRevokeByPath.
The TUI permission-card "Allow always (exact path)" option is intentionally left for
the TUI feature branches (it overlaps #187/#188); the engine + CLI now cover the
full create/match/revoke round-trip for file-scoped grants.
* audit batch 26 (sandbox): Engine.Precheck — report violations before execution
Add Engine.Precheck(ctx, request) []Violation, which reports the sandbox
violations that would block a tool request before it runs (sandbox report / prompt 6).
It reuses Evaluate so policy is not duplicated: an allowed or merely-prompted
request yields no violations; a denied request yields its violation (via the shared
violationsFromDecision helper, with a ViolationPolicyDenied fallback for a deny that
carries no structured violation). A nil engine yields nothing.
+ TestEnginePrecheckReportsViolationsBeforeExecution.
Wiring Precheck into the batch-confirmation prompt is intentionally left to the TUI
branch: it touches prompt rendering, and short-circuiting the agent loop's deny path
would skip the before/after-tool hooks that currently fire around a policy-denied
call. The engine API is in place for that consumer.
* audit batch 27 (sandbox): scoped-egress domain prompt with timeout + caching
The scoped-egress proxy failed closed on any host outside the allowlist. Add an
opt-in DomainPrompt callback (prompt 8): an UNKNOWN host (neither allowed nor
explicitly denied) is authorized at request time via the callback instead of being
refused outright. An explicitly denied host is still refused without prompting; the
callback is bounded by PromptTimeout (default 60s, timeout/error => deny); decisions
are cached for the proxy's lifetime so a host is prompted at most once. With no
callback set the strict fail-closed behavior is unchanged. Wired through both the
CONNECT and HTTP handlers via authorizeTarget.
+ TestEgressProxyAuthorizeDomainPrompt / ...TimeoutDenies / ...NilPromptFailsClosed.
The user-facing prompt that supplies the callback is left to the permission UI (TUI)
branch; the egress mechanism (callback + timeout + cache) is in place and tested.
* audit batch 28 (sandbox): AST-based shell analysis (AnalyzeCommand)
Add AnalyzeCommand(script) AnalysisResult, a static shell analyzer built on the
mvdan.cc/sh/v3 parser (prompt 12). Walking the parsed command tree makes it a more
precise second opinion than the regex detector: a program name only counts when it
is an actual command, so `echo "vim ..."`, `printf 'open with vim'`, and
`git commit -m "rm -rf /"` are clean, while real commands are flagged —
interactive (editors/pagers + REPLs, with the same -e/script suppression as
nonInteractiveREPLFlags), destructive (rm -rf/-fr, dd, mkfs, shred, ...), and
network (curl/wget/ssh/nc/...). An unparseable script yields TooComplex so callers
can treat it as higher-risk. + TestAnalyzeCommand (16 cases) / ...EmptyIsClean.
Dependency: mvdan.cc/sh/v3 pinned to v3.11.0 — compatible with the existing go 1.24
directive (the latest v3.13.1 would have bumped the module to go 1.25). It is kept
as a standalone analyzer for callers to consult; feeding it into Engine.Evaluate is
deliberately not done here, since that would change allow/deny classification.
* audit batch 29 (sandbox): unify proxy-env generation (ProxyEnv)
Export ProxyEnv as the single source of truth for proxy-env injection (prompt 9
req 1), replacing the unexported scopedProxyEnv used by the sandboxed-shell path.
+ TestProxyEnv.
Investigation that bounds the rest of prompt 9 (documented in the ProxyEnv doc):
- web_fetch already honors HTTP_PROXY/HTTPS_PROXY — its transport clones
http.DefaultTransport, whose Proxy is http.ProxyFromEnvironment.
- MCP children already inherit the parent env (mergeProcessEnv appends os.Environ),
so they pick up any proxy vars the agent has.
So the only missing link to scope FetchUrl/MCP is a SESSION-level egress proxy whose
address reaches the agent process. That is deliberately NOT wired here: doing it
safely requires allowlisting the active LLM provider's own domain first, or the
agent's provider calls would route through the scoped proxy and be denied — a
correctness pitfall that also can't be verified end-to-end on this host. Flagged
rather than shipped blind.
* audit batch 30 (sandbox): Linux seccomp Unix-socket block — UNVERIFIED, needs Linux CI
Adds a pure-Go (no cgo, no vendored binary) seccomp BPF filter that denies
socket(AF_UNIX) with EPERM on x86-64/arm64, closing the Unix-socket gap bubblewrap's
filesystem/network isolation leaves open (prompt 11):
- internal/sandbox/seccomp.go: the classic-BPF program as a platform-neutral
[]sockFilter, built and UNIT-TESTED (structure + every jump offset in range — the
classic way a hand-written BPF filter goes silently wrong).
- seccomp_linux.go: ApplyUnixSocketBlock installs it via prctl(NO_NEW_PRIVS) +
prctl(SET_SECCOMP, MODE_FILTER). seccomp_other.go is a no-op everywhere else.
- cmd/zero-seccomp: a tiny linux-only exec wrapper that applies the filter then
execs the real command (the standard way to install seccomp in a child with
os/exec); degrades gracefully (warns + runs) if the kernel lacks seccomp.
⚠️ HONEST CAVEAT: I developed this on macOS and CANNOT run the actual AF_UNIX block
here. darwin build, `GOOS=linux` cross-compile, vet, and the structure test all pass,
but the unit test verifies the program's SHAPE, not that the kernel actually blocks
AF_UNIX — that MUST be verified on Linux CI (create a Unix socket under the wrapper
and assert EPERM). Safe to ship as-is: it is linux-only, no-op on every other
platform, and not wired into the bwrap command yet (opt-in prefixing the sandboxed
command with zero-seccomp is the remaining integration), so it cannot regress any
current path.
* audit batch 31 (sandbox): macOS denial log monitor (opt-in, default off)
Re-adds the macOS seatbelt-denial monitor as an opt-in feature (prompt 10). When
Policy.MonitorDenials is set, the sandbox-exec profile tags its default-deny with a
marker and bash tails `log stream` for that tag during the command, appending any
captured denials as a <sandbox_violations> block to the command's stderr so the
model can see what was blocked.
- DenialMonitor (darwin) tails/parses tagged denials with a capped, deduped ring
buffer and noise filtering; no-op stub on other platforms. parseSandboxDenyLine is
unit-tested. + profile-tag test + appendSandboxViolations test.
- Default OFF: with MonitorTag empty, StartDenialMonitor is a no-op and the command
path is byte-for-byte unchanged, so there is no regression or per-command overhead
unless enabled.
CAVEAT: on at least one macOS host (verified live) seatbelt denials are NOT delivered
to the `log stream`-queryable unified log, so the monitor captures nothing there. It
is the standard mechanism and works on macOS versions that do log denials; shipping
it default-off means it never affects whether a command runs.
* audit batch 32 (sandbox): wire seccomp Unix-socket block into bubblewrap (opt-in)
Completes prompt 11: the zero-seccomp helper (which installs the AF_UNIX seccomp
filter) is now actually invoked by the sandbox instead of only existing as a
standalone binary.
- bubblewrapCommandPlan, when Policy.BlockUnixSockets is set, ro-binds the
discovered zero-seccomp helper into the sandbox at its host path and prefixes the
command with it so the filter is installed before the real argv runs.
- findSeccompHelper discovers the helper next to the running executable first, then
on PATH; it returns "" when absent, so the sandbox degrades gracefully (runs
without the extra filter — bubblewrap's fs/net isolation still applies) rather
than failing the command. Exposed as a package var so the wiring is unit-tested
on any OS.
- Reachable from config: SandboxConfig.BlockUnixSockets and .MonitorDenials now map
onto the policy in applyConfiguredSandboxPolicy (opt-in only — config can turn a
hardening flag on, never off). Both default OFF.
- Tests: helper-prefix present when enabled / absent by default; config flags apply
independently and stay off when omitted.
Default OFF and Linux-only effect; the runtime seccomp enforcement still needs a
Linux host/CI to exercise end-to-end (the wiring and BPF program are unit-tested).
* audit batch 33 (sandbox): enforce scoped/deny network policy in web_fetch & web_search
Completes prompt 9: the built-in network tools now honor the SAME allow/deny/scoped
policy the sandboxed shell egress proxy enforces, instead of bypassing it. Before
this, web_fetch/web_search would reach any public host even when the policy was
NetworkDeny or NetworkScoped — the egress proxy only constrained shell commands.
- New shared gate Engine.NetworkHostAllowed(host) reuses the egress domain matcher
(domainAllowed/normalizeDomains/effectiveNetwork) as the single source of truth:
allow → all hosts; deny → none; scoped → only AllowedDomains minus DeniedDomains
(empty allowlist collapses to deny). Disabled policy / nil engine impose nothing.
- web_fetch is now sandbox-aware: the host is checked before the request AND on every
redirect, so a denied/unlisted host fails closed before any dial. Plumbed via a
networkPolicyGate so the no-sandbox path is byte-for-byte unchanged.
- web_search is sandbox-aware: the configured backend's endpoint host is checked
before the query leaves the machine; blocked under deny, and under scoped unless the
backend host is allowlisted.
- The registry already routes sandbox-aware tools through RunWithSandbox when an
engine is present, so this activates automatically wherever the sandbox is wired.
- Tests: engine gate matrix (deny/allow/scoped/denylist/empty/disabled/nil + host:port);
web_fetch & web_search block denied/unlisted hosts without dialing and allow listed
ones; endpoint-host parsing.
* audit batch 34 (sandbox): prove network-policy wiring end-to-end + document opt-in flags
Verification follow-up to batches 31-33; no behaviour change.
- Adds a registry-level test that exercises the FULL chain for the scoped-network
enforcement: registry.RunWithOptions -> engine.Evaluate (passes a scoped,
egress-capable sandbox-exec backend) -> routes web_fetch through RunWithSandbox ->
per-host allowlist blocks an unlisted host before any dial. This proves the
Tool-interface value actually satisfies sandboxAwareTool at the registry boundary,
not just in a direct method call.
- Documents the two opt-in hardening flags (sandbox.blockUnixSockets,
sandbox.monitorDenials) and the scoped web_fetch/web_search behaviour in the README
so the config keys are discoverable. Both remain off by default.
Confirmed wiring: agent loop forwards options.Sandbox to the registry; the registry
routes sandbox-aware tools (web_fetch, web_search) through RunWithSandbox when an
engine is present; MonitorTag is set only on the macOS sandbox-exec plan; the
seccomp helper prefix is added only on the bubblewrap plan when BlockUnixSockets is
set; both config flags map onto the policy via applyConfiguredSandboxPolicy.
* audit batch 35 (sessions): fix Windows CI failure in TestWriteFileSyncRoundTrips
The exact-permission assertion (perm == 0o600) added in batch 17 fails on
windows-latest because Windows does not honor Unix permission bits: Go reports
0o666 for a writable file regardless of the mode passed to OpenFile. This was the
sole cause of the failing Smoke (windows-latest) CI job; macOS and Ubuntu passed.
Guard the perm check with runtime.GOOS != "windows" so it still verifies the mode
on the platforms that enforce it, while the round-trip (write + read-back) is still
asserted on every platform.
* audit batch 36: address CodeRabbit review (5 findings)
- background/process_posix: after SIGKILL, poll until the process group is
actually gone (SIGKILL is async) and return an error if it survives past a
second deadline, instead of returning nil while descendants still race.
- sandbox/safe_command: only treat ')' as a substitution boundary when it closes
an active $(...). A literal ')' (e.g. echo "a) less") no longer splits into a
fake `less` segment and is no longer misflagged as interactive. Depth-tracked so
nested $(...) still isolate their inner command. Direct splitShellSegments test
(fails pre-fix: yields ["echo \"a", "less\""]).
- cron/schedule: the DST fall-back collapse now only applies when `after` sits
exactly on the minute boundary (its "last fire time" form). With sub-minute
precision (e.g. 01:30:30 EDT) the first 01:30 fire already passed, so the repeated
01:30 EST is the legitimate next fire and is returned — restoring strictly-after
semantics. New test fails pre-fix (skipped to next day).
- config/types: maxTurns error message now reads "must be >= 0" to match the check
(0 is accepted; only < 0 is rejected). Negative-maxTurns test asserts the message.
- docs/audit: header date/branch metadata now matches the body (2026-06-13,
fix/audit-batch) instead of the stale 2026-06-11/fix/audit-critical-high snapshot.
Verified: gofmt clean; go vet clean; host + linux + windows builds; affected suites
green. Each behavioural fix ships with a test confirmed to fail without the change.
* audit batch 37: address CodeRabbit re-review (12 findings)
Findings on the audit code from batches 21-36; each behavioural fix ships with a
regression test (several confirmed to fail without the change).
sandbox/analyzer (AnalyzeCommand):
- Unwrap launcher prefixes (sudo/env/nice/bash -c/...) and recurse into `sh -c`
payloads so `sudo rm -rf`, `env curl …`, `bash -c 'vim x'` classify on the real
command, not the launcher. Mirrors DetectInteractiveCommand.
- Honor rm's `--` end-of-options marker so `rm -- -rf` (delete a file named -rf) is
not flagged destructive.
sandbox/engine: NetworkHostAllowed now shares one backend-aware effective-mode
helper with Evaluate, so scoped collapses to deny when the backend can't enforce
scoped egress — the per-tool gate can no longer permit traffic the engine-level
decision would fail closed.
sandbox/egress: cache domain-prompt decisions by host:port, not host, so one
"allow" can't authorize other ports on the same host.
sandbox/safe_command: the segment splitter is now escape-aware (\| , \" ) so an
escaped operator/quote can't manufacture a fake interactive segment.
sandbox/log_monitor (darwin): Wait() the `log stream` child after draining stdout
so the cancelled process isn't leaked as a zombie.
tools/web_search: confine the search backend to same-host redirects (with a limit)
so a redirect chain can't egress to a host the scoped/deny policy never authorized.
cli/sandbox: an explicit empty --path (`--path=` / `--path ""`) now fails closed
instead of silently widening an allow to tool-wide or a revoke to all-grants.
cli/skills: warn on stderr when duplicate-skill detection fails instead of
swallowing the error.
specialist/exec: don't mask a failed orphan-kill — join the TerminateProcess error
with the SetPID error so a surviving process is visible.
background/process_posix: resolve the PGID and only group-signal when pid leads its
own group; otherwise signal the individual PID. Avoids both a false-success on a
non-leader ESRCH and the hazard of signalling an unrelated (possibly our own) group.
Verified: gofmt + go vet clean; host + linux + windows builds; full go test ./...
green.
* audit batch 38: address CodeRabbit re-review round 2 (3 findings)
Follow-ups to batch 37, each with a regression test:
- safe_command (splitter): a ')' is now only a substitution boundary when it
closes an active, UNQUOTED $(...). A quote-state stack saved at each '$(' lets a
substitution run in a fresh quoting context and restores the outer quoting on
its matching ')'. Fixes `echo $(printf "a) less")` splitting into a fake `less`
segment, while still isolating the inner command of `echo "$(vim x)"`.
- egress: deduplicate in-flight domain prompts per host:port. Concurrent requests
for the same unknown target now wait on the first prompt and reuse its cached
decision instead of each firing a prompt (a prompt storm) and racing to write
the final decision. Race-tested with 25 concurrent authorizers → one prompt.
- analyzer + safe_command: wrapper option-value consumption is now wrapper-specific
(wrapperValueOptionsByProg) instead of a global option set. A valueless flag like
`sudo -n` no longer swallows the real payload, so `sudo -n rm -rf` /
`sudo -n curl …` are correctly classified destructive/network; `sudo -u root vim`
still consumes its value. Shared by the AST analyzer and the regex detector so
they can't diverge.
Verified: gofmt + go vet clean; host + linux + windows builds; go test ./... and
-race on sandbox/background all green.
* audit batch 39: address CodeRabbit re-review round 3 (12 findings)
All verified against current code; each behavioural fix has a regression test.
Security / correctness:
- tools/web_search: redirect policy now also refuses a scheme change, blocking a
same-host https→http downgrade that would send the query + bearer token over
plaintext.
- sandbox/safe_command: a ')' is only a substitution boundary when it closes an
active UNQUOTED $(...), and backticks now get the same quote-state save/restore
as $( — so `echo "`a | less`"` and `echo "$(true | less)"` no longer hide the
inner pager from interactive detection.
- sandbox/analyzer + safe_command: wrapper option-value consumption stays
wrapper-specific, and unwrapping no longer aborts on a dynamic ($x) wrapper arg
once a wrapper is active, so `env "$opts" curl` / `sudo "$x" rm -rf` are still
classified.
- sandbox/egress: in-flight prompts deduped per host:port (carried from batch 38);
the prompt callback is now cancellable (context cancelled on timeout) so a stuck
prompt can abort instead of leaking; signature threaded through.
- sandbox/runner: per-plan unique denial tag (pid+counter) so concurrent monitored
sandbox-exec runs can't ingest each other's denials; helper selection now
requires an executable regular file so a non-exec sibling degrades gracefully.
- mcp/network_client: aggregate SSE event-size cap (not just per-line) so a server
can't grow memory with many unterminated data: lines.
- specialist/accounting: a previously-recorded stop/usage with an empty runId is a
catch-all and now suppresses a later runId-bearing duplicate.
- sessions/lineage: Tree fetches the root via Get() first so a corrupt root
surfaces its real error instead of degrading to "not found".
Docs/tests:
- background/terminate: docstring now states the leader-only group-kill (PID-only
fallback for non-leaders); test child-liveness probe uses ESRCH specifically.
- cli/sandbox: grant usage strings include --path.
Verified: gofmt + go vet clean; host + linux + windows builds; go test ./... and
-race on sandbox/background all green.
* audit batch 40: address CodeRabbit re-review round 4 (8 fixed, 1 skipped)
Verified each against current code.
Fixed:
- sessions/lineage: PANIC FIX — store.Get returns (nil, nil) for a missing
session, so the batch-39 root prefetch could nil-deref. Guard root == nil and
return a not-found error. Regression test added (was a panic).
- agent/compaction: the reactive recover() now records lowWaterMark in the SAME
combined (messages + tool-defs) token domain maybeCompact uses, so the proactive
shrink-guard compares like with like. recover() takes tools; callers pass
request.Tools.
- mcp/network_client: SSE aggregate counter no longer counts a joining newline for
the first data line (only when prior content exists), avoiding premature rejects.
- sessions/store: fsync the parent directory after the metadata rename so the
rename is durable across a crash (no-op on Windows, which can't fsync a dir).
- sandbox/egress: parse the target port with strconv.Atoi instead of
net.LookupPort (no context-unaware resolver use).
- sandbox/runner: escape the per-plan denial tag through sandboxProfileString when
embedding it in the seatbelt profile.
- sandbox/seccomp_linux: guard against an empty filter program before taking
&kernelFilters[0] (defensive; the program is fixed non-empty today).
- cli/cron_run_test: assert last-error-wins with two error events.
- background/terminate, cli/sandbox usage, process_posix_test ESRCH (carried).
Skipped (with reason):
- zerogit worktree-rename (Y='R') test: git porcelain v1 -z only emits the
rename-source field when the INDEX column is R/C; the worktree column never
carries 'R' with a source field in v1, so the fixture would test output git
doesn't produce, and making parseStatus consume on code[1]=='R' would risk
mis-consuming the following entry for a plain ` R` status. Left as-is.
Verified: gofmt + go vet clean; host + linux + windows builds; go test ./... and
-race on sandbox/background all green.
* audit batch 41: address CodeRabbit re-review round 5 (4 findings)
All verified against current code; code fixes have regression tests.
- agent/compaction: summarizeWithFallback's reduce pass no longer swallows ALL
errors. A context-limit error still falls back to the joined partial summaries
(documented "extreme" case), but a non-context failure (auth/network/provider)
now surfaces unchanged, per the function's contract. Two tests (propagate vs.
joined-fallback); the propagate test fails without the fix.
- sandbox/safe_command + analyzer: wrapper value-consuming options now include the
long spellings (sudo --user root, env --unset NAME, timeout --signal …), so a
long flag's value is no longer mis-read as the program (interactive/destructive/
network detection bypass). A `--flag=value` token carries its own value and never
consumes the next token. Tests for space and = forms in both the AST analyzer and
the regex detector.
- docs/audit: the status-ledger summary table's open/partial cells (64/14) were the
lone outlier — the ledger's own narrative and 2026-06-13-reverification.md both
state 66/12. Corrected to 66/12 (66+12+97 = 175); derivation: body 69 open + 18
partial minus the 9 already-fixed-on-main (3 open + 6 partial) = 66/12 = 78
pending, fixed 88+9 = 97. Added a comment documenting the summary-vs-body
relationship. The reverification doc already states 66/12, so both now agree.
Verified: gofmt + go vet clean; host + linux + windows builds; full go test ./...
green.
* audit batch 42: address CodeRabbit re-review round 6 (1 fixed, 1 kept w/ proof)
CodeRabbit re-surfaced two "duplicate" refinements; CI was already green (its 6th
pass was a COMMENT with no new findings).
- cli/sandbox_test (TestRunSandboxGrantsRejectsEmptyPath): now seeds a tool-wide
grant and asserts grant-store immutability after EACH rejected empty-`--path`
call (reflect.DeepEqual before/after), so a mutation in one call can't be masked
by a later one — and a buggy "revoke all" is actually observable (it's a no-op on
an empty store). Stronger than the suggested patch.
- zerogit.parseStatus: KEPT consuming the rename source on the index column only
(code[0]), with an explanatory comment. Verified empirically that git porcelain
v1 -z reports a rename/copy (and emits the extra source field) only in the index
column: `git mv` → "R new\0old\0"; a worktree-only rename → " D old\0?? new\0"
(delete + untracked, never "R" in code[1]). Consuming on code[1]=='R'/'C' would
match no real git output and would only risk mis-consuming the next entry on
malformed input, so the suggestion is declined as incorrect for v1.
Verified: gofmt + go vet clean; host + linux + windows builds; affected suites green.
|
||
|
Vasanth T
|
cc618bae11 | Expand agent quality eval module (#178) | ||
|
KRATOS
|
47c3cc8937 |
wedge stage 13: launch readiness (first-run + benchmark) (#186)
* stage 13: launch readiness (first-run + benchmark) First-run, model-agnostic-first: - provideronboarding: detect local OpenAI-compatible runtimes (Ollama/LM Studio) on their default ports and offer them with no API key required - provideronboarding: ClassifySetupProbe maps a live connectivity probe to a specific, fixable error class (bad key, wrong base URL, model not found, rate limit, config) instead of a stack trace; never panics - zero setup --verify: probe the provider after saving and report the fixable error on failure; print a concrete 'try this' headless example on success zero doctor hardening: - new sandbox.backend check (bubblewrap/sandbox-exec) with a platform-specific install remedy; warn (not fail) on policy-only fallback - new lsp.servers check listing missing language servers with per-binary install remedies; injectable GOOS/LookupExecutable for deterministic tests Self-correct surface + benchmark: - zero exec --self-correct wires the post-edit verify-and-correct loop via the project verifier; nil corrector when off keeps the agent loop byte-identical - perfbench task harness: reproducible Terminal-Bench-style runner over headless zero exec + stream-json; records model + commit + self-correct flag + date - zero-perf-bench tasks subcommand (with --dry-run) and a sample task set - docs/BENCHMARK.md: the number, the exact command, the model, and the with/without-self-correct methodology * stage 13: address review (verify-state, fixture paths, run_end exit, 404 classification) * stage 13: verifier inherits process environment (PATH/HOME/toolchain) |
||
|
KRATOS
|
5e68c78824 |
wedge stage 10: distribution (skill/plugin install, tools make) (#184)
* stage 10: distribution (skill/plugin install, tools make)
Add a file-backed distribution surface so skills and plugins can be
installed from a git URL or local path, and new plugin-tools can be
scaffolded. No central registry: a source is just a git URL or path.
- internal/skills/install.go: Install/Remove/Info + skills.lock. Validates
SKILL.md frontmatter via the existing parser, copies it into the skills
dir, and records a sha256 content hash. Reinstall surfaces the hash
change; a clash with a different source is refused without --force.
- internal/plugins/install.go: same flow validated against the plugin
manifest schema, copying the whole plugin tree (minus .git) so the
plugin is activatable. plugins.lock records source + hash.
- internal/tools/scaffold.go: generate a prompt-gated plugin-tool skeleton
(manifest + runnable shell/node/python entry stub) in the toolbox dir.
- CLI: zero skill {add,info,remove}, zero plugin {add,remove}, and a new
zero tools {make,list}; wired in app.go/extensions.go/skills.go.
Safety: fetch honors the process environment (proxy/egress) and disables
git credential prompts; install never executes fetched content (skills are
markdown; plugin install scripts are never run) and installed plugins still
go through normal activation with permission gating.
* stage 10: make no-usable-name install test cross-platform
The test used a whitespace-only source dir name to force the no-usable-name rejection, which is not creatable on Windows (Smoke failure). Switched to a dir base that validSkillName rejects on every platform (contains "..") while staying creatable on Windows; the blank frontmatter still falls back to the unusable dir base, exercising the same rejection.
* stage 10: address review (tree hash, tools-list diagnostics, --json guards)
* stage 10: make canonical-source tests cross-platform (Windows)
Two Windows Smoke failures from the canonical-source review fix:
- TestInstallSameLocalSourceDifferentSpellingIsNotAClash (skills + plugins) used filepath.Rel(cwd, tempdir), which errors on Windows when the temp dir (C:) and repo (D:) are on different drives. Switched to a same-directory alternate spelling (redundant /./), which still exercises canonicalSource without a cross-drive relative path.
- TestRunSkillAddInfoRemove asserted the raw source path, but canonicalSource resolves symlinks/8.3-short-paths, so the stored+displayed source differs on Windows. Normalize the source via EvalSymlinks up front so the assertion matches on every platform.
* stage 10: address CodeRabbit nitpicks (variadic append, header comment)
|
||
|
KRATOS
|
6beb0b0908 |
wedge stage 09: finish plugin wiring (tools/hooks/skills) (#185)
* stage 09: finish plugin wiring (tools/hooks/skills)
Activate resolved plugin manifests so their declared tools, hooks, and
skills take effect at runtime instead of being parsed-but-unconsumed.
- internal/plugins/activate.go: Activate() registers each plugin tool into
the tools.Registry via a command-runner adapter (JSON args on stdin,
stdout/exit -> tools.Result, ${AGENT_PLUGIN_ROOT} expansion + env),
builds hooks.Definition values for plugin hooks, and exposes plugin
skill search roots. Permission maps allow/prompt/deny -> tools.Safety;
allow only survives load-time clamping when manifest auto-approval is
enabled, so mutating plugin tools are not auto-approved by default. A
plugin-aware skill tool merges the default skills dir with plugin roots
(recording duplicate names, never crashing). Malformed plugins/extensions
are skipped with a warning; activation order is deterministic.
- internal/cli: activatePlugins() invokes activation in both bootstrap
paths (interactive TUI in app.go and one-shot exec in exec.go), after
core/specialist/MCP registration. newHookDispatcherWithExtra folds plugin
hooks into the dispatcher's hook set. Fails open: a bad plugin warns and
is skipped, never wedging startup.
- internal/plugins/plugins.go: update the stale 'not yet registered' note.
* stage 09: normalize plugin command path separators for Windows
expandPluginRoot kept the manifest's forward slashes, so a ${AGENT_PLUGIN_ROOT}/bin/x command became a mixed-separator path on Windows (C:\...\001/bin/x), failing the path tests. Added expandPluginRootPath (filepath.FromSlash on the placeholder-expanded executable path only; args keep their slashes since they may be URLs/flags) and used it for the tool and hook command paths.
* stage 09: address plugin wiring review
- surface per-plugin load diagnostics on stderr (not just top-level errors)
- skip plugin tools whose name collides with an already-registered tool
- resolve manifest-relative plugin skill paths against the plugin dir
* stage 09: fall back to plugin path/id when manifest path unknown in load diagnostic
|
||
|
KRATOS
|
59c332638d |
wedge M2: MCP server resources/prompts + sandbox scoped egress + MCP OAuth (#179)
* wedge stage 06: MCP server resources + prompts
Advertise resources and prompts capabilities in the initialize response and
handle resources/list, resources/read, prompts/list, and prompts/get alongside
the existing tools methods.
Resources enumerate in-scope workspace files via the shared workspace scanner
(reusing its gitignore-style exclusions) and read them read-only with strict
scope enforcement: traversal and out-of-scope absolute paths are rejected with
a JSON-RPC error and no contents, and binary files come back as base64 blobs.
Prompts expose a curated set of templates with simple {{arg}} substitution;
unknown prompt names return an error.
* wedge stage 07: sandbox scoped egress + auto-allow-bash
Add a middle-ground network mode between full allow and full deny:
- types.go: NetworkScoped network mode; Policy.AllowedDomains /
Policy.DeniedDomains; Policy.AutoAllowBashWhenSandboxed plus its
ZERO_SANDBOX_AUTO_ALLOW_BASH env surface (off by default).
- egress.go: in-process loopback (127.0.0.1:0) filtering proxy handling
HTTP CONNECT (HTTPS tunnel) and plain HTTP. Allows only AllowedDomains
minus DeniedDomains via exact-or-subdomain matching (github.com matches
api.github.com but not notgithub.com or github.com.evil.com); deny wins.
Denied targets get 403 / refused CONNECT. Decisions are logged through
the repo redaction so query tokens never hit the audit trail. Fails
closed: empty allowlist is rejected at construction.
- runner.go: NetworkScoped starts the proxy and routes the sandboxed
process through it (HTTP(S)_PROXY/ALL_PROXY + NO_PROXY into the sandbox
env; Linux keeps --unshare-net; macOS profile denies general network
but permits localhost:proxyPort). Empty-allowlist scoped collapses to
deny (effectiveNetwork). Any proxy-start error denies the build rather
than falling back to open network. NetworkAllow/NetworkDeny unchanged.
- engine.go / registry.go / bash.go: honor AutoAllowBashWhenSandboxed -
auto-allow bash without a prompt only when a wrapping sandbox is active
for the command; ignore the flag when the sandbox is inactive. Preflight
network gate now uses effectiveNetwork so a misconfigured scoped policy
still fails closed. Bash defers plan.Cleanup() to shut the proxy down.
Default behaviour is unchanged for existing NetworkAllow / NetworkDeny
users and for the off-by-default auto-allow flag.
* wedge stage 08: MCP OAuth client
Add an OAuth 2.0 + PKCE authorization-code flow so ZERO can connect to
remote MCP servers that require OAuth.
- internal/mcp/oauth.go: authorization-server metadata discovery with
config fallback/override, S256 PKCE, loopback redirect listener, state
(CSRF) validation, code+verifier token exchange, refresh, and optional
dynamic client registration. Tokens are never logged.
- internal/mcp/oauth_store.go: per-server token persistence in a 0600
JSON file under the config dir, with a redaction-safe status summary
that omits the token material.
- internal/mcp/network_client.go: OAuth-configured servers get a round
tripper that attaches the bearer token and, on 401, refreshes once and
retries; refresh failure surfaces an actionable re-login message.
- internal/mcp/config.go + internal/config: an MCP server entry may
declare auth: "oauth" with optional client registration / explicit
endpoints. Non-OAuth servers are unaffected.
- zero mcp oauth {login,logout,status} wired into the CLI surface;
status reports presence/expiry, never the token.
* wedge M2: address review — 2 security fixes, portability, robustness (#179)
Security (Critical):
- resources: resolve symlinks BEFORE the scope check (and resolve roots) so an
in-workspace symlink to an out-of-scope file (e.g. link -> /etc/passwd) is no
longer readable via resources/read. resources/read echoes the requested URI
(not the resolved path) so list/read stay consistent.
- sandbox egress: authorize the dialed URL host, not the client Host header, so
'GET http://denied/ ... Host: allowed' can't bypass the allowlist.
Robustness/correctness:
- prompts: reject prompts/get that omits a required argument; tokenized
substitution so {{...}} inside supplied values is preserved (not stripped).
- resources: percent-encode/decode file URIs via net/url (spaces, #, %).
- oauth: one timeout-scoped context bounds discovery + registration + callback +
exchange; joinWellKnown preserves issuer path segments (RFC 8414).
- cli: 'mcp oauth login' rejects --json instead of silently ignoring it; the
login test is bounded so a missed callback fails fast instead of hanging.
Portability: XDG token-store path test uses t.TempDir() so it passes on Windows
(fixes the Windows Smoke failure).
Adds regression tests for the symlink escape, Host-header bypass, required-arg
rejection, and placeholder preservation. Full go test ./... + -race green.
* wedge M2: fail closed on unenforceable scoped egress (review #179)
Addresses the two P1 scoped-egress enforcement gaps from review:
1. Policy-only fallback ran NetworkScoped with unrestricted networking. On Windows / any host without a native backend, 'network: scoped' with allowedDomains silently got open host networking because Evaluate permitted network-risk tools (expecting a proxy) while BuildCommandPlan ran a direct plan with no proxy. Evaluate now collapses scoped to deny when the active backend cannot enforce egress, so network-risk tools fail closed.
2. Bubblewrap 'scoped egress' could not reach the proxy. --unshare-net isolates the netns and there is no bridge to the host-loopback proxy (the comments claimed one; no code added it), so allowlisted egress never worked. Bubblewrap no longer starts an unreachable proxy: scoped collapses to deny (--unshare-net, no proxy env), matching Evaluate. Comments corrected; a real relay (e.g. slirp4netns) is left as future work.
Mechanism: new Backend.ScopedEgress capability (true only for sandbox-exec, which shares the host net under a seatbelt profile restricting outbound to the proxy port) + Backend.EnforcesScopedEgress(). sandbox-exec scoped egress is unchanged. Regression tests cover enforcing vs non-enforcing backends in Evaluate and the bubblewrap fail-closed plan.
|
||
|
Vasanth T
|
2192c10299 |
Add agent quality eval foundation (#177)
* Add agent quality eval foundation * Fix agent eval review issues * Address agent eval review feedback |
||
|
KRATOS
|
a6d6939bc5 |
audit: fix isolated remaining findings (redaction, secrets, rune-safety, credential status) (#174)
* audit: fix redaction sibling-DAG false cycle, hoist URL regex, widen OpenAI key pattern - redaction: seen-set now tracks only the current DFS path (delete pointer after recursing) so a shared reference reached via a sibling branch is no longer reported as CircularReference; true cycles still detected. (audit redaction.go:222) - redaction: redactURLPasswords compiles its regexp once at package scope instead of per call. (audit redaction.go:362) - secrets: openai_key body allows - and _ so sk-proj-/sk-svcacct- keys match, not just legacy sk-<alnum>. (audit scanner.go:35) * audit: rune-safe cron run-error truncation and commit-subject length - cronTruncate now cuts on a UTF-8 rune boundary (via cutRuneBoundary) so a persisted cron run-error excerpt can't end in a split rune. (audit cron_run.go:179) - zerogit ValidateMessage counts runes, not bytes, so a valid non-ASCII commit subject under 72 chars is no longer rejected. (audit zerogit.go:207) * audit: report auth-header-only profiles as having a credential ProviderSnapshot.APIKeySet was true only for a non-empty APIKey, so a profile authenticated solely via AuthHeaderValue rendered as 'not set' in the command center. Treat either credential as set. (audit contracts.go:166) * audit: address review on #174 (credential-presence coverage + consistency) - Add ProviderSnapshotFromProfile test covering APIKeySet for api-key-only, auth-header-only, both, and neither (the AuthHeaderValue path CodeRabbit flagged as untested). - doctor.go: report the provider 'apiKey' diagnostic as set when EITHER APIKey or AuthHeaderValue is present, matching ProviderSnapshot.APIKeySet — closing the same auth-header-only gap at the other credential-check site. - scanner_test: also assert the [REDACTED:openai_key] placeholder is present (matches the pattern used by the existing redaction test). * audit: trim credentials + surface doctor credential presence (#174 review) - ProviderSnapshot test: add whitespace-only api-key / auth-header cases so the TrimSpace behavior stays covered. - doctor.go: trim both APIKey and AuthHeaderValue before the presence check (matches ProviderSnapshot.APIKeySet) AND move the indicator off the sensitive 'apiKey' detail key — check() runs RedactValue, so 'apiKey' was always scrubbed to [REDACTED], leaving the set/not-set indicator invisible. Now reported under 'credentialConfigured' so it actually surfaces; added a doctor table test. No consumer read the old detail key. * audit: honor AuthHeaderValue in model discovery + base-URL redaction (#174 review) Two credential-presence consistency gaps flagged on #174: - providermodeldiscovery: the /models probe gated on (and sent Authorization from) profile.APIKey only, so an auth-header-only profile was skipped/sent unauthenticated. Gate via discoveryHasCredential (APIKey OR AuthHeaderValue) and build the request with providerio.ApplyAuthHeaders, matching how the live providers authenticate (honors AuthHeaderValue, AuthHeader, AuthScheme, CustomHeaders). APIKey-only behavior is unchanged. - zerocommands.redactProviderBaseURL now also redacts AuthHeaderValue if it leaks into the base URL (variadic secrets, empties dropped). Adds regression tests for auth-header-only discovery, discoveryHasCredential, and auth-header redaction in the base URL. * audit: doctor credential-presence test parity with contracts_test (#174 review) Add the 'both' and 'whitespace auth header only' cases so TestProviderConfigCheckCredentialPresence matches the six cases in contracts_test.go's TestProviderSnapshotAPIKeySetCountsAuthHeaderValue. |
||
|
Anandan
|
fcfdb873d0 |
Polish main TUI surface (#168)
* Polish main TUI surface Simplify the empty chat surface, replace starter prompt chips with a cleaner brand state, move model and permission state into the composer frame, quiet the footer status, and improve composer word-editing keybindings. Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tui Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check * Polish autocomplete command palette * Polish TUI file mention picker * Address TUI review feedback * Keep autocomplete anchored above composer * Center autocomplete over chat viewport * Layer autocomplete overlays over transcript * Prefill argument commands from palette * Show argument hints for command completions * Polish TUI provider setup * Polish TUI picker and composer metadata * Add two-step mouse selection in TUI * Polish TUI mouse selection flows * Make provider wizard navigation test hermetic * Fix TUI review edge cases |
||
|
KRATOS
|
06ea4f004b |
Edit safety: file read/write tracking, external-modification detection, post-apply hooks (#172)
* Edit safety: file version tracking, conflict detection, hook wiring
Phase 1 — FileTracker (internal/tools/file_tracker.go): per-session
map[absPath]{sha256, size, mtime}; CheckConflict flags drift only against
a known baseline, so first-touch writes are unaffected. nil = no-op.
Phase 2 — external-modification detection: read_file records the
whole-file baseline; edit_file and write_file --overwrite refuse to
clobber a file whose on-disk content drifted from it and tell the model
to re-read first, then re-baseline to what they wrote. apply_patch
forgets the files it rewrote (git apply has its own context guard).
Threaded via RunOptions/agent.Options FileTracker, built once per session
in the exec, spec, and TUI paths.
Phase 3 — post-apply validation via hooks: the dormant hooks.Dispatcher
is wired into executeToolCall. beforeTool hooks can veto a call
(DenialHookBlocked); afterTool hook output is scrubbed and surfaced to
the model. DispatchOutcome now collects hook stdout/stderr. Per-session
dispatcher built from merged user+project hooks.json; fails open and runs
nothing until a hook is configured.
* edit-safety: address review — fail closed on read, redact hook reason
- write_file overwrite: if a tracked file cannot be re-read to verify it,
fail CLOSED (refuse the overwrite) instead of silently proceeding — the
read error previously skipped the conflict check and could clobber an
externally modified or replaced file.
- blockedByHookResult: scrub outcome.Reason through redaction before placing
it in model-visible ToolResult.Output; this intercepted path bypassed the
registry's normal output-redaction boundary.
- TestHashContentIsStableAndDistinguishing: compare separately stored hashes
so the stability check is not a same-expression comparison (SA4000).
* edit-safety: flag ToolResult.Redacted when hook output is scrubbed
appendHookFeedback now reports whether scrubbing changed the afterTool
feedback, and blockedByHookResult reports whether it scrubbed the reason;
both set ToolResult.Redacted so the hook-intercepted paths match the
registry's redaction contract. Adds tests asserting a secret-bearing hook
reason/feedback is scrubbed and flips Redacted.
|
||
|
KRATOS
|
8d4a530423 |
test: isolate session store in exec/tui tests so go test stops writing to $HOME (#171)
TestRunExecStreamJSONRunStartUsesResolvedAPIModel, TestRunExecReadsStreamJSONPromptFromStdin, and TestPromptSubmitInjectsLiveSessionModelContext ran real exec sessions but only isolated cwd (t.TempDir + getwd), not the session store, so the default store resolved sessions.DefaultRoot() off the real $HOME and persisted into ~/.local/share/zero/sessions on every run. Set XDG_DATA_HOME to a per-test temp dir (the existing convention in exec_test.go / exec_scope_test.go) so each run is fully isolated. |
||
|
KRATOSandClaude Fable 5
|
e381d8eab1 |
Additional write roots: --add-dir flag, global config key, /add-dir TUI command (#162)
* docs: spec for additional write roots (--add-dir / /add-dir) Co-Authored-By: Claude Fable 5 <[email protected]> * docs: implementation plan for additional write roots Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: add Scope type for multi-root write access Co-Authored-By: Claude Fable 5 <[email protected]> * docs: write roots honored from global config only, union merge semantics Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: fix multi-root violation reporting and pin prefix-alias behavior - validate: prefer ViolationSymlinkTraversal over ViolationOutsideWorkspace when all roots deny; return original requestedPath in violation; append --add-dir hint only on outside_workspace results; extend doc comment to state that a symlink resolving inside any root is allowed. - normalizePrefixForRoot: dedupe insideRoot check via pathWithinRoot (Fix 3); widen EvalSymlinks comment to cover jump-over case; add POSIX-only note. - WorkspaceRoot: add doc comment noting immutability/lock safety (Fix 4). - Tests: pin multi-root traversal-preferred behavior, deterministic alias test (TestValidateResolvesAliasedPathPrefixes), Add symlink normalization test, and tilde-expansion error path test. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: make engine path validation and risk scope-aware Thread *Scope through EngineOptions/Engine so Evaluate and Classify use multi-root scope validation instead of single-root validateWorkspacePaths, letting extra write roots (--add-dir / /add-dir) be honoured by the policy gate and risk classifier. Remove now-dead validateWorkspacePaths helper. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: pin override-root scope isolation, tidy Evaluate scope handling Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: widen seatbelt/bubblewrap profiles and command cwd to scope roots Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: pin extra-root chdir behavior and document bind invariants Co-Authored-By: Claude Fable 5 <[email protected]> * tools: resolve paths against the shared write scope Add PathScope interface and scoped resolver helpers to workspace.go, then thread a scope field through all 8 file tools (read_file, list_directory, glob, grep, write_file, edit_file, apply_patch, bash). Each existing constructor delegates to a new NewScoped* variant; nil scope is byte-identical to the original behavior. Registry gains CoreReadOnlyToolsScoped, CoreWriteToolsScoped, CoreShellToolsScoped, and CoreToolsScoped. Co-Authored-By: Claude Fable 5 <[email protected]> * tools: restore fail-closed write-target symlink checks, share prefix normalization * tools: report absolute paths for extra-root changes, pin scoped traversal denial - resolveScopedPath and resolveScopedTargetPath now return the absolute path as the second value when the matched root is an extra (non-workspace) root, eliminating ambiguity in ChangedFiles, cwd meta, and display summaries downstream - Add TestScopedWriteRefusesSameRootSymlinkTraversal to pin that same-root symlink traversal within an extra root is denied (pre-existing behavior) - Add TestScopedWriteReportsAbsolutePathForExtraRoot to enforce the new absolute-path contract for ChangedFiles on extra-root writes - Extend doc comments on PathScope, ChangedFiles, changedFilesFromPatch, and both scoped resolvers to capture the workspace-vs-absolute invariant Co-Authored-By: Claude Fable 5 <[email protected]> * config: add sandbox.additionalWriteRoots (global config only, union merge) * cli: add repeatable --add-dir flag wiring scope into registry and engine Co-Authored-By: Claude Fable 5 <[email protected]> * cli: forward --add-dir across dispatch paths, pin scope wiring with tests Co-Authored-By: Claude Fable 5 <[email protected]> * cli: pin --add-dir dispatch forwarding end-to-end, close guard gaps Review fixes for the --add-dir dispatch commit: - Add TestRunAddDirDispatchForwardsGrantIntoExecScope: drives runWithDeps through both the "exec" and "-p" dispatch shapes with a provider that calls write_file inside the extra root, asserting the grant reaches the exec sandbox scope (and a negative control without --add-dir is denied fail-closed). Mutation-verified: dropping addDirFlagArgs forwarding from either case fails the test. - Remove help/version from the --add-dir allowlist so any non-TUI/ non-exec leading --add-dir hard-errors, matching the stated requirement; extend the rejection test with those cases. - Fail loud when --add-dir is hidden behind a stray non-flag arg on the --skip-permissions-unsafe path instead of silently dropping the grant. - Reject flag-like values in the inline --add-dir= spelling to match the space form (a directory named -foo stays reachable as ./-foo). - Move TestExecScopeReRegistrationSwapsCoreToolsByName out of the parse test file into the new exec_scope_test.go alongside the e2e test. Co-Authored-By: Claude Fable 5 <[email protected]> * tui: add /add-dir command for session write-root grants Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: surface write roots in zero sandbox status and plan snapshots Co-Authored-By: Claude Fable 5 <[email protected]> * cli: surface write-roots error in --effective --json, normalize fallback root Review fixes for the Task8 observability range: - runSandboxPolicyEffective JSON payload gains writeRootsError (omitempty) so --json consumers see the same fail-soft signal as the text write_roots_error line instead of a misleading workspace-only writeRoots list. - The fail-soft fallback now derives its write roots from a workspace-only Scope (which cannot fail) so the error path renders the same symlink-resolved root as the success path. - TestRunSandboxPolicyEffectiveWriteRootsFailSoft gains a --json leg asserting writeRootsError names the stale root with a workspace-only fallback; the configured-roots test asserts the key is absent for valid roots. - SandboxPlanSnapshot doc comment reworded to match the converter: no current builder populates WriteRoots. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: seatbelt integration test for extra write roots Co-Authored-By: Claude Fable 5 <[email protected]> * docs: document --add-dir and /add-dir write-root grants Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: pass volume-qualified paths through prefix normalization verbatim NormalizePrefixForRoot's POSIX component walk mangled Windows drive paths (C:\Users -> C:Users), which downstream single-root checks treated as relative and joined under the workspace — failing the policy gate OPEN on Windows. Volume-qualified paths now bypass the walk entirely (the macOS /var alias problem it solves has no Windows equivalent) and windows-gated tests pin both the verbatim pass-through and the engine denial. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: make prefix normalization volume-aware for Windows alias resolution The previous commit disabled NormalizePrefixForRoot on Windows, which broke it the other direction: a workspace created under an 8.3 short path (C:\Users\RUNNER~1\...) resolves via EvalSymlinks to its long form (runneradmin), so raw short-form requests escaped the long-form root and legitimate extra-root writes were denied. This is the same alias problem as macOS /var -> /private/var, which the helper already solves. Start the component walk from the volume root (C:\ or //host/share/) instead of "/" so it resolves the prefix on Windows too; POSIX VolumeName is empty so the walk reduces to the original "/"-rooted behavior byte-for-byte. Windows test now pins allow-inside-root + deny-outside both directions. Co-Authored-By: Claude Fable 5 <[email protected]> * sandbox: address PR review findings for additional write roots Resolve the CHANGES_REQUESTED review on PR #162: - engine: derive workspaceRoot from scope.Roots()[0] when NewEngine is given a Scope without an explicit WorkspaceRoot, so Evaluate's EnforceWorkspace/classification guards no longer silently skip (+ regression test). - tools/workspace: scopedRoots now fails closed (returns an error) when a non-nil PathScope exposes empty Roots(), instead of returning success with an empty path; threaded through the scoped resolve/recheck helpers and resolveGrepRoot. - tools/glob: emit absolute matches when cwd resolves into an extra root so results can't be fed back and resolve to a same-named workspace file (+ regression test). - tools/bash,grep,apply_patch: schema descriptions mention granted extra roots (relative -> workspace, absolute -> granted root). - docs/plan: fix markdownlint heading increment, malformed rule+heading, and bare code fence. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> * tools/grep, tui: address PR review for additional write roots - grep now emits absolute, symlink-resolved paths for matches in a granted extra (non-workspace) root, mirroring the glob fix. A bare workspace-relative name like "report.go" otherwise resolves back under the workspace when fed to read_file/edit_file and hits the wrong file when the name exists in both roots. Covers content and files_with_matches output. - tui: only reset chatScrollOffset for a real submission, not an empty Enter (a no-op), so the viewport is no longer yanked to the bottom when nothing was submitted. Real submissions still snap back. - glob: schema description now mentions granted extra roots, matching the bash/grep/apply_patch wording so the --add-dir/-add-dir flow is discoverable. Regression tests: TestScopedGrepReturnsAbsoluteMatchesForExtraRoot, TestEmptySubmitKeepsChatScrollOffset. * docs: add /add-dir to the TUI commands quick-reference table The command was documented further down but missing from the scannable commands table, so users browsing the table wouldn't discover the additional-write-roots flow. --------- Co-authored-by: Claude Fable 5 <[email protected]> |
||
|
gnanam1990
|
0c37093974 |
audit: address PR review findings (security, correctness, tests)
Fixes from the code review on the critical/high audit branch: - tools/apply_patch: parseHunkCounts now reads only the range section between the opening and closing "@@". A crafted section heading like "@@ -1,1 +1,1 @@ +1,999999" could otherwise overwrite the line count, wedge the parser in hunk mode, and swallow later file headers so they escaped validatePatchPaths / recheckPatchWriteTargets — a workspace-confinement bypass. - providerhealth: the SSRF-hardened dial now tries every validated address in order instead of pinning addrs[0], so a dual-stack/multi-record provider isn't failed by a single dead record. Extracted dialValidatedAddrs for coverage. - hooks/dispatch: a beforeTool hook killed by its deadline now fails CLOSED (vetoes the tool) instead of fail-open. A launch failure (missing binary) still fails open so a misconfigured hook can't wedge every tool call. - cron: Store.Get returns a distinct ErrJobNotFound sentinel; cron_run only treats that as "removed during run" and still records/persists on a transient read error instead of silently dropping the run. - providers/anthropic: closeOpen finalizes thinking buffers still open at stream end (SSE ended after thinking_delta/signature_delta but before content_block_stop), so they survive in the done event's ReasoningBlocks for replay. Finalized in index order. - providers/gemini: corrected the stale thinkingConfig comment (no IncludeThoughts field; thought summaries are suppressed by skipping part.Thought parts). Tests: regression coverage for each fix, plus the requested nitpicks — EOF-terminated MCP line, recipe-prompt propagation assertion, checked file.Close() in session store tests, and a shortened bash process-group timeout test via the bashWaitDelay hook. Note: declined the suggestion to replay a "signature" on redacted_thinking blocks — Anthropic's redacted_thinking carries only an opaque "data" field; the signature is for regular thinking blocks, and the stream never populates one for redacted blocks. |
||
|
gnanam1990
|
24e15e2138 |
providers/openai: forward reasoning effort to the chat completions request
zeroruntime.CompletionRequest had no effort field, so agent.Options.Reasoning Effort never reached the provider (cli/exec.go even noted it was 'not yet forwarded'). Add CompletionRequest.ReasoningEffort, thread it through every request the agent loop builds, and map it to OpenAI's reasoning_effort. The CLI gates the forwarded value against the resolved model: known non-reasoning models send nothing (matching the existing 'ignoring' advisory), known reasoning models send their effective level, and unknown custom endpoints forward the requested value as-is. Anthropic/Gemini mapping is handled separately. |
||
|
gnanam1990
|
32a6922916 |
agent: surface response truncation (finish_reason) to the user
Providers already normalized finish_reason into collected.FinishReason, but the agent loop never read it, so a final answer cut off at the output token cap (or by a content filter) was silently presented as complete. Carry the turn's finish reason into agent.Result, add Truncated()/TruncationNotice(), and emit a warning from the exec writer and a system row in the TUI when the response was truncated. |
||
|
gnanam1990
|
f7c9094e1b |
cli/cron: honor explicit expr over recipe; don't clobber pause mid-run
- `cron add "<expr>" --recipe X` silently dropped the explicit schedule because the recipe set expr before the positional arg was consumed. Consume the positional expr first so it overrides the recipe default. - cron_run persisted the job copy read at tick start, so a pause or removal that happened during the run was clobbered back to active. Re-read before persisting and preserve an external pause/removal (best-effort; full atomicity needs a lock). |
||
|
anandh8x
|
6a93f89c85 |
Add first-run provider setup
Introduce zero setup plus first-run onboarding for missing provider credentials, backed by the provider catalog and config save path. Make the TUI fullscreen in interactive mode, keep setup transitions inside the chat surface, and add managed transcript scrolling so wheel/PageUp/PageDown do not recall composer history. Tested: GOCACHE=/tmp/zero-go-cache go test ./... Tested: git diff --check |
||
|
Vasanthdev2004
|
8caffa5395 | Persist provider wizard credentials | ||
|
Vasanthdev2004
|
74923e1d84 | Expose active provider model to agent | ||
|
Vasanthdev2004
|
e06ee74190 | Split Ollama cloud and local providers | ||
|
Vasanthdev2004
|
f3bfd48dec | Add provider onboarding UX | ||
|
|
6fb0231df8 |
TUI scrollback re-architecture, Lime chat surface, and deep-audit fixes (#158)
* docs(design): add Lime TUI prototype and rebuild spec
* tui: remove the zeroline skin system
One design remains in the tree ahead of the Lime rebuild: the zeroline
package, its TUI view/skin plumbing (Options.Skin/ThemeVariant/ThemeDark,
model skin branches, theme picker), and the zeroline CLI subcommand are
gone. /theme keeps its shell-only message.
* tui: replace the cyan palette with the Lime token table
theme.go is rebuilt around the Lime design tokens (docs/design/
zero_tui_lime.html): near-black panels, one lime accent, solid tint
stand-ins for the prototype's rgba overlays. Existing render sites map
onto the new role styles (ink/line/userPrompt/toolName/toolArg) without
layout changes; no hex exists outside theme.go.
* tui: build the Lime chat-surface components
Title bar with brand badge + context window, empty state replacing the
deleted splash (block-art 0, tagline, 1-3 starter suggestions), stream
blocks (user prompt gutter, muted interim with cursor, final-answer
accent rail, done line, bordered notes), tool cards with diff/read/
bash/grep bodies and MiniDot run spinner, borderless composer with
run-state hints, and the grouped status line. Final answers are marked
at append time; resolved tool calls collapse into their result cards.
* tui: fix Stage 2 review findings
Adversarial review of the chat-surface commit confirmed 15 issues, all
fixed here:
- wrapPlainText sliced wide runes by rune count: CJK/emoji prose could
panic the TUI or emit lines ~2x the measure; split at width instead
- a cancelled run's streamingText leaked into the next turn's interim
block; cancelRun now clears it
- orphaned tool-call cards (cancelled/errored turns) re-animated with
the spinner on any later run; the glyph is now scoped to the call's
own runID
- starter-suggestion digit capture was active while the chips were off
screen (/clear mid-run); the guard now mirrors the render condition
- prompt submits while a deferred Ctrl+C quit is draining could start a
run the quit would orphan; submission is blocked while exiting
- the running placeholder advertised ctrl+c (which quits) as the
interrupt key; it now says esc
- read-card parser expected 'N: text' but read_file emits 'N | text',
so the gutter and L-range never rendered on real output
- toolCard borders were one column short of the body rows
- diff tint bands now span the full row; NEW FILE renders green on
addBg; preamble/no-newline lines are unnumbered meta and no longer
shift hunk numbering
- card bodies and system notes paint the panel surface; the head gains
the separate faintest arg column (grep target + pattern)
- title-bar overflow now truncates the minimal candidate instead of
rendering the deleted design's 'ZERO … READY' line
- untracked the stray snake-game.html (left on disk, ignored)
* tui: restyle the interactive surfaces for Lime
Permission prompts render as the amber permission card (PERMISSION
badge, risk readout, key chips); resolved prompts collapse to one line
(allowed once/always/denied · tool) and unprompted allows fold into the
tool card's [auto] tag. Ask-user and spec-review prompts share the card
language with line borders. Autocomplete and picker overlays move onto
the panel/selection tints with the accent ❯ marker; model-picker rows
gain the provider dot and ctx · KEY_ENV metadata from the registry and
provider catalog. /resume renders its list as stacked session cards
(id + age, title, meta). Key handling is untouched everywhere.
* tui: adaptive width tiers, doc updates, final polish
Width tiers re-evaluate from the live width: ≥100 renders the full
spec; 80–99 drops the tool-arg column, header ctx, and the status
'interactive' group; 58–79 drops the diff/read gutters, bares the
badge, and keeps provider+tokens+mode; <58 renders a single-segment
header, rail-less cards, and a mode-only status line. Table-driven
tests cover {58,70,80,100,120} plus a frame-wide invariant that no
emitted line exceeds the terminal at any tier. Card bodies expand tabs
so width math holds. Dead splash-chip machinery (startupCommandNames,
startupOrder) is gone; README's TUI section now describes Lime.
* tui: fix final-review findings across Stages 3-4
A second adversarial review confirmed 14 issues, all addressed:
- permission collapse and tool-card pairing are now run-scoped (rcKey):
providers with repeating synthetic ToolCallIDs (Gemini gemini_tool_N)
could attribute a decision or result to a different run's call
- rehydrated always-grants no longer mislabel as 'allowed once'
(GrantMatched fallback)
- the width invariant now actually holds everywhere: empty-state
tagline/hint, ask-user rows, permission detail blobs, pending image
chips, and the sessions trailing hint are wrapped or fitted; the
invariant test renders the empty state and all of those rows at
widths 24-120
- every card (permission, ask-user, spec-review, sessions, notes)
drops side borders on tiny terminals, not just tool cards
- /resume card fields are sanitized against separator bytes from
user-controlled titles
- the picker's selected row paints its gap so the selBg band is solid
- dead splash helpers (normalizedStartupWidth, countLines, nonEmpty,
unreachable rowWelcome branch) and stale /theme comments are gone
- README no longer claims transcript scrolling keys or Tab-accept
behavior that never existed; chatWidth documents its 24-col floor
Tag tui-lime-s4 moves to this commit.
* audit: deep-audit report, TUI scrollback re-architecture, and cross-module fixes
TUI: settled-row flush frontier prints history to native terminal
scrollback (tea.Println, ordered + ack-serialized); View() renders only
the live tail. Full diffs (400-line flush cap) and OSC 8 file:// links
land in history; streamed interim text, cancellation markers, ask_user
exchanges, and composer input history are preserved; run-scoped dedup
keys fix Gemini's repeating tool-call ids; /exit, Ctrl+C-after-Esc, and
cross-session late flushes no longer orphan or contaminate checkpoints.
Cross-module: zero search panic + offset mapping, usage report
robustness, 529 retry, SSE keep-alive watchdog, registry corrections,
update --check on dev builds, MCP ping + mcp list --json secret
redaction, sandbox.network config knob, doctor apiKey presence, atomic
config writes, C1-safe notifications, rune-safe truncation everywhere,
gofmt + vet CI gates, dead glamour dependency dropped.
Full audit report: docs/audit/2026-06-10-deep-audit.md (175 findings).
Co-Authored-By: Claude Fable 5 <[email protected]>
* tui: hide malformed ask_user tool errors
* fix tui malformed tool retries
* fix max-turn finalization
* upgrade tui agent guardrails
* fix coderabbit guardrail findings
---------
Co-authored-by: Claude Fable 5 <[email protected]>
Co-authored-by: Vasanthdev2004 <[email protected]>
|
||
|
Vasanth T
|
cf6b1c1db7 |
Add repo intelligence map (#157)
* feat: add repo intelligence map * fix: address repo map review feedback * fix: refine repo map scan limits |
||
|
KRATOS
|
0f8ea50101 |
zero cron: dep-free file-backed scheduled agents (foreground runner) (#155)
* cron: dep-free 5-field cron expression parser * cron: next-run evaluator with Vixie DOM/DOW semantics * cron: preset recipes + loop.md prompt resolution (cap + symlink reject) * cron: file-backed job store (metadata.json + runs.jsonl) * cli: zero cron add/list/rm/pause/resume over the cron store * cli: zero cron run foreground scheduler (--once/--catch-up, exec per fire) * cron: fix adversarial-review findings (DST loop, traversal, runaway re-fire, +) - Next: fix infinite loop on DST spring-forward gaps (advance hours by absolute addition + a forward-progress guard); widen the search window to 9 years so a Feb-29 schedule across a century non-leap year (2096->2104) isn't reported as impossible. - store: reject path-traversal job ids (Get/Update/Remove/AppendRun) so 'cron rm ../..' can't delete outside the store; List now surfaces corrupt jobs as a warning instead of silently dropping them. - runner: reject impossible schedules at add even with --run-now; auto-pause a job whose schedule can no longer advance (was re-firing every tick); only skip-reschedule STRICTLY-overdue jobs on startup (keep exactly-due ones); capture stderr into RunRecord.Error on non-zero exit; pass the prompt via the inline --prompt= form so dash-leading prompts aren't misparsed. - Regression tests for each (DST, leap gap, traversal, corrupt-list, pause, reconcile, dash-prompt). * cron: address #155 review (store-error surfacing, resume guard, Runs traversal, +) - fireJob/reconcile/cronRun now surface AppendRun/Update failures to stderr instead of discarding them (a failed write no longer silently re-fires/loses history); fireJob takes stderr. - cron add rejects extra positional arguments instead of silently ignoring them. - cronResume rejects an unparseable/impossible schedule instead of reactivating a job with a stale/zero NextRunAt. - store.AppendRun returns the Close() error (buffered-write failures surface on close); store.Runs now validates the id (path-traversal guard, matching the other store methods). - tests: Runs traversal rejection, cron add extra-args, resume-impossible. * cron: check store errors in cron CLI tests CodeRabbit (critical): the cron CLI tests ignored errors from store.List/Add/Get, so a store failure would surface as a misleading assertion on a zero-value job/slice instead of a clear failure. Check every store op's error across the test file (not only the two flagged sites). * cron: DefaultRoot falls back to os.UserHomeDir when HOME is unset Vasanth (P1): cron's DefaultRoot mirrored neither sessions.DefaultRoot nor reality — with no XDG_DATA_HOME and no HOME it produced a RELATIVE ".local/share/zero/cron" under the caller's cwd, so on Windows/restricted shells cron jobs scattered per working directory and could write repo-local data. Now falls back to os.UserHomeDir() like sessions.DefaultRoot. Adds TestDefaultRootEmptyHomeFallsBackToUserHome (asserts the path stays absolute). |
||
|
Vasanth T
|
40cec8d476 |
Add provider health diagnostics (#151)
* Add provider health diagnostics * Fix provider health auth review findings * Fix provider health test handler assertion * Harden provider health connectivity probes |
||
|
KRATOS
|
15066216c8 |
zero repo-info: local (network-free) repository characterizer (#150)
* repoinfo: language map + build/test/CI + workspace detection helpers * repoinfo: Collect repo characterization from local git (network-free) * cli: zero repo-info command (text + --json) over the repoinfo package * repoinfo: derive AgeDays from the root commit, not git log --reverse -1 git applies -1 before --reverse, so the old call returned the LATEST commit (age always ~0). Use --max-parents=0 (root commits) and take the oldest. * repoinfo: count passthrough dirs + unicode-safe ls-tree (-z); harden tests Adversarial-review fixes: - DirectoryCount now counts every directory on the path to a file (expand each file's ancestors), so directories holding only subdirectories are included. git ls-tree -r lists blobs only, so the previous parent-of-file count undercounted. Verified against 'git ls-tree -r -t | awk $2==tree'. - ls-tree now uses -z (NUL-terminated, unquoted paths) instead of newline scanning, so non-ASCII/special filenames (which git C-quotes by default) are parsed correctly instead of being silently dropped. Drops the bufio scanner. - Tests: assert DirectoryCount/MaxDepth; add an age-from-oldest-root-commit test (would catch the old log --reverse -1 bug); make the CLI JSON test hermetic via a temp git repo. * repoinfo: strip credentials from remote URL before storing/printing Addresses #150 review (@Vasanthdev2004 P1): a git remote can embed secrets (e.g. https://x-access-token:[email protected]/o/r.git), which would leak in both text and --json output. sanitizeRemoteURL strips userinfo from URL forms and the leading user@ from scp-like forms. Tests: unit table for the sanitizer, Collect-level strip, and end-to-end CLI text + JSON assertions that the credential never appears. |
||
|
KRATOS
|
58e209d021 |
Sound / completion notifier: terminal bell + OSC-9 on turn completion & awaiting-input (#149)
* notify: dep-free terminal bell / OSC-9 completion notifier * config: notify.mode + notify.focusMode (validated, default off) * exec: --notify/--no-notify + completion notify on stderr * tui: focus reporting + completion/awaiting-input notifications * notify: fire on exec --use-spec completion; headless ignores focus; gate focus-reporting on enabled * notify: validate --notify value, skip notify on empty ask_user, deterministic resolver tests Addresses #149 review (CodeRabbit + @Vasanthdev2004): - parseExecArgs rejects an invalid --notify value (only off/bell/notify/both), so a CLI typo errors early instead of silently running with no notification. - TUI OnAskUser fires AwaitingInput only when len(request.Questions) > 0 — a request with no questions auto-resolves without prompting, so the bell/desktop notification was a false positive. - notify resolver tests pass Env: map[string]string{} so Resolve does not fall back to os.Getenv (deterministic, not ambient-env dependent). |
||
|
KRATOS
|
2ea36cf138 |
Deferred (lazy) tool loading: tool_search + auto threshold for large MCP tool sets (#148)
* Deferred (lazy) tool loading: tool_search + auto threshold for large MCP tool sets Adds deferred tool loading so a large MCP tool set no longer sends every tool's full JSON schema each turn. When the number of visible MCP tools crosses a threshold (config tools.deferThreshold, default 10; 0 disables), their schemas are withheld and advertised compactly in a per-turn system-reminder; the model pulls a tool's full schema on demand via a new tool_search tool. Strictly additive: below the threshold behaviour is byte-identical to today. No new module dependencies. - Eligibility: MCP tools opt in via an optional Deferred() bool interface (tools.IsDeferred); built-ins stay eager. No change to the core Tool interface. - Formatting (internal/tools/deferred.go): compact one-liners wrapped in a <system-reminder> by BuildDeferredReminder; server label via MCPServerName(). - tool_search (internal/tools/tool_search.go): SideEffectNone/PermissionAllow/AdvertiseInAuto; select:Name1,Name2 (exact) or keyword (ranked); returns full schemas and signals the loop via Meta["load_tools"]. Operator-filter-aware (never lists tools hidden by --enabled-tools/--disabled-tools). - Agent loop: partitionTools is the single source of truth; eligible counts only ToolVisible deferred tools; Meta["load_tools"] lifts to ToolResult.LoadedTools and unions into a per-run loaded-set advertised next turn. Reminder appended only to the per-turn request copy (never persisted), including both reactive-compaction retry paths. tool_search stays reachable when deferral is active (exempt from the EnabledTools allowlist). - Config + wiring: tools.deferThreshold (default 10; 0 disables; negative rejected); tool_search registered and DeferThreshold threaded in exec and TUI, gated on the same visible-deferred count the loop uses. Recreated on top of current main; resolves the SideEffectNone clash with #146 (escalate_model). Supersedes #147. * deferred-tools: activate deferral only when tool_search is runnable; drop dead deferredTools() Addresses #148 review: - partitionTools gates 'active' on a registered, non-disabled, mode-advertised tool_search (mirrors the executeToolCall dispatch gate). When the loader is unavailable (explicitly disabled, missing, or not advertised e.g. spec-draft), fall back to the eager/inactive path instead of hiding deferred tools behind a loader the model can never call (the dead-end). - Flip TestDisabledToolSearch... to assert the eager fallback; register a real tool_search in the active-path tests that previously relied on activation without a usable loader. - Remove unused toolSearchTool.deferredTools(). * deferred-tools: disable deferral in exec when tool_search disabled; drop unused toolDefinitions shim Further #148 review: - exec computes one effective DeferThreshold, forced to 0 when tool_search is in --disabled-tools, and feeds it to both registerToolSearchIfEligible and agent.Options.DeferThreshold — so the registration gate and the loop partition gate agree, and a disabled loader never enables deferral. - Remove the unused toolDefinitions shim (golangci-lint unused; no callers). |
||
|
Anandan
|
b83367e9d3 | Add spec mode review flow (#145) | ||
|
KRATOSandGnanam
|
69810d5ab0 |
Mid-run model escalation: escalate_model tool + --allow-escalation (#146)
* modelregistry: add UpgradeTargetID and Registry.UpgradeTarget escalation lookup * modelregistry: seed escalation chains in catalog decoration * modelregistry: verification gate for escalation upgrade map * agent: add ModelSwitcher option and ToolResult.RequestedModel field * agent: lift escalate_to_model meta into ToolResult.RequestedModel * agent: perform at-most-one mid-run model switch in the turn loop * tools: add SideEffectNone for control-only tools * tools: add escalate_model control tool (metadata + args) * cli: parse --allow-escalation exec flag * cli: document --allow-escalation in exec help * cli: register escalate_model tool under --allow-escalation * cli: wire model switcher for exec escalation * cli: guard byte-identical exec run without escalation flag * cli: attribute post-switch usage to the escalated model * tools: accept an explicit empty escalate_model reason reason is an optional, informational-only arg, but the previous arg parsing rejected an explicit reason:"" (non-empty-string check), failing the whole escalation. Switch to stringArgWithEmpty so an absent OR empty reason is accepted while a non-string value is still rejected. Add a test covering both the empty and absent forms. * agent: test (nil,nil) escalation switcher keeps original provider When a ModelSwitcher returns (nil, nil) — no error, no replacement — the loop's `else if newProvider != nil` guard already keeps the active provider, so the run continues on the original model. Add an agent-layer test that proves this (it panics if the nil guard is dropped), closing the gap the adversarial review flagged. Also note the deferred compaction limitation near the switch: the compactor's context-window budget is fixed at run start and not updated on a mid-run switch (needs a ModelSwitcher contract change to fix). * cli: gate usage model attribution on the escalation flag and harden tests FIX A (back-compat): the OnUsage closure wrote a "model" key into every persisted EventUsage payload unconditionally, diverging a non-escalation run from the byte-identical origin/main baseline. The model can only change mid-run under --allow-escalation, so include the "model" key only then. A new flag-OFF test asserts the persisted usage payload carries no "model" key (mutation-proven). Test hardening for the CLI escalation boundary: - Wire test: each newProvider build returns a DISTINCT provider with its own turn counter; assert the source provider served exactly the escalation turn and the escalated provider served exactly the post-switch answer. Fails if `provider = newProvider` is dropped. - Mixed usage attribution: emit usage pre- and post-switch and assert the first attributes to the original model, the second to the escalated target (the loop's ordering guarantee). - Switch-error path: a failed rebuild is non-fatal and post-error usage stays attributed to the original model (currentModel unchanged). - Top-tier decline end-to-end: a flag-on run that escalates while already top-tier performs no switch (newProvider built exactly once) and succeeds. * modelregistry: validate UpgradeTargetID resolves at registry construction * cli: update currentModel only when the switched provider is non-nil * sandbox: make 'none' a first-class side effect (control-only tools) --------- Co-authored-by: Gnanam <[email protected]> |
||
|
Anandan
|
2dfd654107 | Add spec mode foundation (#144) | ||
|
|
68facda077 |
Image (multimodal) input: exec --image, stream-json images, TUI /image across all 3 providers (#143)
* zeroruntime: add ImageBlock type and NormalizeImageMediaType * zeroruntime: add optional Images field to Message * zeroruntime: add SeedMessagesWithImages, delegate SeedMessages with nil * Emit Anthropic image source blocks for user image turns * Add inlineData part type to Gemini request schema * Serialize user image attachments as Gemini inlineData parts * test(openai): pin text-only chat request serialization before content refactor * feat(openai): allow structured multimodal content parts in chat request * feat(openai): map image attachments to image_url content parts * test(openai): assert image content parts serialize over the request body * modelregistry: add SupportsVision capability gate * Add shared image-file loader package * cli: parse repeatable exec --image flag into imagePaths * cli: resolve exec image attachments via shared loader with sniff and size cap * agent: thread image attachments through Options into the seeded user turn * cli: resolve and vision-gate exec image attachments into the agent run * cli: document exec --image flag in help output * streamjson: add InputImage type and InputEvent.Images field * streamjson: whitelist images field on message events only * streamjson: allow empty content on message events that carry images * streamjson: add ResolveImages to decode, normalize and cap input images * streamjson: cover image parse-and-resolve round-trip and text-only invariance * Add /image command definition to TUI * Add synchronous vision check for TUI model * Implement /image attach, clear, and non-vision refusal in TUI * Render pending image chips above the input in both skins * Thread pending images into the agent turn and clear on submit * Wire stream-json images through to the agent run Stream-json message images were parsed and validated but never reached agent.Options.Images: streamjson.ResolveImages had no production caller, so images sent over stream-json input were silently dropped. resolveExecPrompt now also returns the resolved stream-json images (nil for text input and for image-free stream-json input). The exec runner merges them with --image attachments before the vision gate, so both input sources flow through the same drop+warn and the same agent.Options.Images wiring. Adds an exec-level end-to-end test that feeds a base64 image on a stream-json message event via stdin on a vision model and asserts the image reaches the agent run (the provider request's user turn). * Re-check TUI vision support at submit, not only at /image attach A user could attach an image on a vision model and then switch to a non-vision model via /model before submitting; the images were threaded unconditionally into the agent turn, reaching a model that rejects them. The commandPrompt submit path now re-runs the vision check against the current effective model. When the active model can't accept images, the pending images are dropped (not threaded) and an inline notice mirroring the headless drop+warn wording is appended. Pending image state is still cleared after submit either way. Adds a test that attaches on a vision model, switches the model id to a non-vision one, submits, and asserts the run received no images, an inline notice was appended, and pending state was cleared. * Bound memory with an os.Stat size pre-check in imageinput.LoadFile LoadFile read the whole file into memory before checking the size cap, so a multi-gigabyte file allocated a huge buffer only to be discarded. Add an os.Stat size pre-check before os.ReadFile: a file whose size exceeds MaxImageBytes is rejected with the same 10 MiB-limit error before any read; a missing file surfaces the same not-found error. The existing post-read len(data) check is kept as a TOCTOU backstop in case the file grows between Stat and ReadFile. * OpenAI: only the user role emits image content-parts The Anthropic and Gemini providers only attach images in their user branches, but OpenAI funnels every role through a single mapMessage, which would emit image content-parts for any role carrying Images. Guard the content-parts path to the user role: a non-user message that happens to carry Images keeps the plain string/nil content path. Pins the invariant with TestMapMessageNonUserRolesNeverCarryImages, asserting assistant/tool/system messages with Images set still serialize plain content (no content-parts array). * Deep-copy image buffers so attachments are never aliased Add a shared CloneImageBlocks helper in zeroruntime that deep-copies a slice of ImageBlock including each Data byte slice (returning nil for nil/empty input). Use it in SeedMessagesWithImages so a seeded user turn owns an independent copy of the caller's image bytes, and in the agent loop's copyMessages so image bytes are deep-copied alongside ToolCalls instead of staying aliased across history/request/result copies. Tests assert that mutating the caller's original Data bytes after seeding leaves the seeded message unchanged, and that copyMessages produces independent image bytes. * Accept image-only stream-json turns and bound base64 image size Resolve stream-json images before the prompt in the exec input path and tolerate an empty prompt when images are present, so an image-only message event (empty content + one image) proceeds with prompt="" and the image reaches the agent run instead of being rejected. A turn carrying neither text nor images still errors. In ResolveImages, reject an oversized payload from its ENCODED length (DecodedLen) BEFORE the base64 decode so a huge blob never allocates a decode buffer just to be capped afterward; the post-decode length check stays as a backstop. Tests cover the image-only exec turn reaching the agent and the pre-decode oversize rejection (asserting the size gate fires before any decode). * Bound the image read in imageinput.LoadFile with a LimitReader Replace os.ReadFile with os.Open + io.ReadAll over an io.LimitReader(f, MaxImageBytes+1). The os.Stat pre-check is only a fast-path hint (a non-regular file reports a misleading size and the file can grow between stat and read); the LimitReader is the real bound, so at most one byte past the cap is ever buffered and an oversized or unbounded source can never allocate a multi-gigabyte buffer. The file is closed via defer and the existing size message is reused. * Fix RenderChat frame-height off-by-one with image chips cmdRegion emits TWO rows when ImageChips is set (a chip row above the input line), but the body/overlay height accounting assumed a one-row command region, overflowing the fixed frame by one line. Compute the fixed-row count from the actual command-region height (1 or 2 rows) so the composed frame stays exactly Height rows in both cases. A RenderChat test asserts the rendered frame line-count equals the requested height both with and without ImageChips set. * Reject an empty inline --image= value Route the inline --image= form through requiredInlineFlagValue, the same empty-rejection the other inline flags use, so --image= errors with "--image requires a value" instead of appending an empty image path. A parse test covers the empty inline value. * imageinput: reject non-regular files (FIFO/dir) before opening --------- Co-authored-by: KRATOS <[email protected]> Co-authored-by: Gnanam <[email protected]> |
||
|
|
9c2e14272b |
CLI: doctor config validation, autonomy ceiling, changes --base, usage report (#142)
* config: add ValidateFile for structured config diagnostics
* doctor: add offsetToLineCol helper for JSON error positions
* doctor: validate config files in config.validation check
* cli: thread config paths into doctor and degrade on bad JSON
* doctor: inline read, single-parse config validation + utf8 col test
- Remove the osReadFile wrapper; call os.ReadFile directly at its one call site
- Add config.ValidateBytes([]byte) so each config file is read and JSON-parsed
once in configValidationCheck instead of twice (once for position, once via
ValidateFile); ValidateFile now reads then delegates to its own unmarshal for
the path-prefixed error message, keeping its public signature unchanged
- Fix the misleading skip comment in configValidationCheck: configFilesCheck
only checks path-string presence; the skip is a defensive guard for the
unreachable case where DefaultResolveOptions passed a non-empty path that
still fails to read
- Add utf8 multibyte case to TestOffsetToLineCol documenting byte-column
semantics (column counts bytes, not runes)
* sandbox: add Policy.MaxAutonomy ceiling defaulting to high
* sandbox: normalize both operands in autonomyAllowed to fail closed
* sandbox: clamp grant-allow and unsafe escalation to policy autonomy ceiling
* sandbox: add RequiredAutonomyForBatch helper with fail-closed mapping
* config: add sandbox.maxAutonomy field to file and resolved config
* config: merge sandbox.maxAutonomy across layers, env, and overrides
* cli: thread configured autonomy ceiling into sandbox engine construction
* cli: surface max_autonomy in sandbox policy formatters
* zerocommands: carry max autonomy in sandbox policy snapshot
* sandbox: default empty policy ceiling to high + reason const
Evaluate now treats an empty Policy.MaxAutonomy (from a directly-constructed
Policy that bypasses DefaultPolicy) as High instead of letting it normalize to
Low, which would silently clamp every Medium/High decision to Prompt. The
ceiling is a no-op unless explicitly configured.
Also extracts the "above policy ceiling" clamp reason into a package const
referenced at both engine clamp sites and the related tests, and adds a
non-vacuous engine test proving the empty-ceiling trap is closed.
* zerogit: add base-ref three-dot diff to changes inspect
* zerogit: cover base-ref backward-compat and real-git diff
* zerogit: propagate base ref through change snapshot
* cli: parse changes --base ref and reject on commit
* cli: thread base ref into changes inspect output and help
* changes: redact base in cli summary + guard --base= + rename test
- redactChangeSummary now redacts the Base field alongside Root/Branch/Commit/etc.
- --base= equals-form rejects leading-dash values via flagValueLooksLikeOption, closing the option-smuggling gap
- TestParseNameStatusRenameAndCopy asserts rename/copy three-field lines use the new/destination path with correct status, and confirms two-field modify is unaffected
* Add ParseDiffStat helper for git diff --stat summaries
* Add usage report aggregation over persisted session usage events
* Add zero usage report command over persisted session usage
* Wire usage command into CLI dispatch and help
* usage: validate --since format, fix redaction comments, add tests
Fix 1 (BLOCKING): parseUsageArgs now validates --since with time.Parse
against YYYY-MM-DD; returns exitUsage + "invalid --since %q: expected
YYYY-MM-DD" for bare strings like "foo", unpadded "2026-6-1", or
wrong-separator "06/01/2026". Valid dates are stored unchanged for the
existing lexical comparison.
Fix 2: Replace false "pre-redaction stat" claims in diffstat.go and
usage.go with accurate comments: the --stat summary line carries no
secret-bearing tokens so parsing the already-redacted DiffStat is safe.
Fix 3: Add three tests to usage_test.go —
- TestRunUsageDaysFilter: stubs deps.now, seeds events on two dates,
asserts --days 3 includes the recent date and excludes the old one.
- TestRunUsageInvalidSince: table-driven; verifies "foo", "2026-6-1",
"06/01/2026" each return exitUsage + validation message, while
"2026-06-01" returns exitSuccess.
- TestRunUsageEmptyStore: zero usage events → exitSuccess, output
contains header and "total" row, no panic.
* doctor: surface type-mismatch offsets and drop colliding flat line/col keys
Probe config validation against config.FileConfig instead of a bare any so
a structurally-valid document with a wrong field type (e.g. maxTurns as a
string) surfaces a *json.UnmarshalTypeError carrying the offset, instead of
losing line/col. Remove the flat top-level details["line"]/["col"] keys
that were overwritten by the last malformed file when several were present;
keep only the unambiguous per-path map entry. Tighten the existing malformed
test to assert concrete per-path line/col and add a type-mismatch test that
exercises the previously-dead UnmarshalTypeError branch.
* sandbox: fail closed on invalid configured autonomy ceiling
An invalid non-empty sandbox.maxAutonomy (e.g. "moderate") previously
survived Resolve (only trimmed, never validated), failed to normalize at the
sandbox bridge, and left the policy unchanged at the default High ceiling.
A typo therefore silently disabled the admin's intended ceiling: fail-open on
a security boundary.
Validate at resolve time (fail loud): Resolve now rejects a non-empty
maxAutonomy that does not normalize, returning a clear error. As defense in
depth, applyConfiguredAutonomyCeiling now clamps an unrecognised non-empty
value to the most restrictive ceiling (low) instead of returning the policy
unchanged, so a bad value can never widen the ceiling even via direct use.
Empty/unset stays valid and keeps the default High ceiling.
Add resolver tests for invalid-fails / valid-resolves and bridge tests for
the fail-closed clamp and valid mappings.
* sandbox: fail closed on malformed autonomy and surface policy resolve errors
Treat a genuinely-invalid request autonomy as the highest tier in the engine
so it exceeds a Medium/Low ceiling and clamps to Prompt instead of being
sanitized to Low and auto-allowing on the grant/unsafe path. Surface
resolveConfig failures in the policy command instead of silently falling back
to the permissive DefaultPolicy, which would misreport trust posture.
* usage: bucket and filter by UTC calendar day, reject empty --session
Normalize RFC3339 timestamps to their UTC calendar date before day bucketing
(report) and before the --since/--days cutoff comparison (CLI) so offset
timestamps land on the correct UTC day and the bucket and cutoff agree;
malformed timestamps fall back to the leading-10 slice. Reject empty
--session/--session-id values with a value-required error matching the --since
validation style.
* doctor: fail on unreadable config files and stop fabricating positions
Surface a present-but-unreadable config path (permissions, is-a-directory) as
a failing per-path validation detail instead of silently skipping it; a
genuinely-missing path stays a skip. Return ok=false for a JSON parse error
that carries no offset so it routes through the no-position ValidateBytes path
rather than fabricating a (1,1) line/col. Assert empty stderr on the JSON
doctor failure path.
* config: make sandbox maxAutonomy resolver tests hermetic
Pass an explicit empty Env to the maxAutonomy Resolve tests so host
environment variables cannot leak in through the nil-Env os.Getenv fallback.
* sandbox: preserve raw autonomy for ceiling check so invalid clamps under default ceiling
Defaulting invalid request.Autonomy to High only clamped under a Medium/Low
ceiling; under the default High ceiling autonomyAllowed(High, High) returned true
and still auto-allowed (fail-open). Keep the raw requested value for the ceiling
check so autonomyAllowed's unknown-tier guard fails it closed under ANY ceiling.
Adds default-High-ceiling regression tests for the unsafe and grant-allow paths.
---------
Co-authored-by: KRATOS <[email protected]>
Co-authored-by: Vasanthdev2004 <[email protected]>
|
||
|
Vasanth T
|
2a8dc75628 |
Add provider catalog foundation (#141)
* feat: add provider catalog foundation * feat: add provider onboarding checks * fix: address provider readiness review * fix: clear provider catalog review threads * fix: accept direct auth values in provider checks * fix: harden provider catalog config * fix: guard project provider credential binding |
||
|
Vasanth T
|
32d0e5fa34 |
Add context budget report (#139)
* add context budget report * fix context report review findings |
||
|
KRATOSandKRATOS
|
ea2acbbef2 |
zeroline TUI: fix slash palette, token readout, @ files, ! bash (#138)
* zeroline TUI: fix slash palette, token readout, @ files, ! bash
The zeroline home footer advertised affordances the code didn't keep. Fixes:
- '/' palette: autocomplete.go suppressed a bare '/' ('/' alone showed nothing). Now a bare '/' lists the full command palette (capped at 8). The footer's '/ commands' works.
- Token readout: the zeroline header was built without Cost/TotalTokens, so the bars always showed $0.00 and no token total. zerolineHeader() now threads m.usageTracker.Summary() (TotalCost + TotalTokens, with the unpriced-token fallback); the bottom bar shows a cumulative 'tok <N>' segment (zeroline.Header gains TotalTokens; humanTokens formats it).
- '@ files': '@' input now drives a workspace file picker in the existing overlay (trailingAtToken/fileSuggestions/replaceTrailingAtToken; bounded, skips .git/node_modules/vendor/dist/hidden). Selecting inserts the @path, preserving preceding prompt text. Previously '@…' was sent to the agent as a chat prompt.
- '! bash': '!cmd' now runs as a shell escape (commandBash) — executed async with a 30s timeout in the workspace, output shown in the transcript. Previously '!cmd' was sent to the agent as chat.
- '1-5 theme': left as-is — it works when the input is empty (the home screen's state); gating on empty input keeps digits typable in prompts.
Tests: bare-/ palette, @-token detection/replacement, file listing+filter+skip-dirs, !cmd parsing, header usage. build/vet/-race/full-suite + GOOS=windows green; no new deps.
* zeroline TUI: emit forward-slash @file paths (Windows fix)
fileSuggestions used filepath.Rel, which returns backslash paths on Windows, so the inserted @path token and the test assertions diverged (TestFileSuggestionsListsAndFiltersWorkspaceFiles failed on the Windows smoke). Normalize with filepath.ToSlash so @paths are forward-slash on every platform.
* zeroline TUI: gate !cmd behind unsafe mode + test humanTokens
CodeRabbit major: the !cmd shell escape ran bash -c directly with no permission check, bypassing the sandbox. It now runs ONLY in unsafe permission mode; in auto/ask it is not executed and the user is told to relaunch with --skip-permissions-unsafe. Adds TestBashEscapeGatedByPermissionMode (auto/ask gated, unsafe runs).
CodeRabbit minor: adds TestHumanTokens covering negative clamp, 0, sub-1k, 1k trim, fractional, and the 999999->1000k rounding edge.
* zeroline TUI: bound @file walk by entry count; humanTokens M-scale
Review fixes for #138:
- fileSuggestions now counts every WalkDir entry (directories included), not
just files, and stops at the budget; a directory-heavy tree could otherwise
be traversed in full each keystroke and stall the TUI. Extracted
fileSuggestionsBounded so the per-keystroke budget is unit-tested.
- humanTokens switches to "M" notation past 1,000,000 instead of rendering
"1000k"+ on large-context models; tests extended to the M boundaries.
* zeroline TUI: make unsafe mode reachable so the ! shell escape works
The "!cmd" shell escape is gated behind unsafe permission mode, but the
interactive TUI hardcoded Ask and shift+tab only cycles auto<->ask, so unsafe
was unreachable and the escape was dead code. Worse, the gate's own remediation
message ("Relaunch with --skip-permissions-unsafe") pointed at a flag the
interactive launch did not parse: "zero --skip-permissions-unsafe" fell through
to the unknown-command path.
Honor --skip-permissions-unsafe at interactive launch (both "zero" and
"zero zeroline"), mirroring exec, so unsafe mode -- and the ! escape -- is
reachable via an explicit opt-in. Thread the resolved mode through
runInteractiveTUI(WithSkin); the default stays Ask. Document the flag in
"zero --help" and add launch tests for both entrypoints.
* zeroline TUI: run the ! escape via the platform shell, not hardcoded bash
Now that --skip-permissions-unsafe makes the "!" escape reachable on every
platform, hardcoding "bash -c" left it broken on stock Windows (no bash on
PATH) and anywhere bash is not at a predictable path -- the advertised
"! bash" footer would just print an exec error.
Resolve the shell the same way the agent bash tool does: cmd.exe /d /s /c on
Windows, /bin/sh -c elsewhere. Add a platform test asserting the command is
wrapped correctly.
---------
Co-authored-by: KRATOS <[email protected]>
|
||
|
KRATOSandKRATOS
|
99a0bea90b |
Real-world hardening: graceful shutdown + crash capture (#136)
* [LOCAL PR-Y] CLI: graceful shutdown of headless exec runs (Ctrl+C) zero exec wrapped agent.Run in context.Background(), so Ctrl+C/SIGTERM killed the process abruptly — leaking the in-flight provider request and any spawned children. runExec now wraps the run in a signal-aware context (signalContext): a signal cancels the run, the loop returns cleanly, and exec reports 'Interrupted.' with exit 130 (stream-json: an 'interrupted' run-end). Scoped to runExec — no dispatch-wide signature churn. NOTE: SIGINT runtime delivery needs a manual Ctrl+C check before PR; unit tests cover the helper + the no-regression of the exec suite. * [LOCAL PR-Y] Observability: crash capture (panic -> saved report) New internal/observability package: a recovered top-level panic is written to a local crash report (timestamp, label, full stack) under ~/.zero/crashes and surfaced as a brief notice instead of a raw stack trace. Wired into runWithDeps via a named-return defer. Fail-open and dependency-free; this is the hook a Sentry/OTEL adapter attaches to for remote reporting (deferred, needs a real endpoint to verify). Tested: report formatting/writing, panic capture sets the crash exit code + writes one report, and the no-panic path is a no-op. --------- Co-authored-by: KRATOS <[email protected]> |
||
|
KRATOSandKRATOS
|
07d8ffc524 |
CLI wiring: skills/zeroline commands, --mode presets, exec checkpoints, sandbox --effective, sessions rewind (#128)
* CLI wiring: skills/zeroline commands, --mode presets, exec checkpoints, sandbox --effective, sessions rewind Final module of the runtime-core split. Wires the new runtime features into the cli, PRESERVING main's merged specialist cli (the source branch predates specialist and drops it; this keeps it via a 3-way merge from the common base, adding the reskin features as ours/theirs-only changes). - skills command (internal/cli/skills.go) listing internal/skills; zeroline-skin TUI launcher (internal/cli/zeroline.go) via runInteractiveTUIWithSkin. - exec: --mode presets (smart/deep/fast/large/precise) via applyExecMode; model-registry alias resolution + deprecation/effort notices; ContextWindow sizing (modelContextWindow); before-mutation checkpoint recording in OnToolCall; enhanced tool-result output (ChangedFiles/Redacted/Display); permission-mode validation fix. - sandbox policy --effective (resolved guards); sessions rewind (executes the rewind plan); interactive TUI defaults to Ask mode (advertises write/edit/bash + gates via prompts). - internal/streamjson: EventCheckpoint/EventRestore + CheckpointInfo for structured checkpoint output. Specialist cli preserved (specialist.go + flags + runtime + tests all intact). build/vet/-race/full-suite + GOOS=windows build green; no new deps. * Address #128 review: make sessions rewind reachable + honor --exclude-target, mode/notice/redaction fixes - sessions: add "rewind" to the parser whitelist (isSessionsCommand) — it was wired in dispatch + help but parseSessionsArgs rejected it as 'unknown sessions command', so it was unreachable. Regression test TestRunSessionsRewindIsReachableAndHonorsExcludeTarget. - sessions rewind: honor --exclude-target — ApplyRewind keeps THROUGH a sequence, so when KeepTarget is false apply through TargetSequence-1, matching rewind-plan (previously rewind always applied TO the target, disagreeing with the plan). The test asserts --exclude-target keeps fewer events than the default. - sessions rewind: redact the session id in the success + not-found messages (consistent with every other human-readable path). - exec_parse: reject empty inline --mode= via requiredInlineFlagValue (was a silent no-op running defaults). - exec: notice writes (model-deprecation + reasoning-effort advisories) now treat write failures as exitCrash, consistent with the rest of the command. build/vet/-race/full-suite + GOOS=windows build green. --------- Co-authored-by: KRATOS <[email protected]> |
||
|
Vasanth T
|
781bc7d3c1 |
Add guarded web fetch tool (#121)
* feat: add guarded web fetch tool * fix: harden web fetch DNS validation * fix: tighten web fetch SSRF guardrails |
||
|
Anandan
|
82314a703c |
Add specialist authoring tools (#117)
* Add specialist authoring tools * Harden specialist authoring boundaries |
||
|
Anandan
|
27f0fe8f30 |
Harden specialist background lifecycle (#116)
* Harden specialist background lifecycle * Preserve killed background task state |
||
|
Anandan
|
936bbea59d |
Add specialist background task tools (#115)
* Add specialist background task tools * Address specialist background review feedback |
||
|
Anandan
|
a60154b70a |
Add specialist Task runtime (#113)
* Add specialist Task runtime * Harden specialist resume identity |
||
|
Anandan
|
561fe0b7ad | Add specialist lifecycle metadata (#106) | ||
|
Anandan
|
ae6c9595db |
feat(specialist): add exec metadata flags (#104)
* Add specialist exec metadata flags * Address exec metadata review |