* test(tui): pin the cwd in tests that assert text is absent from the view
Nine tests built their model with an empty Options.Cwd, so newModel took
the package directory and the title bar showed the developer's real
branch and checkout path. Each then asserted that some text was absent
from the whole view, and failed wherever that branch or path happened to
contain it: TestSuggestionOverlayRenders on a branch with /model in its
name, TestSuggestionOverlayCapsRowsWithoutMoreText on a branch or path
containing "more" or "compact", the provider wizard tests on "offline",
"chatgpt" or "gpt-4.1", and so on.
They now pin Options.Cwd to isolatedCwd, a path that does not exist, so
no branch is read and the path is the same on every machine. A guard
test checks that isolatedCwd stays outside any git checkout.
* test(tui): make isolatedCwd fully qualified and absent by construction
The fixed /workspace/zero path could exist on some machine, a dev
container for one, and newModel would then read whatever is there. On
Windows it was also only drive-relative: \workspace\zero is not a fully
qualified path.
isolatedCwd is now a child of the home TestMain isolates that nothing
creates, so it is fully qualified, absent and outside any checkout on
every platform, and the title bar shows ~/not-a-checkout everywhere. A
t.TempDir path would have put the user name and the test name into the
view. The guard test checks all three properties.
* test(tui): clear the branch in view-test models instead of trusting the cwd
isolatedCwd is a child of the isolated home, which lives under the
temporary directory. With TMPDIR inside a checkout, gitBranch walks up
from that absent path to the checkout and puts its branch in the title
bar, so the guard failed on an ordinary branch and a colliding branch
broke the nine view tests again.
newViewTestModel now builds those models: it sets the isolated cwd and
clears the branch after construction, since no path can promise there
is no checkout above it. The guard keeps only what a path can promise
(fully qualified and absent), and a new test puts the home inside a
checkout on feat/model-picker and checks that neither that branch nor
the real checkout path reaches the view.
* test(tui): assert no checkout path reaches the view in the enclosing-checkout test
The test checked the branch strings and that the isolated cwd shows,
but a view could show both the isolated cwd and a real checkout path
without failing it. It now also asserts that neither the enclosing
checkout nor the directory the tests run from appears in the view.
The variable turns off both the models.dev fetch and the cached overlay,
and four tests in modelsdev_test.go inherited it from the shell. With it
set, TestRefreshModelsDevCacheRejectsBadBodyWithoutClobbering and
TestDefaultModelEntriesAppliesFreshCache fail, and
TestCachedModelsDevProvidersIgnoresStaleCache and
TestCachedModelsDevProvidersRequiresOptIn pass without reaching the
check they are named for: the nil they expect comes from the variable.
Clear it in each of them, as the two sibling refresh tests already do.
GHSA-5r4p-p66f-jhc7 (moderate): hono's serveStatic decodes the request
path a second time, so middleware on static paths can be bypassed.
Versions 4.12.4 to 4.13.10 are affected and main pins 4.13.9 through
tuistory, which release packaging installs as a local-control helper.
npm update takes hono to 4.13.13, inside every range that asks for it.
package.json is unchanged and nothing else in the lock moves.
* fix(oauth): use the crash-safe secret store and sync token publications
The encrypted-file OAuth backend carried its own AES-GCM secret implementation that had drifted from internal/securefile: a crash during first login left <secret>.lock behind and every later Save timed out instead of reclaiming it, and the secret and token blobs were published without fsync.\n\nUse securefile.Crypter with an opt-in publication directory so the secret still publishes through the sandbox-denied .secret.publish directory, add file and directory barriers to the encrypted token blob, and extend securefile with the publication directory plus destination-directory sync.\n\nMove the concurrent-creation and wrong-sized-secret coverage to securefile, add a stale-lock regression through the OAuth store, and test publication-dir use, sync ordering, sync-failure preservation, and retry after a failed secret sync. The stale-lock test fails on the previous code.
* test(oauth): cover post-rename directory-sync failures
The existing failure tests inject a file sync error before the rename. Add coverage for a destination-directory barrier failure after the token or secret has been atomically replaced: the write must report the error and the newly published bytes must remain readable. Also cover the corresponding securefile secret path, including its expected size.
* fix(securefile): never clobber a newer lock when restoring a reclaimed one
reclaimStaleLock could rename a lock that had just been replaced, then restore the quarantined copy with os.Rename, replacing a third holder's new lock. Both holders could then publish different secrets for the same file. Restore with O_CREATE|O_EXCL and discard the quarantined copy when the destination is already held.
Extend the directory-sync failure test to prove a later caller actually loads the published secret, not just its size. Add restore tests for the free and already-held cases.
govulncheck now reports GO-2026-6809 (crypto/x509 distrusts compromised
intermediate certificates) against Go 1.26.9, reachable through
daemon.WriteFrame, so Security & Code Health fails on main and on every
open PR. It is fixed in 1.26.10.
CI and release builds read the toolchain from go.mod. The stated
source-build minimum in README.md, README_ZH.md and docs/INSTALL.md
moves with it.
runExec starts a background models.dev download unless
ZERO_DISABLE_MODELS_FETCH is set. TestRunExecImageAttachmentHistory moved
the user directories into a TempDir by hand but left the fetch on, so a
5.7 MB download landed in the TempDir while its cleanup ran, and the
macOS runner failed with "unlinkat ... directory not empty". The two
older image tests had no isolation and fetched into the real user cache.
Use isolateCLIUserState in all three, which also points the cache path
into the fixture and turns the fetch off.
* fix(tools): grep stops where a file's line breaks stop
scanGrepFile treated EOF after a trailing line break - and an empty file -
as the START of another line, and handed the matcher an empty line one
number past the real content. Every pattern that matches an empty string
then reported a match on a line no reader sees:
a.txt holds "x\n"; grep pattern "^$"
-> a.txt:2: (grep -c '' GNU-style reports no such line)
same file, pattern ".*", count mode
-> "2 matches found" (one real line, one phantom)
an empty file, pattern "^$"
-> a.txt:1: (the file has no lines at all)
read_file and the file tracker's countLines already state the rule the
scan violated: "the trailing newline does not open a line", and an empty
file has no lines. The scan now stops there too, so anchored and
empty-matching patterns can only match the lines the content carries.
Real matches, numbering, count-per-hit accounting for non-empty patterns,
file mode, glob and symlink handling are unchanged.
* test(tools): make the grep line-number parity test catch the phantom
The parity test matched only "needle", a pattern that cannot match the
empty line the old EOF branch invented, so its absence-of-line-4
assertion passed on the code being fixed. The pattern now also matches an
empty line, and the expected output is asserted exactly: on the pre-fix
scanner the same run reported
grep = "a.txt:2: needle two\na.txt:4: ",
want exactly the line read_file numbers 2
so the phantom is visible right beside the real line, and the test is
about the numbering claim it states. Also gofmt-aligns the case table
that made the CI formatting check fail.
Two credentials glued end to end share one recovering property: the second
one's trigger prefix is where redaction can re-anchor. For fixed-length
bodies that anchor is exact (AKIA{16} stops at the next key), which is the
case the anchored end-chaining always covered. For unbounded bodies it
isn't: github's [A-Za-z0-9]{36,} body eats the literal "ghp" of the token
glued behind it, so the first match ends PAST the second trigger, the scan
that re-anchors at the span end finds nothing there, and the second
credential leaves the output as +_bbbb... / .eyJzdWIi... plaintext:
RedactString("ghp_aaa..36a..ghp_bbb..36b..")
-> "[REDACTED]_bbbb......" (second token's body printed in full)
RedactString(jwtA+jwtB)
-> "[REDACTED].eyJzdWIiOiI5ODc2...SA" (second token's payload+sig)
The same holds across shapes (a github key directly feeding "sk-ant-..."
through its body) and for three tokens in one run.
redactAdjacent now follows the run a match starts: at each boundary it
re-binds the match at the EARLIEST trigger occurrence inside the span that
stills validates as its shape (the head before the trigger must match the
anchored pattern exactly, and the run chained from the trigger must reach
past the span, so a mid-body lookalike and a mid-span cut that recovers
nothing leave the span alone), and continues under whichever shape matched
the swallowed trigger. Candidate examination is capped at
maxTriggerOccurrences per span so body spam of trigger lookalikes costs
what the span costs, and the first validating cut wins.
End-chaining for fixed-length bodies, single-credential shapes, custom
replacement templates, and the split path's announced no-op-for-contiguous
text guarantee are all unchanged; the regression tests pin the glued pairs
that previously leaked plus the AWS chaining and lone-token cases.
The paragraph describing publishAdapterLaunch appeared twice: once above
launchedOnce, where it described the wrong closure, and once above
publishAdapterLaunch itself. Keep the second.
#1149 was opened before #1105 moved the module to github.com/Twigpine/zero
and merged right after it, so its two imports of internal/gitsafe still
used the old path. Nothing provides github.com/Gitlawb/zero any more, so
main stopped building at 2ffe1831.
* fix(sessions): preserve image attachment summaries on resume
Persist TUI-compatible attachment counts for ACP and exec user events.
Restore readable markers in live history, session replay and resumed
prompts without storing or resending historical image data.
Cover image-only and mixed prompts, restart recovery, malformed metadata
and the aborted-turn boundary with regression tests.
Fixes#1135
* fix(workspaceindex): follow the current module path
Use github.com/Twigpine/zero/internal/gitsafe in the ignore scanner and its
tests. The upstream module rename preceded the scanner's gitsafe change,
leaving these two imports on the old path and preventing the rebased tree
from compiling.
* fix(sessions): count only images accepted by the model
Take ACP and exec attachment counts after the vision capability gate,
matching the TUI's persisted attachment semantics. Update the ignored-image
regressions to require zero saved images and no historical image marker.
Refs #1135
main moved the module to github.com/Twigpine/zero in #1105. Rename this
branch's imports to match, including the two in internal/workspaceindex
that main itself still carries until #1153 lands.
* fix(workspaceindex): run Scan's git calls through gitsafe
Scan asks git for ignored paths on every turn with its own gitCommand, which
turned off core.fsmonitor and dropped the repository-location variables but
still let a clean filter or diff driver from the repository's own config run
during status. It now goes through gitsafe, like the TUI and the scratch-file
warning: hooks are off as well, and a repository whose config defines a filter
or diff command takes the non-git path. The status call also passes
--ignore-submodules=dirty.
Scan makes two git calls, so gitsafe gains Check, which reads the repository
config once and builds several commands from it. Command is now Check plus one
command, and each command gets its own copy of the environment.
* test(workspaceindex): pin Scan's fallback when gitsafe refuses the repository
A repository whose own config defines a filter or diff command is refused by
gitsafe, and Scan then takes the non-git path. Scan reaches gitsafe.Check
through a package variable now, so a test can stand in for such a repository
without configuring one that names a command, and
TestScanFallsBackWhenGitsafeRefusesTheRepository checks both sides: with git
consulted, .gitignore decides and the fixed list does not apply; refused, the
fixed list applies and .gitignore is not read.
Update the Go module path, imports, install scripts, docs, workflows,
packaging and action defaults from Gitlawb/zero to Twigpine/zero. The npm
scope (already @twigpine/zero on main, with its @gitlawb/zero migration
notes), LICENSE, action author, the OpenGateway provider and historical
CHANGELOG links are unchanged.
The X-AIMLAPI-Integration-Repo header keeps Gitlawb/zero: it is what
AIMLAPI sees for attribution, so changing it needs a maintainer's yes.
docs/INSTALL.md keeps `go install github.com/Gitlawb/zero/...`, because
every tagged release so far declares that module path. The Homebrew
formula comment names the Twigpine/homebrew-tap tap.
Co-authored-by: Claude Opus 5.5 <[email protected]>
* feat(sandbox): support self-exec pattern to eliminate separate helper binary
Linux sandboxing previously required distributing a separate 'zero-linux-sandbox'
binary and looking it up on PATH.
This wires an internal '__sandbox-helper' entrypoint directly in cmd/zero/main.go
and configures findLinuxSandboxHelperCommand to fallback to os.Executable(),
enabling single-binary zero distributions with fully functional sandboxing.
* fix(sandbox): prefer PATH helper before self-exec fallback
Colocated zero-linux-sandbox stays first. A standalone helper on PATH
is used next. Self-exec of the main zero binary is only the last
fallback, so an installed helper is not ignored.
* feat(sandbox): clean formatting in main.go and add self-exec unit tests
* feat(sandbox): insert __sandbox-helper verb in bwrap plan for self-exec binary
* fix(sandbox): drive Linux selection from the self-exec helper resolver
Availability and command-plan construction now share the same helper
resolution and keep the __sandbox-helper prefix on the backend, so a
main-only install with bwrap reaches native sandboxing.
* fix(sandbox): self-exec only the zero main binary, not test binaries
* fix(sandbox): carry standalone vs self-exec identity into the inner bwrap plan
A versioned physical helper (zero-linux-sandbox-v1) must not receive
__sandbox-helper. The inner plan uses an explicit SelfExec flag, threaded
from argv, instead of guessing from filepath.Base.
* docs(install): describe self-exec as the last helper fallback
Self-execution runs only when neither a colocated nor a PATH
zero-linux-sandbox is available.
* docs(install): state colocated, PATH, then self-exec helper order
---------
Co-authored-by: cl-ment <[email protected]>
* perf(redaction): add fast-path byte checks to eliminate regex allocations on clean strings
Evaluating cascading regular expressions across every log line, tool output,
and session event incurred ~23 µs and 51 heap allocations even on strings with
zero secrets.
This introduces fast-path substring checks before invoking expensive regex
substitutions, yielding a 70x speedup and 0 B/op heap allocation on clean
strings.
* test(redaction): lengthen ghp_ fixtures and cover conditional gates
GitHub classic tokens need 36 characters after ghp_. Benchmarks and
shape tests now use a matching fixture. A table-driven test checks each
fast-path gate, including near-misses and case-insensitive Authorization.
* perf(redaction): structure triggered patterns and avoid allocation in header case checks
* test(redaction): cover secret-header and URL-password gates
Keep the redaction marker literal after url.URL rewrites userinfo, so
callers still see [REDACTED] instead of a percent-encoded form.
* fix(redaction): unescape only the generated URL password
A path or query that already contains the encoded redaction marker
must stay encoded. Only the userinfo password is rewritten in the
clear [REDACTED] form.
* fix(redaction): re-escape URL username after parse
Username() is decoded. Re-encoding with url.User keeps control bytes
out of the redacted URL so a %0AAuthorization userinfo cannot inject
a header after the header pass.
* fix(redaction): fail closed on password URLs without a host
* test(redaction): isolate query-gate coverage from assign and text secrets
* fix(redaction): fold Unicode U+017F and U+212A in containsCaseInsensitive
Go (?i) regular expressions apply Unicode simple case folding, where U+017F (ſ) folds to 's' and U+212A (K) folds to 'k'. Teach containsCaseInsensitive to fold these runes to ASCII lowercase with zero allocations so case-insensitive header gates cannot be bypassed by Unicode-folded equivalents.
---------
Co-authored-by: cl-ment <[email protected]>
* feat(providers): add Concentrate provider preset
Register Concentrate (https://api.concentrate.ai/v1) as an
OpenAI-compatible catalog provider. Concentrate is already listed in the
README Partners section; this adds the provider itself.
- models.dev has no Concentrate provider, so the id `concentrate` and the
key name CONCENTRATE_API_KEY match OpenClaude's Concentrate
integration. `Concentrate AI`, `concentrate.ai` and `concentrate-ai`
resolve to it.
- Default model deepseek-v4-flash-0731: the dated id behind the
deepseek-v4-flash alias that OpenClaude uses. Concentrate's /v1/models
keeps the rolling name only in `aliases`, and onboarding keeps the
default selected only when it matches a live id, so an alias default
made setup land on the first model in the list (claude-fable-5).
- Curated models: deepseek-v4-flash-0731 (default), deepseek-v4-pro,
kimi-k3 and glm-5.3. Context windows (1,048,576 each), tool calling and
thinking support come from Concentrate's public /v1/models.
- Regression test: a Concentrate-shaped live list must contain the
preset's default model.
- README and README_ZH: key line and setup command.
* test(providers): prove the Concentrate default check runs on the live list
The regression test passed whenever the default model was in the result,
and the curated fallback also contains it. Assert that the mock served
/v1/models and that the result includes claude-fable-5, a model only the
live response lists, so a silent fall back to the curated catalog fails.
* fix(cli): let providers subcommands run with no active provider
`zero providers list`, `use`, `remove` and `rename` exited 3 with
`no active provider configured: active provider "" not found` as soon as the
environment supplied more than one provider and config.json had no active
provider: normalizeProvidersWithOptions only elects an active provider by
itself when there is exactly one, and resolveCommandCenterConfig treated the
resulting error as fatal, so the command stopped before doing anything. A
base URL without a key counts as a second provider too (applyProviderEnv
builds a profile from a base URL alone).
The resolver already returns the successfully normalized list alongside
ErrNoActiveProvider, and the interactive TUI already falls back to a usable
entry from it in runInteractiveTUIWithSetup. Do the same here instead of
failing, so these subcommands can answer for providers that only come from
the environment.
Also pin the four env-derived provider tests to a fixed environment and a
temporary user config. They set OPENAI_API_KEY but resolved through the real
resolver against the developer's own config.json and ambient provider
variables, so they failed on any machine that also exported another provider
variable and whose user config had no active provider.
Adds regression coverage for the two-env-provider case: `providers list`
must report every detected provider and `use`/`remove`/`rename` must give
their usual env-derived answer. Verified against the fix (pass) and with the
fix reverted (exit 3, `active provider "" not found`).
Fixes#1130
* refactor(cli): report the configured active provider, not an elected one
Provider-management subcommands answer for the SAVED configuration, while
the TUI's recovery path answers "what can I run with". Adopting
firstUsableProvider in resolveCommandCenterConfig conflated the two: with
two env-derived providers it made `providers current`/`list` present a
provider that `zero exec` would still refuse to run with, and a
base-URL-only profile failed `providers list` because the election demands
a credential. Return the resolver's partial config untouched instead, so
nothing is reported as active when nothing is selected.
Also removes the OAuth store read this path picked up with firstUsableProvider.
Adds assertions that `current` shows nothing selected, `list` marks nothing
active, `use --json` leaves activeProvider empty, plus a base-URL-only case.
* fix(cli): scope the no-active-provider exception to provider management
Returning the resolver's partial config made `zero config` exit 0 with a
partial summary, contradicting the resolver's documented headless contract:
`zero config` / `zero exec` still fail with the actionable message and only
the interactive TUI recovers from a missing active provider. Split the
helper so only issue #1130's commands — `providers list`/`current` and the
unpersisted `use`/`remove`/`rename` reporters — tolerate the state; `zero
config`, `detect`, `models` and `check` keep failing as before.
Also pin the OAuth store in providerEnvSetupDeps: summarizeConfig reads
OAuth login state through the default store, so a developer's stored login
could flip these assertions. Point ZERO_OAUTH_TOKENS_PATH at a temp file
and force ZERO_OAUTH_STORAGE=file so an ambient keyring setting cannot pull
the OS keychain in either.
* fix(workspaceindex): skip git-ignored and build-cache dirs in Scan only
Scan has a fixed file budget, and build output (Cargo target/, Python
virtualenvs, caches) could exhaust it before real source was reached:
repo-map reported truncation and ranked fingerprint files above code.
Inside a git work tree, Scan now asks git for the ignored set
(`git status --porcelain=v1 -z --ignored=matching .`), so the repo's own
.gitignore, nested ignore files, info/exclude and core.excludesFile all
apply, including names no fixed list anticipates (.locust_env/). Outside
a work tree, or when git is missing, older than 2.16, fails, or exceeds
5s, a fixed list of common build/cache directory names stands in.
ShouldSkipDir is unchanged: glob, grep, list_directory and path
autocomplete share it and must keep seeing target/, .venv/ and friends.
The lookup runs with GIT_OPTIONAL_LOCKS=0 so a per-turn scan never takes
.git/index.lock next to a concurrent git command.
Fixes#1108
* fix(workspaceindex): skip a rename's original-path field when parsing status
In `git status --porcelain=v1 -z`, a rename or copy record is followed by one
more NUL field holding the original path, with no XY prefix. The parser
inspected every field, so an original name beginning with "!! " read as an
ignored entry: after `git mv "!! a" b`, a tracked file "a" disappeared from
Scan.
Move the parsing into parseIgnoredStatus, which consumes that field for R or C
in either status column, and test it directly and end to end.
* fix(workspaceindex): disable repo fsmonitor and force untracked-files=normal for status
Scan's git status lookup must not run a repository's core.fsmonitor hook or fail
when status.showUntrackedFiles=no would reject --ignored=matching.
* fix(workspaceindex): ignore inherited git repository location
Git exports GIT_DIR and GIT_WORK_TREE to hooks. With those pointing at
another repository, Scan kept .locust_env/lib/x.py, which root's own
.gitignore excludes.
* fix(workspaceindex): quote the fsmonitor hook path in the regression
On Windows, sh treats an unquoted backslash path as escapes, so the hook
ran and the marker the test stats was never created. Require a status
without the override to create that marker first.
* fix(workspaceindex): pass the fsmonitor hook path to git with forward slashes
Smoke (windows-latest) failed TestGitIgnoredPathsDisablesRepoFsmonitor
because core.fsmonitor was set to a backslash script path. Git for Windows
hands that value to sh, which consumes the separators, so the hook never
runs and the marker is never created. The test now stores a quoted forward-slash
path. The product code is unchanged.
* fix(tui,cli): stop a repository's git config from running programs in Zero's own git calls
Zero runs git status and git diff on its own: the TUI's files sweep at
startup and after each command, its pull request diffstat, and the
scratch-file warning after a run. A repository's own config can name
programs those commands run (core.fsmonitor on every status, hooks when
the index is rewritten, clean filters and diff drivers assigned through
.gitattributes), so a workspace unpacked from an archive ran its own code
on the first turn, although Zero refuses the same workspace's project
hooks and plugins until the user trusts it.
internal/gitsafe builds those commands with core.fsmonitor and
core.hooksPath turned off and GIT_OPTIONAL_LOCKS=0, and drops the
repository-location variables git exports to hooks, using the same
environment for its config check and for the command. A repository whose
own config, or per-worktree config when extensions.worktreeConfig is on,
defines a filter or diff driver command is refused, and the callers
already treat that like a directory that is not a repository. The status
and diff calls also pass --ignore-submodules=dirty, because a submodule's
own config is not checked.
* test(gitsafe): keep the real-git setup off an inherited GIT_DIR
The setup helper in TestCommandAgainstRealGit ran git with the inherited
environment, so from a hook, or a shell that exports GIT_DIR or
GIT_WORK_TREE, git init and git config would act on the repository those
name instead of the temporary one. It now uses the same environment as
Command, and the test sets both variables to a decoy repository and checks
after each setup step that the decoy was not touched.
* fix(npm): wait for npm to apply each publish, and drop the one-time @gitlawb/zero steps
npm applies a publish asynchronously: `npm publish` returns once the upload
is accepted, and the version appears later. In the 0.10.0 release the
@twigpine/zero wrapper appeared 9 seconds after publish-packages.sh stopped
checking for it, and the last platform payload about ten minutes after its
upload. Two things went wrong:
- The wrapper became visible before four of its five platform payloads, so
installs in that window resolved aliases that 404 and fell back to the
GitHub-release download. Ordering the uploads does not order what npm
shows.
- The final `latest` check gave up after 25 seconds and failed the job even
though every publish landed, which also skipped the @gitlawb/zero steps
until a re-run.
publish-packages.sh now waits until npm shows every platform version before
it publishes the wrapper, then waits until `latest` is the wrapper version.
One deadline bounds the waits (ZERO_NPM_PUBLISH_WAIT_TIMEOUT, default 30
minutes). On timeout it fails before the wrapper goes out and says what npm
never showed. These lookups treat an error as "not yet", unlike the skip
decision, which still stops on anything but E404.
The test fake can now apply a publish only after a number of later lookups,
and records which aliased versions were missing when the wrapper was
published. Against the script as released in v0.10.0, the two new tests fail
the way the release did: the job exits 1 although everything landed, and the
wrapper goes out while its platform payload is not visible.
The two release steps that published 0.10.0 once more as @gitlawb/zero have
done their job and are removed; docs/NPM_PACKAGING.md and the comments that
pointed at them now describe what happened. The Go and wrapper handling of
the old name stays, since only a build installed under it can trigger it.
* fix(npm): validate the publish wait settings, and correct the legacy publishing claim
ZERO_NPM_PUBLISH_WAIT_TIMEOUT went into Bash arithmetic as typed. A leading
zero reads as octal (0600 waited 384 seconds), and 08 is an arithmetic error
that, in a script, does not stop it: Bash prints the error, leaves the
deadline unset and carries on, so the run only died at its first wait, after
the platform payloads were published. A negative timeout ended the wait
before it started, and a bad ZERO_NPM_PUBLISH_RETRY_DELAY only failed at the
first sleep, also after publishing began.
Both settings are now checked before anything is published: whole seconds
only, converted with 10# so a leading zero is decimal, bounded before the
conversion, and capped (timeout 86400, delay 600). A bad value fails with
the variable's name.
TestPublishPackagesReadsTheWaitTimeoutAsDecimal makes the script wait, which
is where the old code broke: against the reviewed script it publishes the
platform payload and then exits 1 on "08: value too great for base".
TestPublishPackagesRejectsAnInvalidWaitSetting covers -5, abc, 1e3, an
over-range and an overflowing timeout, and a non-numeric delay; the old
script published with five of them and crashed on the sixth with
"abc: unbound variable".
docs/NPM_PACKAGING.md said nothing could publish @gitlawb/zero again.
Removing the trusted publisher stops this workflow; a maintainer could
still publish by hand, and the doc now says that.
A session's lease file lived in the Store that took it, so once that Store
was unreachable the garbage collector closed the file, and closing it drops
the lock. That made TestForkAndChildHoldTheParentTheyAreCreatedFrom fail on
main whenever a collection ran before its dry-run prune. It can also happen
in an exec run: PrepareExec takes the calling or forked session's lease on a
Store the run never touches again, so the parent's lease can disappear part
way through and a prune in another terminal could remove it.
Leases now belong to the process, as Hold documents. Every Store holding a
session shares one lease file in a process-wide table, and the file is
unlocked and closed only when the last Store holding it releases it.
* feat(npm): publish as @twigpine/zero
The npm package moves from @gitlawb/zero to @twigpine/zero, and everything
that finds Zero's GitHub repository by name now names Twigpine/zero.
Package name
- package.json, the lockfile and release-please use @twigpine/zero.
`repository` names Twigpine/zero: npm checks provenance against it, so a
publish from this repo with the old URL fails with E422. The homepage is
zero.twigpine.com.
- The name lives in package.json only. build-platform-packages.mjs names the
wrapper, platform versions, alias names and alias targets from it, and
bin/zero.js reads its own package.json for the alias it resolves and for
every reinstall hint. The Go constant npmPackageName is held equal to
package.json by a test.
Repository defaults
- The update check (DefaultRepository), install.sh, install.ps1, the npm
wrapper's fallback downloader, and the GitHub Action's zero-repo default
all used Gitlawb/zero. That name now belongs to an unrelated GitHub org, and
these paths download and run what they find there, so they point at
Twigpine/zero. Docs, README and issue-template links follow. The Go module
path (and so `go install github.com/Gitlawb/zero/...`) is unchanged here.
Publishing
- The two inline publish steps become scripts/npm/publish-packages.sh, which
reads the name from the assembled payloads, refuses to start when the
wrapper's aliases do not match the platform payloads, skips versions
already on the registry (so re-running a half-finished job is safe), and
publishes with --ignore-scripts. The dist-tag rules are unchanged.
internal/npmwrapper/publish_test.go runs it against a fake registry.
Docs explain the switch for existing installs: uninstall @gitlawb/zero first,
because both packages own the `zero` bin and npm refuses the second (EEXIST).
* feat(npm): publish 0.10.0 once more as @gitlawb/zero to point users at the new name
Deprecating @gitlawb/zero alone would strand every existing npm install.
0.9.0's `zero upgrade` runs `npm install -g @gitlawb/zero@latest`; with
nothing newer under that name npm installs nothing, prints no deprecation
warning (it only warns when it installs something), and Zero then reports
"updated via npm to 0.10.0" while staying on 0.9.0.
So this release goes out once more under the old name, as a bridge:
- The publish job assembles the same verified artifacts a second time with
the new `build-platform-packages.mjs --package-name @gitlawb/zero` and
publishes them with publish-packages.sh, after the @twigpine/zero publish.
A gate runs it only while @gitlawb/zero's latest is still 0.9.0 (or is
this version, on a re-run), so it happens once.
- The build knows its name from the package.json beside it. The Go side
reports InstallMethodNpmLegacy: `zero upgrade` refuses with the switch
steps before any release check (so it works offline too), and
`zero update --check` adds them whether or not an update is out.
- bin/zero.js prints the same steps after zero exits (the TUI would hide a
notice printed first), only when stderr is a terminal, and not for
`update`/`upgrade`, which print it themselves.
- Updating from 0.9.0 now installs a real version, so npm also shows the
deprecation message.
The steps say to uninstall @gitlawb/zero before installing @twigpine/zero:
both own the `zero` bin, npm refuses the second install with EEXIST, and a
forced install loses the bin when the old package is later removed (checked
against npm 11.16).
docs/NPM_PACKAGING.md has the maintainer runbook: bootstrap and trust the new
package, retarget the old one's trusted publisher for the bridge, then
deprecate @gitlawb/zero, revoke its publisher, and delete the bridge steps.
* fix(npm): only skip a publish when npm says the version is not there
publish-packages.sh treated any failed `npm view <name>@<version>` as "not
published". A transient network, auth or registry error on the re-run that the
skip exists for therefore published a version that was already out, and npm
rejected it with EPUBLISHCONFLICT; with the registry unreachable it published
blind. npm 11 reports every kind of missing (no such package, no such version,
including platform versions) as E404, so only E404 now means "not published".
Any other failure is retried three times and then stops the job before it
publishes anything, with npm's own error shown.
The test fake now answers a missing version with E404 the way npm does (it
returned an empty success before), and can fail lookups with a network error.
The two new tests fail on the old lookup: one with EPUBLISHCONFLICT after
transient failures, the other by publishing while the registry is unreachable.
Also renames the unused retry variable in the legacy-name gate step to `_`
(shellcheck SC2034), matching the script.
shell-quote 1.9.0 is inside the range GHSA-pqg4-j6r4-53mv covers
(>= 1.8.4, < 1.11.0). It reaches the lock through tuistory,
@opentui/react and react-devtools-core, which asks for ^1.6.1, so
refreshing the lock moves it to 1.12.0 without touching package.json.
Release packaging installs the local-control helpers from this lock,
so the old version shipped in the helper tree.
* fix(acp): persist token usage events for ACP sessions
ACP turns never set agent.Options.OnUsage, so provider token counts were
dropped on the floor. zero exec and the TUI persist the same counts as
session usage events, and `zero usage report` totals those events, which
left every editor-driven ACP session at zero requests and zero tokens.
runTurn now queues each report through usage.EventUsagePayload, the same
writer exec uses for a non-escalating run, and commits it in the turn's
existing buffered batch under the session id. A hard run failure still
returns before persistTurn, so that turn's usage is dropped with its
history. StopCancelled stays a successful outcome and keeps the usage
already reported. session/load and session/resume still skip event types
other than messages, compaction, and tools, so the restored prompt and
the wire replay do not gain a usage channel. ACP has no model escalation,
so the payload has no "model" key and no prompt, tool argument, or other
raw input.
This sits beside the permission-mode field persisted on the session
(#1134). It does not change the ACP wire protocol, exec, the TUI, or the
report reader.
Refs #1118
* test(acp): pin cancel, resume, and payload shape for usage events
The persistence test showed that a successful turn records token counts,
and the hard-failure test showed that an aborted turn records nothing.
Neither one locked the two edges that are easy to get backwards.
A client cancel is a StopCancelled outcome, not a hard failure, so the
tokens the provider already reported stay in the session log. session/load
and session/resume read that same log; the replay and the next prompt
must still be the conversation, with no usage payload on the wire and no
token counts folded into the resumed prompt. The persisted JSON is also
checked to be token-count keys only, so a prompt or tool argument cannot
ride along in the usage record.
Refs #1118
* test(cli): pin sandbox policy golden temp roots
TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields compared the
policy snapshot to a fixed golden after stripping only the temp roots
NewScope keeps. NewScope drops a candidate that sits inside an earlier one,
while the JSON (built with a nil scope) still lists it, so a TMPDIR inside
/tmp — or TMP inside TEMP on Windows — survived normalization and failed
the golden. Pin the variables that path reads (TMPDIR on Unix, TEMP and TMP
on Windows) and cover a nested temp dir that is not inside the earlier
candidate, so the normalizer removes it instead of leaving it in the JSON.
Refs #1121
* test(cli): run the golden against an ambient nested temp dir
The nested golden created its temp dir in the package working directory and
failed when that path sat inside /tmp, so a checkout under /tmp could not
run it and the test wrote into the source tree. Stage the issue's shape
first — TMPDIR inside /tmp, or TMP inside TEMP — then let the pin override
those variables. Directories come from t.TempDir. If the nested shape cannot
be built, skip.
Refs #1121
* test(cli): give the local adoption probe a test-sized timeout
TestLocalAdoptionSavesAndReloadsEligibleModel relied on the 400ms
defaultLocalProbeTimeout, which treats a slow probe as "runtime not running"
and silently drops the candidate; under -race on a busy runner one slow
loopback round trip can exceed it and empty the detection result. Pass a
5-second test deadline instead. The production default is unchanged and stays
covered by TestDetectLocalRuntimesAppliesDefaultTimeout.
Refs #1132
* test(cli): correct the timeout-coverage note on the adoption probe
TestDetectLocalRuntimesAppliesDefaultTimeout pins the probe timeout with an
explicit 20ms deadline, so it covers timeout enforcement rather than the
400ms production default. Reword the comment accordingly; no assertion or
production behavior changes.
Addresses the CodeRabbit finding on #1136.
ACP kept the permission mode in memory only: both mode doors called setMode,
registerSession always started at PermissionModeAuto, and nothing wrote the
mode to the session store, so session/load and session/resume returned a
plan/ask session as auto after a restart.
Save the mode next to the model on both doors, store first so a failed write
fails the request instead of leaving disk behind memory, and restore it through
registerSession. A session with no saved mode (written before this change)
keeps auto; a saved value outside auto/ask/plan never comes back weaker than
ask, and unsafe stays unreachable over ACP.
Refs #1125
* test(sessions): run the session lock test on Windows and add a cross-process one
TestSessionFileLockSerializesAcrossStores was skipped on Windows with
"flock semantics differ on windows", so the LockFileEx path in
filelock_windows.go was never exercised by a test on the platform that
uses it. LockFileEx locks are per handle, which is what the test relies
on, so the skip was unnecessary: remove it.
Also add a helper-process test (same pattern as lockutil's) proving the
session lock blocks a different process and is released by the OS when
the holder exits without unlocking.
Both fail on Windows when the lock is replaced by a no-op.
Fixes#1104
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
* test(sessions): keep the session lock alive in the helper process
The helper discarded the unlock closure returned by lockSession, which is the only reference to the open lock file/handle. GC finalization could close it while the helper waits on stdin, releasing the OS lock early and making the cross-process test pass for the wrong reason. Retain the closure and runtime.KeepAlive it after io.Copy, as the lockutil helper does.
---------
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
* test(tui): isolate user home, config and cache dirs for the TUI tests
newModel reads <user config>/zero/commands/*.md, and the package has 500+
newModel calls with no isolation, so a developer's own slash commands and
state leaked into the TUI tests (and tests could write to real user
directories), against the AGENTS.md "Hermetic tests" rule.
Add testutil.IsolateUserDirs, which points HOME, USERPROFILE, APPDATA,
LOCALAPPDATA and the XDG_* roots at a temp dir (covering Linux, macOS and
Windows), and call it from a TestMain in internal/tui. Per-test t.Setenv
overrides still win.
Refs #1103
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
* test(testutil): fail closed when user dir isolation cannot be applied
IsolateUserDirs now returns an error when any Setenv fails, rolling back
the variables it had already redirected, and its restore func reports
restoration failures. The TUI TestMain exits non-zero when isolation,
restoration or removal of the temp root fails, and the helper test
restores the inherited XDG_DATA_HOME via t.Setenv before clearing it.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
* test(testutil): cover partial-redirect rollback and compare physical paths
Make Setenv injectable so a test can let HOME be redirected and fail the
next variable, proving IsolateUserDirs rolls back a variable it really
changed (the NUL-root case fails on the first call and cannot).
Add testutil.PathWithin, which resolves the existing part of each path
with EvalSymlinks before the containment check, and use it in both the
helper test and the TUI hermetic test so symlinked temp dirs and Windows
short names cannot cause false results.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---------
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
The peer approval prompt's decide closure called runtimeMessageSink from
inside Update. The sink ends in program.Send, which blocks on bubbletea's
unbuffered message channel; the only reader is the event loop that is
running Update, so answering a held cross-session message hung the TUI.
Give pendingPermissionPrompt a decideCmd that returns a tea.Cmd yielding
the peerDecisionMsg, and have resolvePermissionWithReason return it
instead of calling the sink. Tests now drive the decision through the
returned command and a blocking sink, which hangs on the old code.
Fixes#1099
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
TestExecCommandForegroundServerReturnsSessionAndServesHTTP parsed the
helper's listening address from the first exec_command result only, with
a 500 ms yield. On a loaded Windows runner the helper had not printed its
address yet, so the test failed even though the session was running.
Poll the session with write_stdin until the address appears (20 s
deadline), and register the cleanup before the address check so the
server is stopped on early failure too.
Fixes#1097
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
SetNotify was kept in #1001 as a thin replace-form wrapper over
UpdateNotify for existing tests, but every product surface (the CLI
`zero config notify` and the TUI /notify picker) calls UpdateNotify's
merge form directly, so the advisory deadcode job reports it as
unreachable. Move the replace helper into writer_test.go as the
unexported setNotify, keep every test it served unchanged in behaviour,
and point the two CLI doc comments at UpdateNotify.
`make deadcode` no longer lists internal/config; the other advisory
findings in that job are pre-existing and untouched.
* fix(config): sync atomic config replacements
Persist temporary config contents before rename, then sync the parent directory on platforms that support it. Return persistence failures and cover ordering, cleanup, and pre/post-replacement failure states.
FixesGitlawb/zero#1087
Amp-Thread-ID: https://ampcode.com/threads/T-01a0dcd7-47fc-76eb-82d3-3f6c25384912
Co-authored-by: Pierre Bruno <[email protected]>
* fix(config): sync new config directories and relax unopenable dir sync
Sync each directory entry created by MkdirAll into its parent so a crash
cannot lose the config directory after a successful first write.
Treat a config directory that cannot be opened as best-effort durable,
matching the sessions store's syncDir; only a failed Sync or Close is
reported, since the rename has already replaced the file.
Check the temp file is closed with Seek instead of Stat: on Windows,
File.Stat on a closed file returns "The handle is invalid" rather than
os.ErrClosed.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(config): report directory-open failures except for write-only directories
syncConfigDir treated every os.Open failure as success, so a caller could
acknowledge a save whose directory barrier was never reached (Copilot
review, PR #1093). Keep the maintainer-requested best effort for a
directory the user can write but not read, as in the sessions store, and
return every other open failure.
TestSyncConfigDir now expects os.ErrNotExist for a missing directory on
non-Windows hosts; it fails on the previous code with
"missing directory: error = <nil>, want file does not exist".
* fix(config): create the config directory durably on the lock path and roll back failed barriers
Every exported mutator acquires the config lock before it writes, and the
lock file is a sibling of the config, so lockConfigFile's MkdirAll was what
created the config directory on a first write. writeConfigData then found
nothing missing and never synced the new entries into their parents, so a
locking mutator could report success without the directory creation being
durable (CodeRabbit review, PR #1093).
A failed parent sync also left the newly created directories in place: a
retry found nothing missing, skipped the barrier, and reported success.
Route both creation sites through one helper, ensureConfigDir. It creates
each missing component with os.Mkdir so it knows exactly what it created,
syncs each new entry into its parent deepest first, and on any barrier
failure removes only the directories this call created, deepest first,
with os.Remove so a directory another process has populated survives. A
failed rollback is joined into the error, never swallowed. A component
that appeared between the lookup and the Mkdir is not rolled back but its
parent is still synced.
Regression coverage:
- TestLockingMutatorSyncsCreatedDirectories drives UpdateNotify on a
missing nested directory, asserts the parent syncs happen before the
lock file exists, that a failed barrier rolls the directories back, and
that the retry re-runs the barrier. On the previous code it fails with
"error = <nil>, want injected sync failure" because the lock path
bypassed the barrier entirely.
- TestWriteConfigDataRollsBackOnlyCreatedDirectories asserts a failed
barrier removes the created directories but keeps a pre-existing
directory and its file, and that a directory populated by another
process is kept and the failed rollback reported. On the previous code
it fails with "must be removed after a failed parent sync: <nil>".
---------
Co-authored-by: Amp <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
Co-authored-by: Kevin Codex <[email protected]>
Register GMI Cloud (https://api.gmi-serving.com/v1) as an
OpenAI-compatible catalog provider and add it to the README Partners
section.
- The catalog id `gmicloud` matches the models.dev provider id, so the
remote model catalog resolves without a mapping. `gmi-cloud`,
`GMI Cloud` and `gmi` resolve to it.
- Reads GMI_API_KEY (the name in GMI's docs) and GMICLOUD_API_KEY (the
name models.dev uses).
- Defaults to Qwen/Qwen3.8-Max. The curated list is Qwen 3.8 Max and
Flash (GMI's partner models), then DeepSeek V4 Pro and Flash. All four
are served by GMI's /v1/models.
- README and README_ZH: key line, setup command, and the partner logo in
light and dark variants under docs/assets/.
Verified live with a GMI key: `zero providers check gmicloud
--connectivity` passes, and `zero exec` completes a read-then-write task
on both Qwen 3.8 models.
* feat(providers): opt-in ZERO_RESPONSE_HEADER_TIMEOUT override for slow first-byte deployments
The shared HTTP transport waits at most 120s for a response header. A local
model server that has to load a large model can take longer than that to
produce the first byte (measured 1-5 minutes on a throttled local Ollama), so
an otherwise-alive request is aborted.
Add ZERO_RESPONSE_HEADER_TIMEOUT, mirroring ZERO_STREAM_IDLE_TIMEOUT: a Go
duration or a bare number of seconds; "0", "off", "none" or "disabled" remove
the limit; an unparseable or non-positive value falls back to the default
instead of removing the limit. The default stays 120s, so nothing changes for
anyone who does not set the variable. The constant and the resolver sit next to
the idle-timeout equivalents in providerio.
Checked against a throttled local Ollama: ZERO_RESPONSE_HEADER_TIMEOUT=5s
fails at ~6.4s, =300s succeeds at ~223s (a request that would have hit the
120s ceiling).
Refs #1038
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_0142QEjA7eEFcdXTpcTmcAZk
* fix(providers): keep the 120s header timeout when bare seconds overflow
A value such as 36028797018963968 passes strconv.Atoi and wraps to zero
when multiplied by time.Second, which would disable the default. Values
that do not fit in time.Duration stay on the default.
* fix(providers): keep timeout defaults when bare seconds overflow
ResolveStreamIdleTimeout multiplied bare seconds by time.Second the same way the header timeout did, so a large count wrapped to zero and disabled the watchdog. Both resolvers now share a bounds-checked conversion.
* fix(providers): apply the header timeout when the shared client is taken
The stall-hardened transport baked ResponseHeaderTimeout in at package init, so a later ZERO_RESPONSE_HEADER_TIMEOUT never reached the real client and the 120s check depended on the env at init. Each resolved value now keeps its own transport, and a slow header proves that transport enforces it.
---------
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
Add three Trendshift badges under the tagline (#1 Go repository of the
day, #3 Go repository of the week, #3 on GitHub Trending) and a Partners
section listing Twigpine, Xiaomi MiMo, Novita AI, Concentrate, and Exa,
following the layout used in the OpenClaude README. README_ZH.md gets
the same badge row and a matching section.
The Novita, Concentrate, and Exa logos are vendored under docs/assets
with light and dark variants. The Twigpine and Xiaomi MiMo logos are
loaded from their own sites, as OpenClaude does; Twigpine switches to
its dark variant under a dark color scheme.
Reject starts when all slots are live or reserved, prune only completed history, and release reservations on launch failure. Keep live processes tracked even after failed termination. Add concurrent admission and real-process regressions for #1085.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0dcd5-eb93-74c9-a1ac-5b140edaf973
Co-authored-by: Amp <[email protected]>
* fix(security): harden shell flag parsing, write roots, and formatters
- sandbox: recognize POSIX short-option clusters in shellCommandFlag (ZERO-ESC-02)
Extract payload commands behind combined short flags (-ec, -lc, -xc) and long
--command in dashCPayload and shellDashCPayload, preventing destructive
subcommands from bypassing recursive AST inspection and auto-allowing execution.
- tools: anchor write_file and edit_file to *os.Root descriptor (ZERO-TRAV-01)
Use openScopedWriteRoot and commitRootedFileContents to traverse parents relative
to the root descriptor handle, eliminating residual TOCTOU directory swaps.
- tools: harden format_on_write against untrusted execution and secret leakage
Pass --no-config to prettier to disable dynamic JS config execution, reject
formatter binaries resolved inside the workspace (formatterBinaryInWriteRoots),
and scrub credential-bearing environment variables before running formatters.
* fix(security): stop shell flag parsing at dash-dash and verify root descriptor identity
Stop recursing into shell `-c` payloads for tokens that follow the `--`
separator, which marks positional operands rather than options, in both
shellDashCPayload (text detector) and dashCPayload (AST analyzer). This
prevents `bash -- -ec '...'` from being classified on a command the shell
never runs as a command string.
Verify the identity of the *os.Root descriptor returned by openScopedWriteRoot
against a pre-open os.Stat of the resolved root, closing the check-to-use
window opened by os.OpenRoot re-resolving the path. A root substituted for a
symlink or a different directory is refused and the descriptor closed.
Regression tests fail on the unfixed code:
- TestShellDashCPayloadStopsAtDashDash, TestDetectInteractiveCommandAllowsNonInteractive
- TestAnalyzeCommand (`bash -- -ec ...` must stay non-interactive/non-destructive)
- TestOpenScopedWriteRootRejectsSubstitutedRoot
Bureau: desk-pr-1066
* fix(tools): relax symlink resolution error handling on host formatters and clean up test directory
* fix(security): anchor write preimage on root descriptor and report skipped formatters
---------
Co-authored-by: hazyhaar <[email protected]>
* fix(tools): hint when Windows resolves POSIX paths under the workspace root
On Windows a leading-slash POSIX path is not absolute, so read tools joined
it under the workspace root and surfaced a raw GetFileAttributesEx error.
Append a Windows-only hint naming the host OS and the workspace root when such
a path is missing, so the model switches to a workspace-relative path instead
of retrying the same shape.
Guidance only: which path is opened, confinement errors, and write-path
resolution are unchanged. All read tools share the annotated resolver.
Refs #972
* test(tools): keep the Windows path hint wiring test hermetic
The cross-platform wiring test used a fixed "/home/..." request, but
resolveScopedReadPathForGOOS injects only the annotation GOOS: on Unix the
request is host-absolute, so root was ignored and the test read state it did
not own. Build the request from t.TempDir() instead - joined under root on
Windows, anchored in root on Unix - and resolve an existing file under that
tree to prove the resolution stayed in test-owned storage. The native Windows
end-to-end test is unchanged.
Addresses the P2 finding on #1063.
* fix(tools): reject mixed-separator UNC paths in the Windows hint
Windows treats both separators as valid, so a second leading separator in
either position (/\server\share, /\?\C:...) is the same UNC or device-style
prefix as //. looksLikePosixAbsolutePath excluded only the literal "//", so a
missing mixed-separator target was misreported as a POSIX path resolved under
the workspace root. Exclude either separator byte and cover both mixed forms
in the shape and annotation tables.
Addresses the P3 finding on #1063.
* feat(sessions): prune old sessions, never one another process has open
Store.Prune removes sessions last updated before a cutoff of at least a
day, and reports what it removed, what it kept and why, and what failed.
A dry run makes the same decisions and removes nothing.
A session holds no lock between writes, so nothing told a prune in one
terminal that another terminal still had the session open. Each session
now has a lease file: a process that writes a session, or loads it
through the rehydrated reads the TUI, exec --resume and ACP use to
continue one, holds a shared lock on it for its lifetime, and prune
takes it exclusively and leaves alone any session it cannot get. On
Windows the lease file is opened with FILE_SHARE_DELETE so a held lease
never blocks removing the directory.
Prune also keeps every ancestor of a session that stays, since Lineage
and Tree fail on a missing ancestor, removes descendants before their
ancestors, re-checks each session under its lease and write lock, and
removes the metadata first so a removal that fails part way leaves
nothing that looks like a session.
Part of #971.
* feat(cli): add zero sessions prune, with a retention setting read from user config only
`zero sessions prune --older-than 30d` removes sessions last updated
before the cutoff, `--dry-run` lists what it would remove and why it
keeps the rest, and `--json` prints the report. The cutoff is days or a
Go duration and never under a day. Without --older-than, prune uses
sessions.retentionDays; with neither it refuses and says how to set one,
so nothing is removed unless asked for.
sessions.retentionDays is read straight from the user config file.
Sessions belong to the user rather than to a repository, so a project's
.zero/config.json cannot choose what prune deletes, and no resolver
merge copies the setting from anywhere.
Closes#971.
* test(sessions): a dry run reports a session another process has open as kept
* test(sessions): pin the dry run for undated sessions and the lease a new session takes
* test(sessions): report every failure when a session is written while pruning
* docs(readme): list sessions prune in the Chinese README as well
* fix(sessions): a fork or child holds its parent, and is refused while prune holds it
A lease is taken best effort, so a fork whose parent prune held at that
moment went ahead: the rehydrated read in exec --fork could not hold the
parent, Fork read it anyway, and the fork was created under a parent
prune was removing. Fork and CreateChild now take the parent's lease
themselves before reading it, and refuse when prune holds it, so the
parent of a session being created is never removed under it.
* docs(sessions): say that nothing calls Release yet, and what it is for
* fix(sessions): let go of the lease before removing a pruned session's directory
lease.lock is deleted with the rest of the session while prune still
holds it. Where a delete only takes effect once the last handle closes
(Windows without POSIX delete semantics: older builds, or FAT and exFAT
volumes), prune's own handle kept the directory from being empty, so
the removal failed and left an empty directory behind. Prune now closes
the lease after session.lock is gone and before the directory goes.
* test(cli): an empty --older-than is a usage error, not the retention setting
* fix(sessions): refuse to continue a session prune holds, rather than read it without the lease
A resume that started while prune held the session could not take the
lease, read the session anyway, and went on to use one prune was free to
remove: a read changes nothing prune checks again. The rehydrated read
behind the TUI resume, exec --resume and --fork, and ACP session/load
and session/resume now fails with ErrPruning at that moment, and none of
the three falls back to a raw read on it. ACP load stays best effort
about a history it cannot read, but not about this one. Writes are
unchanged: a write lands under session.lock, where prune checks the
session again.
Prune takes the lease through the new Store.HoldExclusive, which the
TUI and ACP tests use to hold a session the way prune does.
* test(sessions): a lease that cannot be taken keeps the session, and does not stop a resume
Prune fails closed when it cannot take a session's lease at all, so a
process whose own hold fails the same way can carry on: prune cannot
remove the session either. Refusing there instead would stop resume
and fork wherever locking does not work, for people who never prune.
* fix(sessions): keep a parent forked after the plan, and refuse sessions removed under a caller
Three gaps jatmn found in the prune lease rules.
A fork made after prune's plan, by a process that has since exited,
was not in the plan, so its old parent was removed and the fork was
left under a missing ancestor. Once prune holds a parent exclusively it
now looks for sessions the plan did not see that name it as parent,
and keeps it if there is one.
Store.Create with a ParentSessionID, which exec --calling-session-id
and spec implementations use, did not hold the parent at all. It now
holds it the way Fork and CreateChild do, and all three refuse a
parent whose directory is still there without its metadata: prune
removes the metadata first and unlinks lease.lock after it, so a lease
taken at that moment lands on a fresh lease file and proves nothing.
A resume, exec --resume or --fork, or ACP load that picked a session a
moment before prune removed it could take a fresh lease and read the
session as empty. HoldToContinue now holds the picked session and
requires its metadata, and those callers go through it. The
rehydrated read on its own still reads a missing session as empty.
* fix(sessions): make a dry run decide at removal time like a real prune
A dry run appended every planned session to Removed without calling
pruneSession, so none of the checks made once a session is held for removal
ran: the second lease check, the re-read of the last update, and the scan for
a child created after the plan. On the same disk, a dry run could list a
session that a real run keeps.
pruneSession now takes a dry-run flag, makes every one of those checks under
the same locks, and stops before removing anything. The prune command reports
a session a dry run could not check as such, not as one it could not remove.
* test(cron): pass the history limit in the claim tests
#1094 added a limit argument to cron.Store.Runs. #1090 merged shortly
after from a branch that predated it, so its two new tests in
cron_claim_test.go still called Runs with only the job id and the
internal/cli test package stopped compiling on main:
internal/cli/cron_claim_test.go:111:38: not enough arguments in call to stores[0].Runs
have (string)
want (string, int)
That failed vet and every job that runs go test. Pass 0, which selects
the default MaxRunHistory cap, as the other callers in this package do.
* fix: bump Go to 1.26.9 for stdlib vulnerability fixes
govulncheck reports ten reachable standard-library vulnerabilities on
Go 1.26.6, all fixed in 1.26.9: GO-2026-6603, 6605, 6610, 6611, 6612,
6613 and 6617 in net/http, GO-2026-6608 in net/textproto, GO-2026-6607
in crypto/tls, and GO-2026-6604 in os (Windows only).
CI and release workflows read the toolchain from go.mod, so the go
directive is the only version to change. Update the stated source-build
minimum in the READMEs and install guide to match.
Login discovers the token endpoint (RFC 9728 / 8414) and may obtain a
client_id/client_secret through dynamic registration, but persisted none
of it. Refresh read only the config-file OAuthConfig, which is empty for
the standard `auth: oauth` setup, so every expired access token failed
with "no token endpoint configured for refresh" and required a new
`zero mcp oauth login`.
Store the discovered token endpoint (and whether it was server-advertised)
and the dynamically registered client on the token, and fall back to them
in Refresh. Explicit config still wins, and values already in config are
not copied into the store. A stored endpoint is re-validated before use:
the shared https/loopback rule always, and for a server-advertised
endpoint the same public-network policy and pinned client Login used. The
refreshed token keeps the stored fields so later refreshes keep working.
Fixes#1101
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
* fix(cron): preserve newer claims when overlapping runs complete
Let claimFire own schedule state and increment the current fire count under the store lock. Abort completion writes on read errors rather than restoring a stale snapshot.
Cover adjacent slots in both completion orders using independent stores, including failed executions, duplicate claim rejection, and unreadable completion state.
FixesGitlawb/zero#962
Amp-Thread-ID: https://ampcode.com/threads/T-01a0dc9c-8bc2-72cc-ad0b-6c64e3578df8
Co-authored-by: Pierre Bruno <[email protected]>
* fix(cron): avoid stale next-run summary on persist failure
Report the next run as unknown when completion state cannot be persisted, as requested in review. Extend the existing corrupt-state regression to verify the summary.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e862-0c9c-753c-9afc-45f9c1082c17
Co-authored-by: Pierre Bruno <[email protected]>
---------
Co-authored-by: Amp <[email protected]>
The pool stopped retrying only on exit 0 or ExitPermanent (76), so the
other codes `zero exec` uses to say a run is over (2 usage, 3 provider,
4 incomplete, 130 interrupted) were treated as crashes and retried up to
MaxAttempts with backoff. Exit 4 means the agent already ran, so each
retry repeated its side effects, re-billed the provider and appended a
second run to the same session stream.
Treat those four codes as final: return the worker's exit code with
ErrPermanent. Crash-type exits (1, signals) and launch/read errors still
retry.
Fixes#1100
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
* fix(redaction): redact adjacent AWS keys glued end to end
Two AWS keys with no separator between them (AKIA...AKIA...) only had the
first redacted: the leading \b in the secret patterns sees a word character
on the left of the second key and misses it.
Apply each textSecretPattern via a helper that also retries at every redacted
span end with a \b-stripped, start-anchored variant of the pattern, so a
credential starting exactly where the previous one ended is still caught.
Chained runs (three or more glued credentials) are followed to the end. A
mid-word occurrence that does not abut a redacted span still does not match.
Extracted from #1075 per Vasanthdev2004's suggestion on #969; #1067 remains
the fix for the RedactString leak.
* Address CodeRabbit critical: bound redactAdjacent span-chaining loop
The outer loop re-read len(spans) while the inner loop appended chained
spans to the same slice, so appended spans were re-chained and the span
count grew exponentially on long glued-key runs (RedactString handles
untrusted content). Bound the loop to the original match count and add a
regression test with 64 glued AWS keys asserting 64 markers promptly.
* fix(cron): bound run history and read recent records from the tail
Retain the newest 1,000 outcomes under the job lock and publish complete history snapshots. Add bounded reverse reads and regression coverage for legacy logs, failures, and concurrent stores.
FixesGitlawb/zero#1086
Amp-Thread-ID: https://ampcode.com/threads/T-01a0dcd6-ad63-74d8-8396-45573c57dcec
Co-authored-by: Pierre Bruno <[email protected]>
* fix(cron): skip oversized legacy history records
Discard entire oversized lines during bounded tail reads so compaction can record new outcomes without weakening atomic publication. Cover exact size boundaries and oversized first, middle, and unterminated final records.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e861-ebee-7764-be1d-e67517cd914c
Co-authored-by: Pierre Bruno <[email protected]>
* fix(cron): append the outcome when the history log cannot be replaced
On Windows the atomic replace of runs.jsonl fails while another process
holds the log open, which dropped every outcome for as long as it stayed
open. Keep the replace, and only when it fails append the record on a
fresh line. The leading newline terminates any unterminated tail; the
blank line it may leave is already skipped by readRuns. Zero's own
readers take the job lock, so they never observe a partial append.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
* fix(cron): preserve the history DACL and append only on sharing violations
Compacting runs.jsonl renamed a temporary file over it, which on Windows
replaced a DACL applied to the log with the job directory's inherited
one. Publish through ReplaceWithRetry so the log keeps its descriptor.
Fall back to appending only when the replace fails with a Windows
sharing or lock violation, the error a reader holding the log open
produces. Any other failure, such as a persistent access denial, is
returned so a log that can never be replaced cannot grow past the
retention limit. fsutil.IsSharingOrLockViolation exposes the existing
classifier, gated to Windows because errno 32 and 33 mean EPIPE and EDOM
elsewhere.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---------
Co-authored-by: Amp <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
* test(daemon): stop what a worker left running after it was reaped
A daemon worker is launched into its own process group, but Kill and the
Cancel hook pass its PID to background.TerminateProcess, which looks the
group up again with Getpgid when it signals. Once the worker has exited
and been waited, that lookup fails, only the dead PID is signalled, and a
child the worker left running survives. This pins the case with a shell
worker that backgrounds a sleep and exits: after the wait, Kill and the
Cancel hook must still stop the sleep.
Committed ahead of the fix so CI shows it failing on the current
behaviour on Linux and macOS.
* fix(daemon): stop worker process groups by their launch-time identity
Kill and the Cancel hook now go through background.TerminateCommandGroup,
which signals the group the worker was launched into (ConfigureChild-
ProcessGroup's Setpgid) instead of rediscovering it from the PID, so a
worker that has exited, or on Darwin one that is still a zombie, no longer
leaves its children running. It is TerminateCommand without the reap:
the pool waits on its workers itself and exec.Cmd calls Cancel from
inside Wait, so reaping there would be a second Wait.
On Windows it is the same tree kill, with the process handle pinned while
it runs. A command that has already been waited cannot be pinned; that
now reports os.ErrProcessDone, which exec.Cmd's Cancel contract treats
as nothing to cancel, instead of the unexported "already released" error.
Fixes#861
* fix(tui): show provider ownership in recent and favorite models
Display saved provider names in mixed-provider picker groups and include them in row sizing. Preserve provider-grouped rows and selection values.
FixesGitlawb/zero#840
Amp-Thread-ID: https://ampcode.com/threads/T-01a0d8e9-d346-71f9-bdac-4ac762d810a5
Co-authored-by: Pierre Bruno <[email protected]>
* fix(tui): label mixed-group rows with the provider Enter uses
Recent and Favorites rows labeled their owner from recent history, but
selection falls back to the active provider when that owner was renamed
or removed, so the label could name a provider that would not be used.
Derive the label from the same decision selection makes
(modelPickerSwitchOwner) and store it on the row as OwnerLabel.
Cap the owner prefix at 20 cells so an unbounded profile name cannot
consume the model name once the overlay reaches its 76-column maximum.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(tui): distinguish abbreviated model picker owners
Budget owner cues against the rendered row width, preserve model names, and distinguish abbreviated saved profiles with numbered suffix cues. Show the highlighted full owner as a wrapped key before selection. Cover shared-prefix and shared-suffix owners, narrow terminals, Favorites, and Enter routing.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e862-13e1-756e-9608-58d5fed96f05
Co-authored-by: Pierre Bruno <[email protected]>
* fix(tui): preserve model labels when owner cues cannot fit
Split owner cues onto a second line when the model needs the row width. Check the actually rendered row before suppressing the full owner key, so owner text hidden inside a clipped model label cannot hide that key.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e862-13e1-756e-9608-58d5fed96f05
Co-authored-by: Pierre Bruno <[email protected]>
* fix(tui): fit multiline model rows and mouse targets
Fit the model picker item window to its rendered viewport height and share that window with mouse hit-testing. Account for group headers and two-line rows so clicking either line selects the correct model and the keyboard-selected item remains visible.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e862-13e1-756e-9608-58d5fed96f05
Co-authored-by: Pierre Bruno <[email protected]>
---------
Co-authored-by: Amp <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
* fix(sandbox): say when a session's sandbox enforcement is degraded
Only `zero doctor` and `zero sandbox policy` reported a degraded sandbox, so
a TUI session or a `zero exec` run went ahead with the native sandbox
effectively off and nobody saw it. Both now say so once, before the first
prompt or tool call, naming the downgrade reason: exec on stderr next to
the grant migration notice, the TUI as a system notice when it opens.
The line comes from the session's own engine through Backend.BuildPlan,
the plan `zero sandbox policy` prints, so the notice and the command that
explains it cannot describe two different sandboxes. Only the degraded
level speaks: a disabled sandbox is the user's choice, and the unelevated
Windows tier still enforces the write jail.
Exec tests that expect a quiet stderr now compare against the notice this
host prints under the default policy, since they run on the host's own
backend and a Linux runner without the helper is degraded while macOS and
Windows are not.
Fixes#1041
* fix(sandbox): say so when the nesting markers skip wrapping
With ZERO_SANDBOXED and ZERO_SANDBOX_BACKEND set, BuildCommandPlan passes
every command through unwrapped on the word of an outer sandbox nothing
verifies (#727). The notice came from Backend.BuildPlan, which does not
model that path, so a native backend reported full enforcement and a
session started with the markers ran every command unwrapped in silence.
The engine's notice now mirrors the runner and names the markers and the
unverified outer sandbox. A disabled sandbox still says nothing.
hostSandboxNotice builds its expectation through the engine too, and the
tests that pin the notice clear the markers so a suite run inside a Zero
sandbox does not change their answer.
* fix(tui): say the startup notices again when a session is resumed
/resume rebuilds the transcript from an empty one, so the startup notices
went with the old transcript and the session it switched to ran under a
degraded sandbox without a word. The model keeps the notices and puts
them back first, before the resume summary. Rewind and compaction stay in
the same session and keep the notice in the scrollback above their
divider.
Also pins why the notice can be decided once, at startup: grants made
during a session only open the network or add allow and deny paths,
never the mode, and every mode but disabled already needs the platform
sandbox, so no grant reaches a degraded command plan the startup notice
missed, and a deny path the backend cannot enforce is refused instead.
* fix(tui): say the startup notices again when /new opens a session
/new cleared the transcript and added only its own note, so the new
session started under a degraded sandbox without a word, the same gap
/resume had. It now puts the notices back first, the way /resume does.
/clear stays in the same session and, like rewind and compaction, keeps
the notice in the scrollback above its divider.
* fix(modelregistry): start no background refresh when the fetch is turned off
The TUI and exec each started a goroutine that called
RefreshModelsDevCache, which reads ZERO_DISABLE_MODELS_FETCH and the
cache path when it runs. A test that turned the fetch off through
t.Setenv could finish and have the setting restored before that
goroutine ran, and the refresh would then fetch into the developer's
real cache. StartModelsDevRefresh reads the setting before starting
anything, so with the fetch off no refresh exists to outlive the test.
* test(cli): keep the degraded-notice tests out of the developer's user state
runExecWithSandbox and the TUI launch test left the sandbox grant store
and the models.dev cache at their defaults, so they read the
developer's sandbox-grants.json and could migrate it, and could refresh
the real models cache. Both now run under isolateCLIUserState, and both
join TestCLILaunchHelpersLeaveUserStateAlone so the isolation is pinned
for each of them.
* fix(cli): stop exec when the degraded-sandbox notice cannot be written
The notice goes out before any tool runs, and before MCP servers and
plugins start through the same sandbox, but a failed write to stderr
was ignored, so the run went ahead with reduced isolation and no
warning delivered. It now stops with exitCrash, the way the image and
reasoning-effort notices in the same function already do.
The lock pinned hono 4.12.27 and @hono/node-server 1.19.14 through
tuistory 0.10.0, and npm audit reports advisories against both. tuistory
already declares ^4.11.7 and ^1.19.9, so the patched releases fit the
existing ranges: hono 4.13.9 and @hono/node-server 1.19.17. package.json
is unchanged.
npm also re-syncs the lock's root entry with package.json, which main had
already changed without regenerating the lock: license MIT, android in
os, and no install script.
* fix(oauth): give every loopback callback server the bounded shutdown
The loopback OAuth callback server was written out three times: the shared
oauth.LoopbackListener, MCP's Login and OpenRouter's login. Each ended with
Shutdown under a one-second deadline, and Shutdown only closes idle
connections. A client that has sent part of a request header is not idle, so it
survived the deadline, and its goroutine and the Serve goroutine stayed alive
after the flow returned (#1028).
#1051 fixed that for LoopbackListener (a header read bound, force-close when
Shutdown runs out of time, and a join on Serve) and left the other two exactly
as they were, because each was its own copy.
The lifecycle now lives once, in oauth.CallbackServer, with #1051's behavior
unchanged: ReadHeaderTimeout of 5s, Shutdown within 1s, Close() on a missed
deadline, and Close returning only after Serve has. LoopbackListener uses it
through an unexported constructor that keeps its ConnState test hook, and MCP's
Login and OpenRouterLogin use StartCallbackServer, which both packages could
already import.
Each call site gets a regression test that stalls a raw client mid-header ahead
of the real callback and then requires the stalled connection to be closed once
the login returns. The ordering does the synchronising: the stalled client is
dialled before the real callback, Serve accepts in order, and the login cannot
finish until the real callback has been served, so no sleep is involved.
Fixes#1028
* test(oauth): pin the callback server's header bound and its join on Serve
Two of the three things CallbackServer does had no test that noticed their
removal, the same as when #1051 introduced them on LoopbackListener: the
five-second header read bound, and Close waiting for the Serve goroutine.
The header test stalls a client mid-header and never calls Close, so it can
only pass on ReadHeaderTimeout; the bound is a package var so the test can
shorten it. It synchronises on StateNew. Go reports StateActive only after a
request read returns, and a stalled header read returns only when the header
timeout fires, so a helper waiting for StateActive depended on the bound under
test and turned its removal into a SETUP INVALID instead of a failure.
The join test uses the case where the join matters: with no connections open,
Shutdown closes the listener and returns at once, so without the join Close can
return before Serve does. Without it the test failed 10 runs out of 10.
* fix(acp): publish sub-run sessions loaded over ACP as view-only
session/load registered a fully promptable session for any session kind,
including the child, side and spec sub-runs that every other resume path
refuses through sessions.IsResumableKind: the TUI, exec sessions, the exec CLI,
and ACP's own session/resume. So a sub-run could be opened and continued as a
standalone conversation through ACP and nowhere else, with whatever its parent
run relied on about that sub-run's lifecycle no longer true.
Load has a real render-only use, a client rebuilding a transcript it can scroll,
so it still loads those sessions and still replays their history. What changes
is that the session it publishes for a non-resumable kind is marked read-only,
and session/prompt refuses it with an invalid-params error that says why.
runTurn is reached only from session/prompt, so that one gate covers every way a
turn can run; set_mode and the config options only change settings.
The flag is decided at first registration and never changes. Resume refuses a
non-resumable kind before it registers anything, and session/new always creates
a standalone session, so the only path that can publish one of these ids is
load, which now publishes it read-only.
TestACPResumeAppliesStandaloneSessionKindPolicy claimed "session/load should
retain render-only access", but its prompt check ran before the load, so it only
proved the refused resume never registered the session. It now prompts after the
load as well. TestACPLoadPublishesSubRunsReadOnly checks every kind in both
directions, so a gate that refused everything would fail on the resumable kinds,
with a setup guard that the table agrees with IsResumableKind.
Fixes#1045
* test(acp): require resumable sessions to be promptable after load, not merely unrefused
TestACPLoadPublishesSubRunsReadOnly decided the outcome from the error text:
"refused as view-only" was false for any error other than the view-only one, so
a regular or fork session whose prompt failed for some other reason still
passed. With every writable prompt made to fail with an internal error, the old
test stayed green. Raised by CodeRabbit on the PR.
Resumable sessions now have to prompt successfully. For the sub-run kinds the
decision is the RPC code, and the message is checked only to tell the view-only
refusal apart from other invalid-params errors such as an unknown session id.
* fix(mcp): forward image tool results on the existing image channel
Recreated from Gitlawb/zero#1012 (approved but unmerged).
Original: euxaristia/zero:feat/823-forward-mcp-images
* test(mcp): pin the pre-decode encoded cap and MIME sniffing guards
Adds the two reviewer-requested tests for the untrusted-input boundary:
- TestOverCapEncodedPayloadIsRejectedBeforeDecoding: a payload one byte
over EncodedLen(MaxImageBytes) is rejected and the decoder is never
called, so a hostile server cannot force arbitrary decode work.
- TestDeclaredMimeTypeDisagreeingWithSniffedBytesIsDropped: an HTML
payload declared as image/png is sniffed from its bytes and dropped
rather than forwarded to the provider as an image.
Both mutations (removing the cap, trusting the declared mimeType) were
verified to turn their test red locally.
* fix(agent): retry mid-stream transport aborts before tool dispatch
Recreated from Gitlawb/zero#1014 (approved but unmerged).
Original: euxaristia/zero:fix/973-mid-stream-transport-retry
* Address CodeRabbit nit: add Unix ECONNABORTED needle to shouldReconnect
midStreamAbortNeedles claims to be a subset of shouldReconnect but listed
'software caused connection abort' (the Unix/macOS ECONNABORTED text),
which shouldReconnect did not match. Add the needle alongside the other
abort phrases, and cover it in TestShouldReconnectClassification.
* fix(search): redact every text field of session metadata, not ten of thirty
`zero search --json` prints Hit.Session straight to stdout and redactMetadata
covered ten of its string fields. Filling every string-kinded field of
sessions.Metadata with a credential and walking the result names twenty that
came back carrying it: SessionKind, Tag, RootSessionID, AgentName, TaskID,
SpawnedFromEventID, SpecID, SpecFilePath, SpecStatus, SpecDraftModelID,
SpecDraftReasoning, SpecUserComment, SpecRejectReason, SpecSourceSessionID,
SpecImplSessionID, and the five on the Goal (Objective, Status, StatusReason,
CreatedAt, UpdatedAt).
The ones carrying the most free text are the ones a hand-kept list is least
likely to reach: SpecUserComment and SpecRejectReason are review text the user
typed, SpecDraftReasoning is the model's own prose, and Goal.Objective and
Goal.StatusReason are however the goal was phrased.
Goal is redacted through a copy. Metadata is passed by value but Goal is a
pointer inside it, so redacting in place would edit the session the caller still
holds, and a search would quietly rewrite the live record it was only asked to
report on.
TestRedactMetadataCoversEveryTextField walks the struct rather than repeating
the list, so a string field added to Metadata or Goal later fails on the day it
lands. It carries a SETUP INVALID guard that the chosen secret is a shape
RedactString actually recognizes, since otherwise every leg would pass
vacuously, and a second guard that the walk visited any fields at all.
* test(search): make the metadata coverage guard recurse, so nesting cannot slip past it
The first version of the guard walked only the direct string fields of
sessions.Metadata and hand-allocated the Goal beside it. That left it with the
blind spot it exists to close: a nested struct or pointer field added later
would arrive empty, the walk would find nothing in it, and the test would pass
while the new field went out unredacted. Raised by CodeRabbit on the PR.
Filling is now recursive through structs, pointers and slices, allocating what
it finds nil, and the walk is its mirror. The test compares the two counts, so a
shape one traversal understands and the other does not fails as SETUP INVALID
rather than passing quietly.
Both helpers fail rather than skip on a kind they do not understand. A field the
guard cannot populate is a field it is not guarding, and saying so at the point
it is added is the whole value of having it.
`go test ./...` is red on a developer machine and green in CI, for two tests
that take an input from the machine rather than from the test.
TestResolveReportsExplicitMaxTurns left ResolveOptions.Env nil on four of its
five cases, and a nil Env means "read the real process environment" (envValue).
Any one of the ten variables the resolver reads is enough to break it. With
ANTHROPIC_BASE_URL exported, applyProviderEnv synthesizes a second, keyless
"anthropic" profile from a base URL on its own, the single-provider shortcut in
normalizeProviders no longer applies because there are now two, and Resolve
returns `no active provider configured: active provider "" not found` before the
test reaches its assertion. Measured one variable at a time against a clean
environment, all ten fail the same test: ANTHROPIC_BASE_URL, ANTHROPIC_API_KEY,
ANTHROPIC_MODEL, OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL, GEMINI_API_KEY,
GOOGLE_API_KEY, ZERO_PROVIDER and ZERO_MAX_TURNS.
The cases now state an empty environment instead of no environment, which is
what they meant, and the test exports three hostile values itself so it proves
that rather than relying on the machine it runs on being clean. Reverting either
changed case fails the test on a clean environment too, so this cannot regress
silently on CI.
TestAltScreenTranscriptScrollKeepsFooterFixed asserted the branch survives in
the title bar at width 90 while leaving Options.Cwd empty, so newModel took the
real working directory. The outcome is not a monotonic function of that path's
length, which is why this took a measurement rather than a guess. The title bar
picks the widest candidate that fits: workspace carries branch and path, cwdOnly
carries the path alone, compactLeft carries the branch alone. At width 90 with
this branch, a cwd of up to 52 bytes keeps both, 53 to 74 falls to path-only and
drops the branch, and 75 or more falls again to branch-only and restores it. The
test failed from a checkout whose package path is 74 bytes and passed from one
at 80; CI sits at 40. Cwd is now pinned, which also stops newModel reading the
real repository for a branch the test overwrites anyway.
No production code changes. The resolver's nil-Env contract and the header's
candidate ladder are both deliberate; only the tests were leaning on them.
* fix(redaction): redact credentials split by an invisible character
Every shape matcher in RedactString describes a contiguous run of body
characters, and none of their classes admits a control or format character.
One NUL or ESC dropped into the body of a key therefore ended the match and
both fragments were emitted untouched. Whoever read that output next saw the
original key back, because nothing about those characters was ever displayed:
a terminal eats the escape, a log viewer or JSON consumer strips the control
byte, and a copy-paste drops both.
The matchers now also run over a copy with those characters removed, and the
spans they find are mapped back so the replacement covers the original bytes,
separators included. Emitting the compacted copy would hand the caller a
different string than the one they passed in, and leaving a separator behind
would read as two secrets where there was one.
Written against the character classes rather than the two bytes in the report,
because every other Cc and Cf character behaves the same way: BEL, backspace,
DEL, the C1 controls, zero-width space and joiner, word joiner, BOM and soft
hyphen all split a key on main. Tab, newline and carriage return are
deliberately excluded and a test says so: they are real text structure, a
credential broken across a line is visible to whoever reads it rather than
hidden from them, and stripping them would let one match span a line break and
replace unrelated lines.
Text with no such character takes exactly the path it took before, decided by
a byte scan that allocates nothing, so the common case is unchanged at 51
allocations. Text that carries control characters and no credential pays for
one compacted copy and a second pattern pass.
Fixes#969
* test(redaction): assert the exact output around a malformed byte
The malformed-UTF8 legs only asked whether the key was gone, so a decoder that measured an invalid byte as the three bytes of U+FFFD still passed: the replacement covered the secret and moved its edges onto the bytes beside it. They now assert the whole string. One leg also records that an invalid byte is not one of the invisible separators, on the same ground as tab and newline: it decodes to U+FFFD and is drawn.
* fix(redaction): keep credential boundaries while matching across invisible characters
The split-credential pass compacted every Cc and Cf character out of the whole
string before matching, which removed delimiters as well as filler. Two things
followed. A whole credential written after a NUL lost the word boundary the
shape matchers open with, so "prefix<NUL>AKIA...<ESC>..." stopped redacting even
though the unsplit "prefix<NUL>AKIA..." has always redacted. And with the
delimiter between two credentials gone, the unbounded JWT shapes ran out of the
first key and through the header of the next, replacing both with one marker and
exposing the second token's payload and signature. The same reach consumed
ordinary prose written after a key.
Three changes, one rule: a separator is filler inside a credential and a
delimiter outside one, and the difference comes from the source rather than from
the compacted copy.
The pass now runs after the contiguous matchers instead of before them, over
what they left, and never across a replacement. A whole credential is claimed by
its own strict match, so neighbours stay separate and the delimiter between them
survives. The shape matchers used here drop their leading word boundary, and the
byte before the match's original start is what has to be a non-word byte, so a
key after a NUL redacts and a key glued to the end of a word still does not.
The offset table is gone with them. It carried a start and an end for every
source byte, which is sixteen bytes per byte of input on a 64-bit build, and
RedactString is handed whole command output and notification bodies with no
bound in front of it. Only the endpoints of actual matches are resolved now, in
one walk. On a 4 MiB input with a split key, the split path allocated 223.6 MB
before this change and 154.9 MB after, against 146.3 MB for the same text with
no separator in it.
Tests cover a delimiter in front of a split key against its unsplit verdict at
every kind of leading byte, each supported shape placed in front of a JWT with
each class of separator, text immediately behind a key, a caller-supplied
replacement made of word characters, and the allocation ceiling.
* fix(redaction): match split credentials with gap-tolerant shapes on the original text
CodeRabbit found a bypass in the compaction approach:
"xAKIA<NUL>AKIAIOSF<SOH>ODNN7EXAMPLE" leaked while its unsplit counterpart
redacts. With the separators removed first, the leftmost AWS match began at the
decoy AKIA, was then rejected for the "x" in front of it, and had already
consumed the start of the real key, so the real key was never considered. The
ordinary-prose version is any word ending in "sk" followed by a hyphen, "ask-"
or "task-", ahead of a split OpenAI key.
Resuming the scan one byte after each rejection would close that but is
quadratic for the greedy shapes on text like "ask-ask-ask-..." with one NUL in
it. So the boundary goes back inside the pattern instead. Each shape is rewritten
through regexp/syntax so a run of separators may sit between any two characters
it consumes, but never before the first, and matched against the ORIGINAL text
with its leading \b intact. RE2 then judges the word boundary against the real
neighbour in one linear pass, and there is no compaction and no offset mapping.
The separator class is generated from the same Unicode tables as
splitSecretSeparator.
Two further cases came out of rerunning the seeded differential against main:
- The shapes are matched independently and the union of their spans replaced.
Applied one after another, a split key's unbounded body ran on across a
separator into a following JWT, stopped at its first dot, and left the JWT
shape nothing to match, exposing its payload and signature.
- A second pass, ungated, catches a credential glued onto a split one, which
gains its word boundary only once the first is replaced. It is often whole
itself, so its region can hold no separator by then.
On 6,000 control-free inputs the output is byte-for-byte main's. On 6,000 with
invisible characters, 147 outputs still hold a known credential once those are
removed, against 2112 on main; 140 of the 147 leak on main too when the input is
unsplit, and the remaining 7 are the one limit now written into the header
comment: a split key whose first fragment is a complete credential on its own,
with another credential glued onto its tail.
* fix(redaction): keep two split credentials of one shape apart
Matches of one pattern never overlap, so when a split credential's body ran
on through the separator after it, a following credential of the same shape
could not start a match of its own. A JWT body stops at the next token's
first dot and a classic GitHub body at its underscore, so two split JWTs
side by side left the second one's payload and signature in the clear.
A match the OpenAI filter rejects spans what it ran into as well, so a
split key after kebab-case prose was rejected with the prose and printed.
When a match runs through a separator run into a position where the same
shape matches again and reaches at least as far, that credential is taken
on its own, and the first is cut at the separators when it is a complete
match by itself. Each piece is filtered on its own. Candidates are tried
latest first and only where the shape's literal prefix follows the
separators, at most eight per match, so the pass stays linear.
The header comment now says plainly that the rest of a key whose front
part is complete alone stays visible, and drops a stale sentence from
before the shapes were matched independently.
* fix(redaction): recognise the next credential when its prefix is split too
The run-on search looked for a shape's literal prefix contiguously after
the separators, so a token split inside its first few characters was never
seen as a new start and its payload leaked again. The prefix is now matched
with gaps allowed between its characters, the way the shapes are. The new
test splits the second token's header at every character, which also
pins that the bounded search is spent only where a credential can begin.
* fix(redaction): take one run-on step per match, not the whole chain
Following a run-on from the second credential into a third, and on to the
end of the chain, did that walk again from every match inside the chain,
which is quadratic: a megabyte of split JWTs back to back allocated about
seventeen gigabytes and ran for ten minutes. Each match now separates only
the credential it ran into. The scan's own next match starts inside that
credential and takes the following step, so a chain is still covered, and
in chains of three or more the markers after the first pair can join.
A test bounds the allocation on a chain of split JWTs; the linear pass uses
about forty times the input and the walk used far more.
* feat(notify): make permission-prompt alerts on by default and discoverable
The notify system existed but was silent unless the user hand-edited
config.json, with no UI surface to discover or change it.
- resolver: fall back to mode=both, focusMode=unfocused when the notify
block is missing or empty (Fixes#579)
- tui: add /notify slash command with popup picker, mirroring /theme;
explicit choices persist via config.SetNotify
- cli: add `zero config notify` to read/update/reset the preference
(--mode, --focus, --reset, --json)
- config: add SetNotify writer using the existing atomic-write helper,
validating against the same vocab the resolver accepts
The TUI effectiveTUINotifyMode default (empty -> both) now matches the
resolver. exec_test.go seeds notify.mode=off where a test asserted
silent stderr, which the old empty-default implicitly provided.
* fix(notify): apply review feedback from CodeRabbit
- cli: omitted --mode/--focus flags now preserve the current resolved
value instead of wiping it (--reset remains the only clearing path);
aligns the CLI with the TUI's mode-only preservation behavior
- tui: reject /notify inputs with more than two tokens instead of
silently accepting them
- tui: apply /notify choices to the live notifier via the new
notify.Notifier.Configure, so the change takes effect on the next
permission prompt in the same session (the previous message claimed
this but only the persisted value was updated)
- notify: add Notifier.Configure (mutex-guarded policy swap that
preserves sinks, focus state, and the writer)
- tests: mode-only/focus-only CLI preservation, live-notifier apply,
trailing-argument rejection, Configure immediate-effect + sink
retention
* fix(notify): read cfg under the lock in Notify
Configure (334c688) made cfg mutable at runtime, but Notify still read
n.cfg.Mode before acquiring n.mu — a data race with a concurrent
Configure. Move the mode check inside the critical section and copy cfg
to a local for all reads.
Regression test TestConfigureConcurrentWithNotify runs Configure
concurrently with Notify; verified it reports DATA RACE on the unfixed
code and passes after the fix (go test -race -count=5).
* fix(notify): keep defaults out of the resolver; preserve the user's own values
Addresses the maintainer review (Vasanthdev2004) on PR #1001. All three
findings share one root cause: the resolved config was treated as if it
were the user's choice.
- resolver: no longer defaults notify.mode/focusMode. The TUI's
effectiveTUINotifyMode already maps empty -> both on its own, so the
permission-prompt alert still works out of the box, while headless
`zero exec` stays byte-identical to base (no BEL/OSC-9 on stderr under
-o json), the exec empty-stderr fixture is restored, and the
ZERO_NOTIFY_WEBHOOK_URL sink is not armed by an implicit default.
- cli: `zero config notify` seeds omitted fields from the user's own
file (new config.UserNotify), never from the resolved view — a
project config's mode:off can no longer be copied into the user's
global config, and blank stays blank instead of pinning today's
default as an explicit choice. --reset remains the only clearing path.
- tui: /notify mode-only changes preserve the focus stored in the
user's own file (blank stays blank); the /notify picker enumerates the
full 4x3 mode x focus space so every valid pair is a row, the current
pair is always preselected, and Enter can never commit a setting the
user did not choose. State view reads the stored pair.
- tests: the three regressions from the review — exec writes nothing to
stderr on a clean run (fixture restored + resolver empty-default
test), a ProjectConfigPath test proving project notify cannot leak
into the user file, and Enter on an open picker from a pair outside
the old curated list (off, always) keeps the setting unchanged.
* fix(cli): zero config notify no longer requires a configured provider
The command resolved the full config (providers included) before
touching notification settings, so a fresh user with no provider hit
ErrNoActiveProvider and could not read, set, or reset their notify
preference — the exact first-run user this feature targets (CodeRabbit
review, PR #1001).
The command manages a user preference, so it now talks only to the
user's own config file (config.UserNotify / config.SetNotify) and never
runs config resolution. Display reports the user's stored values — a
project config that overrides notify for one repo is not shown here,
matching the write path's source-of-truth from the maintainer review.
* fix(notify): separate live session policy from persisted user preference
Addresses the review from jatmn (PR #1001). The three notification
states — stored user file, resolved startup pair, live session — are
distinct contracts and must not be conflated:
- handleNotifyCommand now keeps two focus values per update: the LIVE
focus (in-session, initialized from the resolved pair, so a project's
focus rule keeps applying) feeds the notifier's Configure and the
status line; the PERSISTED focus seeds from the user's own file
(config.UserNotify), so a mode-only /notify writes the mode while the
user's focus value — blank included — is preserved and project values
never leak into the global file.
- /notify list reports the live pair (model fields), not the stored
file, which can legitimately disagree under a project override.
- the /notify picker preselects the live pair; committing a row is an
explicit choice, so Enter on the preselected row keeps the setting.
- /notify list requires exactly one token, matching /theme, /effort,
and /style (rejects "/notify list junk").
- shell completions: config gained a notify child with its flags
(--mode/--focus/--reset/--json), plus a completion-tree assertion for
both contexts so new CLI surfaces cannot go stale.
- help/copy pass: the CLI help now describes the stored global
notification preference controlling BOTH the completion and
needs-input alerts, --reset as returning the TUI to its effective
default (headless unconfigured stays silent), lists all three focus
values in /notify usage, and drops the stale "resolver default"
wording from effectiveTUINotifyMode and the command doc.
- tests: contract-matrix coverage with the three states deliberately
disagreeing (blank user file + project-resolved off/focused) asserting
list/picker/preselection/Enter, mode-only and explicit-pair updates on
both live and persisted sides, and restart precedence; the SA4006
lint hits are resolved by asserting the returned live model.
lint-static: 0 issues. Affected suites green under -race; the only
remaining local failures are the known environment flakes
(TestRunAuthOpenRouterSavesMintedKey on macOS keychain under a
sanitized env, and TestLoadProviderCommand* 5s subprocess timeouts
under full-suite load — both pass in isolation and fail identically on
clean origin/main here).
* fix(notify): close the remaining review findings (isolation, races, fidelity)
Addresses the third review round (PR #1001) as one boundary pass:
- BTW isolation: /notify mutations and the picker are now behind the
existing btwCommandUnavailable guard (like /theme), because the side
conversation shares the parent's notifier pointer — an unblocked
/notify off inside BTW reconfigured the parent and wrote the global
file while only the side's display changed. /notify list stays
available. Regression: side-conversation attempt then return, checking
the parent's actual notification output and the stored preference.
- Serialization: new UpdateNotify runs read-merge-write as ONE
transaction under a cross-process advisory lock (flock/LockFileEx on a
.lock file beside the config, credstore's pattern), so two concurrent
partial updates (--mode off vs --focus always) cannot interleave and
silently undo each other's explicit change. The CLI and the TUI's
mode-only persistence both go through it; the TUI's stale pre-read of
the stored focus is gone (the merge reads under the lock). Verified
0/30 lost updates driving the real binary with two concurrent
processes per round; a goroutine-level regression test asserts both
explicit fields survive.
- Fidelity: notification writes now edit ONLY the notify member's bytes
(setNotifyJSONObject, on the existing byte-preserving JSON editor),
so unrelated values survive with their explicit presence intact —
including tools.deferThreshold: 0 and MCP disabled: false, which the
typed serializer's omitempty cannot round-trip, and keys FileConfig
does not model at all. A reset removes the notify member instead of
leaving a {} husk.
- Startup recovery: Resolve's ErrNoActiveProvider partial result now
carries the parsed non-provider fields (notify, sandbox, tools, ...),
and the setup-recovery branch that clears the resolved config keeps
the notify block — a stored explicit mode:off no longer resurrects
alerts through the provider-recovery path. Regression: stale
activeProvider + stored off/always through recovery asserts the
launched TUI receives the opt-out, not the both/unfocused default.
- docs nits: zero config --help now describes both alert classes; the
two remaining "resolver default" wordings updated.
lint-static: 0 issues. Affected packages green under -race (the only
local failure is the known TestRunAuthOpenRouterSavesMintedKey macOS
keychain flake, which fails identically on clean origin/main here).
* fix(config): notify reset removes every duplicate notify member
A hand-edited config may contain duplicate notify members (JSON decoders
tolerate them; the last occurrence wins). The reset path removed only
the final member, so an earlier block survived and became the effective
preference on the next decode — `zero config notify --reset` reported
success while the old value still applied (CodeRabbit review, PR #1001).
Loop the removal until no notify member remains, mirroring
setPetPreferenceJSON's duplicate handling. The replace path still edits
the LAST member, which is the one a decoder treats as effective, so an
earlier duplicate stays inert there.
Regression TestSetNotifyResetRemovesEveryDuplicateNotifyMember verifies
the reset-orphaned failure output on the pre-fix code and passes after;
it also pins the no-op reset and the last-member-replace behavior.
* fix(config): join UpdateNotify release error; restore main's test surface
CodeRabbit pass on the rebased head, three findings:
- UpdateNotify discarded the advisory-lock release error while every
other writer joins it into the result (errors.Join). A failed unlock
would leave later config updates blocked while the caller is told
success. Now named-result + deferred Join, matching SetPet et al.
- The rebase had resolved model_test.go by taking the pre-rebase file,
which silently dropped every test upstream added or changed since the
fork point (composer blink/cursor suite, beginRun run-details,
scrim-preserves-semantic-colors, session payload, Termux burst, ...).
Rebuilt the file from origin/main plus only this branch's own additions
(TestEffectiveTUINotifyMode), so main's newer assertions are back:
the stale scrim assertion now expects semantic ANSI preservation, and
the removed TestBeginRunResetsSidebarHidden stays removed exactly as
upstream had already replaced it with TestBeginRunKeepsRunDetailsClosed.
* fix(tui): restore pickerTheme's transcript append; pin the picker failure path
Rebase regression (Vasanthdev2004, PR #1001): resolving the choosePicker
conflict attached the shared transcript-append line to the new
pickerNotify case, so the pickerTheme case lost it — a theme choice
whose preference could not be saved fell out with the session theme
changed and nothing reported, while the /theme text path kept its line,
so no test went red. Restore main's line in the pickerTheme case.
New regression TestThemePickerAppendNoteOnFailedSave drives the exact
reported shape: UserConfigPath under a regular file (the write must
fail), a pickerTheme item, choosePicker, and the could-not-save note
asserted in the transcript. Verified it fails on the pre-fix code with
the reviewer's own repro output and passes after.
Also restores the TestEffectiveTUINotifyMode comment to the post-review
wording (the default lives in the TUI, not config.Resolve, so headless
runs stay silent); the model_test.go rebuild had pulled the older
pre-notify phrasing back in.
* test(tui): restore the default palette after the picker regression test
Vasanthdev2004 follow-up (PR #1001): handleThemeCommand applies its
theme to the package-level zeroTheme, so TestThemePickerAppendNoteOnFailedSave
left dracula installed and the next palette-sensitive test
(TestFocusedPermissionSelectedRowUsesSelectionTintNotBrandChip) read
dracula's selection tint instead of the default. One line, the package's
existing convention: defer applyTheme(themeDark, true), as every other
theme-committing test already does. Verified the tint test fails in
sequence before the defer and the full internal/tui suite passes under
-race after.
* fix(tui): label the effective-default notify mode as "(default)"
jatmn round (PR #1001): the TUI printed the effective default mode as a
bare "both" while the new CLI labeled the same configuration "mode:
(default)" — the two first-party surfaces contradicted each other, and a
user could not tell whether "both" was stored or effective. Focus
already labeled its built-in default ("unfocused (default)"); mode now
mirrors it.
The model keeps the RAW configured mode (notifyConfiguredMode) alongside
the effective one, so state output can distinguish "both because the user
chose both" from "both because nothing is configured" — clearing to the
raw value when an explicit choice is made. effectiveModeLabel renders the
(default) tag on the /notify list card and the /notify change line, the
same wording the CLI read path uses. No defaults are persisted; resolver
and headless behavior untouched.
Regression TestNotifyStateLabelsEffectiveDefaultMode pins the contract:
unconfigured shows "both (default)", explicit stored "both" renders bare,
an explicit /notify bell choice renders bare in both the change line and
later state. Verified it fails on the pre-fix code and passes after.
* fix(config): one lock authority; full-replace across duplicate notify keys
jatmn round (PR #1001), two findings plus one documented precedence:
- UpdateNotify took a SECOND lock primitive over the same config.json.lock.
On Windows its LockFileEx range did not overlap lockutil's, so notify
writes could race provider/theme/credential/MCP writes and silently
discard them; on every platform it also bypassed the established
no-follow path checks and the bounded wait. Delete filelock_unix.go /
filelock_windows.go entirely and use the existing lockConfigFileFn
authority every other mutator already shares. New regression
TestUpdateNotifyRacesExistingConfigMutatorWithoutLoss holds notify vs
SetTheme and requires both acknowledged edits to survive; the real
binary also races 30/30 clean cross-process on the shared lock.
- setNotifyJSONObject replaced only the last duplicate notify member, but
Go's decoder MERGES the fields of duplicate members into one struct, so
a replacement omitting a field inherited it from an earlier duplicate
({"notify":{"mode":"off"},"notify":{"focusMode":"focused"}} +
SetNotify({FocusMode:"always"}) left mode "off" effective). The
nonempty path now removes every duplicate and inserts the replacement
fresh — full replace semantics, nothing inherited. UpdateNotify's
partial merge is unaffected (it seeds omitted fields before the edit).
Regression TestSetNotifyReplacementInheritsNoFieldFromEarlierDuplicate,
verified failing on the pre-fix editor and passing after.
- zero config notify --reset combined with --mode/--focus now documents
its precedence: --reset wins, explicit values are ignored (the help
previously did not define the combination).
* fix(config): replace single notify member in place; reserve wipe for duplicates
Vasanthdev2004 review note (PR #1001, non-blocking): the duplicate-key fix
ran remove-and-reinsert for EVERY notify write, including the ordinary
single-member file — insertJSONMember appends after the last member, so
every /notify toggle moved the notify key to the end of the config object
and collapsed its formatting while everything around kept theirs.
Nothing was lost (member order carries no meaning), but a byte-preserving
write should not reformat what it need not touch.
setNotifyJSONObject now counts notify members first: exactly one is a
positional value replace (key stays where the user put it, surrounding
bytes untouched); two or more keeps the remove-every-duplicate-and-insert
path, where Go's field-merging decode would otherwise let an earlier
block leak fields into the replacement. Regression
TestSetNotifySingleMemberReplacedInPlace asserts exact bytes through an
in-place replace, verified failing on the over-eager version and passing
after; the duplicate regression and the whole config suite stay green.
An open, empty stream blocks Wait forever, which is the defect these tests
pin, so against the old Close they hung until the test binary timed out
instead of failing. Wait now runs behind a bound that reports the stream
open and closes it so the waiting goroutine returns.
A beforeTool hook that vetoed while carrying an enforcement notice put the
notice on two channels: Dispatch appended it to DispatchOutcome.Notices and
blockReason folded it into Reason as well. The one consumer made up for it
by dropping every notice whose text appeared in the reason, which also
dropped an earlier hook's disclosure whenever the blocking hook's output
quoted it. Reason now carries the hook's words only, and the veto result
takes every notice from the typed slice, the same owner hookMessage already
defers to.
Runtime.Close read the disclosure stream field directly. That raced a first
StartupDisclosureStream call, and a Close that came before any call closed
nothing, so a later caller got a fresh, open stream subscribed to launches
from a runtime that was already gone. Close now publishes through the same
Once, as an empty stream when nobody had asked for one, and closes it.
Tests: the launch-state and veto tests assert the typed slice and that the
reason carries no copy; the veto result is checked for the blocker's own
notice and for an earlier notice the reason quotes; a command that launched
and failed keeps its disclosure; the never-launched MCP check reads the
carried notices instead of the error text, which never included them.
The ACP agent spelled its own tool-result payload and wrote result.Output.
After the output/notice split that field is the undecorated text, and the
payload had no notices field, so the enforcement disclosure was dropped at
the moment of persisting. The live tool_call_update for the same result
showed it, and session/load then replayed the sandboxed command as if
nothing had constrained it. Fixing the live translation could not reach
this: the text was already gone on disk.
The payload builder moves from internal/tui to internal/agent, beside
ToolResult, because it now has three writers and only one of them is a
terminal. The TUI keeps its unexported helpers as thin delegates, exec calls
the agent function directly, and ACP persists through the same function. A
TUI resume of an ACP-written session therefore gets the typed notices and the
undecorated card body like any other.
The ACP reader restores the disclosure as what it was, typed notices beside
undecorated text, so toolCallResult composes it exactly once. The stored
output already has the notices composed in, so tools gains the inverse of
WithEnforcementNotices next to it; text that was never decorated passes
through unchanged and still gets its notice. displayPreview is deliberately
not read there: it is a card body, and the live ACP update sends the model
text.
The regression runs a real turn through the production OnToolResult
callback, loads the session in a fresh agent, and requires the replayed
update to equal the live one with the notice exactly once, including for a
command that printed nothing. Reported by jatmn.
The previous version used an unsupported sandbox level, which is refused in the
level switch, so moving the join down to just above that switch still counted
one call and the test passed. A DenyRead profile on a restricted-token tier is
refused before the switch, so a join below any check now reads as not having
happened.
Two problems with the ownership this branch added, both mine.
The handle was closed in a defer. This process is a MEMBER of the job it
creates, so closing the last handle to it is the kill: the defer fired during
runWindowsSandboxCommand's own return and terminated the helper before its exit
code could be handed back. Every sandboxed command reported 0 however it really
ended, refusals included. Measured three ways in one process: no job exits 42,
join and hold exits 42, join and close exits 0. The handle is now held for the
process lifetime, which is what the guarantee was built on in the first place:
process exit is what releases it.
The regression tested it from inside the process that owns it. It called
joinWindowsChildKillJob in the test binary and then closed the handle, so it
terminated the test binary. `go test` printed "ok" and exited 0 with the
assertions never reached, and the 51 tests that had not run yet were silently
dropped: 315 of them completed where skipping this one test gave 366. Everything
is driven out of process now, through the test binary re-executed as a stand-in
for the command helper, parked in the create-to-resume window with a real
suspended child on the real production join.
On top of that the coverage review asked for:
- cancellation and deadline expiry both driven through execution.Runner's
captured path over the sandbox Engine, the wiring hooks and plugins use, with
the call required to return bounded AND the child required to be gone
- the control without the job, which reproduces the orphan and the blocked
caller, so the two above cannot pass for an unrelated reason
- the exit code of the production runner, and that it takes ownership before it
refuses a command
- cleanup that owns the child independently of every assertion
Reported by @jatmn.
Keeps the typed enforcement notices and the Base* accessors this branch
introduced, and carries main's FileDiffs from #999 alongside them in both
ToolResult constructions; the ACP translation keeps composing the disclosure
through ModelOutput while building main's content slice.
* fix(providerio): close idle connections periodically to prevent HTTP/2 PROTOCOL_ERROR
OpenRouter and other HTTP/2 backends can silently reset or drop connections
that sit idle in Go's client connection pool across user turns. When a
subsequent turn reuses the stale connection, it fails on stream connect with
PROTOCOL_ERROR.
Launch a background idle connection closer on non-darwin platforms to evict
idle pooled connections on a 30-second interval, forcing fresh handshakes
after idle pauses while preserving connection reuse across fast agent turns.
Refs #1057
* test(providerio): poll for idle connection eviction with bounded deadline
Replace the fixed 60ms sleep in TestStartIdleConnCloserClosesIdleConnections
with a 2-second bounded polling loop to avoid test flakes under heavy CI
load or under the race detector.
Refs #1057
---------
Co-authored-by: euxaristia <[email protected]>
* feat(execution): add a stdin-only write for interactive processes
* feat(tui): attach overlay for interactive terminal sessions
* feat(tui): add /attach for interactive terminal sessions
* feat(tools): point the model at tty sessions for interactive prompts
* feat(tui): open the terminal automatically for tty exec sessions
* feat(tools): tell the model to keep polling while the user types
* feat(tui): give attached terminal sessions the full viewport
* feat(tools): route sudo through an escalated tty session
* fix(tui): route the auto-attach tick to its run
* fix(tui): defer auto-attach while another view owns the screen
* fix(tui): keep bracketed-paste framing for attached sessions
* fix(tools): let tty sessions run interactive commands
* fix(tui): capture the auto-attach baseline before the exec starts
* fix(mcp): reject a server-initiated request as a streamable-HTTP response
networkClient.request matched the decoded POST body to the in-flight call on
id alone, so a server that answered tools/call with a method-bearing frame
produced an empty, error-free result. Apply the same isRequestOrNotification
guard the SSE paths already use and return a protocol error naming the method.
Fixes#1050
Signed-off-by: Vladimir Babin <[email protected]>
* fix(mcp): describe the unexpected frame by method, not as a request
A method-bearing frame without an id is a notification, so the error
should not call it a request.
Signed-off-by: Vladimir Babin <[email protected]>
---------
Signed-off-by: Vladimir Babin <[email protected]>
Motivation: scripts/action-summary.mjs is invoked by the composite
action in action.yml on every action run, and ships with a 12-test
suite in scripts/action-summary.test.mjs. Neither the action-smoke
workflow nor the main Go-focused ci.yml executed that suite, and the
action-smoke workflow's paths filters did not even include those two
script files, so a change to the helper or its tests triggered no CI
job at all.
Approach: add scripts/action-summary.mjs and
scripts/action-summary.test.mjs to both the pull_request and push
paths filters in .github/workflows/zero-action-smoke.yml, and add a
Setup Node (actions/setup-node, node-version 24, matching the pin
already used in release-artifacts.yml) step followed by a
`node --test scripts/action-summary.test.mjs` step in the validate
job, ahead of the existing action.yml validation steps.
Validation:
- `node --test scripts/action-summary.test.mjs` passes locally: 12/12
tests pass (Node v22.23.1; repo engines field requires >=18, CI pins
24).
- Broke an assertion in a scratch copy of the test file outside the
repo and reran `node --test`: 1 test failed and the process exited
non-zero, confirming the new CI step actually gates on failures as
the issue requires.
- `python3 -c "import yaml; yaml.safe_load(...)"` confirms the edited
workflow YAML still parses.
- Confirmed .github/workflows/ci.yml has no Node step (Go-only
pipeline), so zero-action-smoke.yml is the correct integration
point.
- Base branch CI (upstream/main) is currently green.
Report: https://github.com/Gitlawb/zero/issues/1030
Assisted-by: claude-sonnet-5 (via Claude Code)
Signed-off-by: Pujitha Paladugu <[email protected]>
Co-authored-by: Pujitha Paladugu <[email protected]>
Motivation:
release-artifacts.yml's three actions/checkout steps (ordinary checkout,
workflow-dispatch requested-ref checkout, and the npm-publish job
checkout) and pr-auto-review.yml's single checkout step all leave
persist-credentials at its default, so the workflow token stays in the
job's local git config for every later step, including build/test/package
steps that never need it. ci.yml and zero-action-smoke.yml already
disable this. Neither modified file runs git push/commit/tag: the GitHub
Release publish step supplies its own explicit GH_TOKEN, and npm
publishing uses OIDC trusted publishing, so removing the persisted
credential changes no behavior.
Approach:
Add `persist-credentials: false` to each of the four checkout steps,
matching the exact convention already used in ci.yml/zero-action-smoke.yml.
Validation:
- go build ./... and go vet ./... pass.
- gofmt -l . reports nothing.
- go test ./internal/installtest/... passes.
- actionlint on both changed files reports only two pre-existing shellcheck
warnings in release-artifacts.yml (SC2016, SC2034) that are unrelated to
this diff and were confirmed present before the change as well (verified
by stashing and re-running actionlint against the original files).
- A live GitHub Actions run of the release/publish workflows was not
reproducible locally; this change only disables a default the workflows
never rely on, so no functional change is expected.
Report: https://github.com/Gitlawb/zero/issues/1029
Assisted-by: claude-sonnet-5 (via Claude Code)
Signed-off-by: Pujitha Paladugu <[email protected]>
Co-authored-by: Pujitha Paladugu <[email protected]>
* fix(oauth): force-close loopback callback connections past the shutdown deadline
Motivation:
LoopbackListener.Close shuts the callback server down with a 1-second
context and discards the result. http.Server.Shutdown only closes idle
connections, so a client that sends a partial request header and never
finishes it counts as active; Shutdown blocks out the full deadline,
times out, and Close returns while that connection and its Serve
goroutine remain open indefinitely. This is a loopback-only (127.0.0.1),
short-lived, single-use server, so the impact is a resource leak past
the end of the flow, not a remote vulnerability or crash.
Approach:
- add a ReadHeaderTimeout to the server so a stalled request header is
bounded on its own;
- follow a timed-out or otherwise failed Shutdown with server.Close()
to forcibly tear down any connection still open past the deadline;
- join the Serve goroutine (via a done channel) before Close returns,
so its completion is observable instead of left dangling.
internal/mcp/oauth.go and internal/provideroauth/openrouter.go build
their own loopback servers with the same pattern; they are left
untouched here to keep this change scoped to the file the issue's
reproduction demonstrates.
Validation:
- go build ./internal/oauth/... and go vet ./internal/oauth/...: clean.
- gofmt -l internal/oauth/: clean (make fmt-check clean repo-wide).
- go test ./internal/oauth/... -race -v -count=1: all pass, including
three new tests (TestLoopbackCloseForceClosesSlowHeaderConnection,
TestLoopbackCloseIsIdempotent,
TestLoopbackCloseReturnsPromptlyAfterSuccessfulCallback).
- Confirmed TestLoopbackCloseForceClosesSlowHeaderConnection fails
against the pre-fix loopback.go (with the new test kept) and passes
with the fix applied.
- Full repo go build ./..., go vet ./..., and go test ./... pass.
- Could not run make lint-static (golangci-lint) locally: it needs a
very large dependency download and the local environment ran out of
disk space fetching it. That check is continue-on-error (advisory)
in .github/workflows/ci.yml, not a required gate.
Report: https://github.com/Gitlawb/zero/issues/1028
Signed-off-by: Pujitha Paladugu <[email protected]>
Assisted-by: claude-sonnet-5 (via Claude Code)
* test(oauth): synchronize loopback regression tests instead of sleeping
TestLoopbackCloseForceClosesSlowHeaderConnection relied on a fixed
100ms sleep to let Serve accept a connection before sending a stalled
header; under load this could fail or silently skip the force-close
path it's meant to exercise. Use an http.Server.ConnState hook (via a
new unexported newLoopbackListener, set before Serve starts to avoid
a race) to wait for the real StateNew signal instead.
TestLoopbackCloseReturnsPromptlyAfterSuccessfulCallback measured
Close's elapsed time without joining its http.Get goroutine or
draining/closing the response body; join it first so the timing
starts from a connection that's actually back to idle.
Also register t.Cleanup(l.Close) in both tests so a Fatalf between
construction and the existing explicit Close doesn't leak the
listener's Serve goroutine.
Signed-off-by: Pujitha Paladugu <[email protected]>
---------
Signed-off-by: Pujitha Paladugu <[email protected]>
Co-authored-by: Pujitha Paladugu <[email protected]>
* feat(config): report whether the tool-turn budget was explicitly set
ResolvedConfig.MaxTurns is ambiguous today: a configured 80 and the
built-in default 80 are indistinguishable, so a caller that wants a
different default cannot tell "user chose this" from "nobody set it".
FileConfig now records at each merge gate (config files, provider
command, ZERO_MAX_TURNS, CLI overrides) when a source supplies a
positive maxTurns, and Resolve surfaces it as MaxTurnsSet.
* feat(exec): default headless runs to the turn-budget ceiling
The shared 80-turn default is sized for the TUI, where the user can
say "continue" and the run resumes. Headless exec has nobody to do
that, so benchmark evals truncated at 80 turns mid-implementation.
An unset budget now resolves to MaxTurnsCeiling for exec only; any
explicit source (flag, mode preset, ZERO_MAX_TURNS, config file) and
exec-profile budgets keep their precedence. The TUI default is
unchanged.
* test(cli,config): cover the headless turn-budget default
* refactor(cli): extract the headless turn-budget decision
* feat(tui): support an optional detail line on picker rows
Add a faint second line under picker items so richer surfaces can show
secondary metadata (project, model, size) without crowding the label.
The line renders on the selection band, joins the search haystack, and
only appears at medium width and above; detail rows halve the visible
window so the overlay keeps its height, and mouse hit-testing maps a
click on either line to the same item.
* feat(tui): show project, model, and size under /resume rows
Each session row now carries a detail line with the project directory
(~-contracted), the model that ran it, and its event count, plus a
status chip for forks and tagged sessions. The line also joins the
filter haystack, so searching by project path or model name works.
Narrow terminals keep the existing single-line list.
* feat(tui): show relative last-activity age in /resume rows
Replace the absolute timestamp column with relative age ("2h ago",
"12d ago") so recency reads at a glance; distant history still shows
the calendar date. Also adds coverage for the detail line: rendering
at wide vs narrow tiers, status chips, and filtering by model.
* refactor(tui): drop the event count from /resume detail rows
Event count is an internal metric — the detail line now carries just
project, model, and the status chip, which are the fields that
actually discriminate sessions.
* fix(tui): sanitize session metadata before it reaches picker rows
Persisted titles (--session-title, /rename) and other metadata can
legally contain newlines; a stray line would break the picker's fixed
row geometry and misalign mouse hit-testing. Reuse sanitizeCardField
for the label and detail line, and cover the detail-line click path.
* fix(tui): strip carriage returns from rendered session metadata
A \r in a persisted title or model id rewinds the cursor to column 0
and overwrites the row. Fold it into sanitizeCardField alongside the
existing newline handling, and cover it in the picker test.
* fix(tui): pad picker detail lines on the selection band
The detail line only painted its text, so the selection highlight
stopped at the end of the text instead of spanning the row like the
label line above it. Pad the remainder with the row surface.
* test(tui): cover carriage returns in sanitizeCardField
* fix(tui): drop all control runes from rendered session metadata
The picker detail line newly renders cwd, model id, and tag, and any
of them can legally carry ESC/CSI/OSC/BEL/BS/VT/NEL bytes — a project
directory name containing \x1b[2J would repaint the terminal when the
picker opens. sanitizeCardField now maps line separators and tabs to
spaces and drops every remaining control rune, matching the package's
other sanitizers; this also closes the same hole for titles, which
predates the detail line.
* fix(tui): include exactly-30-day sessions in the relative-age branch
* fix(sandbox): keep a derived ACL target inside the write root it came from
FILE_FLAG_OPEN_REPARSE_POINT refuses a reparse point at the final path
component and resolves every component above it, which is right for a path
the operator named and not enough for one this package derived. The
write-root carveouts are derived: <root>/.git/hooks and <root>/.git/config
are constructed from a root, and <root>/.git is a name an unprivileged
workspace writer can create before setup runs. mklink /J needs no
privilege, so a junction there had the apply open <junction-target>/hooks,
an ordinary directory with nothing wrong about its final component, and
zero sandbox setup wrote a deny ACE on it as Administrator, outside the
workspace.
A derived entry now carries the root it came from, and the apply requires
the object it finally holds to still live under it. The check is on the
handle rather than the name: GetFinalPathNameByHandle answers where the
open object actually is, and both sides go through it so the spelling
normalizes the same way. Materialization is checked before it creates,
since os.MkdirAll follows the same reparse points.
Deliberately not a refusal of every reparse point on the path. Above the
write root the path is the operator's, who may keep a workspace under a
junction or a mapped directory; only the tail the sandbox appended is held
strict.
Closes#1024
* fix(sandbox): reject the reparse ancestor itself before materializing
The pre-create check asked where the deepest existing ancestor lives, and
a junction answers with its own path: the open does not follow a
final-component reparse point, so <root>/.git came back as <root>/.git and
matched. os.MkdirAll does follow it. What disqualifies that ancestor is
that it IS a reparse point, not where it reports living.
Found by reverting the check and watching the test still pass, which said
the guard was doing nothing rather than that the test was weak. Pinned
against the function now, because through the whole apply the create is
made and then removed on the failure path, so the filesystem afterwards
looks identical either way. The plan wiring gets its own test for the same
reason: every apply-level case here hands the group an anchor directly, so
none of them would notice the builder never setting one.
* fix(sandbox): anchor only the carveouts that are under the root, and put the DACL back
Two from review, both right.
ReadOnlySubpaths is a profile field an operator can set to any path, and one
placed outside the write root is a configuration that works today; anchoring
it unconditionally turned that into a containment refusal. Only paths
lexically under the root are anchored now, which is where the derived
carveouts are anyway, and anything else keeps the final-component guard it
always had.
The successful-apply tests left their deny ACE in place, so t.TempDir could
fail to remove the tree. Restoring the snapshot exposed the sharper half:
the ACE denied the group the test runs as, which revoked its own WRITE_DAC
and left the rollback unable to reopen the target. The tests deny a group
this process is not a member of instead, which is what a capability SID is
in production, and roll back afterwards.
* test(sandbox): check the plan anchoring on every platform, out-of-root case included
The plan builder is cross-platform and its anchor tests were in a Windows-only
file, so nothing checked the wiring on Linux or macOS. Moved beside the other
BuildWindowsACLPlan tests, with the case review raised: an out-of-root
ReadOnlySubpath stays unanchored, and an in-root one is still held to its
write root.
* test(sandbox): give the cross-platform anchor tests native paths
Moving these beside the other BuildWindowsACLPlan tests made them run on
Linux and macOS, where their Windows path literals stop meaning what they
say: pathWithinRoot is filepath.Rel underneath, a backslash is an ordinary
character off Windows, and C:\workspace\.git\hooks is then one component
that is not under C:\workspace, so every anchor came back empty. The
neighbouring tests get away with such literals because they only compare
strings they built the same way and never ask whether one contains another.
Paths are built with filepath.Join from temp roots now, so the containment
question is asked in the separator the running platform actually uses.
Creating the child suspended is what makes the launch report trustworthy: the
fact is published before the child can execute, so an absent report means
nothing ran. It also creates a state the helper's ordinary unwind does not
cover. Between CreateProcessAsUser and ResumeThread the child exists, holds the
inherited stdin/stdout/stderr handles, and can run nothing.
Hook and plugin commands reach the helper through Engine.CommandContext, which
uses the default exec.CommandContext cancellation, so a timeout or a cancel
kills the helper outright. It never reaches terminateSuspendedWindowsChild, its
defers, or the resume. Windows does not terminate a child because its parent
died, so the suspended process survives holding the pipe write ends and a
parent still reading them waits on a process that will never write and never
exit.
The helper now joins a job object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE
before any child can exist, so every child created afterwards inherits
membership at creation and there is no window where one is alive and unowned.
Terminating the helper closes the last handle and the kernel does the killing,
which is the only mechanism that survives the helper being killed rather than
unwound. A failure to create the job is not fatal: it costs the guarantee, not
the sandbox.
Deliberately not fixed with exec.Cmd.WaitDelay in the parent. That releases the
caller while the orphan keeps running and keeps the pipes, trading a visible
hang for an invisible leak; it is a reasonable backstop, not the fix.
The regression drives the real kernel: join the job, create a suspended child,
close the last handle the way a terminated helper would, and require the child
to be gone. With a plain job instead, the child outlives it.
Reported by @jatmn.
* feat(acp): add standard session list and resume
Origin-Session: local-abff1c | Claude Code | 7 prompts
Origin-Snapshot: b7d0806d49f9
Origin-Session: local-13d543 | Claude Code | 5 prompts
Origin-Snapshot: 6b5eb8ba4e5b
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* fix(acp): preserve notification wire order
Origin-Session: local-abff1c | Claude Code | 7 prompts
Origin-Snapshot: b7d0806d49f9
Origin-Session: local-13d543 | Claude Code | 5 prompts
Origin-Snapshot: 6b5eb8ba4e5b
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* fix(acp): bind sessions to persisted workspaces
Origin-Session: local-abff1c | Claude Code | 7 prompts
Origin-Snapshot: b7d0806d49f9
Origin-Session: local-13d543 | Claude Code | 5 prompts
Origin-Snapshot: 6b5eb8ba4e5b
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* fix(acp): compare workspaces by filesystem identity, not by spelling
@Vasanthdev2004's three findings, all reproduced before changing anything.
TWO SPELLINGS OF ONE DIRECTORY WERE TWO WORKSPACES. Both comparisons were string
equality on ResolveWorkspaceRoot output, and that resolver is abs plus
filepath.Clean plus a stat — it does not fold case and does not resolve
junctions. A session persisted from the TUI was unresumable from an editor
holding a different spelling of the same project folder, and session/list
filtered by the other spelling returned nothing, which makes it an invisible
failure rather than a reported one. It lands on exactly the case this feature
exists for, and on the two processes most likely to disagree about spelling.
os.SameFile asks the filesystem which directories these are, which is the
question. filepath.EvalSymlinks is NOT the fix on Windows — it normalises a
drive letter and returns a junction path unchanged, so the alias survives it,
and junctions need no privilege. String equality stays as the fast path, and a
stat failure falls back to it rather than widening the match: this gate refuses
access to another workspace's files and configuration, so an unanswerable
comparison denies.
THE TEST COULD NOT SEE ANY OF IT. testDeps resolves with the identity function,
so the guard degenerated to "are these two strings different" fed two unrelated
temp directories — it could only ever answer yes. The rejection direction was
pinned and the acceptance direction was asserted nowhere. The new test uses a
resolver reproducing the production normalisation and drives the ACCEPTANCE
direction through an alias, skipping if the filesystem folds the alias away so
it never passes vacuously. Reverting to string equality fails it three ways:
load, resume, and a list that returns zero.
THE WIRE KEYS ARE AN EXTERNAL CONTRACT. Nothing pinned sessionId, cwd, title,
updatedAt, _meta, modelId, createdAt, sessions, nextCursor, cursor, loadSession,
promptCapabilities, sessionCapabilities, list or resume, so renaming a Go field
would break every client and leave the suite green. Renaming modelId to model_id
now fails.
Origin-Session: local-79d7a0 | Claude Code | 5 prompts
Origin-Snapshot: c175cabb9d50
Origin-Session: local-13d543 | Claude Code | 5 prompts
Origin-Snapshot: 6b5eb8ba4e5b
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* fix(acp): test the alias guard where the bug lives, and stop listing unusable sessions
THE TEST COULD NOT RUN ON THE PLATFORM THE BUG IS FROM. @Vasanthdev2004's point,
and he is right that it is not a nit. The alias test built its second name with
os.Symlink, which needs a privilege an ordinary Windows session does not hold, so
it SKIPPED there — and Windows is where junctions exist and where this defect came
from. The identity guard was verified by CI on the two platforms that never had
the problem. It now builds the alias with mklink /J on Windows, which needs no
privilege and is how he found the defect in the first place, and keeps the symlink
arm elsewhere.
This is the second time in this series a correct fix shipped with a test that
could not exercise it: the same helper shape was added to internal/memory for the
same reason a day earlier.
A SESSION WITH NO PERSISTED WORKSPACE IS NOT RESUMABLE, SO IT IS NOT LISTED.
CodeRabbit's finding. activatePersistedSession refuses an empty Cwd, but the
listing advertised it anyway — a menu entry that only fails when taken. The test
asserts both halves, because the listing is only correct relative to what resume
will accept: the omitted session is checked to really fail on resume, and a usable
session is checked to survive the filter.
Mutation-checked: removing the filter advertises the unusable session again.
Origin-Session: local-13d543 | Claude Code | 5 prompts
Origin-Snapshot: 6b5eb8ba4e5b
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* fix(acp): resolve every listed workspace, not only when a filter was supplied
@anandh8x's P1, both halves reproduced before changing anything.
The loop resolved item.Cwd only when a cwd filter was present, and skipped only a
blank one. Two shapes stayed on the menu that session/resume then refuses:
- a session whose persisted workspace has since been deleted, advertised as
resumable
- a legacy entry holding a relative path, reported as cwd "." although ACP
requires SessionInfo.cwd to be absolute
Every entry is now resolved unconditionally, anything that cannot resolve is
omitted, and the RESOLVED root is what SessionInfo carries — absolute as the
contract requires, and the same value the client hands back on resume. The
optional identity filter then applies to the resolved roots, which is also where
it belonged.
Listing is a menu: activatePersistedSession resolves and refuses what it cannot
reach, so anything this loop cannot resolve is something a client would be
offered and then denied.
Mutation-checked: restoring the resolve-only-when-filtered shape re-advertises
the deleted workspace and re-emits the relative cwd.
Origin-Session: local-76c8d7 | Claude Code | 6 prompts
Origin-Snapshot: 259b715cf0fd
Origin-Session: local-13d543 | Claude Code | 5 prompts
Origin-Snapshot: 6b5eb8ba4e5b
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* test(acp): assert the relative entry is retained, not merely absolute
CodeRabbit's catch, and the test was genuinely weaker than it looked. It checked
that the deleted workspace was gone, the live one kept, and every listed cwd
absolute — all of which a "fix" that simply DISCARDED any non-absolute entry
would satisfy, while losing a resumable session.
Presence is now asserted separately from spelling: the relative entry must still
be listed, and listed with an absolute path.
Mutation-checked: skipping non-absolute entries instead of resolving them now
fails with "a session with a resolvable relative workspace was dropped rather
than normalised". The first attempt at that mutation did not compile, so it
proved nothing until it was rewritten — worth saying, because a mutation that
fails to build looks exactly like a test that passes.
Origin-Session: local-13d543 | Claude Code | 5 prompts
Origin-Snapshot: 6b5eb8ba4e5b
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* fix(acp): a relative persisted workspace is omitted, not rebased
Reported by @anandh8x. Resolving a stored relative cwd does not recover the
session's workspace, it invents one: ResolveWorkspaceRoot joins it against
whatever directory the ACP server happens to be running in, and that invented
absolute path was then advertised as the session's workspace and accepted as
its home on resume.
Reproduced on f2c6fc9a, a session persisted with cwd ".":
LISTED legacy-rel as cwd="/Users/kratos/dev/f914/internal/acp"
resume with an UNRELATED workspace -> err=... cwd does not match its persisted workspace
resume with NO cwd (falls back to ".") -> err=<nil>
The mismatch check does its job when the client names a workspace, so the only
opening was the fallback path, where the rebased value was compared against
itself and always agreed. A conversation created for one project could be
resumed against another project's files, configuration and tools.
Both doors now take the same guard: handleSessionList omits an entry whose
persisted cwd is not absolute, and activatePersistedSession refuses one rather
than resolving it. The original base is not knowable from the metadata, so
guessing at it is not an option a fix can take.
This reverses an earlier assertion in TestSessionListResolvesEveryWorkspace,
which expected the relative entry to be normalised and retained. That was
requested in review on the grounds that dropping it loses a resumable session.
It does, but the entry was never resumable into its own workspace, only into
this process's. The test now asserts it is dropped, and a new
TestResumeRefusesARelativePersistedWorkspace covers the fallback path that the
listing filter alone leaves open.
Both guards mutation-checked: removing either one fails its test.
Pre-existing on this branch and on its merge-base, unrelated to this change:
TestRunDoctorFormatsRedactedProviderDiagnostics and
TestRunDoctorConnectivityProbesProvider both exit 3 in this environment.
Origin-Session: local-8cd239 | Claude Code | 11 prompts
Origin-Snapshot: 365efe3045f2
Origin-Session: local-13d543 | Claude Code | 5 prompts
Origin-Snapshot: 6b5eb8ba4e5b
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* test(acp): the resume guard is asserted through both activating methods
Raised by CodeRabbit: the test is named for resume and called session/load.
session/load and session/resume are separate entry points that today share
activatePersistedSession, so an assertion through either one passes while the
guard holds — but the name promised a surface it was not touching. Both are now
named explicitly, which keeps that true: if resume is ever given its own path,
this fails rather than quietly covering half of what it claims to.
With the guard removed, both methods accept a relative persisted workspace on
the no-cwd fallback. The named-workspace case was already refused by the
existing mismatch check; the fallback was the only door open, and it is open on
both.
Not taken in this PR, from the same review: threading a rooted directory handle
through ResolveWorkspaceRoot and workspace construction so a root rename or link
swap cannot redirect later file operations. That is a real question and a
pre-existing one — this change adds a refusal and no path handling — but it is a
capability refactor across workspace and tool access with its own race test, not
something to fold into a session-list fix. Worth its own issue.
Origin-Session: local-8cd239 | Claude Code | 11 prompts
Origin-Snapshot: 365efe3045f2
Origin-Session: local-13d543 | Claude Code | 5 prompts
Origin-Snapshot: 6b5eb8ba4e5b
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* fix(acp): activation requires an absolute cwd in the REQUEST, not just the record
Reported by @jatmn.
ResumeSessionParams is a type alias for LoadSessionParams, so JSON decoding turns
an OMITTED resume cwd into an empty string. That blank reached the shared
activation path, whose blank-cwd fallback substitutes meta.Cwd — so
{"sessionId":"known"} activated a persisted session, even though ACP v1 requires
session/resume to carry an absolute working directory.
This is a different hole from the persisted-cwd one fixed earlier on this branch.
That guard asks whether the STORED workspace is identifiable; this asks whether
the CALLER named one at all. The earlier fix does not cover it, because a stored
absolute cwd passes that check and the blank request then silently inherits it.
requestedWorkspace validates the request's own cwd before anything reaches the
fallback: absent, empty and whitespace-only are all invalid params, and so is a
relative path. Applied to both activating methods rather than to resume alone —
load's omitted-cwd fallback was inheriting the same way, and leaving one door
open is how this class survived the last fix.
Mutation-checked at the wire, which is where the defect lives: making the blank
case return no error compiles and fails the regression on four separate requests
— session/load and session/resume, each with cwd omitted and with cwd blank. A
Go-level test would not have caught it, since the defect is in decoding an absent
field.
Two notes from a verification pass, neither a defect:
Error precedence changed: a blank cwd with an UNKNOWN session id now reports the
cwd problem instead of "session not found". Kept deliberately and pinned — it
stops the server confirming whether a session exists to a request that named no
workspace.
AdditionalDirectories is declared on two params structs and consumed nowhere in
the repo. Not a hole today, but it is the same shape — client-supplied paths with
no absoluteness rule — so the field now carries a note saying it must go through
requestedWorkspace when wired up.
Rebased onto ad34dc8d. go test -race ./internal/acp/ -count=5: clean.
Pre-existing here and on main: TestRunDoctorFormatsRedactedProviderDiagnostics
and TestRunDoctorConnectivityProbesProvider exit 3 in this environment.
Origin-Session: local-c962d7 | Claude Code | 3 prompts
Origin-Snapshot: d2b8f44a9abc
* fix(acp): resume the conversation a session became, and fail closed when it cannot
Three defects in ACP session restoration, all reported by @jatmn.
loadHistory read the raw event log and kept only EventMessage. A compacted
session stores its original prefix alongside an EventCompaction naming the
events it replaced and carrying their summary, so restoring from the raw log
replayed superseded turns AND dropped the summary that replaced them. It now
reads the same rehydrated view the TUI and exec paths use, and explicitly
projects the compaction summary -- switching readers alone would still drop it,
because rehydration substitutes the compaction event in place of what it
replaced.
historyErr only suppressed replay and raised a warning: the session was
registered and reported ready regardless, so an unreadable events file left the
caller holding a live, promptable session ID whose next prompt ran as a fresh
conversation under the old identity. Resume now fails. Load keeps the
best-effort policy deliberately rather than by inheriting the shared helper.
Tool calls and their results were dropped from session/load, so a restored
transcript showed prose asserting edits with no record that any tool ran. They
now replay through the same toolCallStart/toolCallResult mapping a live turn
uses, keyed on the stored toolCallId so results pair with their calls. They do
not enter turnRecord, so load and resume still consume the same effective
history. Resume stays replay-free.
Also asserts that an unset sessionCapabilities is omitted rather than
serialized as null, raised by CodeRabbit.
* fix(acp): persist tool activity for session replay
* test(acp): bound replay notification waits
* fix(acp): preserve session restore invariants
* fix(acp): restore session lifecycle boundaries
* fix(acp): preserve replay and restore identity
* fix(acp): commit a completed turn as one store batch
A completed turn was buffered until its outcome was known and then persisted
one event per call. Each call took and released the store's session lock, so a
second ACP instance holding the same session -- its turnMu is its own, only the
store is shared -- could append its whole turn between two of them: user A,
user B, answer B, answer A. Every write was individually locked and the log
was valid, but a fresh load pairs prose by order and reconstructed three wrong
turns from two right ones. Before this branch the user and assistant events
went to AppendEvents together; buffering until the outcome is known was the
right change and splitting the commit into singleton writes was not.
The whole buffer now goes through one Store.AppendEvents call, which writes the
batch under one lock and one file write. That is the contiguity being restored:
not a disk transaction, and not a promise about a crash mid-fsync. The outcome
gate still runs first; a hard failure still commits nothing; cancellation still
commits through the same boundary. A persistence failure is now the failure of
the whole batch, so there is no dependent suffix to protect and the rewritten
regression asserts that nothing of the turn is durable, the warning is raised,
and a fresh load replays none of it.
The two-instance regression holds both writers at the commit boundary until
each has arrived and then delegates to the real batch API under the real lock;
it observes what production hands it rather than batching anything itself, so
reverting to singleton writes fails it before any interleaving has to be
provoked. The restored prompt keeps both prose pairs and no historical tool
output.
Also normalizes the expected workspace in the listing test through
filepath.Clean, matching the resolver's output contract, so a TMPDIR spelled
with a ".." component no longer fails a correct result.
Rebased onto main to pick up the ACP browser metadata, specialist model
restoration and worktree pointer protection changes. Both reported by @jatmn.
Since #1006 a Windows profile with denyRead is rejected before setup or token creation on both restricted-token tiers, so there is no run left for the notice to describe. Keeping it meant a plan could carry a note saying reads are denied and writes unconfined while the runner refused to start that plan at all: two statements about one fact, disagreeing.
Removed: windowsDenyReadWarnings and its host indirection, denyReadDiagnosticWarnings and the append into the execution request warnings, the plan-notes append in the command-plan funnel, and the two predicates that only served it (windowsRestrictedTokenWillRun, willBuildWindowsRestrictedToken). The three test files that pinned the producer go with it.
CommandPlan.Notes now has no producer in the tree. The channel from plan notes into Enforcement.Notices stays, since it is what hooks, plugins and MCP read, and the end-to-end pin that reached it through the real producer is replaced by a fixture-plan test so deleting that one line still fails somewhere. The comment in loop.go that justified skipping the rebudget on the strength of that one producer now says so.
Closes the last part of #869, which #1006 settled by refusal.
* fix(tools): say when format-on-write ran out of time
The 10s bound exists so a wedged formatter cannot hang a tool call, which
is right, and on expiry the unformatted write stood with nothing said. Every
other fallback here is a standing fact about the environment (toggle off, no
formatter for the extension, binary not installed) and is silent because
nothing is wrong. A deadline firing is not that: formatting was configured,
available and expected, and the file was written unformatted because the
machine was slow. Left silent, the caller believes it wrote canonical style
and finds out from a CI format check it cannot see, which is the failure
this feature exists to prevent.
The timeout is now reported on the write and edit summaries, naming the file
and the formatter. The caller cancelling and the formatter exiting non-zero
are deliberately not reported: the first is already surfaced as a cancelled
run, and the second usually means content the formatter could not parse,
which a write does not promise to fix.
The bound stays 10s rather than being raised. Raising it makes a wedged
formatter block longer, which is the thing the bound is for; what was
missing was knowing when it fired.
Closes#1022
* fix(tools): put the written bytes back when the formatter fails
These formatters edit in place, so one killed by the deadline, killed by
the caller, or exiting partway through its own rewrite can leave the target
truncated: neither the input nor the output. Returning the written bytes on
top of that left the tracker baseline and the diff preview describing a
file that is not on disk, so the next edit would compare against content the
file does not have. That is a worse failure than the missing formatting the
previous commit set out to disclose.
The failure paths write the bytes back, and when even that fails the user
hears about it, since it is their file. Written back unconditionally on that
path rather than only when the bytes differ: comparing first means reading
the file to find out, and a read that fails leaves the same ambiguity this
exists to remove.
The fixture is proven to damage the file on a throwaway copy, because a
successful restore erases the evidence from the real one. My first attempt
asserted that on the target itself, where it could only pass vacuously or
report the opposite of what it saw.
* fix(sessions): render resume context fields in key order
extractText walked a map[string]any directly and Go randomizes map
iteration, so the same session put the same fields in a different order in
every process: one run rendered a tool call as c1 read_file {"path":...},
the next as {"path":...} c1 read_file. That block is part of the user
prompt, so the prompt itself changed run to run, which leaves a provider
nothing stable to prefix-cache for it and makes two resumes of one session
impossible to diff while debugging.
Only the ordering is decided here. The summary short-circuit is untouched,
a list keeps the order it was written in, and which fields survive at all
stays the caller's projection.
Pinned as exact rendered strings rather than as two renders agreeing: a
same-process repeat passes whenever the random order happens to repeat,
which for a small map is often. The raw-JSON case proved it while being
written, matching on the first five attempts before diverging on the sixth.
Closes#1020
* test(sessions): name the expected context block instead of the first render
Comparing later renders against the first only asks whether the renderer is
stable, and a renderer that is stably wrong passes it: any fixed field order
satisfies it, including the unsorted one this change replaces.
It also hid something. The events were tool calls, which
promptContextEvents filters out of the resume context on this branch, so
the block being compared was a single message line and nothing said so. The
test now names the block it expects, on events that survive the filter, and
both failures are visible.
Creating a symlink on Windows needs SeCreateSymbolicLinkPrivilege, which an
ordinary developer account does not hold, so this reported a failure with
nothing wrong in the code: "A required privilege is not held by the client".
CI runners are elevated, so it only ever appeared locally, where it cost a
round of triage on #1003 before being ruled environmental and would do the
same to any contributor on a stock Windows machine.
The two other symlink tests in this package, and most of the rest of the
repo, already answer this way. Swept the remaining os.Symlink-then-Fatal
sites rather than assuming: internal/release passes, the one in
internal/agent is never reached, and the unix-only files cannot hit it. This
was the outlier.
Closes#1021
* feat(tui): offer mid-run model escalation behind --allow-escalation
Mid-run escalation existed but only exec offered it. The TUI already handled
every consequence of a switch, re-deriving the compaction threshold "after a
mid-run escalate_model switch" and resolving the summarizer against the active
profile, while nothing on that surface could cause one: escalate_model was
registered by exec alone. This closes that, opt-in, the same conservative way
exec answers the question.
Opt-in rather than default because escalation moves a run onto a different model,
which changes what it costs and which provider sees the conversation. That is the
operator's call, and the exec flag already answers it.
The switchers are built per turn from m.providerProfile rather than once at
launch. A TUI session can change models with /model, so a closure capturing the
startup profile would escalate from whatever the session began with instead of
what is in force now, and would carry that stale profile's base URL and
credential with it. exec has no such problem because its profile cannot change
mid-run, which is why the wiring lives in different places on the two surfaces.
The nil-contract logic is now shared rather than copied. providers.
EscalationSwitchers is the single implementation both surfaces call: the loop
swaps only on a non-nil provider, so a (nil, nil) return has to leave everything
untouched including the caller's usage attribution, and the session switcher is
installed only when the run STARTED optimized so escalation cannot change the
transport underneath a session. A second copy of that would only ever have been
exercised on one surface. exec's behaviour is unchanged: its six existing
escalation tests pass against the shared builder.
The tool and the switchers ride on one flag and a test asserts they cannot be
separated. Registering escalate_model without wiring a switcher ships a tool the
loop silently ignores, so the model reports an escalation that never happened;
that is the specific failure worth a guard rather than a comment.
Note this does not answer all of #554. Assigning models per phase, plan with one
and execute with another, is still not built. This makes the mechanism reachable
from the interactive surface, which is the part that was one flag away.
Refs #554
* fix(tui): attribute usage after an escalation to the model in force
exec reassigns currentModel when the escalation switcher fires; the TUI
captured usageModelID once per run and handed the switcher no callback,
so every usage event after a mid-run escalation was billed to the model
the run started on. The switcher now updates usageModelID, and the final
response carries the model per usage event so the batch fallback does
not swing the other way and bill the events before the switch to the
escalated model. Covered by a run through a scripted escalation that
checks both the live and the batch attribution.
* fix(cli): accept the root flags in any order
Each leading-flag splitter stops at the first token it does not own, so
running them once in a fixed sequence made the order load-bearing:
`zero --allow-escalation --theme auto` stranded the theme as an unknown
command and exited with an argument error instead of launching, and the
same happened after --skip-permissions-unsafe. The three splitters now
run until they make no progress, at the root and after the unsafe flag,
so every ordering reaches the TUI with every flag applied. A --theme
written before --skip-permissions-unsafe was dropped on that path; it is
kept now, with a later one winning as the last occurrence does.
* fix(cli): forward a root --allow-escalation into exec, reject it elsewhere
The root flag was stripped before dispatch, so `zero --allow-escalation
exec ...` and `zero --allow-escalation -p ...` ran without the opt-in the
help text promises, and `zero --allow-escalation version` accepted a flag
it could only discard. It now follows --add-dir: re-synthesised into the
exec argument list for the exec and -p shapes, so exec parses it as its
own flag, and rejected loudly for every other command. The ordering test
now generates every permutation of the three root flags, with
--skip-permissions-unsafe absent or at any position.
* fix(tui): record the model on an escalated run persisted usage events
The live usage record carried the model in force; the persisted session
payload did not. The usage report rebuilds cost from that payload and
falls back to the session-wide model when an event does not name one, so
an escalated interactive run was priced end to end at the model it started
on. Written only under escalation, matching what exec records under the
same flag, so an ordinary run persists the same compact payload as before.
* test(cli): isolate the new launch helpers from real user state
A temporary working directory isolates project files and nothing else.
runWithDeps fills the dependencies a test leaves out with production ones,
and the interactive launch path reads user config, opens stores, refreshes
the models.dev cache and migrates any inline plaintext API key into the
credential store before it reaches an injected runTUI callback. Running
these tests on a developer machine therefore rewrote that developer's
config.json and wrote a credential file beside it, and the results depended
on whatever providers, MCP servers and plugins the machine had.
Both new helpers now point every per-user base directory at a throwaway
root, pin the models.dev cache path, disable its background fetch, and
force the file credential backend so nothing reaches the host keyring. A
new test seeds a config with an inline key outside those fixture roots and
fails if either helper rewrites it or leaves a file beside it; without the
isolation it reports the rewritten config and the credentials.enc files.
* test(cli): check each launch helper isolation on its own
t.Setenv restores at the end of the test, not when the helper returns, so
one test exercising both helpers let the first isolation cover for the
second. Each helper now seeds its own emulated user config in a subtest,
and removing the isolation from either one fails that subtest.
* test(cli): pin the isolation itself, not only what this platform writes
The exec helper writes its models.dev cache from a background goroutine,
so a footprint check could not prove its isolation deterministically. Each
subtest now also asserts the helper moved the per-user paths off the
seeded root and disabled the fetch and the keyring backend, which fails
for either helper when the isolation call is removed.
* fix(sessions): carry an interrupted turn's work into the resume prompt
A turn that read six files and then died on a provider error left this behind
for the next turn:
- #1 message: add retries to the http client
- #6 error: provider error: upstream timeout
Nothing named the files, so the next turn re-read them from scratch (#913).
The work was never lost from the session: tool calls and results are recorded as
they happen, and the error path carries them out with everything else.
promptContextEvents then dropped both types on the way into the prompt, so the
record existed and nothing read it.
Filtering them was deliberate (#460) and is right for work an answer already
describes: forty read_file results add length and no information next to the
assistant message explaining them. It is wrong only for work with no answer
after it, and that is exactly what an interruption leaves. A turn that ends
normally was survivable for that reason, which is why this went unnoticed: the
answer was doing the remembering.
So the events after the last assistant message come along and the rest stays
filtered. The tail gets its own allowance rather than sharing the conversation
budget, so a tool-heavy interrupted turn still cannot push an earlier message
out of the context. That is the property #460 added the filter for and it is
asserted directly.
This changes a tested contract. TestFormatExecPromptKeepsConversationMessagesWhenNoisyEventsFollow
asserted that tool results are omitted, and its fixture is an interrupted turn:
43 results after an assistant answer with nothing speaking for them. That is the
#913 shape, so no rule satisfies both and the contract had to move. The test
keeps the guarantee it was written for, that conversation survives a noisy turn,
and a counterpart now pins the other half: work an answer already covers is
still filtered.
Not fixed here, and worth saying plainly: this carries what was DONE, not what
was READ. Zero has no conversation-history parameter at all, agent.Run seeds
from the system prompt and the user prompt every time, so file contents cannot
survive a turn boundary today. The next turn learns it already read a path and
can decide, rather than starting blind.
* fix(sessions): carry the tool outcome, not the tool output
The tail was carrying whole tool-result events, so up to 500 bytes of raw tool
output rode into a later turn prompt. Nothing redacts on the way in, and the
prompt renderer truncates rather than filters, so the only thing standing between
a file read and a later prompt was a length limit.
It bought almost nothing. The tail exists so a resumed turn knows what the
interrupted one did, and the CALL already says that: the tool name and its
arguments, which is the path for a read. The result body is the one part of the
pair that can carry file contents.
The status is kept rather than dropping the result entirely. A bare call reads as
work that succeeded, so a failed read would come back as a file the next turn
believes it already has.
Raised by CodeRabbit on the PR, and right.
* test(sessions): scope the outcome assertion to the tool_result lines
The status check searched the whole prompt for "error", which the provider error
message in the same fixture also contains. Blanking the status out left the test
passing, so it was asserting the presence of an unrelated line.
Scoped to the tool_result lines, it fails on that mutation naming the outcome it
lost.
* test(sessions): pin the whole of every tool line, not a search for one string
Two assertions here were weaker than what they claimed.
Searching the prompt for the fixture's whole secret only rejects that exact
string, so a change that carried a truncated or partly redacted prefix of the
output into the prompt would have passed. And asking whether "ok" and "error"
each appear somewhere among the result lines passes just as well with the two
statuses swapped, or both hung off the wrong call.
Both tool calls and both results are now pinned outright, which says what the
trim actually promises: nothing but the tool name and the outcome survives a
result, and the line order pairs each outcome with its own call. Compared as a
set of fields per line rather than as a string, because the renderer walks the
payload map and Go randomizes that order, so the same events come out in a
different field order on every run.
Reported by CodeRabbit on this PR.
* fix(sessions): keep a tool call's identity and drop its payload in resume context
toolResultOutcome strips a result down to name and status, and calls were left
verbatim, so an interrupted write_file put its content into the next turn's
prompt while the matching result body did not. Same secret, one door left open.
Admitting calls is the #913 fix and has to stay: a resumed turn knowing which
file was read is the whole point. What had to change is the symmetry.
toolCallIdentity is the call-side half of toolResultOutcome. It decodes the
arguments, keeps the fields that say what a call was about, and drops the ones
that carry what it was about to write, run or search for. Paths, directories,
urls, names, patterns and a read window survive; write_file's content, edit_file's
old and new strings, apply_patch's hunks and a shell command line do not.
An allow-list rather than a deny-list, so an argument this file has never seen is
dropped rather than replayed. A new tool with a new body field then shows up as a
missing path in a resume prompt, which is visible, instead of a new leak, which
is not. Keyed by argument name rather than tool name, so an MCP tool whose
argument is a url keeps it without being enumerated. Arguments that do not decode
as an object are removed outright: text that could not be read is text that
cannot be checked.
The tail budget gets a comment rather than a change. Tail slots exist only when
the conversation uses fewer than the 80-event cap, so a session already at the
cap carries no interrupted tool work, which is what every session did before
tool events were admitted and what the #460 cap is there to hold. Reserving a
minimum tail by trimming conversation further is a product decision, and it is
not this one.
Reported by jatmn.
* fix(sessions): scrub credentials out of retained tool-call values
An allow-listed key is not a safe value. web_fetch accepts a credential in
the query string and redacts the URL it reports back, so an interrupted
fetch had its token dropped from the result and kept verbatim in the call,
and this projection then carried it into the next resume or fork prompt.
Retained string values now go through the same redaction web_fetch uses on
its own returned URL: host, path and ordinary query survive, the token does
not, and non-string values are untouched.
* fix(sessions): project a tool call's top-level fields instead of patching one
toolResultOutcome builds its output from the fields it keeps, so a field it
has never heard of cannot reach a prompt through it. The call side edited
arguments in place and returned every sibling key, and returned the payload
untouched when there were no arguments at all, so a producer recording one
more top-level field would put it into the next turn. Both halves now name
what survives and drop the rest.
* fix(sessions): name the value shapes a projected argument may keep, and scope the ambiguous aliases
Two halves of the same gap: the projection decided which KEYS survive and
said nothing about the values under them or about which tool gave them
meaning.
Values. Strings were scrubbed and everything else passed through, which
reads as "a number is nothing to scrub" and is true of a number and not of
an object. MCP schemas allow object and array properties, so
{"query":{"api_key":"...","content":"private body"}} arrived under a
permitted key and carried a whole payload into the next prompt. Redacting a
serialized object would not have helped, since ordinary private text has no
credential shape to match. The retained shapes are named now: a scrubbed
string, and the scalars a read window is made of. A container is dropped and
the call keeps its name and id, so a resumed turn still knows which tool ran.
Meaning. grep accepts "search" for its pattern and edit_file accepts the
same word for the text being replaced, so admitting it globally would have
replayed file contents. It is scoped to grep, and the rule is kept as a
test: an argument name any mutating tool accepts for body content belongs in
the per-tool table or nowhere. The unambiguous supported forms are added to
the shared table: glob's match, and read_file's start_line, end_line,
max_lines, byte_offset and byte_limit.
Driven through the store on both resume and fork, including a call the tool
would have rejected, since the agent records OnToolCall before execution. The
persisted events are asserted unchanged: this is a view for the prompt, not
an edit to the record.
* test(sessions): assert the window key with its value, not the number alone
strings.Contains(out, "40") finds it inside "4096", so the start_line
assertion passed whether or not start_line survived the projection, which
is the opposite of what it claimed. The rendered arguments carry the pairs,
so the assertions name them.
* fix(installtxn): put back an install a killed commit left behind
CommitDir publishes by two renames: the live target moves into a workspace
backup, then the staged copy is renamed into place. Neither is journaled, so a
process killed between them left the target absent with the install's only copy
retained in a workspace nothing ever read. plugins.Load and skills.Load
enumerate directories, so the extension simply disappeared, while the lockfile
went on listing it. Recovering needed one fact the transaction never wrote
down: which install a backup belonged to. CommitDir now records that before it
moves anything, and Recover puts the backup back when the target is absent.
Recovery is a second data-loss surface, so it is bounded on every side. It
refuses a name that is not a single element inside the install root, never
replaces a live target, identifies a workspace by the name StageDir gives one
rather than by contents alone, and leaves intact anything it cannot attribute.
Every path that takes the install lock recovers, not just the two that install.
Recovering on the install path alone is worse than not recovering: a removal
takes the not-present branch, drops the lockfile entry and reports success
while the backup it never looked at stays on disk, and the next install
republishes it, reinstating an extension the user deleted. Removal and the
terminalpet install hold the same lock and now do the same thing first.
Recovery stays an explicit call rather than a side effect of Lock, matching how
the other staged-swap transactions here invoke their repair pass and keeping a
filesystem mutation visible at the sites that cause it.
* fix(installtxn): retire a backup the publish rename already superseded
Recover skipped any workspace whose target was occupied, which left the backup
of a commit that was killed after its publish rename but before its cleanup.
Removing that install then deleted the live target and its lockfile entry but
not the skipped backup, and the next recovery read the absent target as an
interrupted swap and published the stale tree again. plugins.Load and
skills.Load enumerate directories, so the removed extension was loadable again
with nothing in the lockfile naming it.
The target already records how far the commit got, so recovery reads it as the
phase rather than carrying a phase file. Absent means the swap never finished
and the backup is put back as before. Present means the publish rename
committed, so the backup beside it is superseded and its workspace is retired.
The live install is never replaced or removed either way, and the guards that
skip a workspace with no backup, an unreadable or missing marker, or a recorded
name that is not a single element inside the install root are unchanged. The
retire path removes the workspace directly rather than through
cleanupWorkspace, which refuses one holding a previous precisely because it
cannot tell a superseded backup from one still owed a restore.
Reading the target that way is only safe once nothing can leave a partial tree
there. rollback deleted the failed install in place before restoring, so a
process killed partway through that delete left a husk at the target while the
backup was still the only complete copy, and recovery would have taken the husk
for a committed publish and deleted the last good tree. The renames are now
ordered so the target is never partial: the failed install moves aside into the
workspace, the backup moves back to the target, and only then is the set aside
tree removed. A first install has no backup to protect and recorded no target,
so it still deletes in place.
The move aside can fail too, and then the failed install stays live at the
target with the backup still the only copy of what it replaced. Rollback drops
the workspace marker on that path, which leaves a workspace nothing can
attribute, and recovery already leaves those alone.
* fix(installtxn): reconcile both halves of an interrupted commit
Recovery inferred three facts from incidental filesystem shape: target
presence stood in for the transaction phase, a public dot prefix plus two
ordinary filenames stood in for ownership, and a void best effort call stood
in for both "nothing to recover" and "recovery failed".
CommitDir swaps the trees and only then publishes the lockfile, so a kill
between the two leaves exactly what a kill after both leaves. A phase bit
written after the publish only inverts which side of the write the ambiguity
falls on, so Recover now asks the caller which of the two trees its published
metadata records. plugins and skills answer from the recorded hash; the
terminalpet install publishes no metadata and passes nil, where a live target
is the committed one. A backup the metadata still records goes back over the
target through the same move aside ordering rollback uses.
Recover returns an error, and every caller that takes the install lock aborts
on it before reading the lockfile, inspecting the target, installing over it,
or reporting a successful removal. Malformed, legacy, and unattributable
workspaces are still skipped without error; a workspace we own and could not
resolve is reported, and every workspace is processed before returning.
Ownership is now proven rather than inferred. The workspace marker carries a
magic and version first line, so a user authored skill directory that happens
to be named with the workspace prefix and to hold a previous directory is left
alone. A recorded name carrying the workspace prefix is refused too, since it
names another transaction rather than an install. Retirement clears the backup
before the workspace around it: os.RemoveAll unlinks the marker first, so a
failure inside the backup used to cost the workspace the attribution that gets
it reported at all.
* fix(installtxn): do not read a target we could not probe as absent
recoverWorkspace read any os.Lstat failure on the target as nothing being
there, which enters the restore branch: the one branch that moves a tree,
taken on the strength of a question that never got an answer. Only a
not-exist error means the swap did not finish; every other probe failure
leaves the phase unknown, so it is reported and the workspace is left as it
was.
The same distinction the reconcilers already draw between a missing file and
a file that could not be read, applied to the probe that decides which branch
recovery takes at all.
* fix(installtxn): ask the reconciler about the name that resolved the path
recoverableTarget trims the recorded name before resolving the path, but the
raw line went to the reconciler, so the two halves of one decision ran on two
different values. A marker recording "demo " resolves dir/demo and then asks
the lockfile about "demo ". The lookup misses, a miss proves the publish never
ran, and the committed target is replaced with the tree it superseded. Found
by CodeRabbit on the round-2 push; the test observes that replacement before
the fix.
Also fixes the two CI failures on the new tests, neither of which was a
product defect:
macOS and Windows: the skills recovery matrix compared the lock source against
the path the test handed the installer, while Install records
canonicalSource(source). t.TempDir returns /var on macOS where the recorded
value is /private/var, and an 8.3 short name on Windows where it is the long
one. The expectation now comes from the same resolver the installer uses.
Windows: the two plugins failure-injection tests make a directory unwritable to
force a rename or removal to fail. Windows does not block either that way and
root ignores the bits, so the injected failure never fired and the tests
asserted against a success they never meant to produce. They now skip there, as
the installtxn and skills equivalents already did.
* fix(installtxn): stop recovery acting on trees the lockfile never described
An adversarial pass over the recovery protocol, driven end to end through the
built binary rather than package tests, found that the protocol still acted on
evidence it did not have.
The one that loses data: a missing lock entry was read as proof the publish
never ran, so recovery replaced whatever was at the target with the retained
backup. A missing entry proves no such thing. The entry can be lost after a
publish that did run, since a truncated or deleted lockfile reads as an empty
one, and a directory with no entry is a state these packages already support
(Load returns it, Remove handles it). Worse, the tree at the target may have
nothing to do with the transaction: `zero tools make foo` scaffolds into the
plugins root, and a hand-written skill directory is an ordinary thing to find
in the skills root. Both were destroyed and replaced by a stale backup, and the
install reported success. Reproduced through the CLI both ways before the fix.
A missing entry is now PhaseUnknown, which stops every caller and touches
nothing.
The rest are the same class as the target probe fixed in 005fff72, a probe
whose failure was read as a fact:
- A marker that could not be read was skipped as somebody else's content, so
the only copy of an install stayed stranded while the caller was told there
was nothing to do. Present but unreadable is now reported.
- The retained backup was probed with Stat, which follows symlinks and never
checked the type, so a symlink there published content from outside the
install root and a regular file published a file where a directory belongs.
Only a directory is a tree this package set aside.
- An entry at the target was read as an install at the target. A dangling
symlink satisfies Lstat, and treating it as live retired the only real copy.
- Two workspaces claiming one install were acted on in turn, which published
one over the other and deleted the rest. Nothing ranks two claims, so both
are reported.
- A stale `failed` tree from an earlier rollback made the restore rename fail
for good, and since every caller aborts on a recovery error that wedged every
install and removal in the root. The workspace is proven ours, so the stale
tree is cleared first.
- A recorded name carrying a NUL or longer than a path element can be reached
Lstat and came back with an error that is not not-exist, reported on every
pass forever with no way for a caller to clear it. Such a name is not an
install to recover and is skipped.
Unresolved-transaction errors now name the workspace directory, because the
only remedy is to delete it by hand and nothing in the CLI can reach it.
Left alone deliberately: an interrupted RemoveDir still writes no marker, which
the PR body already discloses as a pre-existing residual.
* fix(redaction): split camelCase keys in normalizeKey
A lower-to-upper boundary now inserts an underscore, so accessToken
and refreshToken match the registry instead of leaking into logs.
Token-count fields such as promptTokens stay unredacted.
Fixes#922
* fix(config): replace deprecated reflect.Ptr with reflect.Pointer
* fix(doctor): report auth presence under a non-sensitive key
camelCase normalization treats credentialConfigured as a credential
segment, so check() redacted the set/not-set indicator. authConfigured
does not match the registry.
* docs: name the five supported targets and drop Plan 9 as a review gate
Plan 9 has never compiled and is not a release or CI target. Reviews still
asked for plan9 shims. Document the five release platforms as the supported
set so unsupported GOOS is not a review criterion.
FixesGitlawb/zero#817
* docs: exclude unsupported GOOS/GOARCH from review criteria
CONTRIBUTING.md names five exact release combinations. Align AGENTS.md and
.coderabbit.yaml so other GOOS/GOARCH pairs are not a review criterion.
Unsupported targets such as plan9 remain allowed as fixtures.
* docs: retain Android support in target policy
---------
Co-authored-by: cairn-intern <[email protected]>
* fix(config): lock config read-modify-write across processes
Every config mutator loads the whole document, edits independent fields, and
publishes a complete replacement by rename. The rename is atomic, so a reader
never sees partial JSON — but two processes that loaded the same revision each
write a full document, and the second rename silently discards the first one's
acknowledged update. The result is valid JSON with one update missing and no
error anywhere.
lockConfigFile takes a cross-process advisory lock through lockutil, using the
retry-with-deadline idiom cron, hooks and oauth already share (10s timeout, 20ms
retry). Callers acquire BEFORE their first read, so the lock spans load,
mutation, validation and publication and the read inside it is authoritative —
holding it only around the write would still let both processes start from the
same stale revision.
The lock file is a sibling (config.json.lock), never the config itself: an
advisory lock is held against an inode, and publishing by rename installs a new
one, so locking the config directly would leave each process holding a
different inode.
Covered: all 14 mutators in writer.go, plus ClearProviderKeyStored and
MigratePlaintextProviderKeys in credentials.go. The migration matters most — it
rewrites the config on every startup, so it is the likeliest writer to collide
with an interactive mutation in another Zero. The SetProviderDescription test
seam locks too, so it cannot stand in as the one unsynchronized writer.
Two shapes needed care:
- The lock is not reentrant. EnsureCatalogProvider scans for an existing
profile and then upserts, so UpsertProvider is split into a locking wrapper
and upsertProviderLocked; one lock now spans the scan and the upsert, which
also closes the window where two processes could both create the same
catalog profile.
- SetPet edits raw bytes to preserve unknown members and formatting rather than
round-tripping the struct. It takes the same lock, so it neither clobbers nor
is clobbered by the struct writers.
Regression tests, each verified to FAIL with the lock disabled:
- TestConcurrentMutationsDoNotLoseUpdates — theme, pet, recaps, favorites and a
provider mutated at once; all five are independent fields, so a lost update
shows up as a zero value in exactly one of them.
- TestConcurrentProviderUpsertsAllSurvive — 16 distinct providers added
concurrently, all must be present.
- TestConcurrentSameFieldMutationsSerialize — 24 writers contending on one
field; every call succeeds and the document stays readable.
- TestCrossProcessMutationExcludesAndPreserves — the coordinated two-process
case. Goroutines share this process's descriptors, so only a second OS
process shows the lock is kernel-held. The child announces itself, then the
parent asserts its write cannot land while the lock is held elsewhere, does
its own mutation, releases, and requires both updates to survive.
Fixes#832
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01U389qmQUhoZB3YtXkFmiTq
* fix(cli): take the config lock for MCP config edits
The previous commit locked internal/config's mutators, but that is not every
writer of the user config document. internal/cli's MCP editor reads the SAME
file, edits it, and republishes it with the identical temp-file+rename shape,
at three sites (add/update, remove, disable/enable). Locking only one package
left `zero mcp add` free to clobber a concurrent provider or preference write —
and to be clobbered by one — with the file still valid JSON afterwards, which
is the same silent lost update issue #832 reports.
config.LockFile exports the existing helper so the lock is one authority across
packages rather than a private detail of internal/config. A second, adjacent
implementation would drift from the first exactly the way this writer already
drifted from the mutators.
TestRunMCPAddParticipatesInConfigLock is the regression. Racing the two writers
and waiting to observe a lost update is NOT reliable — the losing interleaving
is narrow, and a straightforward concurrent version of this test passed five
consecutive runs against the unlocked code, so it would have shipped as
reassurance that proved nothing. It instead asserts the deterministic property:
while the config lock is held elsewhere, the MCP writer's update cannot land,
and it completes once the lock is released. That fails immediately against the
unlocked code and passes with it.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01U389qmQUhoZB3YtXkFmiTq
* fix(config): report config unlock failures instead of discarding them
AGENTS.md requires that a mutator never report success when unlock failed, and
the release error was being dropped on the floor. A failed Release can leave the
lock held for the rest of the process, so returning (cfg, nil) after one claims
a state the next mutation cannot reproduce — it will block for the full lock
timeout and then fail.
lockConfigFile now returns lock.Release directly, and every caller joins it into
its own result with the idiom credstore already uses:
defer func() { err = errors.Join(err, unlock()) }()
Joined, not chosen between: a release failure annotates the result rather than
masking the mutation error that explains what actually went wrong. That covers
all 16 mutators in writer.go, both in credentials.go, and the test seam. The
three MCP sites return an exit code rather than an error, so they convert a
release failure into a crash exit and a stderr message, without overwriting a
failure the command had already reported.
A release failure cannot be provoked through the public API — lockutil.Release
is idempotent and reports nil once released — so the mutators call through a
lockConfigFileFn seam that a test can substitute. My first attempt at this test
could only ever t.Skip, which asserts nothing; the seam is what turns it into a
real assertion.
- TestMutationReportsUnlockFailure: SetTheme surfaces the release error AND the
mutation is still published, since a release failure annotates the result
rather than undoing the write.
- TestMutationErrorSurvivesUnlockFailure: an unknown-provider error and the
release failure are both present in the joined error.
Both verified to fail when the defer discards the release error.
Note for review: cron, hooks, oauth and swarm all still discard their
lockutil.Release error, so this makes internal/config stricter than its
siblings. Worth deciding whether the guideline should be applied to them too —
out of scope here.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01U389qmQUhoZB3YtXkFmiTq
* fix(cli): complete MCP config lock handling
Amp-Thread-ID: https://ampcode.com/threads/T-01a043da-1703-70c5-9d8d-904cd8fd964b
Co-authored-by: Pierre Bruno <[email protected]>
* test(cli): synchronize MCP config lock assertion
Amp-Thread-ID: https://ampcode.com/threads/T-01a0448f-5860-721c-8a47-5119fc57f685
Co-authored-by: Amp <[email protected]>
* fix(tui): persist provider model selection atomically
Amp-Thread-ID: https://ampcode.com/threads/T-01a04c92-2d1d-7508-91bc-416341b7e8b0
Co-authored-by: Amp <[email protected]>
* fix(tui): surface unsaved model selections on every switch path
choosePicker replaces handleModelCommand's status text with its own
applied-notice, so the "· not saved (…)" qualifier that text carried was
dropped and the selection reported an unqualified success. The user only
found out at the next launch, when the old persisted model came back.
dispatchCommand drops the same text on a committed switch, so typed
/model had the identical hole — the cross-provider path already returned
the outcome as a value precisely because display copy is not a contract.
handleModelCommand now returns that outcome too, and one
modelAppliedNoticeFor decides how a committed switch is announced for
both entry points, so neither can regain the gap independently.
Recent-model history is written under the same lock, so a selection that
already lost the lock would pay a SECOND full timeout for one keystroke
and add a redundant save error. It now falls back to the in-session
updateRecentModels for exactly that case, keyed on a new
config.ErrLockUnavailable rather than on any persistence failure: a
missing config file also fails the lookup, and that write still has its
own reason to be attempted.
Regressions cover both entry points. The picker test holds the config
lock — the case the review asked for — and bounds elapsed time, which is
what proves one keystroke costs one wait; it fails at 20s against the
previous code. The typed test uses an unwritable config path so it
proves the same plumbing without a second ten-second timeout.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01REzorhNj3F1DGPXyn5Uq7j
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
Co-authored-by: Amp <[email protected]>
* fix(providers): register Atomic Chat as local OpenAI-compatible runtime
Point atomic-chat at the default local API (http://127.0.0.1:1337/v1),
mark it Local with no API key, and auto-detect it alongside Ollama and
LM Studio. Keep opencode-go-anthropic-compatible in the provider catalog
and curated model list after rebasing onto latest main.
Co-authored-by: Cursor <[email protected]>
* docs(zh): sync Atomic Chat local runtime into README_ZH
The English README lists Atomic Chat in the provider list and tells
local-model users to run its desktop app on http://127.0.0.1:1337/v1,
but the Chinese README still mentioned only Ollama and LM Studio.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
* fix(providers): give the local Atomic Chat runtime its own catalog ID
Repurposing the existing atomic-chat ID from the hosted, key-gated preset
to a keyless local one silently downgraded credential checks for profiles
already saved against the remote endpoint: MissingCredentialEnv resolves
RequiresAuth from the catalog descriptor, so those profiles were reported
as ready and only failed later with an unauthenticated call to
api.atomic.chat.
Keep the hosted atomic-chat descriptor as it was and register the local
runtime under a separate atomic-chat-local ID, mirroring the existing
ollama-cloud/ollama split. Stored profiles keep their identity and no
migration is needed.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
* test(onboarding): assert the full Atomic Chat local endpoint and default model
Co-Authored-By: Claude Opus 4.8 <[email protected]>
* fix(models): drop the placeholder curated list for atomic-chat-local
Atomic Chat's model catalog is a Hugging Face scrape refreshed every 12h
plus arbitrary owner/repo lookups, so the served model ID is whichever
model the user loaded and no hardcoded list stays accurate.
The single local-model entry short-circuited Models() without adding
anything: the descriptor is openai-compatible, so modelMatchesProvider
never matches a registry entry and the fallback already yields the
descriptor's DefaultModel. Output is unchanged; the difference is that
local-model is no longer presented as a curated contract.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
* fix(onboarding): pin the probed model in the local-runtime adopt command
probeLocalRuntime already reads the runtime's /v1/models, but SetupAction
dropped the result and emitted 'zero providers add <id> --set-active'
with no --model. The add path then persisted the catalog DefaultModel,
which for a local runtime is the placeholder 'local-model', and the first
completion failed with an unknown-model response.
SetupAction now pins the probed id via SetupCommandWithModel. The catalog
default wins when the server actually serves it, so an existing Ollama
setup does not switch models; otherwise the first advertised id is used.
An empty probe result leaves the command on the catalog default rather
than inventing an id.
Covers a server whose returned id is not 'local-model'.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
* fix(onboarding): shell-safe adopt command + reject atomic-chat-local placeholder model
commandArg now POSIX single-quotes rendered args instead of strconv.Quote, so a model id from an untrusted /v1/models response cannot expand when the adopt command is pasted into a shell.
providerProfileForAdd now errors when atomic-chat-local is added without --model instead of persisting the local-model placeholder that fails the first completion.
Adds regression tests for command substitution, backticks, and the require-model guard.
* fix(onboarding): finish Atomic Chat setup across supported shells
Rebase on current main and keep the hosted Atomic Chat preset intact.
Only emit setup arguments that are portable across POSIX, cmd.exe and
PowerShell; show interactive guidance for other model IDs. Reject the
Atomic Chat placeholder before both CLI and TUI persistence, and stop
advertising adoption when discovery finds no usable model.
Validation: full Go test suite, focused race suite, vet, release build,
smoke, formatting, advisory lint and vulncheck pass with isolated config
and inherited Anthropic overrides removed. Onboarding, CLI and TUI test
packages cross-compile for Windows and Linux. Regression tests fail on
the rebased pre-fix code and pass with these changes.
Refs Gitlawb/zero#550 and Gitlawb/zero#592.
* fix(onboarding): preserve option-looking model IDs in adopt commands
Use the existing --model=value grammar when a discovered model begins
with a hyphen. Validate the model separately from the fixed separator so
shell safety remains unchanged and '=' is not added to the allowlist.
Exercise generated detection actions through shell tokenization, the actual
add parser, and profile construction for Atomic Chat, LM Studio and Ollama.
Cover leading hyphens, flag-looking IDs, ordinary IDs, spaces, and unsafe
inline values. Before the fix, the new regression failed with:
generated adoption command rejected by add parser: --model requires a value
Validation: full Go suite, focused race tests, formatting, vet, release
build, smoke, lint-static and vulncheck passed. Affected test packages
cross-compiled for Windows and Linux.
Refs Gitlawb/zero#550.
* fix(onboarding): select eligible chat models and preserve Ollama defaults
P2: apply the existing known-noncoding exclusion before default preference
or first-model fallback. When a nonempty list has no eligible model, offer
commandless guidance instead of persisting the rejected/default model.
P3: recognize an Ollama untagged default's :latest spelling, preserving the
advertised ID. Prefer exact matches and leave other tags/providers alone.
Add selector tests plus a mock-server integration test through discovery,
the generated action, the real add parser, profile persistence, a fresh
process, and the chat completion request. Before this fix, that test failed:
fresh process sent model "text-embedding-local", want eligible model "-loaded chat model"
The Ollama alias case also failed before the fix:
AdoptModel() = "qwen3:8b", want "llama3.1:latest"
Explicit model choices, shell checks, hosted/local provider identities and
Atomic placeholder guards retain their existing behavior.
Validation: full Go suite, focused race tests, formatting, vet, release
build, smoke, lint-static and vulncheck passed. Affected test packages
cross-compiled for Windows and Linux. Integration evidence uses a mock
server; native Windows shell and actual runtime behavior are not claimed.
Refs Gitlawb/zero#550 and Gitlawb/zero#592.
---------
Co-authored-by: Cursor <[email protected]>
Co-authored-by: Claude Opus 4.8 <[email protected]>
Co-authored-by: alex <[email protected]>
* fix(background): treat POSIX zombie leader as already-exited
terminateOwnedProcess always returned leaderAlreadyExited=false, so
TerminateCommand surfaced "did not exit after SIGKILL" when ps was
unavailable and the leader was already a waitable zombie. Probe the
leader the way Windows uses GetExitCodeProcess, then still signal the
group so live descendants are not skipped.
* fix(background): preserve live-group termination failures
---------
Co-authored-by: cairn-intern <[email protected]>
* ci: harden workflow permissions, add timeouts, concurrency and coverage report
* ci: reuse make fmt-check and refine coverage table formatting
* ci: scan Windows build graph via vulncheck-windows and collapse coverage summary to single-pass awk
* ci: invoke the pinned Windows govulncheck via a local GOBIN
go install writes the binary to GOBIN, which is not necessarily on PATH.
Install into .cache/gobin and run that executable. GOOS=windows applies
only to the scan so the scanner stays a host binary.
* ci: declare contents:read on the action smoke workflow
Pull-request workflows inherit the default GITHUB_TOKEN unless they
set an explicit permission block. The smoke workflow is now covered
by the same least-privilege policy as ci.yml.
* ci: parse workflow permissions structurally and quote the Windows scanner path
The pull-request token check now ignores commented lookalikes and accepts
an inline on: [pull_request] trigger. vulncheck-windows keeps the cache
path as one shell word.
* ci: drop -race from this PR so issue 939 stays with PR 940
Coverage profile remains. The dedicated race-detector job is owned by
the open race PR.
* ci: parse workflow permissions as YAML and scope GOWORK=off
* ci: label the non-race smoke test explicitly
---------
Co-authored-by: cl-ment <[email protected]>
* perf(providers): parallelize isolated provider unit tests and shrink retry backoffs
Enable t.Parallel() on independent provider test cases and shrink default
retry backoffs from seconds to 1ms during test runs, reducing provider test
suite duration from ~300s to ~2.4s without flakiness.
* test(providerio): add unit coverage for ShrinkBackoffForTest and restoration
* test(openai): keep Codex idle-timeout acceptance test serial
StreamIdleTimeout arms a wall-clock watchdog. Parallel scheduling can
lose the 50ms race against response.completed even though the test
only checks that the option is accepted.
* fix(config): replace deprecated reflect.Ptr with reflect.Pointer
---------
Co-authored-by: cl-ment <[email protected]>
* security(mcp): prevent server requests from matching client pending responses
* fix(mcp): send courtesy -32601 reply asynchronously off read loop
* fix(mcp): restore jsonRPCIDEchoable and validate request IDs before responding
* fix(mcp): queue and drop courtesy replies without blocking client callers
* fix(mcp): drop canceled queued requests
* fix(mcp): preserve valid numeric request IDs
* fix(mcp): treat JSON method member presence as the request discriminator
An explicit empty method string is a request, not a response.
Courtesy -32601 is still dropped when the write queue is full: failAll
on that path stopped the reader and blocked inbound pipe writes
(TestStdioClientUndrainedServerDoesNotBlockCallerWithDeadline).
* fix(mcp): count dropped courtesy replies when the write queue is full
The inbound reader must not stop. An empty select default is now an
atomic counter rather than a silent discard.
* fix(mcp): retain method presence, writer lifecycle, and original JSON-RPC IDs
* fix(mcp): bound courtesy overflow, pin writer queue, match exponent IDs
* fix(mcp): skip SSE frames that carry a method member
Treat method presence the same on stdio and SSE so empty or null method
never completes a pending client request.
---------
Co-authored-by: cl-ment <[email protected]>
Co-authored-by: KRATOS <[email protected]>
* fix(oauth): split an oversized keyring token blob across entries
Store every provider and MCP token in one keyring entry and the store stops
working once the logins outgrow it. On macOS the secret rides inside a
`security -i` command line capped at 4095 bytes, which leaves 3027 bytes of
JSON for all logins combined, so a second OIDC login fails to save and every
write after it fails too.
Split a blob that does not fit across numbered entries and put a manifest in
the anchor account. Chunks live in two alternating generations: a write fills
the one that is not live, then replaces the manifest, so that single write is
the commit point and a crash partway through still reads the previous
generation. `zc1:` cannot prefix base64, so an entry written by an existing
build is still recognised and read without a migration step.
Reserve the range a write will occupy before occupying it. Without that, a
write interrupted while filling a longer generation leaves chunks above the
count the manifest records, and no later cleanup knows to delete them: a
fragment of a token blob would stay in the keychain for good.
Expose the per-entry budget from internal/keyring rather than hardcoding the
macOS figure in the oauth store, sharing one line builder with Set so the
budget and the boundary it describes cannot drift. Backends with no limit
report so and keep the single-entry layout, so Linux is untouched.
Refs #937
* fix(oauth): lock keyring reads, validate manifest digest, and harden chunk tests
* test(oauth): assert Status blocks during keyring lock regression test
* fix(oauth): sweep both chunk generations when a store regrows
A shrink writes the blob back under the anchor, which replaces the manifest
and takes the per-generation chunk counts with it. From then on nothing can
name the chunks a failed cleanup left behind, so the growth branch's sweep of
the target generation is the only one that will ever reach them — and it only
ever targets family A. A keychain that refused one delete during the shrink
therefore kept a superseded generation of access, ID and refresh tokens
indefinitely, with no way for the user to know.
Sweep the other generation alongside the target, and document that the
reclaim waits for the next growth: a store that shrinks once and never grows
again keeps the residue, which sweeping on every whole write would close at a
cost of 128 `security` invocations per save on macOS.
Also record why Load and Status take the cross-process lock, and give the
missing-chunk error the same "log in again" advice the digest failure carries.
Refs #937
* fix(oauth): serialize keyring operations under user-level lock
Derive the keyring backend lock file path from the user's home directory rather than file store configuration, ensuring processes with distinct store paths share the same lock domain for the OS keychain.
Refs #938
* Address review feedback on chunked keyring token store and recovery
Refs #938
* Refine auth reset message and chunk format notation
Refs #938
* Harden OAuth store reset lifecycle, keyring migration recovery, and completions.
Refs #938
* Validate keyring cleanup markers, propagate lock resolution errors, and test reader concurrency.
Refs #938
* Preserve live OAuth credentials when stale cleanup markers survive migration.
Refs #937
* Verify JSON output from OAuth reset to cover its recovery response.
Refs #937
* Verify that JSON OAuth reset actually removes stored logins.
Refs #937
* Require confirmation before resetting shared OAuth credentials.
Refs #937
---------
Co-authored-by: euxaristia <[email protected]>
* fix(update): keep Windows failed-restore state in trusted recovery records
A failed restore currently records the last verified binary only as a
.deletable .keep marker in the install directory. Write an unresolved
identity-bound record to the existing per-user update-recovery store on
that failure path too, and consult it during preflight so deleting the
marker cannot silence the tamper refusal.
Fixes#868
* fix(update): keep a fail-closed signal if unresolved recovery records cannot be written
Ignoring a failed appendUnresolvedRecoveryRecord left only the sibling
.keep marker. An install-directory writer can delete that marker, after
which preflight finds no unresolved state and a retry can overwrite the
last verified recovery copy. Relocate that copy to a .recovery name when
the per-user record cannot be persisted so promotion still refuses.
Fixes#868
* fix(update): do not relocate the live binary when failed-restore compensation cannot record
persistFailedRestoreSignal relocated through the handle when the per-user
unresolved record could not be written, without checking that the object
was still the aside copy. A restore that had already put the verified
bytes back at targetPath then had that binary moved out of the executable
path. Verify identity with verifyPromotedTarget before relocating.
Unreadable recovery-state JSON now fails closed instead of looking like
an empty queue. The per-user vs per-machine split is documented: a second
account still relies on the install-directory .keep marker.
Fixes#868
* fix(update): durably commit recovery state on Windows
---------
Co-authored-by: cairn-intern <[email protected]>
* security(update): prevent tar extraction from traversing escaping symlinks (fixes#920)
When extracting tar archives, lexical path cleaning alone does not prevent
subsequent file entries from being written through a previously extracted
directory symlink that resolves outside destDir (Zip/Tar Slip).
This introduces verifyNoSymlinkEscape in safeExtractPath, which recursively
inspects existing path components between destDir and target using os.Lstat
and filepath.EvalSymlinks to reject entries attempting to write through escaping
symlinks.
* test(update): exercise chained symlink escape during file extract
The previous case was a single escaping symlink already rejected by the
existing Linkname check. Seed a mid -> outside chain under destDir and
extract a regular file through it so verifyNoSymlinkEscape is what fails,
with no write outside destDir.
* fix(update): resolve destDir symlinks and permit valid dangling symlinks in archive
* docs(update): document POSIX-only scope for verifyNoSymlinkEscape
* internal/update: test extracting files through directory symlinks
* fix(update): reject dangling symlinks instead of lexical fallback
EvalSymlinks failure used filepath.Clean on the link text, which does
not preserve already-traversed links. A dangling member is now an
error.
* fix(update): resolve dangling symlink chains from the physical parent
* fix(update): authorize tar paths through OpenRoot and reject outbound symlink entries
* fix(update): resolve intermediate symlink parents and cap Root link depth
Walk each path component from the physical parent, reject a ninth link,
and look up release binaries through OpenRoot as regular files only.
---------
Co-authored-by: cl-ment <[email protected]>
* ci: run the race detector on pull requests
The Makefile declares `go test ./... -race -count=1` as the project's test
command, but no workflow invokes that target. ci.yml and pr-auto-review.yml both
run plain `go test ./...`, and `-race` appears nowhere under .github/workflows,
so the detector has never run on a pull request. The concurrent paths where a
race is easiest to introduce and hardest to see in review, the turn loop,
streaming provider I/O, the session store, cron, swarm mailboxes, and the
cross-process lock paths in the credential and OAuth stores, are all unchecked.
Add a dedicated job rather than putting -race on the existing matrix step. A
race run is typically several times a plain one, and the matrix Test step is
followed by build and smoke, so folding it in would serialize behind those and
make that job the new long pole. On its own it runs alongside the Windows job,
which is the current long pole at 9m29s, and a failure reads as a race rather
than as a smoke failure.
Run `make test` instead of repeating its flags, so the project's declared test
command and what CI executes cannot drift apart again.
Refs #939
* ci: add permissions to race detector job
---------
Co-authored-by: euxaristia <[email protected]>
* fix(providerhealth,tools): let the configured forward proxy through the loopback guard
With HTTPS_PROXY pointing at a local forward proxy, the transport dials the
proxy first and tunnels the request through it. The SSRF-guarded dialer in
providerhealth refused that dial as a loopback address, so
`zero doctor --connectivity` failed with "proxyconnect tcp: provider
connectivity URL is unsafe: loopback hosts are blocked" for a public
provider that had already been validated, and web_fetch had no proxy
support at all (#569).
Each dialer is now handed its transport's own Proxy function and lets a dial
through unpinned only when the address is exactly the proxy endpoint that
function names for the request scheme: same host, same port, with a missing
proxy port taken as the scheme default. A sibling loopback port is still
refused. The shared check lives in internal/proxydial since providerhealth
and tools otherwise share nothing.
web_fetch honours http.ProxyFromEnvironment again. The guarded fetch had set
Proxy to nil as part of its SSRF hardening; the target is still validated
before the request, and what a forward proxy removes is only the dial-time
rebind pin, which is inherent to using any forward proxy. The pre-permission
URL validation stays in place.
Started from #594 by baoyu0. This version keeps the pre-permission check,
matches the proxy port exactly instead of any port when the proxy URL has
none, and injects the transport's Proxy function instead of reading the
environment a second time.
Closes#569
Co-authored-by: baoyu0 <[email protected]>
* fix(proxydial): have the dialer ask its transport for the proxy at dial time
Copying transport.Proxy into the dialer at construction left the wiring
between the two unpinned: a dialer handed nil would pass every unit test
while a proxied request still died as loopback. The dialer now consults
the transport it is installed on, and each package drives a real request
through a loopback listener acting as the proxy and checks that the
CONNECT arrives. The proxy is substituted on the transport rather than
through HTTPS_PROXY, since http.ProxyFromEnvironment reads the
environment once per process and an env-based test would depend on
test order.
* fix(proxydial): bind the proxy exemption to the route the transport chose
Asking the Proxy function a question of our own invention answered for a
request the transport never made. With NO_PROXY excluding a host, the real
request to that host goes direct while a synthetic probe still comes back
proxied, so a direct dial whose address happened to equal the configured
proxy endpoint skipped the guard and reached whatever the name resolved to
at connect time. A probe on the previous head dials rebind.example:80 with
zero safety lookups; the guard now runs and refuses its private answer.
Host-dependent proxy callbacks make the mismatch general, and no spelling
of the probe host closes it, because the two calls are about different
requests.
The decision is now recorded where it is made. proxydial.Transport wraps
the transport, asks its Proxy function about the actual request, and puts
the answer in that request's context; the transport derives the dial
context from the request context, so the dialer reads back the route this
very connection is being opened for. Per-request, so concurrent requests
cannot overwrite each other, and the context key is unexported so nothing
outside the package can plant one.
Address matching is gone with it. A transport opens one connection dial,
and when a proxy is selected that dial goes to the proxy, so the recorded
route already identifies the dial. Comparing the address on top could only
disagree with the transport, which is what a Unicode proxy hostname did:
Go dials xn--bro-hoa.example while the URL still spells it as configured,
and the proxy was refused as an ordinary private address. Both spellings
are now one endpoint with no comparison to get wrong.
Documents where destination enforcement ends: with a proxy configured, the
target hostname is sent to the proxy and the proxy resolves it, so the
local pre-request checks no longer bind the address that is reached. A
forward proxy already sees and can rewrite every request through it, so
that trust comes with configuring one; the alternative is refusing to use
proxies at all, which is the outage in #569.
---------
Co-authored-by: baoyu0 <[email protected]>
* fix(agent): stop the tool-schema ratchet meaning a different thing per host
TestEagerToolSchemaTokenBudget counted exec_command shell guidance along with
the schemas, and that guidance is host-specific. The same tree measured 3498 on
Linux, about 3600 on a Windows host with pwsh 7, and 3653 on Windows PowerShell
5.1, because shellGuidanceForRuntime appends a longer block for 5.1 (it has to
explain that && and || do not work there).
5.1 is stock Windows, and that case has been over the 3650 ceiling since
view_image landed. So `go test ./...` failed for anyone on a stock Windows box
while CI stayed green: the runners have pwsh 7 and take the cheaper branch. The
workflow does run the tests on windows-latest, so this was never a coverage gap,
just a ceiling sized against whichever shell the runner happened to have.
The two halves are now charged separately. The schema ceiling measures the
schemas alone and reads 3498 on Linux and on Windows PowerShell 5.1, so it
ratchets the same thing everywhere. The host guidance gets its own ceiling at
200, above the worst case rather than above this machine: 5.1 is 155 tokens,
pwsh 7 is 110, cmd.exe is 87, POSIX is 0.
No guidance text was trimmed. It is what stops the model writing bash on Windows,
and the 5.1 note is the reason a chained command does not silently do half its
work. Paying 155 tokens for it is the right trade; measuring it as if it were a
schema was not.
HostExecCommandShellGuidance is exported for the budget, which is production
surface added for a test. It is one accessor over existing logic, and the
alternative is a ratchet that cannot tell the two costs apart. A guard in
internal/tools pins that the guidance really is the tail of the description, so
the subtraction cannot silently start measuring the wrong thing.
* test(tools,agent): make the guidance contract checkable off Windows
The ratchet separates the fixed tool schemas from this host's exec_command
shell guidance, and both halves of that separation were only being checked on
Windows.
The description test returned early off Windows once the accessor reported no
guidance, so it never looked at the description. Dropping the GOOS condition in
the constructor would have a POSIX host append its own guidance (about 150
characters on darwin) with the accessor still reporting none and the ratchet
charging every byte of it to the schemas, and nothing would fail. Both the
description and the accessor now take goos, so the contract is checked for all
three platforms from any one of them.
The budget also went from subtracting a separately rounded guidance estimate to
removing the guidance from the description before estimating. ApproxTextTokens
is an integer division, so floor((schema+guidance)/4) - floor(guidance/4) is one
token high whenever the two remainders add past four, and which side of that a
host lands on depends on the length of its guidance. That is the same
host-dependence the ratchet was being fixed for, one token smaller.
No behaviour change: exec_command's description is byte for byte what it was on
every platform.
The unwind test asserted the report was removed on a resume failure, which
is the contract this branch just replaced: absence is what a normal cleanup
leaves too, so it does not revoke a launch a live reader already saw. It
now asserts an explicit false, with a companion for the case where the
retraction write itself fails and the file is discarded after all.
The report is published before ResumeThread, so the fact is readable while
the child has executed nothing. A live poll landing in that window latched
it, and the latch outlived the file: the terminal read finds the report
cleaned away and restores what was observed, so a child that never became
runnable was reported as launched and its write-jail trade disclosed as if
it had been made.
Deleting the report on that path cannot fix it, because deletion is also
what a normal cleanup does and the restore exists for exactly that. The
helper now retracts with an explicit false, which is the one answer that
outranks a latch, and the manager repeats the observation while the command
runs instead of caching it. Silence still changes nothing, so a genuine
launch still survives its own cleanup.
The report is published before ResumeThread so the inherited-pipe race is
closed, and that ordering stays. But a failure between the publish and the
resume reaped a process that had executed nothing while leaving a report on disk
saying a child launched. AppliedEnforcementNotices gates on that report and
ResolveChildLaunched treats it as authoritative, so the operator would have been
told a write-jail trade applied to a child that never became runnable.
The comment above the publish already claimed the stronger invariant, that every
failure between creation and resume leaves "no child launched" true for the
parent. The code held it only for failures before the write.
publishThenResume now owns the sequence: the published flag it returns is true
only when the child actually resumed, so a resume failure hands the deferred
close a false and the report is removed. The docstring on the terminate helper
now distinguishes the pre-publish path, where nothing was written, from the
post-publish pre-resume path, where the record has to be taken back.
Reported by jatmn.
beforeTool was moved onto the typed EnforcementNotices slice and afterTool was
left folding its notices into the prose feedback, so the same fact had two
writers on the normal tool tail: the typed slice, which every surface composes
through ModelOutput and HumanDisplay, and the hook feedback block appended to the
body. Both carry the identical fixed deny_read string, so a hook running under
the same token shape as the tool it follows made the model see the disclosure
twice, and a bash or exec card show it in the amber furniture and again in the
body. Neither half existed on the merge base; this branch introduced both.
hookMessage now returns the hook's own stdout or stderr and nothing else.
dispatchAfterTool returns its notices alongside that output, and the loop merges
them into the result through the same finalization beforeTool uses, with the same
order and dedupe. A veto's Reason still carries its notice inline, because that
field is prose that reaches a person on its own.
Ordinary afterTool validator output is untouched: a formatter diff or vet warning
still arrives as feedback in the body, which is what an afterTool hook asked for.
A silent hook that exists only to disclose a token trade now reaches the typed
slice instead of becoming model input.
The hooks tests that asserted the disclosure through hookMessage now assert it on
Notices and additionally that it does NOT ride along in Messages, so the property
they protected is unchanged and its carrier moved with it.
Reported by jatmn.
A successful beforeTool hook's disclosure was folded into result.Output as hook
prose. That reached the provider, which reads the output, and reached nothing
else.
Every interactive surface builds its enforcement furniture from the typed
EnforcementNotices slice, and for an edit or a write the card renders
Display.Preview instead of Output. So on exactly the results where something was
written, the operator saw the diff and no disclosure at all, collapsed and
expanded, and the session payload persisted that same omission for the restored
card. The model was told the token had been weakened and the person was not.
The notice now merges into the typed slice at one finalization point that the
normal path, the hook veto and the retry denial all return through, so no return
path can reopen the loss. Ordering is the hook's disclosures ahead of the tool's
own, exact repeats dropped, so a surface rendering the slice shows each one once.
Nothing is written into Output as well: decoration has one owner per surface, or
the disclosure appears twice. ModelOutput, HumanDisplay, the card renderer and
session serialization keep the ownership they already had.
Hook notices are third-party text on a path that bypasses the registry's
redaction boundary, so the merge scrubs them the way appendHookFeedback did while
they travelled as prose, and reports it so Redacted stays accurate.
The joiner that folded notices in with afterTool feedback is gone with its last
caller. afterTool output was never enforcement data and still arrives as prose.
Reported by jatmn.
Three positive-path tests skipped when the manager came back with an unwrapped
plan. Nothing about those fixtures is an environment prerequisite: they pin GOOS,
an available native backend, command wrapping and an explicit executable, so an
unwrapped result is not a machine that cannot run the test, it is the producer
whose disclosure they exist to check having gone away. CI would have reported all
of the deny_read disclosure coverage passing after the real route disappeared,
and the hand-built predicate tests would have stayed green with it because they
start from CommandPlan{Wrapped: true}.
They now fail through one shared assertion that names the wrapping state, the
target backend and the enforcement level.
The fixtures were also still reading the real machine in one place.
windowsSandboxInitialized stats the per-host setup marker and BuildCommandPlan
consults it, so a box where `zero sandbox setup` has run resolved these requests
at native enforcement while every CI runner got unelevated. The plan came out
wrapped either way, which is why nothing failed, but the level being asserted
against was whichever machine ran it. Both fixtures now pin it.
Reported by jatmn.
The gate is redundant today: every route that reaches StartupNotices has already
settled the decision positive through the handshake confirmation, so removing it
left every test passing. Unfalsifiable code is a stop signal, so it is pinned
directly rather than left as an unchecked claim.
It stays because the redundancy is on the safe side. An edit that moves or loses
the confirmation makes this return nothing rather than announce a confinement on
the strength of the helper having started.
connectAndList reads client.StartupNotices() on the success path and hands the
result straight to the disclosure sources, with no launch check anywhere in
between. For a wrapped plan those notices were recorded because cmd.Start
returned, which is the HELPER starting: the one thing the report exists because
it does not prove. The answer happened to be right, since a completed handshake
does imply the child ran, but by coincidence rather than by rule, while the
failure path beside it was already asking the adapter.
Found by falsifying: dropping the handshake confirmation left every test passing,
because the success disclosure never consulted the decision at all.
StartupNotices now goes through the same decision as the sink and the error, so
one rule covers all three carriers.
The ordering test also asserts the failure disclosure travels through the ERROR,
not only the sink. Registration merges the two, so a sink-only assertion passed
while the decision was taken before the adapter settled and the failure the
operator reads carried nothing.
ChildLaunchTracker gets its own tests, because the rule that a negative read is
not remembered is invisible through the MCP paths: they each ask once, at a point
where the answer is already terminal. Pinned where the rule lives instead.
The cross-process launch report has three states, not two: not settled yet,
settled with no child, and child created. An absent file, the empty file the
adapter opens before launching, a half-written one and a decode error are all the
first state. Collapsing them to "no child" is a claim about a process that may be
running at that instant, and every consumer was making that collapse on its own
and compensating for the timing separately, which is why fixing one presentation
site kept exposing the next.
Three changes, one contract.
The Windows helper now creates the sandboxed child with CREATE_SUSPENDED and
resumes it only after the report is published. The child inherits the MCP pipes,
so created runnable it could answer initialize, emit a malformed response or close
stdout before the helper was next scheduled, and a parent reading the report then
saw the empty file and cached "no child" about a server already running
unconfined. It also closes the second manifestation: publishing could fail AFTER a
runnable child had begun making external changes, and reaping it afterwards does
not undo the work it did. A suspended child has executed nothing, so every failure
between creation and resume terminates a process that never ran and the missing
report is then true.
ChildLaunchTracker is the settlement rule, at the adapter boundary where every
consumer can share it. It caches a launch and never an absence: a negative read
stays provisional until the answer is terminal. Two things make it terminal.
Confirm, when the consumer observed the child itself, and Settle, when the adapter
process has exited. Settle runs from Cleanup, before the report file is deleted,
so reading the evidence and destroying it are one step rather than a race the
consumer has to win.
The MCP client uses both. A successful initialize response is Confirm: the adapter
speaks no MCP and the child is created suspended, so a well-formed response can
only have come from the requested server. On the failing exit the decision now
comes after Close instead of before it, because Close waits out the adapter and
cleanup settles with the report still on disk. Asking first asked an adapter that
may be between creating the child and recording it, and the connect timeout ends
the attempt exactly there.
The existing fixture writes the finished report during PrepareExecution, before
the helper command starts, so every ordering above is over before the parent
looks. The new tests drive a real helper process that publishes at a controlled
point: after answering the handshake, after failing it, and not at all.
For an adapter-owned launch, cmd.Start proves only that the sandbox helper
started. It can then fail setup-marker validation, ACL application, network
validation, token construction, or CreateProcessAsUser without ever creating
the requested MCP server. The launch sink made that distinction; the
initialize-error path did not, and carried the planned notices out
unconditionally. The operator was told a server had run without write
confinement when no server had run at all.
The adapter-gated path was also dead on that same route. client.Close runs the
plan cleanup, which deletes the report file, and it ran before
publishAdapterLaunch read it, so a wrapped server that really did launch and
then failed its handshake published nothing either. Two competing definitions
of applied enforcement, and both were wrong in opposite directions.
connectStdio now resolves the launch fact once, through
execution.ResolveChildLaunched, memoized, and reads it before Close so the
evidence still exists. The sink and the error carrier consume that one answer.
The success path is unchanged: a completed handshake is independent proof the
child existed, and gating it on a mis-written report would suppress a true
disclosure. Direct stdio and bwrap are unaffected, since an unwrapped plan
resolves as launched exactly as before.
Regressions drive RegisterTools with no ClientFactory, so the real connectStdio
runs, and with a preparer whose cleanup deletes the report the way the Windows
plan does. A helper reporting no child, and one that wrote no report at all,
disclose nothing while still being recorded as skipped; a helper that reports
the child discloses exactly once.
The Windows helper publishes childLaunched immediately after
CreateProcessAsUser creates the restricted child, and only then waits for it.
ProcessManager read that report exclusively in the post-Wait goroutine, so for
the entire live lifetime of a retained wrapped session the report was the zero
value. The first exec_command reply and every write_stdin poll resolved
Launched=false and disclosed nothing, while the fact sat readable on disk. A
watcher, or a retained session nobody polls to completion, would never be told
the write jail had been traded away.
The launch fact is a monotonic lifecycle transition, not terminal process data.
managedProcess now observes it once, latches it, and hands it out on live
results as well as the terminal one. markDone will not overwrite a latched
launch with a terminal read, because the plan's cleanup has already removed the
report file by then on some orderings.
Only the positive is promoted. An absent, partial, or undecodable report, and a
helper that failed before it created the child, all leave the live result
exactly as before: not confirmed, nothing disclosed. Latching false or
surfacing a read error from the live read would let a mid-flight poll rewrite a
running command into a setup failure. Direct commands and bwrap are untouched:
the read is gated on the plan being adapter-owned, so an unwrapped plan does no
extra work.
Regressions drive the real ProcessManager with a real child that publishes the
way the helper does and then stays alive: a live start and a live poll both
disclose, a helper that reported no child stays silent, and a report published
mid-flight is observed on the next poll.
Delivering DispatchOutcome.Messages for a successful beforeTool hook fixed the
silent disclosure and overshot. hookMessage builds that slice by folding the
enforcement notice together with the hook's ordinary stdout, or stderr when
stdout is empty, because afterTool validators want both. So every successful
hook's routine logging, large diagnostics, and whatever text the hook happened
to process became a standing input channel into the next model request. main is
silent for a successful hook.
DispatchOutcome now carries Notices separately: only the disclosures, one entry
per notice, accumulated across every hook that ran. Messages keeps its old
meaning and its old consumer.
The notices are also accumulated BEFORE the veto short-circuit, because they
describe something that already happened. Dispatch stops at the first veto, so
a hook that ran under the weakened token ahead of the vetoing one used to leave
its disclosure in the audit record and nowhere else. Two exits after the hook
runs now share one finalization helper with the normal tail: the veto result,
and a denied, cancelled or ungrantable unsandboxed retry. The blocking hook's
own notices are already inside its reason, so they are not repeated.
The regression that expected `go version` stdout to reach the provider was
locking in the wrong behaviour and is replaced. One hook run now emits both a
notice and ordinary output, and the test asserts the notice arrives exactly
once while the output stays silent, driven through Run and asserted on what the
provider received. A second test covers the veto path. Falsified by delivering
Messages again, by returning the veto result without the notices, and by
dropping the accumulation ahead of the veto.
Smoke (windows-latest) failed lint on cc85f2ee: SA1019, runtime.GOROOT has been
deprecated since Go 1.24. The fallback was copied from an older test and was
never needed here, since the test cannot run without a go binary anyway. Skipping
when one is not on PATH is the honest answer.
executeToolCall inspected the beforeTool DispatchOutcome only when Blocked was
true, so a hook that ran fine and produced output put it in the audit record and
on no surface anyone could see. A beforeTool process that ran under the weakened
DenyRead token said so to nobody; only vetoes and afterTool feedback reached the
model.
Its messages now ride out on the tool result, the same delivery afterTool
feedback already uses, ahead of that feedback and without displacing it. Blank
messages contribute nothing, so a run with no hook output stays silent rather
than appending an empty header, and veto behaviour is untouched.
The regression drives the real Run loop with a real hook process and asserts on
what the provider received on the next turn; dropping the capture fails it on
that assertion. A unit test on the joining helper passed with the capture
removed, so it could not have caught this.
sessionStart and sessionEnd still discard their outcomes. Routing those needs a
delivery surface that does not exist in agent.Options today, and choosing one is
a product decision rather than a mechanical fix, so it is raised on the PR
instead of invented here.
Joining the pump at stop bounded writes to the pump's lifetime but said nothing
about the overlap. Headless and interactive startup keep writing plugin, trust,
peer, provider, trace and validation output to the same caller-supplied writer
for the whole time the pump is live, and Run accepts an arbitrary io.Writer: a
plain bytes.Buffer corrupts under concurrent use, and even a concurrency-safe
terminal writer interleaves logical lines. A mutex private to the pump could not
fix that, because the foreground writes do not go through it.
reportMCPStartupDisclosures now returns a guarded view of the caller's writer
alongside stop, and both startup paths adopt it for the rest of startup, so the
pump and the foreground path take the same lock. Machine-readable stdout is
untouched and the stop-before-TUI boundary is unchanged.
The regression is deterministic rather than hopeful: a writer that parks inside
Write holds the foreground message there while the late launch resolves and the
pump tries to print, and it counts concurrent entries. Removing the lock from the
guarded writer fails it on that count; the test also asserts stop drains the
final notice exactly once and that the foreground message is not lost.
The adapter-owned child-launch contract was interpreted only by
Runner.ExecuteCaptured. bash, exec_command/ProcessManager and durable stdio MCP
each copied a subset of the prepared state and decided launch independently, so
the same false disclosure survived in every path the earlier fix did not touch:
a Windows helper that starts and then fails during marker, ACL, network, SID,
token or CreateProcessAsUser setup still promoted the planned DenyRead notice
even though no restricted child ever existed.
ResolveChildLaunched is now the single answer: the adapter's report wins in both
directions, an adapter that owns the fact and stays silent means not launched,
and anything else keeps the caller's own observation, which is correct for a
direct command and for bwrap. The captured runner, bash, and the exec_command
conversion all call it. ProcessManager carries the ownership bit into
ProcessResult so the retained write_stdin shape has it too, where the helper can
be returned before it has even attempted the inner launch. connectStdio keeps
prepared.Report and the ownership bit and no longer publishes at cmd.Start for a
wrapped plan; it publishes once the adapter confirms, on both ways the attempt
can end, which is also where an attempt abandoned at the connect timeout lands.
Separately, the helper no longer drops ownership of a child it created. The
report file is claimed BEFORE CreateProcessAsUser, so a failure to obtain the
side channel happens while there is still nothing to own, and a failure to
publish afterwards terminates and reaps the child instead of returning while it
runs with nobody waiting on it and the parent free to start a second one. A
report that was never published is removed, so a truncated file cannot be read
back as a launch.
The regression drives the production conversion rather than the shared helper:
asserting the launch again fails it on three property assertions naming the
notice it wrongly disclosed.
For a Windows restricted-token plan the command the runner starts is the sandbox
helper, not the requested executable, so exec.Cmd.Process becomes non-nil as soon
as the ordinary helper process starts. Setup-marker validation, unelevated ACL
application, network-policy validation, capability and offline SID construction,
restricted-token creation and CreateProcessAsUser all happen inside that helper
and can each return without creating the requested process. On those paths
AppliedEnforcementNotices still reported that reads were denied as requested,
when the only thing that ran was the unsandboxed adapter.
The fact belongs to whoever sees the transition. AdapterReport gains
ChildLaunched, which the helper writes to the adapter-owned report file at the
moment CreateProcessAsUser succeeds, and the runner believes over its own
observation. A plan whose adapter owns the fact is marked, and silence from that
adapter now means not launched rather than falling back to the wrapper's start,
so a missing report cannot be read as proof that enforcement applied. The mark is
per adapter, not plan.Wrapped: a bwrap plan is wrapped too and reports only
denials, and treating its silence as "no child" would drop the disclosure from
every successful Linux sandbox run.
Direct, unwrapped commands are unchanged, since the process the runner starts is
the requested one. The report file is created with O_EXCL under the per-user temp
directory, so a name another local user pre-created makes the helper fail rather
than supply the fact the parent reads back.
The regression drives both sides: a helper that runs and never creates the child
discloses nothing, an adapter that owns the fact and says nothing discloses
nothing, a restricted child that starts and then exits non-zero discloses exactly
once, and a direct command keeps its own observation. Ignoring the reported fact,
or removing the fail-closed branch, fails it on the notice it wrongly disclosed.
ReportStartupDisclosures took the CLI's presentation function and invoked it
from whichever goroutine resolved the launch. For a server still inside
cmd.Start when registration gave up, that is the abandoned connect goroutine,
so the write to stderr happened off the output owner's goroutine and could
arrive after runExec had returned or after Bubble Tea had taken the alt screen.
The regression added with it demonstrated the bug rather than catching it: it
raced bytes.Buffer.String under -race.
The runtime owns the fact that a process started; it does not own anyone's
writer. StartupDisclosureStream carries the disclosures as typed events and the
caller drains them on the goroutine that owns stderr: the set known at
registration is printed before the reporter returns, keeping server order, and
later arrivals are printed by a single pump. The returned stop ends delivery,
joins the pump and flushes what is queued, so no write outlives the caller's
ownership. A disclosure arriving after stop is dropped, which is the defined
answer to what close does with pending delivery.
Headless defers stop so it runs before the runtime is closed. The interactive
path calls it on the line before deps.runTUI, so a launch resolving later is
dropped instead of writing raw text over the alt screen. Runtime.Close also
ends delivery, so a launch that resolves after the runtime is gone has nowhere
defined to land.
Removing the join reintroduces the original data race; removing the close lets
a post-stop launch write to the abandoned writer, failing the new regression on
the bytes it wrote.
Two findings, one contract: a fact that outlives the consumer that first
looked for it.
The retained launch sink stopped a late cmd.Start from being lost, but a value
nobody re-reads is still a lost disclosure. Both production reporters sampled
StartupDisclosures once, right after RegisterTools returned, and a stdio
attempt abandoned at the connect timeout could still be inside Start at that
moment. The process then ran under the reduced write confinement, the reaper
closed its client, and the operator saw the skipped-server warning and never
the disclosure. The previous regression masked this by polling
StartupDisclosures by hand after release rather than exercising a reporter.
The sink is now an event as well as a value. Runtime.ReportStartupDisclosures
reports servers whose notices were known at commit immediately, in server
order, and subscribes every other server's sink; a subscriber whose launch has
already happened runs before subscribe returns, otherwise it runs from
publishLaunch, and either way exactly once. Servers that never start never
publish, so prepare, pipe and Start failures stay silent, and network servers
contribute nothing. reportMCPStartupDisclosures uses the push form and keeps
the pull form for a runtime that lacks it. The new test drives the REAL
reporter against a REAL runtime: reporting before release prints nothing,
release prints the notice exactly once. Reverting the reporter to pull-only
fails it with "disclosed 0 time(s), want exactly 1".
The headless session writers built their own payload with decorated output
only, while the TUI writer stored typed notices and an undecorated body. Both
append to the same store the TUI resumes from, so a CLI-written long collapsed
result restored into the TUI had no body to carry the decorated text and no
notice furniture to draw it: the disclosure the run had shown was gone.
tui.ToolResultSessionPayload is now the one owner and the CLI delegates to it;
the one field the CLI had added on its own, truncated, moves into the shared
shape so it is not lost. A tui test restores the CLI's exact bytes into a
collapsed card and asserts the notice exactly once; a cli test asserts the
delegation, since reverting it leaves the tui test green and fails the cli one
on the notice, the body and the truncated flag.
The settle grace is only another timeout. When it expires, registration reaps
the abandoned attempt in the background and returns, but the process can still
be inside cmd.Start at that moment. It then starts under the reduced write
confinement, publishes to its sink, and nobody is left who can say so: the
reaper closes the late client, and Runtime had already frozen its disclosures
into a snapshot taken during the serial commit. Startup reported such a server
only as skipped.
Registration is bounded and a launch is not, so the two cannot share a
lifetime. The sink already carries the authoritative fact and outlives the
attempt; Runtime now retains it per server and StartupDisclosures reads through
it instead of copying out of it. A server whose notices were known at commit
never re-reads its sink, and entries stay in server order, so repeated reads
cannot duplicate or reorder anything.
Nothing else moves: pre-launch failures still publish nothing and stay silent,
network servers still launch no process, and the reaper still closes the late
client.
The existing regression releases its launch at 120ms, deliberately inside the
250ms grace, so it only proved the grace covered that interval. The new test
releases strictly after the bound, asserts the disclosure was legitimately
absent beforehand, and asserts a second read does not duplicate it. Reverting
StartupDisclosures to the snapshot fails the new test and leaves the in-grace
one passing, which is the point: the old shape could not see this.
The enforcement disclosure reached the card in two forms: typed
EnforcementNotices, which the card renders as its own furniture, and
ModelOutput, which has the notice composed into the text. toolResultDetail
returned the undecorated Display.Preview when one existed and fell back to the
decorated ModelOutput when one did not, so every result without a preview drew
the warning twice: once in the notice lines and once at the top of the body.
That is every bash and exec card and every error card. The existing regression
used a rich-preview edit result, which selects the one representation that
masks the path.
Give the body one owner. toolResultDetail now returns the base text, and the
card decorates once. agent.ToolResult gains BaseModelOutput and BaseDisplay
mirroring tools.Result, so a surface that renders the typed notice has a
canonical accessor to build from rather than re-deriving it, and ModelOutput
and HumanDisplay are expressed in terms of them so the base is computed once.
The durable path had the same mismatch: the payload stores the decorated output
beside the typed notices, and restoration used that output as the body whenever
no distinct preview was stored. The undecorated body is now always written when
it differs, and restoration keys on the field being PRESENT rather than
non-empty, because a command that printed nothing under an enforced profile has
an empty body and a real notice.
Tests cover live and restored, success and error, and assert that the notice
and the underlying output each appear exactly once. Reverting toolResultDetail
alone fails all of them.
connectStdio publishes only once cmd.Start has returned, and the timeout branch
sampled the sink the instant it fired. Those interleave: the sample reads
empty, the result commits with no notice, and the background reaper closes the
late client without being able to amend a commit that has already happened. A
process that started under reduced write confinement is then never represented
in StartupDisclosures.
Wait briefly for the abandoned attempt to say whether it had started, before
concluding it had not. cancel() has already fired, so an attempt that never
reached Start fails fast and the grace costs nothing; only one that did start
can still be inside Start, and it publishes on the way out. A test asserts that
the never-started case is not delayed, so the grace cannot quietly become a
startup cost.
The window itself is microseconds wide and cannot be hit from a test seam: an
attempt to widen it with a slow second server failed, because every per-server
goroutine returns at its own timeout and nothing holds wg.Wait open. So the
tests drive the contract instead, a start that lands after the timeout but
inside the grace, which is the case the synchronization exists to catch.
The serial phase also re-reads the sink for an index whose result carried no
notices, as a second net that costs nothing.
execExecutionOutcome is shared between exec_command and bash, and it set
Launched unconditionally. That is true for exec_command, where a start failure
returns an errorResult before an execution outcome is ever built, and it is not
true for bash, which hands EVERY Run error to the same conversion: a missing
executable and a context cancelled before os.StartProcess both arrive here with
a prepared plan and no child.
So a bash command that never created a process reported the DenyRead token
trade as applied. Measured before the fix: Launched=true, ChildLaunched()=true,
and one applied enforcement notice for a command that did not exist.
Observe it where it is known instead. exec.Cmd sets Process only once
os.StartProcess has succeeded, so bash captures that at the Run boundary and
threads it through withBashExecution; exec_command sets it true at its own call
site, with the reason written down rather than assumed.
The regressions drive the real tool rather than constructing an outcome, since
the bug was exactly that the constructed shape and the real one disagreed.
A stdio server that started and then hung in initialize or tools/list was
abandoned at the connect timeout and recorded as skipped, with nothing said
about the confinement its process ran under. The notices left connectStdio
only on the returned client or the returned error, and an abandoned attempt
produces neither before the serial commit phase is over. The reaper that
collects it later runs after that phase, so it cannot contribute without
breaking the deterministic ordering the phase exists to provide.
Publish the launch fact at Start instead, on a sink carried in the context, and
read it in the timeout branch. Launch and connection usability are separate
facts with separate lifetimes, which is the distinction that was missing.
Carried on the context rather than in the factory signature so an injected or
third-party factory that knows nothing about it still works and simply
discloses nothing.
A timeout before Start stays silent, since the sink is only ever published to
after Start returns. That placement is load-bearing, so it is pinned by a test
that drives the real connectStdio with an executable that cannot start: moving
the call one line up leaves the registry-level tests green while every failed
launch begins claiming the trade.
currentUserSIDForTest swallowed the GetTokenUser error and returned empty,
and its one caller guarded on the result being non-empty. On a machine where
the call fails, the assertion ran against nothing and the test reported a
pass.
The check exists to catch the restricted token keying itself to the very SID
it has to be stricter than, which is the whole point of the shape, so failing
to read the prerequisite is a failure rather than a silent skip. Confirmed
both directions with a simulated unreadable SID: the old shape passes, this
one fails naming what it could not read.
The notices are prepended to the model view on the way out, so a disclosed
result costs more context than result.Output alone. The outcome diagnostics
measured the bare output, which undercounts every disclosed call, and the
undercount scales with the notice rather than being fixed slack. A short
command output measures 13 bytes against the 114 the model is handed.
Measure the canonical text instead. ModelView stays the bare output on
purpose: ModelOutput prepends the notices itself, so storing them here would
send them twice, and a test pins that too.
The notice was prepended to ModelOutput, which toolResultRowText carries into
row.text, and toolCardHead is handed row.text. But the head renders the action
and the target, so the notice went nowhere. Every result with a rich preview,
which is every edit and write card, rendered with no disclosure at all,
collapsed or expanded.
Resume lost it too. The session payload carried the notice only inside the
"output" string, and the restored card is rebuilt from displayPreview, which
never had it.
Carry the notices as their own field on the row, persist and restore them
alongside changedFiles, and render them above the body on all three card
paths. Shown collapsed as well as expanded: a trade the operator has to expand
a card to discover has not been disclosed.
Kept out of row.detail deliberately. That field is parsed as a diff by the
files panel and rendered line by line by the file view, so prefixing it with
the notice would have been the shorter fix and would have corrupted both. A
test pins the diff stats against exactly that.
The render cache keys on the notices for the same reason. It distinguished
them already, but only through row.text, and that incidental coupling is what
hid the notice from the card to begin with.
BackendPlan derived the DenyRead warning from request.Backend and the
requested profile. request.Backend is always the AVAILABLE backend, so on a
Windows host it stays the restricted-token backend with NativeIsolation set
even when the resolution disables sandboxing outright.
With deny_read configured and --sandbox forbid, the plan resolves to
enforcement disabled and target none, builds no token and enforces no read
rule, and `zero sandbox policy` still reported that the write jail had been
traded for read denial. The reassuring half was the false one: it claimed
reads were denied as requested on a run that denies nothing.
The execution path already keyed this on the resolved plan through
windowsRestrictedTokenWillRun. Split the request-side half of that predicate
into willBuildWindowsRestrictedToken and reuse it for the diagnostic, so both
describe the plan that will run rather than the backend that happens to be
installed.
plan.Wrapped stays with the execution caller rather than moving into the
shared predicate. It is the produced execution state and the request cannot
speak for it, so folding it in would buy symmetry by handing the execution
path back a bug the request-side checks alone cannot catch. A test pins that
split.
addSandboxMeta writes the plan's notices at plan time, before anything runs, and
finalizeToolOutcome promoted them into the user-visible disclosure
unconditionally. That claims a token trade for a command that may never have
started, which is the same substitution the hooks and plugins paths already
stopped making.
The promotion now comes from the execution outcome when there is one, so it
follows the recorded launch state and the planned notices together. The plan
metadata is untouched and stays as diagnostics, since what was intended is still
worth having in the record, and a tool with no execution outcome still promotes
from metadata rather than silently losing its disclosure.
execExecutionOutcome states that its outcomes describe a started process rather
than leaving it to be inferred: a command that could not be started returns an
error result before reaching it.
connectStdio records the notices once cmd.Start returns, which is the right
moment, but the initialize failure path closes and discards the client. The
client was the only carrier, so a server that started, did filesystem work and
then failed its handshake told the operator it was unavailable and never that it
had already run without the write jail.
A launched process is a fact about the past: once Start has succeeded the
disclosure is true whatever the handshake does next. The failure now carries it
out, and registration recovers it from the error, so the fact no longer dies
with the connection it was attached to. A connect that never launched still
discloses nothing.
OutcomeKind is not a launch-state field, and reading it as one was wrong in both
directions. The adapter report is read AFTER Run, so a child that really ran and
then produced an unreadable report is rewritten to a setup failure and the
disclosure was dropped although it applied. And a context already cancelled
before os.StartProcess still selects a cancellation, so the disclosure was
claimed for a process that never existed.
ExecuteCaptured now records whether an OS process was created, taken from the
only thing that knows: exec.Cmd sets Process only once os.StartProcess has
succeeded. That is false for a missing executable and for a context cancelled
before Start, and true for anything that ran, including a later timeout or
cancellation. Report decoding can now fail after launch without rewriting the
historical launch fact.
The plugins test that asserted the kind decides was encoding the defect, so it
now expresses the recorded-fact contract instead, including the two shapes the
kind gets wrong.
`zero exec` registers workspace MCP servers through the same sandbox-backed
runner interactive startup uses, so a stdio server here can launch under the
weakened token and serve the whole run. Only the TUI reported the disclosure, so
every text, JSON, stream-JSON and --list-tools caller was told nothing about the
enforcement trade for a process that was already running.
Reported immediately after registration, before --list-tools and before the
first result, since both return early. On stderr, which is where the
skipped-server and trust notices already go, so stdout framing is untouched; the
regression asserts the JSON and stream-JSON output still parses.
The test isolates HOME, APPDATA, LOCALAPPDATA and the XDG roots. Without that it
builds a sandbox engine against the real config dir, triggers the one-time grant
migration there, and the notice surfaces on a later test's stderr, failing
whichever test happens to assert an empty one. That moves between runs and reads
as flakiness rather than as contamination.
Two defects in the disclosure collection, both found by jatmn.
The append ran inside the per-server goroutine, so it raced the shared slice
header: entries could be lost or overwritten, and whichever survived were
ordered by completion time rather than by server. The comment directly above
promises that the concurrent phase touches no shared state and that the serial
phase is therefore deterministic, and this broke both halves of it. The notices
now travel on the indexed connectResult and are committed in the serial loop, in
server order. Reproduced with 32 simultaneous servers under -race before the fix.
The disclosure was also reachable only through the client, so a server that
started, did filesystem work, and then failed initialize or tools/list lost the
fact when that path closed the client and returned nil. The operator was told the
server was unavailable and not that the process had already run without the write
jail. connectAndList returns the notices separately now, so they survive the
failure that discards the client. A factory error still discloses nothing,
because nothing launched.
A hook audit record kept an exit code, stdout and stderr, and the notice is
deliberately in none of those. Once the dispatch result was gone, nothing could
tell an audit or recovery reader that a hook had run under the weakened DenyRead
token. AuditResult carries the notices typed and omitempty, so historical
records read back unchanged and an ordinary hook writes what it wrote before.
An MCP stdio server's launch is the same shape one level up. connectStdio
received the prepared command and kept only the command and its cleanup, so a
server started under the weakened token served the whole session with nothing
able to say so, and no later tool result could recover it because the fact
describes startup rather than any response. The client keeps the applied
enforcement, recorded after Start returns so a prepare failure or a missing
executable claims nothing, registration collects it per server, and startup
states it once next to the skipped-server warnings. Network servers launch no
local process and report nothing.
Enforcement.Notices is PLANNED: it describes the shape a command was prepared to
run under, and planning is not proof that anything ran. Hooks copied the field
straight out, so a sandbox setup failure or a missing executable told the
operator the write jail had been traded away for a child that never existed.
Outcome.AppliedEnforcementNotices now makes that call once, where the outcome
kind is known, and hooks and plugins both use it; the plugin-local copy of the
rule is gone. A new pre-launch outcome kind is classified in one place instead
of being disclosed by whichever consumer was not updated.
MCP tools/call serialized Result.Output directly. That was a complete value
before this branch and is not one now: Output holds the undecorated base text
and ModelOutput is the model-facing projection. An affected Windows command
reached an MCP client with its ordinary output and no statement about the token
shape it ran under.
Making agent.ToolResult store the undecorated model text plus the typed notices
was right, but I only audited the consumers that render to a terminal. Three
others read the raw field and lost the disclosure the moment that change
landed.
ACP sends the tool result straight to its client, so an ACP client saw the
output with the warning removed, on the one surface that has no other way to
learn the sandbox narrowed what the command could do.
Both headless session writers persisted the raw field, and replay reads that
value directly into the transcript without rebuilding a ToolResult, so a
warning visible during the original run vanished from resumed and compacted
context with nothing failing to say so. Those two writers also spelled the same
payload separately and had already drifted, since the stream writer used the
accessor; they now share one helper.
The rule is that presentation and durable consumers both go through
ModelOutput, because the accessor is the only thing that composes the text with
the notices.
executeToolCall copied the already-rendered ModelOutput/HumanDisplay into
agent.ToolResult while also copying the typed EnforcementNotices slice, so the
same disclosure lived in two places with no contract between them. It renders
once today only because the outcome arrives finalized and the agent accessor
then reads Outcome.ModelView rather than the stored field, which also means the
stored field disagreed with the outcome it came from. A result reaching the
accessor without a finalized outcome would have shown the notice twice.
Split the undecorated base out into BaseModelOutput/BaseDisplay and have the
projection store that. Decoration now happens in exactly one place, the
accessors, and the stored text agrees with the finalized outcome.
Every other notice assertion in this PR hands a constructor a Notices slice and
checks it comes out the other side. That proves the consumers and never the
producer: deleting the one line in EnforcementFor that puts plan.Notes into
Enforcement.Notices left every notice test in the repo green, and that line is
the entire reason hooks, plugins and MCP see anything at all.
This starts from a plan the manager built rather than a literal, so the chain
from profile through plan.Notes to Enforcement.Notices is covered end to end,
with a silent-plan case so it cannot be satisfied by a field that is never
empty. It fails with the projection removed.
Two more places the notice was dropped, both the same shape as the last round:
one path assembles the result and another path, taken under different
circumstances, rebuilds it from fewer fields.
A plugin that timed out or was cancelled took invoke's error branch, which
constructed a result from status, output and metadata alone. The child had
already launched under the non-WRITE_RESTRICTED token, so the disclosure was
still true of it, and the model saw only the timeout.
The launched-or-not question is answered once now, in
execPluginCommandWithExecution where the outcome kind is known, rather than at
each constructor. A setup failure or a missing executable started nothing and
carries no notice; everything past launch does, however it ended. Every return
in invoke now carries whatever that decision produced, so the disclosure cannot
depend on which branch runs.
A vetoing beforeTool hook took the blocking branch, which builds
DispatchOutcome.Reason through blockReason and returns immediately, never
reaching hookMessage. Reason is the field the agent turns into the
model-visible result, so a hook that blocked an action while running without
write confinement said only that it blocked. blockReason composes the notices
now; blockCause keeps the wording it had.
No double render: blockedByHookResult reads Reason only, and the advisory path
reads Messages only, so the two channels stay separate.
The hook regression drives Dispatch rather than calling blockReason with a
hand-built commandResult, because a test that assembles the shape it expects
proves the consumer and not the producer. Both fail with their fix reverted.
Two halves of the same disclosure, neither of which reached a user.
The predicate keyed on request.CommandWrapped, read as "something already
wrapped this, so we are re-entrant". That is the opposite of what the field
means: BuildExecutionRequest sets it TRUE for exactly the native and unelevated
requests that buildPlatformCommandPlan then routes to
windowsRestrictedTokenCommandPlan. So the notice was suppressed on every plan
that builds the restricted token and fired on none of them. Every real
file_system.deny_read execution got the non-WRITE_RESTRICTED token and was told
nothing.
It keys on the produced plan's Wrapped state now, which is the resulting
execution state and cannot be read backwards: the direct plan sets it false, the
restricted-token plan sets it true, and both arrive through the same funnel.
The old test passed because its hand-built request left CommandWrapped false,
which is a shape no real execution has, and the whole cluster around it did the
same by passing an empty CommandPlan. Those are rewritten to be plan-based, and
the new regression drives the manager so the request carries the state the
transition actually produces. One of its silent cases named the misreading
outright and is gone.
The second half: plugin and hook results discarded Outcome.Enforcement.Notices.
Both projections copied stdout, stderr and an exit code out of the structured
outcome and dropped the rest, so once the predicate above is fixed a plugin tool
or a hook runs under the weakened token and still says nothing. Both carry the
notices now, plugins onto Result.EnforcementNotices and hooks into the surfaced
message, prepended so a hook that prints nothing still discloses. Covered on
both paths with an assertion that the notice appears exactly once.
Three findings from review.
The disclosure went into Result.Meta and stopped there. That looked like the
established channel because sandbox_downgrade_reason travels the same way, and
it is not one: nothing in production reads those keys, ModelOutput and
HumanDisplay never consult Meta, and the durable history drops it. A Windows
user configuring deny_read could take the non-WRITE_RESTRICTED token, lose write
confinement, and see nothing but ordinary command output. It is a field on the
canonical result now, surfaced by both accessors, so every surface reads it
through one contract. Prepended rather than appended, because the output budget
trims from the end and a disclosure that survives only on short results is not
one. The metadata copy stays for integrations reading the result JSON.
Promoted at finalizeToolOutcome, the single seam every tool result crosses,
rather than at each construction site. Setting it where results are built would
have been a third hand-maintained projection of the same fact, which is how it
went missing from the generic adapter to begin with.
That generic adapter is the second finding. PrepareExecution built
execution.Enforcement by hand for the wrapper hooks, plugins and MCP processes
go through, while exec_command built the same struct by hand for the tool path,
so Notices reached one and not the other. Both go through EnforcementFor now,
which copies the slice defensively.
And the notice claimed a trade nobody had made. The predicate asked only about
the host, the backend and DenyRead, so a disabled sandbox or a re-entrant
command, both of which take the direct unwrapped plan while still carrying the
Windows backend and profile, were told the write jail was gone. Neither half was
true there: no restricted token is created and deny-read is not enforced either.
It is keyed on the resolved execution state now, with the disabled, degraded,
already-wrapped, no-platform-sandbox and no-backend cases covered as explicit
silent cases.
My own fixture from the previous round was one of the things that had to change:
it named the backend without the fields that make a plan wrapped, so it was
asserting against a request that would never have produced a token.
The warning this PR added was reachable only from BackendPlan, which is what
`zero sandbox policy` and `zero sandbox check` render. A real tool call takes a
different path: the resolved profile becomes a CommandPlan, and the Windows
runner picks the token shape from that profile alone. DenyRead being non-empty
drops WRITE_RESTRICTED, which is the shape #869 is about. So an operator could
approve file_system.deny_read for one command, lose the workspace write jail,
and never see the disclosure, because it lived on a diagnostic view they had no
reason to run.
The notice is derived in withSandboxExecutionMetadata rather than at each
caller. That is the single funnel every plan passes through, including the
Windows one, so an execution caller cannot be added that quietly misses it. It
travels on CommandPlan.Notes, reaches the tool boundary as the sandbox_notices
metadata key alongside the downgrade reason that already goes that way, and
reaches the typed execution path as Enforcement.Notices.
The policy and check warning stays as the diagnostic view.
Covered in both directions and at both layers: a plan resolved with DenyRead
carries the notice and an ordinary profile carries none, and the tool metadata
gains the key only when there is something to say. Dropping the derivation fails
the plan test, dropping the emission fails the metadata test.
The message advertised `--sandbox forbid`, an option that does not
exist, and it survived because nothing drove the branch. The text was
only ever correct by inspection, and inspection is what missed it.
Route the apply through a seam so a test can fail it, then assert what
the operator actually reads: the cause is still wrapped, the option that
does not exist never returns, and both surviving remedies are named.
Restoring the old wording fails the test on both counts.
Also assert the failure does not record the applied-plan marker. That
marker is what makes later commands skip the re-apply, so recording it
here would turn one refusal into a sandbox that silently stops applying
its ACLs entirely.
The unelevated ACL failure told the reader to re-run with `--sandbox
forbid`. There is no such option: SandboxPreferenceForbid is an internal
engine state with no flag behind it, so following the advice produced an
unknown option and left them stuck on a failure they had just been told
how to clear. A recovery instruction that does not work costs more than
no instruction, because the reader spends time discovering it is wrong.
Point at the real way out instead: turn the sandbox off in the user
config, which is honored from global config only. The elevated-setup
remedy beside it was already correct and stays.
Reported by jatmn against the same string on #640. It predates both
branches, having arrived with the unelevated fallback tier in #427.
The warning fired on any plan targeting the Windows backend, including one built
somewhere else, which broke the smoke job on macOS and ubuntu.
credentialDenyReadPaths returns empty ON Windows and populates itself from the
host everywhere else, so a Windows-targeted plan built on a Linux runner carries
that machine's credential paths and drew a warning about a token nothing would
ever build. TestSelectBackendChoosesPlatformAdapterWithFallback asserts a Windows
plan has no warnings, and it only ever builds Windows plans from another host.
Gating on the host is more accurate rather than merely convenient: this describes
a token the Windows command runner will build, and that runner only runs on
Windows. Behaviour on a real Windows host is unchanged, which is why local
testing missed it.
The host is read through a var so both sides stay testable anywhere, matching
windowsSandboxInitialized. Added a regression test for the case that actually
broke, and mutation-verified it: dropping the gate fails it for linux and darwin.
#865 removed the World SID from the WRITE_RESTRICTED token, which is what makes
the write jail hold: every principal carries Everyone, so while it was a
restricting SID the write half of the check passed for free on any
Everyone-writable path. That fix had no CI protection. The only test covering it
sits behind ZERO_SANDBOX_REAL_SMOKE=1 and no workflow sets that variable, so a
rebase or refactor restoring the unconditional World SID goes green. #640's
branch conflicts on exactly that hunk.
CreateRestrictedToken works unelevated against the caller's own token, so the
invariant can be checked in an ordinary unit test. Added four:
- the WRITE_RESTRICTED token must not carry the World SID (this fails against a
reverted #865, verified by mutation)
- neither token shape may carry Users, Authenticated Users, INTERACTIVE, BATCH,
Administrators, SYSTEM, SERVICE, NETWORK, or the user's own SID. #869 names
these as the ones that would reopen the same class of bypass
- the capability SID must be present, so a token with an empty list cannot pass
by having no keys at all
- the non-WRITE_RESTRICTED shape still carries the World SID, which documents the
open gap rather than asserting the end state. It skips with a note if that
stops being true, so whoever closes#869 is told to replace it
Second half: the trade was invisible. Setting denyRead selects the token shape
without WRITE_RESTRICTED, and nothing told the person who set it that they had
given up write confinement to get read-deny. The plan now carries a warning
saying so, keyed off the same field the runner reads and scoped to the Windows
restricted-token backend, so the default posture stays quiet. Zero never
populates denyRead on Windows itself, so this only reaches users who configured
it.
This does NOT close#869. Closing it needs a read-side grant that is not a
universal group (AppContainer, or the per-workspace principals in #808), which
is a different piece of work. What changes here is that the fixed shape can no
longer regress silently, and the unfixed shape no longer looks enforced.
Refs #869, #865, #612, #640
* fix(sandbox): resolve git metadata carveouts for worktrees
.git is a file (gitdir: ...) in worktrees/submodules, not a dir. The
hooks/config write-deny carveout assumed a directory and pointed at
<root>/.git/hooks, which has no mkdir-able parent under the .git file.
bwrap then failed 'Can't mkdir parents ... Not a directory', blocking
every sandboxed tool in worktree checkouts.
Resolve the real (common) git dir via the gitdir pointer + commondir so
worktrees deny the shared hooks/config like a plain clone. Plain
checkouts (.git dir) and .git-absent roots keep the literal paths.
* fix(sandbox): bound worktree git metadata carveouts to the write root
Refuse gitdir/commondir pointers that escape the workspace instead of
carving outside confinement. Invalid, stale, or non-directory pointers
return no carveouts so bwrap does not mkdir under a .git file.
* fix(sandbox): evaluate gitdir/commondir through symlinks
Containment must use the real path so an in-root symlink cannot punch
hooks/config carveouts outside the write root.
* fix(sandbox): fail closed on gitdir eval and protect the pointer file
Refuse carveouts when EvalSymlinks cannot produce a real in-root path,
compare evaluated roots and targets, and write-deny the .git pointer so
it cannot redirect hooks/config between commands.
* fix(sandbox): deny worktree-local gitdir metadata and parse first gitdir line
Keep hooks/config write-denied on both the common dir and the in-root
worktree gitdir, emit those paths under the lexical write root, and
accept a gitdir: pointer that has trailing comment lines.
* fix(sandbox): skip special git metadata files and re-check carveouts at apply
Read .git and commondir only when they are regular files so a FIFO cannot
block profile construction. At sandbox apply, drop ReadOnlySubpaths that
no longer sit inside the write root after a symlink swap.
* fix(sandbox): follow directory .git symlinks and unix-tag FIFO tests
A .git symlink to a directory is a normal checkout, so keep lexical
hooks/config carveouts. FIFO coverage stays on unix so Windows still
compiles.
* fix(sandbox): protect worktree git pointer directly
* test(sandbox): skip unavailable symlink fixture
* fix(sandbox): preserve git symlink semantics
* fix(sandbox): limit git pointer carveout to regular files
---------
Co-authored-by: jatmn <[email protected]>
Co-authored-by: jatmn <[email protected]>
* fix(specialist): keep the pinned model when a specialist is resumed
Metadata.Model exists so a bounded, delegated task can run on a cheaper model
than its parent, and BuildArgs appends it to the child argv through
appendModelArgs. BuildResumeArgs never did.
So a specialist pinned to a cheap model ran on that model exactly once. The
moment the orchestrator resumed it, the child fell back to whatever the
parent's configured model resolved to. Nothing surfaced it: the resumed child
starts normally and does the work, so the only symptom is the bill.
Cost-motivated delegation quietly stopped saving anything.
Resuming does not restore the recorded model on its own. sessions.PrepareExec
records the model a run used but never feeds it back into provider
construction, so the flag has to be passed again rather than relied upon.
BuildResumeArgsInput now carries ParentModel and ParentReasoningEffort, the
same fallbacks the fresh path takes, and runResume passes what TaskRunOptions
already held. The reasoning-effort rule travels with the model unchanged: the
parent's effort is inherited only when the manifest pins no model of its own,
because a manifest that chose a different model has not agreed to the parent's
effort for it.
Regressions drive both builders and compare them, so the two paths cannot
drift again: a pinned model survives resume, an unpinned one still inherits the
parent's, both halves of the effort rule hold, and the flag keeps its position
relative to --auto in both.
Refs #554
* test(specialist): cover the resume call site, not just its builder
The builder tests all call BuildResumeArgs directly, so dropping the
ParentModel field from the runResume call site still compiled and still passed
every one of them. The defect this fixes lived at the call site, so it needs a
test that goes through Run.
Driven through the real dispatch with the RunChild seam capturing argv.
* test(specialist): guard the fresh call site as well
runFresh and runResume each construct their builder input by hand and carry a
byte-identical ParentModel line. Deleting either compiles and, until now,
deleting the fresh one was silent.
The resume half is what this branch repairs. This covers the other half so the
pair cannot drift again in the direction nobody was looking.
Review tests for Bun test idioms, deterministic behavior, and meaningful coverage of safety decisions, provider/runtime flows, session events, and TUI command behavior.
- path:"**/*.go"
instructions:|
Do not request Plan 9 or other unsupported GOOS/GOARCH shims. The five native prebuilt release targets are linux-x64, linux-arm64, macos-arm64, macos-x64, and windows-x64. Android/Termux is also a supported runtime through the documented android-arm64 source build and Linux artifact fallback. Other GOOS/GOARCH combinations are not a review criterion.
echo "::error::dist-tag latest points at platform version ${latest} — a platform publish clobbered it. Fix the dist-tag before the wrapper publish." >&2
exit 1
;;
esac
echo "dist-tag latest is ${latest} (unchanged by platform publishes)."
* **cli:** let providers subcommands run with no active provider ([#1143](https://github.com/Twigpine/zero/issues/1143)) ([d1a0d04](https://github.com/Twigpine/zero/commit/d1a0d048ee2c1e7cbfb75160ec4c9f099f3c4472))
* **npm:** wait for npm to apply each publish, and drop the one-time @gitlawb/zero steps ([#1145](https://github.com/Twigpine/zero/issues/1145)) ([8e363fc](https://github.com/Twigpine/zero/commit/8e363fcaa5d51bf5b5b244692dc61bf8d4ae8799))
* **tui,cli:** stop a repository's git config from running programs in Zero's own git calls ([#1133](https://github.com/Twigpine/zero/issues/1133)) ([640d936](https://github.com/Twigpine/zero/commit/640d936bfbe00c4cd3017ca35fb7b5c15d91a1c5))
* **workspaceindex:** skip git-ignored and build-cache dirs in Scan only ([#1117](https://github.com/Twigpine/zero/issues/1117)) ([c88c77b](https://github.com/Twigpine/zero/commit/c88c77bf701039dca01d9786bf5648d779c949b3))
### Performance Improvements
* **redaction:** add fast-path byte checks to eliminate regex allocations on clean strings ([#945](https://github.com/Twigpine/zero/issues/945)) ([ce2fcff](https://github.com/Twigpine/zero/commit/ce2fcffaf10d8e8c8c5a13e06fd7809f09fc9867))
* **notify:** make permission-prompt alerts on by default and discoverable ([#1001](https://github.com/Twigpine/zero/issues/1001)) ([7f8e5f0](https://github.com/Twigpine/zero/commit/7f8e5f08bb83371ce16cc54c97ab032a010026cf))
* **npm:** publish as @twigpine/zero ([#1142](https://github.com/Twigpine/zero/issues/1142)) ([530563a](https://github.com/Twigpine/zero/commit/530563a0bb58d3cbadceb97b3ee234ef763124df))
* **sessions:** add zero sessions prune, which never removes a session another process has open ([#1089](https://github.com/Twigpine/zero/issues/1089)) ([23c27e0](https://github.com/Twigpine/zero/commit/23c27e0778e3af54c04b8749d1eda846d3253f4e))
### Bug Fixes
* **acp:** persist the session permission mode across restarts ([#1134](https://github.com/Twigpine/zero/issues/1134)) ([bb96206](https://github.com/Twigpine/zero/commit/bb96206c2781e846fd13437113eec5790afe9c81)), closes [#1125](https://github.com/Twigpine/zero/issues/1125)
* **cron:** bound run history and read recent records from the tail ([#1094](https://github.com/Twigpine/zero/issues/1094)) ([42eb6ab](https://github.com/Twigpine/zero/commit/42eb6ab5fbe4ae9037f1f4cd42a00243c456a22e))
* **daemon:** bound pending sessions and prune on completion ([#1091](https://github.com/Twigpine/zero/issues/1091)) ([8a68660](https://github.com/Twigpine/zero/commit/8a68660161b04ce9668363faa14ff72c9fbad223))
* **daemon:** do not retry workers that finished with a final exit code ([#1112](https://github.com/Twigpine/zero/issues/1112)) ([eefa978](https://github.com/Twigpine/zero/commit/eefa978f6183332b7416c70c25242bcdfba1a75c)), closes [#1100](https://github.com/Twigpine/zero/issues/1100)
* **daemon:** stop worker process groups by their launch-time identity ([#1088](https://github.com/Twigpine/zero/issues/1088)) ([ab018b3](https://github.com/Twigpine/zero/commit/ab018b3098b5cb103cf8f90b8698d32a6687bdf7)), closes [#861](https://github.com/Twigpine/zero/issues/861)
* **deps:** refresh the lockfile to patched hono and @hono/node-server ([#1080](https://github.com/Twigpine/zero/issues/1080)) ([aa52040](https://github.com/Twigpine/zero/commit/aa52040498e012c41d5e8f6027a91c18eff861d4))
* **deps:** refresh the lockfile to patched shell-quote ([#1131](https://github.com/Twigpine/zero/issues/1131)) ([c56a4c4](https://github.com/Twigpine/zero/commit/c56a4c4e6e598504ab417e8cc4f5ab8d69f1b42f))
* **execution:** reserve process capacity before launching ([#1092](https://github.com/Twigpine/zero/issues/1092)) ([bb3346a](https://github.com/Twigpine/zero/commit/bb3346a82883c19ed8815fe1e874f54950711d69))
* **mcp:** forward image tool results on the existing image channel ([#1077](https://github.com/Twigpine/zero/issues/1077)) ([0324c1d](https://github.com/Twigpine/zero/commit/0324c1d4103328eb90cb24a0b582dba335255b5c))
* **mcp:** refresh OAuth tokens for discovered endpoints and registered clients ([#1113](https://github.com/Twigpine/zero/issues/1113)) ([571af70](https://github.com/Twigpine/zero/commit/571af701bd3d8dfaf40f87b5b112b25e7def5b29)), closes [#1101](https://github.com/Twigpine/zero/issues/1101)
* **oauth:** give every loopback callback server the bounded shutdown ([#1079](https://github.com/Twigpine/zero/issues/1079)) ([5473301](https://github.com/Twigpine/zero/commit/547330197b20ece591e077e876a4b8ffd015cb6e))
* **providers:** send stable session headers to OpenCode ([#1060](https://github.com/Twigpine/zero/issues/1060)) ([99721c7](https://github.com/Twigpine/zero/commit/99721c762f37cd43ac511007a5f51d1846df959e))
* **redaction:** redact adjacent AWS keys glued end to end ([#1095](https://github.com/Twigpine/zero/issues/1095)) ([df67ede](https://github.com/Twigpine/zero/commit/df67edec13345b73a85327adc003cacc7bdd8009))
* **redaction:** redact credentials split by an invisible character ([#1067](https://github.com/Twigpine/zero/issues/1067)) ([e20b762](https://github.com/Twigpine/zero/commit/e20b7624261cef8336a8eb0fbdd44d536ad77fc2))
* **sandbox:** say when a session's sandbox enforcement is degraded ([#1082](https://github.com/Twigpine/zero/issues/1082)) ([6b5852d](https://github.com/Twigpine/zero/commit/6b5852dc18bb256af1f25745697c2f2e3ea6f112))
* **search:** redact every text field of session metadata, not ten of thirty ([#1073](https://github.com/Twigpine/zero/issues/1073)) ([9520f2b](https://github.com/Twigpine/zero/commit/9520f2bad7e09ce4a8d0d42be7ce77d2ae928ddd))
* **security:** harden shell flag parsing, write roots, and formatters ([#1066](https://github.com/Twigpine/zero/issues/1066)) ([ac5fe0d](https://github.com/Twigpine/zero/commit/ac5fe0d21849b0f74af428dd3ca12e3b80a8b7aa))
* **sessions:** keep a session lease for the life of the process, not of the Store ([#1144](https://github.com/Twigpine/zero/issues/1144)) ([883e5be](https://github.com/Twigpine/zero/commit/883e5bec24dac791d1d60618c7f4927f4659b687))
* **tools:** hint when Windows resolves POSIX paths under root ([#1063](https://github.com/Twigpine/zero/issues/1063)) ([0c6f179](https://github.com/Twigpine/zero/commit/0c6f179a8443d4dfdfbbefe5cc6caf11c2a525f0))
* **tui:** bound live dictation regions ([#985](https://github.com/Twigpine/zero/issues/985)) ([feb9927](https://github.com/Twigpine/zero/commit/feb992757c582c68f0397e070e206d4bf07071e1))
* **tui:** show provider ownership in Recent and Favorites ([#1083](https://github.com/Twigpine/zero/issues/1083)) ([1697b7a](https://github.com/Twigpine/zero/commit/1697b7a5ce948e781877cdbe9223adac62c3fde6))
* **tui:** stop held peer message Allow/Deny from deadlocking Update ([#1111](https://github.com/Twigpine/zero/issues/1111)) ([a195e88](https://github.com/Twigpine/zero/commit/a195e8848cec1a27b4fe1789ecc572b943c32186)), closes [#1099](https://github.com/Twigpine/zero/issues/1099)
* unbreak main CI (cron claim tests, Go 1.26.9) ([#1124](https://github.com/Twigpine/zero/issues/1124)) ([2c418fc](https://github.com/Twigpine/zero/commit/2c418fc8ff6763de16703128f372fe526fddd331))
* **acp:** add standard session list and resume ([#914](https://github.com/Gitlawb/zero/issues/914)) ([c1937df](https://github.com/Gitlawb/zero/commit/c1937dfac72e6ad0e5ade6e48e2d9c17d9c3e5d6))
* **acp:** make configured MCP tools callable ([#1003](https://github.com/Gitlawb/zero/issues/1003)) ([7f5e5af](https://github.com/Gitlawb/zero/commit/7f5e5af7bbdbe5bb4d91706146072d461c0234a4))
* **agent:** stop the tool-schema ratchet meaning a different thing per host ([#1017](https://github.com/Gitlawb/zero/issues/1017)) ([f909266](https://github.com/Gitlawb/zero/commit/f9092668a832ea73a69551f0ea2df4f6eaf356df))
* **background:** treat POSIX zombie leader as already-exited ([#981](https://github.com/Gitlawb/zero/issues/981)) ([1aacf38](https://github.com/Gitlawb/zero/commit/1aacf38b727d31b5ca09949316d6afa8292aec46))
* **config:** lock config read-modify-write across processes ([#960](https://github.com/Gitlawb/zero/issues/960)) ([b04fe96](https://github.com/Gitlawb/zero/commit/b04fe96e9ef3f2deda4fb2a972270300629079d2))
* **daemon:** publish status files atomically ([#949](https://github.com/Gitlawb/zero/issues/949)) ([9001421](https://github.com/Gitlawb/zero/commit/90014210d2ada2e3d61f4eca5087d3f35a3c4b8b))
* **installtxn:** put back an install a killed commit left behind ([#997](https://github.com/Gitlawb/zero/issues/997)) ([2837f5e](https://github.com/Gitlawb/zero/commit/2837f5efb38dcd3c69771b0ba31565f98a206120))
* let the configured forward proxy through the loopback guard ([#1025](https://github.com/Gitlawb/zero/issues/1025)) ([1e4db7c](https://github.com/Gitlawb/zero/commit/1e4db7c6643ca7e2c3d8e4b8b30371cfbe43561e))
* **lockutil:** use kernel-held advisory locks ([#950](https://github.com/Gitlawb/zero/issues/950)) ([6fe0d1e](https://github.com/Gitlawb/zero/commit/6fe0d1edcdbadea6d566800fbcca739c1cd54ea1))
* **mcp:** discover OAuth through protected resource metadata ([#991](https://github.com/Gitlawb/zero/issues/991)) ([1b5db17](https://github.com/Gitlawb/zero/commit/1b5db1765672820caac1684b168c9898b5ba3593))
* **mcp:** reject a server-initiated request as a streamable-HTTP response ([#1054](https://github.com/Gitlawb/zero/issues/1054)) ([2479d94](https://github.com/Gitlawb/zero/commit/2479d946c15dbf74ee3c04a637e97181d1786f08))
* **oauth:** force-close loopback callback connections past the shutdown deadline ([#1051](https://github.com/Gitlawb/zero/issues/1051)) ([176f5a2](https://github.com/Gitlawb/zero/commit/176f5a242b754a86ea4c7121a8dcc1c0d72cf2e4))
* **oauth:** Split an oversized keyring token blob across entries ([#1007](https://github.com/Gitlawb/zero/issues/1007)) ([275e248](https://github.com/Gitlawb/zero/commit/275e2486695338ce38193a6e3319f8b6a9a5c33c))
* **providerio:** close idle connections periodically to prevent HTTP/2 PROTOCOL_ERROR ([#1058](https://github.com/Gitlawb/zero/issues/1058)) ([b76c764](https://github.com/Gitlawb/zero/commit/b76c764b77926891aa67163e6605a2d8540aacaa))
* **providers:** register Atomic Chat as local OpenAI-compatible runtime ([#550](https://github.com/Gitlawb/zero/issues/550)) ([79fa142](https://github.com/Gitlawb/zero/commit/79fa1422ef1a3691b93342788453ebc284d57f9c))
* **redaction:** split camelCase keys in normalizeKey ([#994](https://github.com/Gitlawb/zero/issues/994)) ([08fbea7](https://github.com/Gitlawb/zero/commit/08fbea7a3221d2061d88038924f90a4cba9af1ac))
* **sandbox:** keep a derived ACL target inside the write root it came from ([#1040](https://github.com/Gitlawb/zero/issues/1040)) ([3b43c43](https://github.com/Gitlawb/zero/commit/3b43c43eca968c8619adadc81fcc1a17e220216a))
* **sandbox:** keep Windows restricted-token SIDs narrow and fail closed on DenyRead ([#1006](https://github.com/Gitlawb/zero/issues/1006)) ([0207f2b](https://github.com/Gitlawb/zero/commit/0207f2b2697149a73ecc89a5ab5db0bd665f1911))
* **sandbox:** Make WSL backend tests deterministic. ([#948](https://github.com/Gitlawb/zero/issues/948)) ([e59bf78](https://github.com/Gitlawb/zero/commit/e59bf78cabc2703678778b904c27c8bc8ef82bc8))
* **sandbox:** refcount temporary roots so one holder's cleanup cannot revoke another's ([#911](https://github.com/Gitlawb/zero/issues/911)) ([7acac7a](https://github.com/Gitlawb/zero/commit/7acac7acee65565e25089f2a7b5813c086ce65eb))
* **sessions:** carry an interrupted turn's work into the resume prompt ([#1016](https://github.com/Gitlawb/zero/issues/1016)) ([6ae8396](https://github.com/Gitlawb/zero/commit/6ae8396f0282a5f18a431f0c0ef1a272ecc2ca2b))
* **sessions:** render resume context fields in key order ([#1043](https://github.com/Gitlawb/zero/issues/1043)) ([da244bc](https://github.com/Gitlawb/zero/commit/da244bcbfcfa05e414567ac0acc3cf8ba745ad7d))
* **specialist:** keep the pinned model when a specialist is resumed ([#1009](https://github.com/Gitlawb/zero/issues/1009)) ([aadb4a2](https://github.com/Gitlawb/zero/commit/aadb4a27e9cdb41e774e8e1a8f23cf9c2fb50db2))
* **swarm:** stop original member before handoff ([#957](https://github.com/Gitlawb/zero/issues/957)) ([0021475](https://github.com/Gitlawb/zero/commit/0021475d4c58c91911098f414795512cf60a0b56))
* **tools:** an edit keeps the reads it did not disturb ([#908](https://github.com/Gitlawb/zero/issues/908)) ([1649008](https://github.com/Gitlawb/zero/commit/1649008506da0e675431dc056251a01891b15cc8))
* **tools:** make apply_patch tolerant and re-expose edit_file ([#956](https://github.com/Gitlawb/zero/issues/956)) ([2749447](https://github.com/Gitlawb/zero/commit/2749447cdce4ae5dd333538b36fa919b80ecc66d))
* **tools:** reject fuzzy edits with drifted content ([#983](https://github.com/Gitlawb/zero/issues/983)) ([8b3d1b1](https://github.com/Gitlawb/zero/commit/8b3d1b1597b04bfa6d4bc7f912abc13ca80a6298))
* **tools:** say when format-on-write ran out of time ([#1044](https://github.com/Gitlawb/zero/issues/1044)) ([6937a30](https://github.com/Gitlawb/zero/commit/6937a309cf00825572210a7610a1f3ea8b74c2f9))
* **tui:** move voice capture to Ctrl+Space ([#968](https://github.com/Gitlawb/zero/issues/968)) ([eeea330](https://github.com/Gitlawb/zero/commit/eeea3308fed38b37b203522624babaf2f30df91f))
* **update:** keep Windows failed-restore state in trusted recovery records ([#979](https://github.com/Gitlawb/zero/issues/979)) ([a3852ba](https://github.com/Gitlawb/zero/commit/a3852bac595e06c5d3ca9da22212b281b8796802))
Installer scripts download the matching release asset for the local platform and
verify its `.sha256` file. If Zero is already installed, run `zero upgrade`
@@ -93,14 +97,24 @@ binary:
| State on disk | Meaning |
|---|---|
| `<binary>.old` (or `<binary>.<suffix>.old`) plus a `.keep` marker | A previous update could not restore the original binary. The `.old` file may be the last binary the updater verified; the installed one may be unverified. |
| `<binary>.old` (or `<binary>.<suffix>.old`) plus a `.keep` marker, or a per-user update-recovery record naming that copy | A previous update could not restore the original binary. The `.old` file may be the last binary the updater verified; the installed one may be unverified. The `.keep` marker is in the installation directory and can be deleted; the identity-bound record is not, so deleting the marker alone does not end the refusal. |
| `<binary>.…old.<suffix>.recovery` | A previous update could not even write the marker, so it moved the last verified binary to that name. |
| The binary is missing and one or more `*.old` files exist | The previous attempt was interrupted between moves. |
The refusal names the paths involved and the two moves that end the state:
either move the recovery binary back over the executable path, or — if the
installed binary is the one you want — delete the `.keep` marker (or the
`.recovery` copy, once you have verified the installed binary) and update again.
installed binary is the one you want — delete the recovery copy itself (not
only the `.keep` marker) after verifying the installed binary, then update
again. Deleting the marker alone is not enough: a failed restore also records
that copy in per-user state outside the installation directory, and the next
update consults that record even if the marker is gone.
That trusted record lives under `%AppData%\zero\update-recovery` (per-user).
The recovery copy and its `.keep` marker sit in the installation directory
(per-machine). A second Windows account on the same machine does not see the
first account's unresolved record; the sibling marker is the machine-visible
signal for every account. There is no separate machine-wide store: writing one
would need a shared writable location and ACL rules the updater does not own.
This is deliberately fail-closed and differs from older releases, which deleted
`<binary>.old` and proceeded. The trade-off is that anyone who can write in the
iferr:=live.client.Call(ctx,MethodSessionPrompt,PromptParams{SessionID:created.SessionID,Prompt:[]ContentBlock{TextBlock("list the files")}},&PromptResult{});err!=nil{
ifgot:=browserToolTitle("open",`{"URL":"https://different.example.test","url":"https://actual.example.test/path"}`);got!="browser open https://actual.example.test"{
t.Fatalf("exact URL key title = %q",got)
}
ifgot:=browserToolTitle("action",`{"command":"not an action"}`);got!="browser action"{
t.Errorf("the hook's disclosure did not reach the typed field, so the card and the restored card show the diff and no disclosure: %v",result.EnforcementNotices)
}
// AND NOT IN THE BODY AS WELL. Decoration has one owner per surface; carrying
// it in both places renders it twice wherever the surface draws the slice.
constmaxTurnsAnswer="Agent reached maximum number of turns without a final answer."
constmaxTurnsFinalAnswerPrompt="You have reached the tool-turn limit. Do not call tools. Give a concise final answer now: summarize what you completed, what you found, and any remaining blockers."
// maxStreamStallRetries bounds how many times a turn that timed out (idle/stall)
// WITH NO OUTPUT yet is re-issued on a fresh connection before giving up. Only
// the no-output case is retried (a partial turn would duplicate), so this is a
// safe recovery for a stalled/dead pooled connection.
// OR hit a mid-stream transport abort (#973) with no answer text yet is re-issued
// on a fresh connection before giving up. Incomplete tool-call previews are
// allowed through the gate (they are never dispatched before the error return).
// The safety claim is only that visible answer prose is not duplicated on retry —
// not that every partial stream shape is excluded. Covers a stalled/dead pooled
// connection or a socket abort before tool dispatch.
//
// Set to 1 (not 2): each attempt can itself idle for the full stream timeout
// (~5min) before the stall is even detected, so 2 retries left an interactive
// session frozen for ~15min. One retry keeps the common single-hiccup recovery
// while bounding the worst case to ~2× the idle timeout.
// while bounding the worst case to ~2× the idle timeout. Mid-stream transport
// aborts share this bound (do not raise it for #973).
Content:"Image output from "+label+" was not sent because the current model does not support image input. Switch to a vision-capable model before retrying image delivery, including any images skipped by the tool's image budget.",
// imageEchoProvider records the messages of the first request it receives, then
Loaded 100 of 961 files, more files were not shown because too many files have changed in this diff.
Show more
Reference in new issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Vasanthdev2004
