mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-08 09:49:34 -10:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
945eb06d2b | ||
|
|
d82b94defe | ||
|
|
88fb7b6192 | ||
|
|
0125e70d0f | ||
|
|
f5fa4ffc64 | ||
|
|
9fbdc34155 | ||
|
|
583e6530bd | ||
|
|
ac50a5a840 | ||
|
|
a4a805a6c8 | ||
|
|
9fe02ae52e | ||
|
|
9d5c82d441 |
No files matched your search
@@ -0,0 +1,3 @@
|
||||
*.go text eol=lf
|
||||
*.mod text eol=lf
|
||||
*.sum text eol=lf
|
||||
+33
-12
@@ -30,6 +30,36 @@ jobs:
|
||||
go-version-file: go.mod
|
||||
cache: true
|
||||
|
||||
- name: Install Make on Windows
|
||||
if: matrix.os == 'windows-latest'
|
||||
shell: pwsh
|
||||
run: choco install make --no-progress -y
|
||||
|
||||
- name: Validate quality targets with stock macOS Make
|
||||
if: matrix.os == 'macos-latest'
|
||||
env:
|
||||
GIT_CONFIG_COUNT: 1
|
||||
GIT_CONFIG_KEY_0: grep.patternType
|
||||
GIT_CONFIG_VALUE_0: fixed
|
||||
run: make -n vulncheck
|
||||
|
||||
- name: Check vulnerabilities through native Windows Make
|
||||
if: matrix.os == 'windows-latest'
|
||||
shell: cmd
|
||||
run: make vulncheck
|
||||
|
||||
- name: Check dead code through native Windows Make
|
||||
if: matrix.os == 'windows-latest'
|
||||
continue-on-error: true
|
||||
shell: cmd
|
||||
run: make deadcode
|
||||
|
||||
- name: Run static lint through native Windows Make
|
||||
if: matrix.os == 'windows-latest'
|
||||
continue-on-error: true
|
||||
shell: cmd
|
||||
run: make lint-static
|
||||
|
||||
- name: Check formatting
|
||||
if: matrix.os == 'ubuntu-latest'
|
||||
shell: bash
|
||||
@@ -100,27 +130,18 @@ jobs:
|
||||
go-version-file: go.mod
|
||||
cache: true
|
||||
|
||||
# govulncheck, deadcode, and golangci-lint each resolve a toolchain from
|
||||
# their own modules, which can drop below the version go.mod requires and
|
||||
# then fail to load our packages. Pin GOTOOLCHAIN to the go.mod toolchain
|
||||
# so all three run under it.
|
||||
- name: Pin toolchain from go.mod
|
||||
run: |
|
||||
toolchain="$(awk '/^toolchain /{print $2}' go.mod)"
|
||||
echo "GOTOOLCHAIN=${toolchain:-auto}" >> "$GITHUB_ENV"
|
||||
|
||||
# Hard gate: fails the build when code reaches a known vulnerability. A stdlib
|
||||
# CVE is cleared by a toolchain bump (see go.mod). May also flag a newly
|
||||
# published advisory on an unrelated PR — intentional: do not ship known vulns.
|
||||
- name: govulncheck
|
||||
run: go run golang.org/x/vuln/cmd/[email protected] ./...
|
||||
run: make vulncheck
|
||||
|
||||
# Advisory: reports functions unreachable from any cmd/* main so dormant
|
||||
# code is visible in CI. Non-blocking while the dormant subsystems are
|
||||
# still being wired or removed.
|
||||
- name: deadcode (advisory)
|
||||
continue-on-error: true
|
||||
run: go run golang.org/x/tools/cmd/[email protected] -test=false ./...
|
||||
run: make deadcode
|
||||
|
||||
# Advisory: catches what deadcode's whole-program reachability analysis
|
||||
# doesn't, unused private functions/assignments reachable within a
|
||||
@@ -130,4 +151,4 @@ jobs:
|
||||
# the repo are cleaned up incrementally (see #527).
|
||||
- name: golangci-lint (advisory)
|
||||
continue-on-error: true
|
||||
run: go run github.com/golangci/golangci-lint/v2/cmd/[email protected] run --enable-only unused,ineffassign,staticcheck ./...
|
||||
run: make lint-static
|
||||
@@ -51,10 +51,8 @@ request, or completing an implementation task:
|
||||
or run focused tests with `-race`, when concurrency is affected.
|
||||
4. **Build**: `go run ./cmd/zero-release build`.
|
||||
5. **Smoke test**: `go run ./cmd/zero-release smoke`.
|
||||
6. **Advisory lint**:
|
||||
`go run github.com/golangci/golangci-lint/v2/cmd/[email protected] run --enable-only unused,ineffassign,staticcheck ./...`.
|
||||
7. **Security**:
|
||||
`go run golang.org/x/vuln/cmd/[email protected] ./...`.
|
||||
6. **Advisory lint**: `make lint-static`.
|
||||
7. **Security**: `make vulncheck`.
|
||||
8. **Diff hygiene**: `git diff HEAD --check` (covers staged and unstaged
|
||||
tracked changes).
|
||||
|
||||
|
||||
@@ -1,7 +1,13 @@
|
||||
# Zero build/test/lint targets. AGENTS.md says "Build with `make`" and "Run `make
|
||||
# lint` before opening a PR" — these targets back those instructions.
|
||||
# Zero build/test/lint targets. AGENTS.md says to build and run quality checks
|
||||
# with `make` — these targets back those instructions.
|
||||
.DEFAULT_GOAL := build
|
||||
.PHONY: build build-all test test-race vet fmt fmt-check lint tidy clean baseline help
|
||||
GO_VERSION = $(word 2,$(shell git grep -G -h "^go[[:space:]]" -- go.mod))
|
||||
GO_TOOLCHAIN = go$(GO_VERSION)
|
||||
DEADCODE_VERSION := v0.46.0
|
||||
GOLANGCI_LINT_VERSION := v2.12.2
|
||||
GOVULNCHECK_VERSION := v1.3.0
|
||||
|
||||
.PHONY: build build-all test test-race vet fmt fmt-check lint lint-static deadcode vulncheck tidy clean baseline help
|
||||
|
||||
# Build the main CLI binary into ./zero.
|
||||
build:
|
||||
@@ -33,6 +39,22 @@ fmt-check:
|
||||
# Lint = formatting check + vet (no extra tooling required).
|
||||
lint: fmt-check vet
|
||||
|
||||
# Versioned tools select the toolchain from their own modules when invoked with
|
||||
# package@version. Read this module's go directive directly, without invoking
|
||||
# the possibly stale Go toolchain or consulting a multi-module GOWORK. git grep
|
||||
# works with both POSIX shells and cmd.exe, including GNU Make 3.81 on macOS.
|
||||
# The target-specific export is shell-independent.
|
||||
lint-static deadcode vulncheck: export GOTOOLCHAIN = $(GO_TOOLCHAIN)
|
||||
|
||||
lint-static:
|
||||
go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) run --enable-only unused,ineffassign,staticcheck ./...
|
||||
|
||||
deadcode:
|
||||
go run golang.org/x/tools/cmd/deadcode@$(DEADCODE_VERSION) -test=false ./...
|
||||
|
||||
vulncheck:
|
||||
go run golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION) ./...
|
||||
|
||||
tidy:
|
||||
go mod tidy
|
||||
|
||||
@@ -55,4 +77,4 @@ baseline: build
|
||||
--output internal/perfbench/reports/baseline.json
|
||||
|
||||
help:
|
||||
@echo "Targets: build (default), build-all, test, test-quick, vet, fmt, fmt-check, lint, tidy, clean, baseline"
|
||||
@echo "Targets: build (default), build-all, test, test-quick, vet, fmt, fmt-check, lint, lint-static, deadcode, vulncheck, tidy, clean, baseline"
|
||||
@@ -367,22 +367,16 @@ variable unset and set.
|
||||
|
||||
### Code Quality and Security Checks
|
||||
|
||||
Before committing any changes, ensure all Go code quality and security checks pass. Pinned `go run` commands matching CI constraints can be used directly without prior installation:
|
||||
Before committing any changes, ensure all Go code quality and security checks pass. The `make` targets below pin each tool to this module's Go version, so they load correctly even when your default `go` toolchain is older — running the plain `go run ...@version` form yourself can select the tool module's own (older) toolchain and fail to load this module instead.
|
||||
|
||||
1. **Formatting**: Run `go fmt ./...` (or `make fmt`).
|
||||
2. **Vetting**: Run `go vet ./...` (or `make vet`).
|
||||
3. **Linting**: Run `go run github.com/golangci/golangci-lint/v2/cmd/[email protected] run --enable-only unused,ineffassign,staticcheck ./...`.
|
||||
4. **Vulnerability Scan**: Run `go run golang.org/x/vuln/cmd/[email protected] ./...`.
|
||||
1. **Formatting**: Run `make fmt` (or `go fmt ./...`).
|
||||
2. **Vetting**: Run `make vet` (or `go vet ./...`).
|
||||
3. **Linting**: Run `make lint-static`.
|
||||
4. **Vulnerability Scan**: Run `make vulncheck`.
|
||||
|
||||
If you prefer to install these tools globally on your path, you can run:
|
||||
|
||||
```bash
|
||||
# Install golangci-lint
|
||||
go install github.com/golangci/golangci-lint/v2/cmd/[email protected]
|
||||
|
||||
# Install govulncheck
|
||||
go install golang.org/x/vuln/cmd/[email protected]
|
||||
```
|
||||
Use the repository-managed targets rather than globally installed binaries:
|
||||
the targets apply the module's required Go toolchain as well as the reviewed,
|
||||
pinned tool versions.
|
||||
|
||||
The installed binaries land in `$GOBIN` when it is set, otherwise in
|
||||
`$GOPATH/bin` (default `~/go/bin`). That directory must be on your `PATH` to
|
||||
|
||||
+2
-2
@@ -108,10 +108,10 @@ Discovery roots (earlier wins on name collisions):
|
||||
2. **Shared multi-agent dir** — `~/.agents/skills/` when present (read-only discovery; never an install target)
|
||||
3. **Plugin skill roots** — skills bundled by active plugins (section 6)
|
||||
|
||||
A missing directory is fine — Zero just omits it. Management commands (`zero skills add` / `remove` / `lock`) always write to the primary Zero dir only; `list` / `info` search primary + `~/.agents/skills`.
|
||||
A missing directory is fine — Zero just omits it. Management commands (`zero skill add` / `zero skill remove`) always write to the primary Zero dir only; `list` / `info` search primary + `~/.agents/skills`. `skill info` reports lock source/hash when present and whether the on-disk `SKILL.md` has drifted from the recorded hash.
|
||||
|
||||
```text
|
||||
~/.local/share/zero/skills/ # primary (install/remove/lock target)
|
||||
~/.local/share/zero/skills/ # primary (install/remove target)
|
||||
run-benchmarks/
|
||||
SKILL.md
|
||||
write-changelog/
|
||||
|
||||
@@ -91,7 +91,7 @@ func (c *profileController) maybeEscalate() (PostureEscalation, bool) {
|
||||
if c.policy == nil || c.escalated {
|
||||
return PostureEscalation{}, false
|
||||
}
|
||||
if !(c.failureTripped || c.riskTripped || c.scTripped || c.uncertainTrip) {
|
||||
if !c.failureTripped && !c.riskTripped && !c.scTripped && !c.uncertainTrip {
|
||||
return PostureEscalation{}, false
|
||||
}
|
||||
c.escalated = true
|
||||
|
||||
@@ -125,9 +125,7 @@ func Score(suite Suite, input ScoreInput) Report {
|
||||
if len(task.RequiredTraceEvents) > 0 {
|
||||
report.Results = append(report.Results, scoreTraceEvents(task.RequiredTraceEvents, input))
|
||||
}
|
||||
for _, result := range unknownCommandResults(input.CommandResults, seenCommands, input) {
|
||||
report.Results = append(report.Results, result)
|
||||
}
|
||||
report.Results = append(report.Results, unknownCommandResults(input.CommandResults, seenCommands, input)...)
|
||||
report.finishSummary()
|
||||
return report
|
||||
}
|
||||
|
||||
@@ -40,14 +40,14 @@ func FormatFacts(info repoinfo.Info) string {
|
||||
b.WriteString("Pre-computed repository facts (from a local scan — verify and fill gaps):\n")
|
||||
|
||||
if info.PrimaryLanguage != "" {
|
||||
b.WriteString(fmt.Sprintf("- Primary language: %s (of %d detected)\n", info.PrimaryLanguage, info.LanguageCount))
|
||||
fmt.Fprintf(&b, "- Primary language: %s (of %d detected)\n", info.PrimaryLanguage, info.LanguageCount)
|
||||
}
|
||||
if langs := topLanguages(info.Languages, 5); langs != "" {
|
||||
b.WriteString("- Languages: " + langs + "\n")
|
||||
}
|
||||
b.WriteString(fmt.Sprintf("- Size: ~%d files, ~%d LOC, max dir depth %d\n", info.FileCount, info.LOCEstimate, info.MaxDepth))
|
||||
fmt.Fprintf(&b, "- Size: ~%d files, ~%d LOC, max dir depth %d\n", info.FileCount, info.LOCEstimate, info.MaxDepth)
|
||||
if info.WorkspaceType != "" && info.WorkspaceType != "none" {
|
||||
b.WriteString(fmt.Sprintf("- Workspace: %s (%d packages)\n", info.WorkspaceType, info.WorkspacePackageCount))
|
||||
fmt.Fprintf(&b, "- Workspace: %s (%d packages)\n", info.WorkspaceType, info.WorkspacePackageCount)
|
||||
}
|
||||
if len(info.BuildTools) > 0 {
|
||||
b.WriteString("- Build tools: " + strings.Join(info.BuildTools, ", ") + "\n")
|
||||
|
||||
@@ -149,12 +149,9 @@ func TestTerminateProcessKillsForkedChildren(t *testing.T) {
|
||||
|
||||
// The forked child must be gone too (poll until reaped by init).
|
||||
deadline := time.Now().Add(2 * time.Second)
|
||||
for {
|
||||
// Only ESRCH proves the child is gone; any other error (e.g. EPERM) would
|
||||
// wrongly pass the test, so treat it as still-present and keep polling.
|
||||
if errors.Is(syscall.Kill(childPID, syscall.Signal(0)), syscall.ESRCH) {
|
||||
break // child no longer exists
|
||||
}
|
||||
// Only ESRCH proves the child is gone; any other error (e.g. EPERM) would
|
||||
// wrongly pass the test, so treat it as still-present and keep polling.
|
||||
for !errors.Is(syscall.Kill(childPID, syscall.Signal(0)), syscall.ESRCH) {
|
||||
if time.Now().After(deadline) {
|
||||
_ = syscall.Kill(childPID, syscall.SIGKILL)
|
||||
t.Fatalf("forked child %d survived terminateProcess — group kill failed", childPID)
|
||||
|
||||
@@ -515,9 +515,7 @@ func agentEvalReportFromBenchmark(suite agenteval.Suite, report agenteval.Benchm
|
||||
if task.Agent.Truncated {
|
||||
converted.Truncated = true
|
||||
}
|
||||
for _, failure := range agentEvalFailuresFromTaskReport(task) {
|
||||
converted.Failures = append(converted.Failures, failure)
|
||||
}
|
||||
converted.Failures = append(converted.Failures, agentEvalFailuresFromTaskReport(task)...)
|
||||
}
|
||||
return converted
|
||||
}
|
||||
|
||||
@@ -139,6 +139,7 @@ func runSkillInfo(args []string, dir string, stdout io.Writer, stderr io.Writer)
|
||||
}
|
||||
if info.Hash != "" {
|
||||
lines = append(lines, " hash: "+info.Hash)
|
||||
lines = append(lines, fmt.Sprintf(" hash drift: %t", info.HashDrift))
|
||||
}
|
||||
lines = append(lines, " path: "+info.Skill.Path)
|
||||
if _, err := fmt.Fprintln(stdout, redaction.RedactString(strings.Join(lines, "\n"), redaction.Options{})); err != nil {
|
||||
|
||||
@@ -83,6 +83,48 @@ func TestRunSkillAddInfoRemove(t *testing.T) {
|
||||
if !strings.Contains(info, src) || !strings.Contains(info, "sha256:") {
|
||||
t.Fatalf("info missing source/hash:\n%s", info)
|
||||
}
|
||||
if !strings.Contains(info, "hash drift: false") {
|
||||
t.Fatalf("info missing hash drift status:\n%s", info)
|
||||
}
|
||||
|
||||
stdout.Reset()
|
||||
stderr.Reset()
|
||||
if exit := runWithDeps([]string{"skill", "info", "confirmation-policy", "--json"}, &stdout, &stderr, deps); exit != 0 {
|
||||
t.Fatalf("skill info --json exit = %d, stderr = %s", exit, stderr.String())
|
||||
}
|
||||
var beforeJSON map[string]any
|
||||
if err := json.Unmarshal(stdout.Bytes(), &beforeJSON); err != nil {
|
||||
t.Fatalf("parse skill info json: %v\n%s", err, stdout.String())
|
||||
}
|
||||
if beforeJSON["hashDrift"] != false {
|
||||
t.Fatalf("hashDrift before edit = %#v, want false", beforeJSON["hashDrift"])
|
||||
}
|
||||
|
||||
skillPath := filepath.Join(skillsDir, "confirmation-policy", "SKILL.md")
|
||||
if err := os.WriteFile(skillPath, []byte("---\ndescription: edited\n---\nchanged\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
stdout.Reset()
|
||||
stderr.Reset()
|
||||
if exit := runWithDeps([]string{"skill", "info", "confirmation-policy"}, &stdout, &stderr, deps); exit != 0 {
|
||||
t.Fatalf("skill info after edit exit = %d, stderr = %s", exit, stderr.String())
|
||||
}
|
||||
if !strings.Contains(stdout.String(), "hash drift: true") {
|
||||
t.Fatalf("info missing hash drift after edit:\n%s", stdout.String())
|
||||
}
|
||||
|
||||
stdout.Reset()
|
||||
stderr.Reset()
|
||||
if exit := runWithDeps([]string{"skill", "info", "confirmation-policy", "--json"}, &stdout, &stderr, deps); exit != 0 {
|
||||
t.Fatalf("skill info --json after edit exit = %d, stderr = %s", exit, stderr.String())
|
||||
}
|
||||
var afterJSON map[string]any
|
||||
if err := json.Unmarshal(stdout.Bytes(), &afterJSON); err != nil {
|
||||
t.Fatalf("parse skill info json after edit: %v\n%s", err, stdout.String())
|
||||
}
|
||||
if afterJSON["hashDrift"] != true {
|
||||
t.Fatalf("hashDrift after edit = %#v, want true", afterJSON["hashDrift"])
|
||||
}
|
||||
|
||||
// remove deletes it
|
||||
stdout.Reset()
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
@@ -24,12 +25,47 @@ func TestRunExecOptimizedSessionUnderGate(t *testing.T) {
|
||||
cwd := t.TempDir()
|
||||
|
||||
var heads, posts atomic.Int64
|
||||
// Closed when the prewarm probe lands, so the turn can be held open until it
|
||||
// does. See the POST handler for why that ordering has to be forced.
|
||||
headSeen := make(chan struct{})
|
||||
var headOnce sync.Once
|
||||
var prewarmWaitTimedOut atomic.Bool
|
||||
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
switch {
|
||||
case r.Method == http.MethodHead:
|
||||
heads.Add(1)
|
||||
headOnce.Do(func() { close(headSeen) })
|
||||
w.WriteHeader(http.StatusMethodNotAllowed)
|
||||
case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/chat/completions"):
|
||||
// Hold the turn open until the prewarm probe arrives. The probe is
|
||||
// issued on the run's context, and runExec cancels that context the
|
||||
// moment it returns, because the stop function signalContext hands
|
||||
// back is a CancelFunc. A probe that has not reached the wire by the
|
||||
// end of the run therefore never reaches it at all. That is why the
|
||||
// poll this replaced could not work at any deadline: once the run is
|
||||
// over the probe is cancelled, not merely slow.
|
||||
//
|
||||
// This widens the window, it does not close it. Once the goroutine is
|
||||
// scheduled it still has only prewarmTimeout to dial loopback and
|
||||
// write the request, and that timeout is armed inside the goroutine
|
||||
// itself, so holding the turn here cannot extend it. If this test
|
||||
// ever fails again, prewarmTimeout is where to look, not this hold.
|
||||
//
|
||||
// No self-deadlock: the pinned transport leaves MaxConnsPerHost at 0,
|
||||
// so the HEAD dials its own connection instead of queueing behind
|
||||
// this held POST. Do not set a per-host connection limit there.
|
||||
//
|
||||
// Bounded, so a probe that genuinely never fires fails the assertion
|
||||
// below with a useful message instead of hanging the test.
|
||||
select {
|
||||
case <-headSeen:
|
||||
case <-time.After(10 * time.Second):
|
||||
// Record it. A probe that shows up after this gave up would still
|
||||
// leave heads at 1, so the count alone proves the probe was
|
||||
// eventually received, not that it was ordered before the turn,
|
||||
// which is the whole claim of this test.
|
||||
prewarmWaitTimedOut.Store(true)
|
||||
}
|
||||
posts.Add(1)
|
||||
w.Header().Set("Content-Type", "text/event-stream")
|
||||
_, _ = w.Write([]byte("data: {\"choices\":[{\"delta\":{\"content\":\"done\"}}]}\n\n"))
|
||||
@@ -75,14 +111,13 @@ func TestRunExecOptimizedSessionUnderGate(t *testing.T) {
|
||||
if got := posts.Load(); got < 1 {
|
||||
t.Fatalf("server saw %d chat-completions POSTs, want >= 1", got)
|
||||
}
|
||||
// The prewarm probe is asynchronous; it fires well before the model turn
|
||||
// finishes, but poll rather than assuming scheduling order. The deadline
|
||||
// exceeds the production prewarmTimeout (3s) so a slow probe cannot flake
|
||||
// this assertion.
|
||||
deadline := time.Now().Add(5 * time.Second)
|
||||
for heads.Load() == 0 && time.Now().Before(deadline) {
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
// Checked before the count, because the count cannot tell the two apart: a
|
||||
// probe that arrived only after the guard expired still leaves heads at 1.
|
||||
if prewarmWaitTimedOut.Load() {
|
||||
t.Fatal("the turn was not held until the prewarm probe arrived; the probe did not precede it")
|
||||
}
|
||||
// No polling: the turn above could not complete until the probe landed, so
|
||||
// the count is settled by this point.
|
||||
if got := heads.Load(); got != 1 {
|
||||
t.Fatalf("server saw %d prewarm HEAD probes, want exactly 1", got)
|
||||
}
|
||||
|
||||
@@ -734,14 +734,12 @@ func (cfg *mcpWritableConfig) setServerDisabled(name string, disabled bool) (boo
|
||||
switch {
|
||||
case legacyFound:
|
||||
raw = legacyRaw
|
||||
found = true
|
||||
case config.IsDefaultMCPServer(name):
|
||||
// A built-in default server isn't written to the file until the user
|
||||
// overrides it. Treat it as present with an empty base so disabling it
|
||||
// writes a minimal {"disabled":true} entry that merges over the default —
|
||||
// letting `zero mcp disable <default>` work even though it lives in code.
|
||||
raw = nil
|
||||
found = true
|
||||
default:
|
||||
return false, false, nil
|
||||
}
|
||||
|
||||
@@ -83,10 +83,10 @@ func runProvidersDetect(args []string, stdout io.Writer, stderr io.Writer, deps
|
||||
func parseProviderDetectArgs(args []string) (providerDetectOptions, bool, error) {
|
||||
options := providerDetectOptions{}
|
||||
for _, arg := range args {
|
||||
switch {
|
||||
case arg == "-h" || arg == "--help" || arg == "help":
|
||||
switch arg {
|
||||
case "-h", "--help", "help":
|
||||
return options, true, nil
|
||||
case arg == "--json":
|
||||
case "--json":
|
||||
options.json = true
|
||||
default:
|
||||
return options, false, execUsageError{fmt.Sprintf("unexpected argument %q", arg)}
|
||||
|
||||
@@ -244,19 +244,19 @@ func parseUsageArgs(args []string) (usageOptions, bool, error) {
|
||||
func FormatReport(report usage.Report, insertions int, deletions int) string {
|
||||
var builder strings.Builder
|
||||
builder.WriteString("Usage report (cost is a reconstructed estimate)\n\n")
|
||||
builder.WriteString(fmt.Sprintf("%-12s %10s %14s %14s\n", "date", "requests", "tokens", "est. cost"))
|
||||
fmt.Fprintf(&builder, "%-12s %10s %14s %14s\n", "date", "requests", "tokens", "est. cost")
|
||||
for _, bucket := range report.Buckets {
|
||||
builder.WriteString(fmt.Sprintf("%-12s %10d %14s %14s\n",
|
||||
bucket.Date, bucket.Requests, groupThousands(bucket.TotalTokens), formatUSD(bucket.TotalCost)))
|
||||
fmt.Fprintf(&builder, "%-12s %10d %14s %14s\n",
|
||||
bucket.Date, bucket.Requests, groupThousands(bucket.TotalTokens), formatUSD(bucket.TotalCost))
|
||||
}
|
||||
builder.WriteString(fmt.Sprintf("\n%-12s %10d %14s %14s\n",
|
||||
"total", report.Total.Requests, groupThousands(report.Total.TotalTokens), formatUSD(report.Total.TotalCost)))
|
||||
fmt.Fprintf(&builder, "\n%-12s %10d %14s %14s\n",
|
||||
"total", report.Total.Requests, groupThousands(report.Total.TotalTokens), formatUSD(report.Total.TotalCost))
|
||||
|
||||
builder.WriteString(fmt.Sprintf("\nnet LOC (estimate): +%d / -%d = %d\n",
|
||||
insertions, deletions, report.NetLOC))
|
||||
fmt.Fprintf(&builder, "\nnet LOC (estimate): +%d / -%d = %d\n",
|
||||
insertions, deletions, report.NetLOC)
|
||||
if report.NetLOCPositive {
|
||||
builder.WriteString(fmt.Sprintf("tokens per net LOC: %.1f\n", report.TokensPerNetLOC))
|
||||
builder.WriteString(fmt.Sprintf("est. cost per net LOC: %s\n", formatUSD(report.CostPerNetLOC)))
|
||||
fmt.Fprintf(&builder, "tokens per net LOC: %.1f\n", report.TokensPerNetLOC)
|
||||
fmt.Fprintf(&builder, "est. cost per net LOC: %s\n", formatUSD(report.CostPerNetLOC))
|
||||
} else {
|
||||
builder.WriteString("tokens per net LOC: n/a (net LOC <= 0)\n")
|
||||
builder.WriteString("est. cost per net LOC: n/a (net LOC <= 0)\n")
|
||||
|
||||
+121
-16
@@ -8,6 +8,7 @@ import (
|
||||
"os/exec"
|
||||
"runtime"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -17,7 +18,11 @@ func LoadProviderCommand(command string) (FileConfig, error) {
|
||||
stdout, stderr, err := runProviderCommand(command, providerCommandTimeout)
|
||||
if err != nil {
|
||||
if errors.Is(err, errProviderCommandTimeout) {
|
||||
return FileConfig{}, fmt.Errorf("provider command timed out after 5s")
|
||||
// Report which phase ran out of time, and anything the command
|
||||
// managed to say. This used to be a bare string with the underlying
|
||||
// error and stderr both dropped, which left a user whose command was
|
||||
// slow on a cold machine with nothing to act on.
|
||||
return FileConfig{}, fmt.Errorf("provider command timed out after %s: %w%s", providerCommandTimeout, err, commandOutput(stderr))
|
||||
}
|
||||
return FileConfig{}, fmt.Errorf("provider command failed: %w%s", err, commandOutput(stderr))
|
||||
}
|
||||
@@ -40,20 +45,111 @@ func LoadProviderCommand(command string) (FileConfig, error) {
|
||||
|
||||
var errProviderCommandTimeout = errors.New("provider command timeout")
|
||||
|
||||
// providerCommandDrainGrace bounds how long the timeout path waits for cmd.Wait
|
||||
// to return after the process tree has been terminated. Terminate normally makes
|
||||
// that immediate; this exists so a descendant that refuses to die cannot extend
|
||||
// a call whose deadline has already expired.
|
||||
const providerCommandDrainGrace = 2 * time.Second
|
||||
|
||||
// syncBuffer serialises the command's I/O pump against reads from this package.
|
||||
// On the timeout path runProviderCommand can return while cmd.Wait is still
|
||||
// draining, so the buffers outlive the call and a plain bytes.Buffer would be
|
||||
// read and written concurrently.
|
||||
type syncBuffer struct {
|
||||
mu sync.Mutex
|
||||
buf bytes.Buffer
|
||||
}
|
||||
|
||||
func (b *syncBuffer) Write(p []byte) (int, error) {
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
return b.buf.Write(p)
|
||||
}
|
||||
|
||||
// snapshot copies out what has been written so far. A copy rather than the
|
||||
// buffer's own slice, which the pump may still append to.
|
||||
func (b *syncBuffer) snapshot() []byte {
|
||||
b.mu.Lock()
|
||||
defer b.mu.Unlock()
|
||||
return append([]byte(nil), b.buf.Bytes()...)
|
||||
}
|
||||
|
||||
// awaitWithGrace waits for done and gives up after grace, reporting whether the
|
||||
// wait completed rather than expired.
|
||||
//
|
||||
// Split out from the timeout path so the bound can be asserted on its own. In
|
||||
// place it is effectively untestable: Terminate kills the tree, so Wait returns
|
||||
// promptly and an unbounded receive would behave identically in every test that
|
||||
// is not pathological, which is exactly how the old unbounded receive survived.
|
||||
func awaitWithGrace(done <-chan error, grace time.Duration) bool {
|
||||
timer := time.NewTimer(grace)
|
||||
defer timer.Stop()
|
||||
select {
|
||||
case <-done:
|
||||
return true
|
||||
case <-timer.C:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// runProviderCommand runs a provider command under a real deadline.
|
||||
//
|
||||
// "Real" is the point: timeout used to bound only the wait, so the call could
|
||||
// take arbitrarily longer than it. Two phases sat outside it. Starting the
|
||||
// process ran before the clock began, and on Windows that is CREATE_SUSPENDED,
|
||||
// job-object creation and assignment, and a system-wide thread snapshot, none of
|
||||
// which is fast on a cold or contended machine. Draining after termination was
|
||||
// unbounded as well. Both are inside the budget now, so the deadline is an upper
|
||||
// bound rather than a floor.
|
||||
func runProviderCommand(command string, timeout time.Duration) ([]byte, []byte, error) {
|
||||
cmd := shellCommand(command)
|
||||
// Bound the time Wait spends draining I/O pipes after the process exits,
|
||||
// in case an orphaned descendant still holds the write ends open.
|
||||
cmd.WaitDelay = time.Second
|
||||
|
||||
var stdout bytes.Buffer
|
||||
var stderr bytes.Buffer
|
||||
cmd.Stdout = &stdout
|
||||
cmd.Stderr = &stderr
|
||||
stdout := &syncBuffer{}
|
||||
stderr := &syncBuffer{}
|
||||
cmd.Stdout = stdout
|
||||
cmd.Stderr = stderr
|
||||
|
||||
proc, err := startCommandProcess(cmd)
|
||||
if err != nil {
|
||||
return nil, nil, err
|
||||
// Armed before the process is started, so process creation is inside the
|
||||
// budget rather than ahead of it.
|
||||
deadline := time.NewTimer(timeout)
|
||||
defer deadline.Stop()
|
||||
|
||||
type startResult struct {
|
||||
proc *commandProcess
|
||||
err error
|
||||
}
|
||||
// Buffered, so the goroutine can finish and exit even when nobody is left
|
||||
// waiting on it because the deadline already fired.
|
||||
started := make(chan startResult, 1)
|
||||
go func() {
|
||||
proc, err := startCommandProcess(cmd)
|
||||
started <- startResult{proc: proc, err: err}
|
||||
}()
|
||||
|
||||
var proc *commandProcess
|
||||
select {
|
||||
case result := <-started:
|
||||
if result.err != nil {
|
||||
return stdout.snapshot(), stderr.snapshot(), result.err
|
||||
}
|
||||
proc = result.proc
|
||||
case <-deadline.C:
|
||||
// The start is still in flight and will eventually hand back whatever it
|
||||
// created, so cleanup is handed off rather than abandoned. Wait reaps the
|
||||
// process; without it a started-but-orphaned command becomes a zombie.
|
||||
go func() {
|
||||
result := <-started
|
||||
if result.err != nil {
|
||||
return
|
||||
}
|
||||
result.proc.Terminate()
|
||||
_ = cmd.Wait()
|
||||
result.proc.Close()
|
||||
}()
|
||||
return stdout.snapshot(), stderr.snapshot(), fmt.Errorf("%w: starting the command exceeded %s", errProviderCommandTimeout, timeout)
|
||||
}
|
||||
defer proc.Close()
|
||||
|
||||
@@ -62,9 +158,6 @@ func runProviderCommand(command string, timeout time.Duration) ([]byte, []byte,
|
||||
done <- cmd.Wait()
|
||||
}()
|
||||
|
||||
timer := time.NewTimer(timeout)
|
||||
defer timer.Stop()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
if err != nil {
|
||||
@@ -77,13 +170,25 @@ func runProviderCommand(command string, timeout time.Duration) ([]byte, []byte,
|
||||
proc.Terminate()
|
||||
}
|
||||
if errors.Is(err, exec.ErrWaitDelay) {
|
||||
return stdout.Bytes(), stderr.Bytes(), errProviderCommandTimeout
|
||||
// Wrap rather than replace. A descendant holding the inherited pipes
|
||||
// open and the command never finishing at all are different failures
|
||||
// that both collapsed into the same bare sentinel, which left callers
|
||||
// and tests unable to tell which one happened. Callers that only ask
|
||||
// errors.Is(err, errProviderCommandTimeout) are unaffected, so the
|
||||
// message LoadProviderCommand reports is unchanged.
|
||||
return stdout.snapshot(), stderr.snapshot(), fmt.Errorf("%w: %w", errProviderCommandTimeout, err)
|
||||
}
|
||||
return stdout.Bytes(), stderr.Bytes(), err
|
||||
case <-timer.C:
|
||||
return stdout.snapshot(), stderr.snapshot(), err
|
||||
case <-deadline.C:
|
||||
proc.Terminate()
|
||||
<-done
|
||||
return stdout.Bytes(), stderr.Bytes(), errProviderCommandTimeout
|
||||
// Bounded, unlike the bare receive this replaces. Terminate kills the
|
||||
// tree so Wait almost always returns at once, but "almost always" was
|
||||
// doing real work here: a tree that resists termination used to hold the
|
||||
// call open indefinitely, long past the deadline that had just expired.
|
||||
// Returning early leaves cmd.Wait running, which is why the buffers are
|
||||
// synchronised.
|
||||
awaitWithGrace(done, providerCommandDrainGrace)
|
||||
return stdout.snapshot(), stderr.snapshot(), fmt.Errorf("%w: the command did not finish within %s", errProviderCommandTimeout, timeout)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+271
-26
@@ -1,7 +1,9 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strconv"
|
||||
@@ -87,9 +89,32 @@ func TestLoadProviderCommandTimeout(t *testing.T) {
|
||||
if elapsed > maxElapsed {
|
||||
t.Fatalf("timeout returned after %s, want roughly 5s", elapsed)
|
||||
}
|
||||
assertProcessTerminated(t, pidFile)
|
||||
assertProcessTerminatedIfStarted(t, pidFile)
|
||||
}
|
||||
|
||||
// providerCommandTestBudget bounds the two background-child tests below, and is
|
||||
// deliberately NOT providerCommandTimeout. Those tests assert which error each
|
||||
// cmd.Wait path yields and that the leftover descendant is killed; neither is a
|
||||
// claim about how fast Windows can start a process. Driving them through
|
||||
// LoadProviderCommand billed the fixture's own startup to the production 5s
|
||||
// budget, so on a loaded runner the outer timer won the select in
|
||||
// runProviderCommand: the nonzero-exit test saw a timeout instead of the exit
|
||||
// status it asserts, and the timer's Terminate killed the background child
|
||||
// before it recorded its PID, putting the PID file permanently out of reach no
|
||||
// matter how long the assertion polled for it.
|
||||
//
|
||||
// This is a hang-breaker, not a tuned margin. Nothing in a healthy run waits on
|
||||
// it, since both tests return about one WaitDelay after the fixture exits.
|
||||
const providerCommandTestBudget = 60 * time.Second
|
||||
|
||||
// backgroundChildLifetime must exceed providerCommandTestBudget so that a child
|
||||
// dying of old age is unobservable: any run where runProviderCommand has not
|
||||
// returned yet trips the budget first. That is what lets both tests drop their
|
||||
// old elapsed-versus-lifetime bounds. "Still alive when the call returned" is
|
||||
// now true by construction, so a child found dead afterwards can only have been
|
||||
// killed by proc.Terminate.
|
||||
const backgroundChildLifetime = 120 * time.Second
|
||||
|
||||
// TestLoadProviderCommandTerminatesBackgroundChild covers a command that
|
||||
// exits immediately but leaves a detached child holding the inherited
|
||||
// stdout/stderr pipes open (e.g. `sleep 600 & exit`). cmd.Wait() only
|
||||
@@ -99,21 +124,20 @@ func TestLoadProviderCommandTerminatesBackgroundChild(t *testing.T) {
|
||||
pidFile := filepath.Join(t.TempDir(), "bg.pid")
|
||||
command := writeCommand(t, commandScript{
|
||||
Stdout: `{"name":"cmd","provider":"openai","apiKey":"sk-command","model":"gpt-command"}`,
|
||||
BackgroundSleepSeconds: 10,
|
||||
BackgroundSleepSeconds: int(backgroundChildLifetime / time.Second),
|
||||
BackgroundPidFile: pidFile,
|
||||
})
|
||||
|
||||
start := time.Now()
|
||||
_, err := LoadProviderCommand(command)
|
||||
elapsed := time.Since(start)
|
||||
_, _, err := runProviderCommand(command, providerCommandTestBudget)
|
||||
if err == nil {
|
||||
t.Fatal("LoadProviderCommand() error = nil, want error from WaitDelay-bounded wait")
|
||||
t.Fatal("runProviderCommand() error = nil, want error from WaitDelay-bounded wait")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "timed out after 5s") {
|
||||
t.Fatalf("error = %q, want timeout", err.Error())
|
||||
}
|
||||
if elapsed > 4*time.Second {
|
||||
t.Fatalf("returned after %s, want well under the 5s provider-command timeout since WaitDelay (1s) should trigger termination", elapsed)
|
||||
// Name the exact path. The outer-timer path reports the bare sentinel, so
|
||||
// matching only that, or the "timed out after 5s" text this test used to
|
||||
// match, would also be satisfied by a slow runner taking precisely the path
|
||||
// this test exists to rule out.
|
||||
if !errors.Is(err, errProviderCommandTimeout) || !errors.Is(err, exec.ErrWaitDelay) {
|
||||
t.Fatalf("error = %v, want errProviderCommandTimeout wrapping exec.ErrWaitDelay", err)
|
||||
}
|
||||
assertProcessTerminated(t, pidFile)
|
||||
}
|
||||
@@ -129,21 +153,62 @@ func TestLoadProviderCommandTerminatesBackgroundChildOnFailure(t *testing.T) {
|
||||
command := writeCommand(t, commandScript{
|
||||
Stderr: "boom",
|
||||
ExitCode: 7,
|
||||
BackgroundSleepSeconds: 10,
|
||||
BackgroundSleepSeconds: int(backgroundChildLifetime / time.Second),
|
||||
BackgroundPidFile: pidFile,
|
||||
})
|
||||
|
||||
start := time.Now()
|
||||
_, err := LoadProviderCommand(command)
|
||||
elapsed := time.Since(start)
|
||||
if err == nil {
|
||||
t.Fatal("LoadProviderCommand() error = nil, want failure from nonzero exit")
|
||||
_, _, err := runProviderCommand(command, providerCommandTestBudget)
|
||||
// The shell's own exit status has to survive. A timeout here means the
|
||||
// budget swallowed it, which is exactly what this test kept reporting on CI.
|
||||
if errors.Is(err, errProviderCommandTimeout) {
|
||||
t.Fatalf("error = %v, want the shell's exit status rather than a timeout", err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "exit status") {
|
||||
t.Fatalf("error = %q, want exit status failure", err.Error())
|
||||
var exitErr *exec.ExitError
|
||||
if !errors.As(err, &exitErr) {
|
||||
t.Fatalf("error = %v, want *exec.ExitError from the nonzero exit", err)
|
||||
}
|
||||
if elapsed > 4*time.Second {
|
||||
t.Fatalf("returned after %s, want well under the 5s provider-command timeout since WaitDelay (1s) should trigger termination", elapsed)
|
||||
if exitErr.ExitCode() != 7 {
|
||||
t.Fatalf("exit code = %d, want 7", exitErr.ExitCode())
|
||||
}
|
||||
assertProcessTerminated(t, pidFile)
|
||||
}
|
||||
|
||||
func TestAssertProcessTerminatedWaitsForDelayedPIDFile(t *testing.T) {
|
||||
pidFile := filepath.Join(t.TempDir(), "delayed.pid")
|
||||
firstMissing := make(chan struct{})
|
||||
writeDone := make(chan error, 1)
|
||||
go func() {
|
||||
<-firstMissing
|
||||
tempFile := pidFile + ".tmp"
|
||||
if err := os.WriteFile(tempFile, []byte("2147483647\n"), 0o600); err != nil {
|
||||
writeDone <- err
|
||||
return
|
||||
}
|
||||
writeDone <- os.Rename(tempFile, pidFile)
|
||||
}()
|
||||
|
||||
missingObserved := false
|
||||
assertProcessTerminatedAfterMissing(t, pidFile, func() {
|
||||
missingObserved = true
|
||||
close(firstMissing)
|
||||
})
|
||||
if !missingObserved {
|
||||
t.Fatal("PID file existed before the missing-file polling path was exercised")
|
||||
}
|
||||
if err := <-writeDone; err != nil {
|
||||
t.Fatalf("write delayed pid file: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func assertProcessTerminatedIfStarted(t *testing.T, pidFile string) {
|
||||
t.Helper()
|
||||
|
||||
// The basic timeout fixture is intentionally unsynchronized. On a loaded
|
||||
// Windows runner, the cold PowerShell child can be terminated before it
|
||||
// records its PID; the synchronized background-child tests below retain
|
||||
// the strict PID and liveness checks.
|
||||
if _, err := os.Stat(pidFile); errors.Is(err, os.ErrNotExist) {
|
||||
return
|
||||
}
|
||||
assertProcessTerminated(t, pidFile)
|
||||
}
|
||||
@@ -151,16 +216,41 @@ func TestLoadProviderCommandTerminatesBackgroundChildOnFailure(t *testing.T) {
|
||||
func assertProcessTerminated(t *testing.T, pidFile string) {
|
||||
t.Helper()
|
||||
|
||||
data, err := os.ReadFile(pidFile)
|
||||
assertProcessTerminatedAfterMissing(t, pidFile, nil)
|
||||
}
|
||||
|
||||
func assertProcessTerminatedAfterMissing(t *testing.T, pidFile string, onMissing func()) {
|
||||
t.Helper()
|
||||
|
||||
deadline := time.Now().Add(3 * time.Second)
|
||||
var data []byte
|
||||
var err error
|
||||
missingObserved := false
|
||||
for time.Now().Before(deadline) {
|
||||
data, err = os.ReadFile(pidFile)
|
||||
if err == nil {
|
||||
break
|
||||
}
|
||||
if !errors.Is(err, os.ErrNotExist) {
|
||||
t.Fatalf("read sleeper pid file: %v", err)
|
||||
}
|
||||
if !missingObserved {
|
||||
missingObserved = true
|
||||
if onMissing != nil {
|
||||
onMissing()
|
||||
}
|
||||
}
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("read sleeper pid file: %v", err)
|
||||
t.Fatalf("read sleeper pid file before timeout: %v", err)
|
||||
}
|
||||
pid, err := strconv.Atoi(strings.TrimSpace(string(data)))
|
||||
if err != nil {
|
||||
t.Fatalf("parse sleeper pid %q: %v", data, err)
|
||||
}
|
||||
|
||||
deadline := time.Now().Add(3 * time.Second)
|
||||
deadline = time.Now().Add(3 * time.Second)
|
||||
for time.Now().Before(deadline) {
|
||||
if !processAlive(pid) {
|
||||
return
|
||||
@@ -237,8 +327,33 @@ func writeCommand(t *testing.T, script commandScript) string {
|
||||
"Set-Content -LiteralPath '" + psSingleQuote(readyFile) + "' -Value ready -Encoding Ascii; " +
|
||||
"Start-Sleep -Seconds " + itoa(script.BackgroundSleepSeconds)
|
||||
lines = append(lines, "start /B powershell -NoProfile -Command \""+bgSleep+"\"")
|
||||
waitForReady := "while (-not (Test-Path -LiteralPath '" + psSingleQuote(readyFile) + "')) { Start-Sleep -Milliseconds 10 }"
|
||||
lines = append(lines, "powershell -NoProfile -Command \""+waitForReady+"\"")
|
||||
// Wait for the child to record its PID before exiting, so the PID is
|
||||
// on disk while the tree is still alive: runProviderCommand kills the
|
||||
// tree one WaitDelay after the script exits, and a child that has not
|
||||
// written by then never will.
|
||||
//
|
||||
// This waits in cmd itself. Spawning a second PowerShell just to poll
|
||||
// cost another interpreter cold start, and that start is pure overhead
|
||||
// added AFTER the PID is already written. On a loaded runner it pushed
|
||||
// the script's exit past the 5s provider-command timeout, so the
|
||||
// nonzero-exit test saw a timeout instead of the exit status it
|
||||
// asserts. The wait is also bounded now: giving up and letting the PID
|
||||
// assertion fail is far more diagnosable than blocking until the
|
||||
// timeout fires and reporting an unrelated error. Each iteration is
|
||||
// about a second, since `ping -n 2` sleeps between its two echoes,
|
||||
// so the ceiling is roughly 15s: far longer than any cold PowerShell
|
||||
// start seen on a runner, and far shorter than
|
||||
// providerCommandTestBudget, so giving up surfaces a legible
|
||||
// PID-file failure rather than a timeout that hides the real cause.
|
||||
lines = append(lines,
|
||||
":zeroWaitReady",
|
||||
"if exist \""+readyFile+"\" goto zeroReady",
|
||||
"set /a ZERO_READY_TRIES+=1",
|
||||
"if %ZERO_READY_TRIES% GEQ 15 goto zeroReady",
|
||||
"ping -n 2 127.0.0.1 >nul",
|
||||
"goto zeroWaitReady",
|
||||
":zeroReady",
|
||||
)
|
||||
}
|
||||
lines = append(lines, "exit /b "+itoa(script.ExitCode))
|
||||
if err := os.WriteFile(path, []byte(strings.Join(lines, "\r\n")), 0o700); err != nil {
|
||||
@@ -299,3 +414,133 @@ func itoa(value int) string {
|
||||
}
|
||||
return string(digits[index:])
|
||||
}
|
||||
|
||||
// The whole point of #811: providerCommandTimeout has to be an upper bound on
|
||||
// how long the call takes, not a floor. Process creation used to run before the
|
||||
// clock started and the post-termination drain was unbounded, so a slow or
|
||||
// contended machine could block startup for far longer than the error text
|
||||
// promised.
|
||||
//
|
||||
// This one guards the end-to-end contract against gross regressions. It is NOT
|
||||
// the assertion that pins the drain bound: Terminate kills the tree, so Wait
|
||||
// returns promptly and this passes even with the drain unbounded. That property
|
||||
// is pinned by TestAwaitWithGraceGivesUp, and the start-phase bound by
|
||||
// TestRunProviderCommandCountsProcessStartAgainstTheDeadline.
|
||||
func TestRunProviderCommandBoundsTotalDuration(t *testing.T) {
|
||||
pidFile := filepath.Join(t.TempDir(), "bound.pid")
|
||||
command := writeCommand(t, commandScript{SleepSeconds: 60, PidFile: pidFile})
|
||||
|
||||
const budget = 2 * time.Second
|
||||
start := time.Now()
|
||||
_, _, err := runProviderCommand(command, budget)
|
||||
elapsed := time.Since(start)
|
||||
|
||||
if !errors.Is(err, errProviderCommandTimeout) {
|
||||
t.Fatalf("error = %v, want the timeout sentinel", err)
|
||||
}
|
||||
// Budget, plus the bounded drain, plus room for a loaded runner. A failure
|
||||
// here means some phase is outside the deadline again, which is the bug.
|
||||
ceiling := budget + providerCommandDrainGrace + 8*time.Second
|
||||
if elapsed > ceiling {
|
||||
t.Fatalf("returned after %s with a %s budget; the timeout is not bounding the call (ceiling %s)", elapsed, budget, ceiling)
|
||||
}
|
||||
assertProcessTerminatedIfStarted(t, pidFile)
|
||||
}
|
||||
|
||||
// The timeout error has to carry something an operator can act on. It used to be
|
||||
// a bare string with the underlying error and the command's stderr both dropped,
|
||||
// so a user whose provider command was slow got no clue which phase ran out or
|
||||
// what the command had said.
|
||||
func TestLoadProviderCommandTimeoutReportsDiagnostics(t *testing.T) {
|
||||
command := writeCommand(t, commandScript{
|
||||
Stderr: "provider is warming up",
|
||||
SleepSeconds: 30,
|
||||
PidFile: filepath.Join(t.TempDir(), "diag.pid"),
|
||||
})
|
||||
|
||||
_, err := LoadProviderCommand(command)
|
||||
if err == nil {
|
||||
t.Fatal("LoadProviderCommand() error = nil, want timeout")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "timed out") {
|
||||
t.Fatalf("error = %q, want a timeout message", err.Error())
|
||||
}
|
||||
// Wrapped, so callers can still classify it.
|
||||
if !errors.Is(err, errProviderCommandTimeout) {
|
||||
t.Fatalf("error = %v, want errors.Is against the timeout sentinel", err)
|
||||
}
|
||||
// And it must say which phase, rather than only that time ran out.
|
||||
if !strings.Contains(err.Error(), "did not finish") && !strings.Contains(err.Error(), "starting the command") {
|
||||
t.Fatalf("error = %q, want it to name the phase that exceeded the deadline", err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
// The output buffers are written by cmd.Wait's I/O pump, which keeps running
|
||||
// after the timeout path has returned them to the caller, so a plain
|
||||
// bytes.Buffer would be read and written concurrently. Asserted on the buffer
|
||||
// directly: driving it end to end does not reliably produce the overlap, because
|
||||
// a terminated command usually stops writing before the snapshot is taken, so
|
||||
// such a test passes with the locking removed and proves nothing.
|
||||
//
|
||||
// Only meaningful under -race, which CI runs.
|
||||
func TestSyncBufferAllowsConcurrentWriteAndSnapshot(t *testing.T) {
|
||||
buffer := &syncBuffer{}
|
||||
writing := make(chan struct{})
|
||||
go func() {
|
||||
defer close(writing)
|
||||
for i := 0; i < 2000; i++ {
|
||||
_, _ = buffer.Write([]byte("x"))
|
||||
}
|
||||
}()
|
||||
for i := 0; i < 2000; i++ {
|
||||
_ = buffer.snapshot()
|
||||
}
|
||||
<-writing
|
||||
if len(buffer.snapshot()) != 2000 {
|
||||
t.Fatalf("buffer holds %d bytes, want 2000", len(buffer.snapshot()))
|
||||
}
|
||||
}
|
||||
|
||||
// The drain after termination must be bounded. Asserted directly on the helper,
|
||||
// because in place it is untestable without a tree that refuses to die: Wait
|
||||
// normally returns at once, which is precisely why an unbounded receive sat
|
||||
// there unnoticed. A channel that never fires is the pathological case made
|
||||
// cheap.
|
||||
func TestAwaitWithGraceGivesUp(t *testing.T) {
|
||||
never := make(chan error)
|
||||
start := time.Now()
|
||||
if awaitWithGrace(never, 50*time.Millisecond) {
|
||||
t.Fatal("reported completion for a wait that never finished")
|
||||
}
|
||||
if elapsed := time.Since(start); elapsed > 5*time.Second {
|
||||
t.Fatalf("gave up after %s, want roughly the grace period", elapsed)
|
||||
}
|
||||
// Still reports completion when the wait does finish, or the timeout path
|
||||
// would always claim the tree hung.
|
||||
done := make(chan error, 1)
|
||||
done <- nil
|
||||
if !awaitWithGrace(done, time.Second) {
|
||||
t.Fatal("reported expiry for a wait that had already finished")
|
||||
}
|
||||
}
|
||||
|
||||
// Process creation has to be inside the budget. It used to run before the clock
|
||||
// started, and on Windows that phase is CREATE_SUSPENDED plus job-object setup
|
||||
// plus a system-wide thread snapshot, none of it fast on a contended machine.
|
||||
//
|
||||
// A budget that is already spent when the call begins can only be exceeded
|
||||
// during the start, so the phase named in the error is what distinguishes the
|
||||
// two orderings: with the clock started afterwards, the start would always
|
||||
// complete first and the failure would come from the wait instead.
|
||||
func TestRunProviderCommandCountsProcessStartAgainstTheDeadline(t *testing.T) {
|
||||
command := writeCommand(t, commandScript{Stdout: `{"name":"cmd","provider":"openai","apiKey":"sk-x","model":"m"}`})
|
||||
|
||||
_, _, err := runProviderCommand(command, time.Nanosecond)
|
||||
if !errors.Is(err, errProviderCommandTimeout) {
|
||||
t.Fatalf("error = %v, want the timeout sentinel", err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "starting the command") {
|
||||
t.Fatalf("error = %q, want the start phase to be the one that exceeded the deadline; "+
|
||||
"anything else means process creation is outside the budget again", err.Error())
|
||||
}
|
||||
}
|
||||
@@ -87,7 +87,7 @@ func mergeMCPServer(base MCPServerConfig, next MCPServerConfig, canReenable bool
|
||||
// server the user explicitly enabled. The only layer allowed to
|
||||
// override an explicit higher-scope decision is the CLI override scope
|
||||
// (canReenable=true).
|
||||
if canReenable || !(baseDisabledSet && baseDisabled != next.Disabled) {
|
||||
if canReenable || !baseDisabledSet || baseDisabled == next.Disabled {
|
||||
base.Disabled = next.Disabled
|
||||
}
|
||||
}
|
||||
|
||||
@@ -100,7 +100,7 @@ func (d *deepgramTranscriber) StreamTranscribe(ctx context.Context, chunks <-cha
|
||||
}
|
||||
default:
|
||||
}
|
||||
return compose(), fmt.Errorf("Deepgram stream error: %w", err)
|
||||
return compose(), fmt.Errorf("Deepgram stream error: %w", err) //nolint:staticcheck // Preserve established user-facing error text.
|
||||
}
|
||||
if typ != websocket.MessageText {
|
||||
continue
|
||||
|
||||
@@ -48,7 +48,7 @@ func buildMinimalPDF(text string) []byte {
|
||||
buf.WriteString("0 " + strconv.Itoa(len(offsets)+1) + "\n")
|
||||
buf.WriteString("0000000000 65535 f \n")
|
||||
for _, off := range offsets {
|
||||
buf.WriteString(fmt.Sprintf("%010d 00000 n \n", off))
|
||||
fmt.Fprintf(&buf, "%010d 00000 n \n", off)
|
||||
}
|
||||
buf.WriteString("trailer\n<< /Size " + strconv.Itoa(len(offsets)+1) + " /Root 1 0 R >>\n")
|
||||
buf.WriteString("startxref\n" + strconv.Itoa(xrefStart) + "\n%%EOF\n")
|
||||
@@ -277,7 +277,7 @@ func buildEmptyTextPDF() []byte {
|
||||
buf.WriteString("0 " + strconv.Itoa(len(offsets)+1) + "\n")
|
||||
buf.WriteString("0000000000 65535 f \n")
|
||||
for _, off := range offsets {
|
||||
buf.WriteString(fmt.Sprintf("%010d 00000 n \n", off))
|
||||
fmt.Fprintf(&buf, "%010d 00000 n \n", off)
|
||||
}
|
||||
buf.WriteString("trailer\n<< /Size " + strconv.Itoa(len(offsets)+1) + " /Root 1 0 R >>\n")
|
||||
buf.WriteString("startxref\n" + strconv.Itoa(xrefStart) + "\n%%EOF\n")
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package lsp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
)
|
||||
@@ -64,7 +65,7 @@ func TestDecodeSymbols(t *testing.T) {
|
||||
|
||||
func TestNavigateUnknownOp(t *testing.T) {
|
||||
m := NewManager(t.TempDir())
|
||||
_, _, ok, err := m.Navigate(nil, NavRequest{Op: "bogus", Path: "x.go"})
|
||||
_, _, ok, err := m.Navigate(context.TODO(), NavRequest{Op: "bogus", Path: "x.go"})
|
||||
if err == nil {
|
||||
t.Fatal("expected an error for an unknown nav op")
|
||||
}
|
||||
@@ -76,7 +77,7 @@ func TestNavigateUnknownOp(t *testing.T) {
|
||||
func TestNavigateUnsupportedExtensionDegrades(t *testing.T) {
|
||||
// A file type with no configured server degrades to ok=false, no error.
|
||||
m := NewManager(t.TempDir())
|
||||
_, _, ok, err := m.Navigate(nil, NavRequest{Op: NavDefinition, Path: "notes.unknownext", Line: 1, Character: 1})
|
||||
_, _, ok, err := m.Navigate(context.TODO(), NavRequest{Op: NavDefinition, Path: "notes.unknownext", Line: 1, Character: 1})
|
||||
if err != nil {
|
||||
t.Fatalf("unsupported extension should not error, got %v", err)
|
||||
}
|
||||
|
||||
@@ -391,7 +391,7 @@ func validateEndpoint(ctx context.Context, endpoint string, resolver Resolver, a
|
||||
return endpointSafetyError{message: "provider connectivity URL is unsafe: localhost hosts are blocked"}
|
||||
}
|
||||
if addr, err := netip.ParseAddr(normalized); err == nil {
|
||||
if reason := blockedAddrReason(addr); reason != "" && !(allowLoopbackOrPrivate && (addr.IsLoopback() || addr.IsPrivate())) {
|
||||
if reason := blockedAddrReason(addr); reason != "" && (!allowLoopbackOrPrivate || (!addr.IsLoopback() && !addr.IsPrivate())) {
|
||||
return endpointSafetyError{message: "provider connectivity URL is unsafe: " + reason}
|
||||
}
|
||||
return nil
|
||||
@@ -407,7 +407,7 @@ func validateEndpoint(ctx context.Context, endpoint string, resolver Resolver, a
|
||||
return endpointSafetyError{message: "provider connectivity host resolved to no addresses"}
|
||||
}
|
||||
for _, addr := range addrs {
|
||||
if reason := blockedAddrReason(addr); reason != "" && !(allowLoopbackOrPrivate && (addr.IsLoopback() || addr.IsPrivate())) {
|
||||
if reason := blockedAddrReason(addr); reason != "" && (!allowLoopbackOrPrivate || (!addr.IsLoopback() && !addr.IsPrivate())) {
|
||||
return endpointSafetyError{message: "provider connectivity URL is unsafe: " + reason}
|
||||
}
|
||||
}
|
||||
@@ -529,7 +529,7 @@ func safeDialContext(resolver Resolver, allowLoopbackOrPrivate bool) func(contex
|
||||
return nil, err
|
||||
}
|
||||
if addr, parseErr := netip.ParseAddr(host); parseErr == nil {
|
||||
if reason := blockedAddrReason(addr); reason != "" && !(allowLoopbackOrPrivate && (addr.IsLoopback() || addr.IsPrivate())) {
|
||||
if reason := blockedAddrReason(addr); reason != "" && (!allowLoopbackOrPrivate || (!addr.IsLoopback() && !addr.IsPrivate())) {
|
||||
return nil, endpointSafetyError{message: "provider connectivity URL is unsafe: " + reason}
|
||||
}
|
||||
return dialer.DialContext(ctx, network, address)
|
||||
@@ -542,7 +542,7 @@ func safeDialContext(resolver Resolver, allowLoopbackOrPrivate bool) func(contex
|
||||
return nil, endpointSafetyError{message: "provider connectivity host resolved to no addresses"}
|
||||
}
|
||||
for _, addr := range addrs {
|
||||
if reason := blockedAddrReason(addr); reason != "" && !(allowLoopbackOrPrivate && (addr.IsLoopback() || addr.IsPrivate())) {
|
||||
if reason := blockedAddrReason(addr); reason != "" && (!allowLoopbackOrPrivate || (!addr.IsLoopback() && !addr.IsPrivate())) {
|
||||
return nil, endpointSafetyError{message: "provider connectivity URL is unsafe: " + reason}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -62,7 +62,7 @@ func TestDetectLocalRuntimesReportsReachableRuntime(t *testing.T) {
|
||||
if !detected[0].Reachable {
|
||||
t.Fatalf("runtime should be reachable: %#v", detected[0])
|
||||
}
|
||||
if detected[0].Models == nil || len(detected[0].Models) == 0 || detected[0].Models[0] != "llama3.1" {
|
||||
if len(detected[0].Models) == 0 || detected[0].Models[0] != "llama3.1" {
|
||||
t.Fatalf("expected discovered model list, got %#v", detected[0].Models)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -188,7 +188,7 @@ func writePromptLine(builder *strings.Builder, format string, args ...any) {
|
||||
if builder.Len() > 0 {
|
||||
builder.WriteByte('\n')
|
||||
}
|
||||
builder.WriteString(fmt.Sprintf(format, args...))
|
||||
fmt.Fprintf(builder, format, args...)
|
||||
}
|
||||
|
||||
func clampBudget(text string, budget int) string {
|
||||
|
||||
@@ -225,7 +225,7 @@ func (backend Backend) SandboxEnvMarkers(policy Policy) []string {
|
||||
if policy.Mode == ModeDisabled {
|
||||
return nil
|
||||
}
|
||||
if !(backend.CommandWrapping && backend.Available) && backend.Name != BackendWSL {
|
||||
if (!backend.CommandWrapping || !backend.Available) && backend.Name != BackendWSL {
|
||||
return nil
|
||||
}
|
||||
name := backend.Name
|
||||
|
||||
@@ -153,10 +153,7 @@ func unsafeCommandPrefix(prefix []string) bool {
|
||||
return true
|
||||
}
|
||||
}
|
||||
if unsafeCommandPrefixLauncher(prefix[0]) {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
return unsafeCommandPrefixLauncher(prefix[0])
|
||||
}
|
||||
|
||||
func unsafeCommandPrefixPart(part string) bool {
|
||||
|
||||
@@ -249,7 +249,7 @@ func (engine *Engine) shellSandboxActive(policy Policy) bool {
|
||||
return false
|
||||
}
|
||||
backend := engine.backend
|
||||
if !(backend.Available && backend.Executable != "" && backend.CommandWrapping && backend.NativeIsolation) {
|
||||
if !backend.Available || backend.Executable == "" || !backend.CommandWrapping || !backend.NativeIsolation {
|
||||
return false
|
||||
}
|
||||
// On Windows the command is only actually wrapped once `zero sandbox setup`
|
||||
|
||||
@@ -2,6 +2,7 @@ package sandbox
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
@@ -485,6 +486,28 @@ func TestEngineDeniesOutOfWorkspacePaths(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnginePromptsForOutOfWorkspaceReadWithReadSpecificReason(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
outside := outsideDefaultTempPath(root, "external")
|
||||
engine := NewEngine(EngineOptions{WorkspaceRoot: root, Policy: DefaultPolicy()})
|
||||
|
||||
decision := engine.Evaluate(context.Background(), Request{
|
||||
ToolName: "list_directory",
|
||||
SideEffect: SideEffectRead,
|
||||
Permission: PermissionAllow,
|
||||
WorkspaceRoot: root,
|
||||
Args: map[string]any{"path": outside},
|
||||
})
|
||||
|
||||
want := fmt.Sprintf("Reading %s requires access outside the workspace.", outside)
|
||||
if decision.Action != ActionPrompt || decision.Block == nil || !decision.Block.Recoverable {
|
||||
t.Fatalf("outside read decision = %#v, want recoverable prompt", decision)
|
||||
}
|
||||
if decision.Reason != want || decision.Block.Reason != want {
|
||||
t.Fatalf("outside read reasons = decision %q, block %q; want %q", decision.Reason, decision.Block.Reason, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEnginePrecheckReportsBlocksBeforeExecution(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
outside := outsideDefaultTempPath(root, "escape.txt")
|
||||
|
||||
+155
-7
@@ -16,7 +16,11 @@ import (
|
||||
"github.com/Gitlawb/zero/internal/redaction"
|
||||
)
|
||||
|
||||
const grantSchemaVersion = 3
|
||||
const (
|
||||
grantSchemaVersion = 3
|
||||
grantLockTimeout = 5 * time.Second
|
||||
grantLockRetry = 10 * time.Millisecond
|
||||
)
|
||||
|
||||
type Grant struct {
|
||||
ToolName string `json:"toolName"`
|
||||
@@ -139,7 +143,40 @@ func (store *GrantStore) ConsumeMigrationNotice() (string, error) {
|
||||
}
|
||||
store.mu.Lock()
|
||||
defer store.mu.Unlock()
|
||||
state, err := store.readState()
|
||||
// Look for a pending notice WITHOUT taking the interprocess lock. Acquiring
|
||||
// that lock creates the grants directory and opens <grants>.lockfile, so doing
|
||||
// it unconditionally would make this call require a WRITABLE grants directory
|
||||
// even when it has nothing to write. Every startup and every `zero exec` calls
|
||||
// this, so a user with no grants file whose configured grants path (or its
|
||||
// parent) sits on a read-only mount would fail outright with "failed to
|
||||
// migrate sandbox grants", where reading alone reports an empty state and
|
||||
// writes nothing.
|
||||
state, needsMigration, err := store.readStateFile(false)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if !needsMigration && (state.Migration == nil || !state.Migration.NoticePending) {
|
||||
return "", nil
|
||||
}
|
||||
// Past here a write is genuinely required — either a schema/policy migration,
|
||||
// which only a lock holder may perform, or clearing NoticePending — so the
|
||||
// lock is warranted and a read-only grants path legitimately fails (as it did
|
||||
// before this lock existed, via readState's own migration path).
|
||||
//
|
||||
// Clearing the pending flag is a read-modify-write like any other mutator, so
|
||||
// it takes the same interprocess lock: two frontends starting at once must not
|
||||
// both read NoticePending, both print the notice, and clobber each other's
|
||||
// write of the rest of the state.
|
||||
unlock, err := store.lockStateFile()
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
defer unlock()
|
||||
// Re-read under the lock. The unlocked peek above is only a snapshot: another
|
||||
// process may have consumed the notice or migrated the file while this one
|
||||
// waited, and when needsMigration was set that snapshot's contents are
|
||||
// meaningless by readStateFile's contract.
|
||||
state, err = store.readStateLocked()
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
@@ -161,7 +198,13 @@ func (store *GrantStore) Grant(input GrantInput) (Grant, error) {
|
||||
}
|
||||
store.mu.Lock()
|
||||
defer store.mu.Unlock()
|
||||
state, err := store.readState()
|
||||
unlock, err := store.lockStateFile()
|
||||
if err != nil {
|
||||
return Grant{}, err
|
||||
}
|
||||
defer unlock()
|
||||
|
||||
state, err := store.readStateLocked()
|
||||
if err != nil {
|
||||
return Grant{}, err
|
||||
}
|
||||
@@ -193,7 +236,13 @@ func (store *GrantStore) GrantCommandPrefix(input CommandPrefixInput) (CommandPr
|
||||
}
|
||||
store.mu.Lock()
|
||||
defer store.mu.Unlock()
|
||||
state, err := store.readState()
|
||||
unlock, err := store.lockStateFile()
|
||||
if err != nil {
|
||||
return CommandPrefixGrant{}, err
|
||||
}
|
||||
defer unlock()
|
||||
|
||||
state, err := store.readStateLocked()
|
||||
if err != nil {
|
||||
return CommandPrefixGrant{}, err
|
||||
}
|
||||
@@ -337,7 +386,13 @@ func (store *GrantStore) Revoke(toolName string) (int, error) {
|
||||
}
|
||||
store.mu.Lock()
|
||||
defer store.mu.Unlock()
|
||||
state, err := store.readState()
|
||||
unlock, err := store.lockStateFile()
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
defer unlock()
|
||||
|
||||
state, err := store.readStateLocked()
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
@@ -372,7 +427,13 @@ func (store *GrantStore) RevokePath(toolName string, scopePath string) (int, err
|
||||
}
|
||||
store.mu.Lock()
|
||||
defer store.mu.Unlock()
|
||||
state, err := store.readState()
|
||||
unlock, err := store.lockStateFile()
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
defer unlock()
|
||||
|
||||
state, err := store.readStateLocked()
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
@@ -407,7 +468,13 @@ func (store *GrantStore) RevokePath(toolName string, scopePath string) (int, err
|
||||
func (store *GrantStore) Clear() (int, error) {
|
||||
store.mu.Lock()
|
||||
defer store.mu.Unlock()
|
||||
state, err := store.readState()
|
||||
unlock, err := store.lockStateFile()
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
defer unlock()
|
||||
|
||||
state, err := store.readStateLocked()
|
||||
if err != nil {
|
||||
return 0, err
|
||||
}
|
||||
@@ -537,7 +604,52 @@ func reconcileScope(scope string, kind ScopeKind) (string, ScopeKind) {
|
||||
return scope, kind
|
||||
}
|
||||
|
||||
// readState reads the grant file for a caller that does NOT hold the
|
||||
// interprocess lock (the lookup/list paths). A schema or policy migration has to
|
||||
// write — a backup plus the migrated state — so it is performed under the lock,
|
||||
// and the read is retaken there because another process may have migrated the
|
||||
// file while this one waited.
|
||||
func (store *GrantStore) readState() (grantFile, error) {
|
||||
state, needsMigration, err := store.readStateFile(false)
|
||||
if err != nil || !needsMigration {
|
||||
return state, err
|
||||
}
|
||||
unlock, err := store.lockStateFile()
|
||||
if err != nil {
|
||||
return grantFile{}, err
|
||||
}
|
||||
defer unlock()
|
||||
return store.readStateLocked()
|
||||
}
|
||||
|
||||
// readStateLocked reads the grant file for a caller that already holds the
|
||||
// interprocess lock, so a migration writes through directly instead of
|
||||
// re-acquiring it (which would deadlock against the caller's own lock).
|
||||
func (store *GrantStore) readStateLocked() (grantFile, error) {
|
||||
state, _, err := store.readStateFile(true)
|
||||
return state, err
|
||||
}
|
||||
|
||||
// readStateFile decodes the grant file. needsMigration reports that the state on
|
||||
// disk must be rewritten (legacy schema or a changed policy version) and that the
|
||||
// caller has to retake this read while holding the interprocess lock; the
|
||||
// returned state is meaningless in that case.
|
||||
func (store *GrantStore) readStateFile(locked bool) (grantFile, bool, error) {
|
||||
state, err := store.decodeState(locked)
|
||||
if err != nil {
|
||||
if errors.Is(err, errGrantMigrationNeedsLock) {
|
||||
return grantFile{}, true, nil
|
||||
}
|
||||
return grantFile{}, false, err
|
||||
}
|
||||
return state, false, nil
|
||||
}
|
||||
|
||||
// errGrantMigrationNeedsLock is the internal signal from decodeState that the
|
||||
// file needs a migration write, which only a lock holder may perform.
|
||||
var errGrantMigrationNeedsLock = errors.New("sandbox grants file needs a migration write")
|
||||
|
||||
func (store *GrantStore) decodeState(locked bool) (grantFile, error) {
|
||||
data, err := os.ReadFile(store.filePath)
|
||||
if err != nil {
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
@@ -559,12 +671,18 @@ func (store *GrantStore) readState() (grantFile, error) {
|
||||
return grantFile{}, store.invalidGrantFile(err)
|
||||
}
|
||||
if head.SchemaVersion == 1 || head.SchemaVersion == 2 {
|
||||
if !locked {
|
||||
return grantFile{}, errGrantMigrationNeedsLock
|
||||
}
|
||||
return store.migrateLegacyState(data, head.SchemaVersion, head.Grants, head.CommandPrefixes)
|
||||
}
|
||||
if head.SchemaVersion != grantSchemaVersion {
|
||||
return grantFile{}, fmt.Errorf("invalid sandbox grants file at %s: unsupported schemaVersion", store.filePath)
|
||||
}
|
||||
if head.PolicyVersion != execution.PolicyVersion {
|
||||
if !locked {
|
||||
return grantFile{}, errGrantMigrationNeedsLock
|
||||
}
|
||||
return store.migrateChangedPolicy(data, head.PolicyVersion, head.Grants, head.CommandPrefixes)
|
||||
}
|
||||
buckets := map[string][]Grant{}
|
||||
@@ -800,6 +918,36 @@ func (store *GrantStore) writeState(state grantFile) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (store *GrantStore) lockStateFile() (func(), error) {
|
||||
lockPath := store.filePath + ".lockfile"
|
||||
if err := os.MkdirAll(filepath.Dir(store.filePath), 0o700); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
file, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
deadline := time.Now().Add(grantLockTimeout)
|
||||
for {
|
||||
locked, err := tryLockGrantFile(file)
|
||||
if err != nil {
|
||||
_ = file.Close()
|
||||
return nil, err
|
||||
}
|
||||
if locked {
|
||||
return func() {
|
||||
_ = unlockGrantFile(file)
|
||||
_ = file.Close()
|
||||
}, nil
|
||||
}
|
||||
if time.Now().After(deadline) {
|
||||
_ = file.Close()
|
||||
return nil, fmt.Errorf("timed out waiting for sandbox grants lock at %s", lockPath)
|
||||
}
|
||||
time.Sleep(grantLockRetry)
|
||||
}
|
||||
}
|
||||
|
||||
func normalizeStoredGrant(name string, grant Grant) (Grant, error) {
|
||||
if strings.TrimSpace(grant.ToolName) == "" {
|
||||
grant.ToolName = name
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
//go:build !windows
|
||||
|
||||
package sandbox
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
"syscall"
|
||||
)
|
||||
|
||||
func tryLockGrantFile(file *os.File) (bool, error) {
|
||||
err := syscall.Flock(int(file.Fd()), syscall.LOCK_EX|syscall.LOCK_NB)
|
||||
if err == nil {
|
||||
return true, nil
|
||||
}
|
||||
if errors.Is(err, syscall.EWOULDBLOCK) || errors.Is(err, syscall.EAGAIN) {
|
||||
return false, nil
|
||||
}
|
||||
return false, err
|
||||
}
|
||||
|
||||
func unlockGrantFile(file *os.File) error {
|
||||
return syscall.Flock(int(file.Fd()), syscall.LOCK_UN)
|
||||
}
|
||||
@@ -0,0 +1,60 @@
|
||||
//go:build windows
|
||||
|
||||
package sandbox
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
"syscall"
|
||||
"unsafe"
|
||||
)
|
||||
|
||||
const (
|
||||
grantLockfileFailImmediately = 0x00000001
|
||||
grantLockfileExclusiveLock = 0x00000002
|
||||
grantErrorLockViolation = syscall.Errno(33)
|
||||
grantErrorSharingViolation = syscall.Errno(32)
|
||||
)
|
||||
|
||||
var (
|
||||
grantKernel32 = syscall.NewLazyDLL("kernel32.dll")
|
||||
grantProcLockFileEx = grantKernel32.NewProc("LockFileEx")
|
||||
grantProcUnlockFileEx = grantKernel32.NewProc("UnlockFileEx")
|
||||
)
|
||||
|
||||
func tryLockGrantFile(file *os.File) (bool, error) {
|
||||
var overlapped syscall.Overlapped
|
||||
result, _, err := grantProcLockFileEx.Call(
|
||||
file.Fd(),
|
||||
uintptr(grantLockfileExclusiveLock|grantLockfileFailImmediately),
|
||||
0,
|
||||
1,
|
||||
0,
|
||||
uintptr(unsafe.Pointer(&overlapped)),
|
||||
)
|
||||
if result != 0 {
|
||||
return true, nil
|
||||
}
|
||||
if errors.Is(err, grantErrorLockViolation) || errors.Is(err, grantErrorSharingViolation) {
|
||||
return false, nil
|
||||
}
|
||||
if err == syscall.Errno(0) {
|
||||
return false, nil
|
||||
}
|
||||
return false, err
|
||||
}
|
||||
|
||||
func unlockGrantFile(file *os.File) error {
|
||||
var overlapped syscall.Overlapped
|
||||
result, _, err := grantProcUnlockFileEx.Call(
|
||||
file.Fd(),
|
||||
0,
|
||||
1,
|
||||
0,
|
||||
uintptr(unsafe.Pointer(&overlapped)),
|
||||
)
|
||||
if result != 0 || err == syscall.Errno(0) {
|
||||
return nil
|
||||
}
|
||||
return err
|
||||
}
|
||||
@@ -1,10 +1,13 @@
|
||||
package sandbox
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestGrantStorePersistsListsRevokesAndClears(t *testing.T) {
|
||||
@@ -70,6 +73,246 @@ func TestGrantStorePersistsListsRevokesAndClears(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestGrantStoreSerializesWritesAcrossStores(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "sandbox-grants.json")
|
||||
storeA, err := NewGrantStore(StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewGrantStore A returned error: %v", err)
|
||||
}
|
||||
storeB, err := NewGrantStore(StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewGrantStore B returned error: %v", err)
|
||||
}
|
||||
|
||||
unlock, err := storeA.lockStateFile()
|
||||
if err != nil {
|
||||
t.Fatalf("lockStateFile returned error: %v", err)
|
||||
}
|
||||
|
||||
done := make(chan error, 1)
|
||||
go func() {
|
||||
_, err := storeB.Grant(GrantInput{ToolName: "write_file", Decision: GrantAllow})
|
||||
done <- err
|
||||
}()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
unlock()
|
||||
t.Fatalf("Grant completed while another store held the file lock: %v", err)
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
// Expected: storeB is waiting for storeA to release the lock.
|
||||
}
|
||||
|
||||
unlock()
|
||||
select {
|
||||
case err := <-done:
|
||||
if err != nil {
|
||||
t.Fatalf("Grant returned error after file lock release: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("Grant did not complete after file lock release")
|
||||
}
|
||||
}
|
||||
|
||||
// TestGrantStoreSerializesMigrationNoticeAcrossStores covers the other
|
||||
// read-modify-write on the state file: clearing the pending flag. Two frontends
|
||||
// starting at once must not both read it as pending and clobber each other.
|
||||
func TestGrantStoreSerializesMigrationNoticeAcrossStores(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "sandbox-grants.json")
|
||||
if err := writeText(path, `{"schemaVersion":1,"grants":{"write_file":{"toolName":"write_file","decision":"allow","approvedAt":"2026-06-05T14:30:00Z"}}}`); err != nil {
|
||||
t.Fatalf("write v1 grants: %v", err)
|
||||
}
|
||||
storeA, err := NewGrantStore(StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewGrantStore A returned error: %v", err)
|
||||
}
|
||||
storeB, err := NewGrantStore(StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewGrantStore B returned error: %v", err)
|
||||
}
|
||||
// Migrate first, so the notice is pending and the lock contention below is
|
||||
// about consuming it rather than about the migration write.
|
||||
if _, err := storeA.List(); err != nil {
|
||||
t.Fatalf("List returned error: %v", err)
|
||||
}
|
||||
|
||||
unlock, err := storeA.lockStateFile()
|
||||
if err != nil {
|
||||
t.Fatalf("lockStateFile returned error: %v", err)
|
||||
}
|
||||
done := make(chan error, 1)
|
||||
go func() {
|
||||
notice, err := storeB.ConsumeMigrationNotice()
|
||||
if err == nil && notice == "" {
|
||||
err = errors.New("expected the pending migration notice")
|
||||
}
|
||||
done <- err
|
||||
}()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
unlock()
|
||||
t.Fatalf("ConsumeMigrationNotice completed while another store held the file lock: %v", err)
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
// Expected: storeB is waiting for storeA to release the lock.
|
||||
}
|
||||
|
||||
unlock()
|
||||
select {
|
||||
case err := <-done:
|
||||
if err != nil {
|
||||
t.Fatalf("ConsumeMigrationNotice after lock release: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("ConsumeMigrationNotice did not complete after file lock release")
|
||||
}
|
||||
// The flag write landed, so the notice is not reported twice.
|
||||
if again, err := storeA.ConsumeMigrationNotice(); err != nil || again != "" {
|
||||
t.Fatalf("second ConsumeMigrationNotice = %q err=%v, want empty", again, err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestConsumeMigrationNoticeWritesNothingWithoutAPendingNotice is the regression
|
||||
// test for jatmn's #755 finding: ConsumeMigrationNotice took the interprocess
|
||||
// lock unconditionally, and acquiring that lock MkdirAlls the grants directory
|
||||
// and creates <grants>.lockfile. Every startup and every `zero exec` calls this,
|
||||
// so it turned "read the grants state" into "require a writable grants
|
||||
// directory" — a user with no grants file whose grants path sat on a read-only
|
||||
// mount failed outright with "failed to migrate sandbox grants".
|
||||
//
|
||||
// Asserting that nothing is created is the portable form of that requirement: it
|
||||
// holds identically on every OS and as any user, unlike a read-only-directory
|
||||
// test (see the sibling test, which cannot run everywhere).
|
||||
func TestConsumeMigrationNoticeWritesNothingWithoutAPendingNotice(t *testing.T) {
|
||||
dir := filepath.Join(t.TempDir(), "config")
|
||||
path := filepath.Join(dir, "sandbox-grants.json")
|
||||
store, err := NewGrantStore(StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewGrantStore returned error: %v", err)
|
||||
}
|
||||
|
||||
notice, err := store.ConsumeMigrationNotice()
|
||||
if err != nil {
|
||||
t.Fatalf("ConsumeMigrationNotice with no grants file: %v", err)
|
||||
}
|
||||
if notice != "" {
|
||||
t.Fatalf("notice = %q, want empty when nothing was migrated", notice)
|
||||
}
|
||||
// Nothing at all may be created: not the lock file, not the grants file, not
|
||||
// even the directory that would hold them.
|
||||
if _, err := os.Lstat(dir); !os.IsNotExist(err) {
|
||||
entries, _ := os.ReadDir(dir)
|
||||
names := make([]string, 0, len(entries))
|
||||
for _, entry := range entries {
|
||||
names = append(names, entry.Name())
|
||||
}
|
||||
t.Fatalf("the grants directory was created (contents: %v); reading a state with no pending notice must not require a writable directory", names)
|
||||
}
|
||||
}
|
||||
|
||||
// TestConsumeMigrationNoticeToleratesAReadOnlyGrantsDirectory exercises the
|
||||
// actual failure from the same finding, rather than only its portable proxy
|
||||
// above: with the grants directory read-only and no grants file, the pre-fix
|
||||
// code failed at MkdirAll/OpenFile for the lock file.
|
||||
//
|
||||
// It cannot run everywhere — Windows ignores the mode bits used here, and root
|
||||
// bypasses directory permissions entirely — so it skips instead of pretending to
|
||||
// cover those platforms.
|
||||
func TestConsumeMigrationNoticeToleratesAReadOnlyGrantsDirectory(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("Unix directory mode bits do not deny writes on Windows")
|
||||
}
|
||||
if os.Geteuid() == 0 {
|
||||
t.Skip("root bypasses directory permissions, so a read-only directory cannot be simulated")
|
||||
}
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "sandbox-grants.json")
|
||||
store, err := NewGrantStore(StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewGrantStore returned error: %v", err)
|
||||
}
|
||||
if err := os.Chmod(dir, 0o500); err != nil {
|
||||
t.Skipf("cannot make the grants directory read-only: %v", err)
|
||||
}
|
||||
// Restore write permission so t.TempDir's cleanup can remove the directory.
|
||||
t.Cleanup(func() { _ = os.Chmod(dir, 0o700) })
|
||||
// Confirm the directory really is read-only here; otherwise this test would
|
||||
// pass without exercising anything.
|
||||
if probe, err := os.OpenFile(filepath.Join(dir, ".probe"), os.O_CREATE|os.O_RDWR, 0o600); err == nil {
|
||||
_ = probe.Close()
|
||||
t.Skip("this filesystem still allows writes to a 0500 directory")
|
||||
}
|
||||
|
||||
notice, err := store.ConsumeMigrationNotice()
|
||||
if err != nil {
|
||||
t.Fatalf("ConsumeMigrationNotice on a read-only grants directory: %v", err)
|
||||
}
|
||||
if notice != "" {
|
||||
t.Fatalf("notice = %q, want empty when nothing was migrated", notice)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGrantStoreMigrationTakesTheFileLock covers the migrations that write
|
||||
// through readState: a legacy file must not be rewritten (nor its backup written)
|
||||
// while another process holds the lock, or two processes could both migrate and
|
||||
// one would clobber the other's result.
|
||||
func TestGrantStoreMigrationTakesTheFileLock(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "sandbox-grants.json")
|
||||
original := `{"schemaVersion":1,"grants":{"write_file":{"toolName":"write_file","decision":"allow","approvedAt":"2026-06-05T14:30:00Z"}}}`
|
||||
if err := writeText(path, original); err != nil {
|
||||
t.Fatalf("write v1 grants: %v", err)
|
||||
}
|
||||
storeA, err := NewGrantStore(StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewGrantStore A returned error: %v", err)
|
||||
}
|
||||
storeB, err := NewGrantStore(StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewGrantStore B returned error: %v", err)
|
||||
}
|
||||
|
||||
unlock, err := storeA.lockStateFile()
|
||||
if err != nil {
|
||||
t.Fatalf("lockStateFile returned error: %v", err)
|
||||
}
|
||||
done := make(chan error, 1)
|
||||
go func() {
|
||||
_, err := storeB.List()
|
||||
done <- err
|
||||
}()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
unlock()
|
||||
t.Fatalf("the migration ran while another store held the file lock: %v", err)
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
if raw, err := os.ReadFile(path); err != nil {
|
||||
unlock()
|
||||
t.Fatalf("ReadFile grants: %v", err)
|
||||
} else if string(raw) != original {
|
||||
unlock()
|
||||
t.Fatalf("the grant file was rewritten while the lock was held:\n%s", raw)
|
||||
}
|
||||
}
|
||||
|
||||
unlock()
|
||||
select {
|
||||
case err := <-done:
|
||||
if err != nil {
|
||||
t.Fatalf("List after lock release: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("the migration did not complete after file lock release")
|
||||
}
|
||||
raw, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatalf("ReadFile migrated grants: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(raw), `"schemaVersion": 3`) {
|
||||
t.Fatalf("grant file was not migrated after the lock was released:\n%s", raw)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGrantStoreMigratesExactV1GrantAndReportsOnce(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
path := filepath.Join(root, "sandbox-grants.json")
|
||||
|
||||
@@ -38,7 +38,7 @@ func ApplyLandlockFilesystemProfile(profile PermissionProfile, cwd string) error
|
||||
}
|
||||
writeRoots := landlockWritableRoots(profile.FileSystem, cwd)
|
||||
if len(writeRoots) == 0 {
|
||||
return errors.New("Landlock requires at least one writable root")
|
||||
return errors.New("Landlock requires at least one writable root") //nolint:staticcheck // Preserve the proper name and established user-facing text.
|
||||
}
|
||||
return installLandlockFilesystemRules(writeRoots)
|
||||
}
|
||||
|
||||
@@ -4,7 +4,7 @@ package sandbox
|
||||
|
||||
import "errors"
|
||||
|
||||
var ErrLandlockUnsupported = errors.New("Landlock is only supported on Linux")
|
||||
var ErrLandlockUnsupported = errors.New("Landlock is only supported on Linux") //nolint:staticcheck // Preserve the exported sentinel's established text.
|
||||
|
||||
func ApplyLandlockFilesystemProfile(profile PermissionProfile, cwd string) error {
|
||||
return ErrLandlockUnsupported
|
||||
|
||||
@@ -203,11 +203,24 @@ func dedupeScopeRoots(roots []string) []string {
|
||||
// only on outside_workspace results. The returned block always carries
|
||||
// the caller's original requestedPath.
|
||||
func (s *Scope) validate(requestedPath string) *pathBlock {
|
||||
return s.validateAgainstRoots(requestedPath, s.Roots())
|
||||
return withOutsideWorkspaceAccessReason(s.validateAgainstRoots(requestedPath, s.Roots()), requestedPath, SideEffectWrite)
|
||||
}
|
||||
|
||||
func (s *Scope) validateRead(requestedPath string) *pathBlock {
|
||||
return s.validateAgainstRoots(requestedPath, s.ReadRoots())
|
||||
return withOutsideWorkspaceAccessReason(s.validateAgainstRoots(requestedPath, s.ReadRoots()), requestedPath, SideEffectRead)
|
||||
}
|
||||
|
||||
func withOutsideWorkspaceAccessReason(block *pathBlock, requestedPath string, sideEffect SideEffect) *pathBlock {
|
||||
if block == nil || block.Code != BlockOutsideWorkspace || !strings.Contains(block.Reason, " is outside the workspace") {
|
||||
return block
|
||||
}
|
||||
switch sideEffect {
|
||||
case SideEffectRead:
|
||||
block.Reason = fmt.Sprintf("Reading %s requires access outside the workspace.", requestedPath)
|
||||
case SideEffectWrite, SideEffectOutOfWorkspace:
|
||||
block.Reason = fmt.Sprintf("Writing to %s requires access outside the workspace. Use /add-dir or --add-dir to allow writes there.", requestedPath)
|
||||
}
|
||||
return block
|
||||
}
|
||||
|
||||
func (s *Scope) validateAgainstRoots(requestedPath string, roots []string) *pathBlock {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package sandbox
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
@@ -279,6 +280,45 @@ func TestScopeValidateAllowsAnyRootButRelativeOnlyWorkspace(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestScopeValidateReadExplainsReadAccessOutsideWorkspace(t *testing.T) {
|
||||
workspace := t.TempDir()
|
||||
scope, err := NewScope(workspace, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("NewScope: %v", err)
|
||||
}
|
||||
|
||||
outside := outsideDefaultTempPath(workspace, "elsewhere.txt")
|
||||
block := scope.validateRead(outside)
|
||||
if block == nil {
|
||||
t.Fatal("validateRead(outside workspace) = nil, want block")
|
||||
}
|
||||
want := fmt.Sprintf("Reading %s requires access outside the workspace.", outside)
|
||||
if block.Reason != want {
|
||||
t.Fatalf("block.Reason = %q, want %q", block.Reason, want)
|
||||
}
|
||||
if strings.Contains(block.Reason, "--add-dir") || strings.Contains(block.Reason, "writes") {
|
||||
t.Fatalf("read block reason must not suggest write access: %q", block.Reason)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScopeValidateWriteExplainsWriteAccessOutsideWorkspace(t *testing.T) {
|
||||
workspace := t.TempDir()
|
||||
scope, err := NewScope(workspace, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("NewScope: %v", err)
|
||||
}
|
||||
|
||||
outside := outsideDefaultTempPath(workspace, "elsewhere.txt")
|
||||
block := scope.validate(outside)
|
||||
if block == nil {
|
||||
t.Fatal("validate(outside workspace) = nil, want block")
|
||||
}
|
||||
want := fmt.Sprintf("Writing to %s requires access outside the workspace. Use /add-dir or --add-dir to allow writes there.", outside)
|
||||
if block.Reason != want {
|
||||
t.Fatalf("block.Reason = %q, want %q", block.Reason, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestScopeValidateKeepsSymlinkTraversalProtection(t *testing.T) {
|
||||
workspace := t.TempDir()
|
||||
outside := outsideDefaultTempPath(workspace, "symlink-target")
|
||||
|
||||
@@ -10,7 +10,7 @@ import (
|
||||
"strings"
|
||||
)
|
||||
|
||||
var ErrWindowsNetworkEnforcementUnavailable = errors.New("Windows sandbox network enforcement is not available")
|
||||
var ErrWindowsNetworkEnforcementUnavailable = errors.New("Windows sandbox network enforcement is not available") //nolint:staticcheck // Preserve the exported sentinel's established text.
|
||||
|
||||
const (
|
||||
windowsWFPProviderKey = "2e31d31c-3948-4753-9117-e5d1a6496f41"
|
||||
|
||||
@@ -17,6 +17,7 @@ func Info(dir string, name string) (SkillInfo, bool) {
|
||||
info.Hash = entry.Hash
|
||||
}
|
||||
}
|
||||
info.HashDrift = skillHashDrift(skill, info.Hash)
|
||||
return info, true
|
||||
}
|
||||
|
||||
|
||||
@@ -76,9 +76,10 @@ type LockEntry struct {
|
||||
// SkillInfo bundles a discovered skill with its recorded source and hash, for
|
||||
// `skill info`.
|
||||
type SkillInfo struct {
|
||||
Skill Skill `json:"skill"`
|
||||
Source string `json:"source,omitempty"`
|
||||
Hash string `json:"hash,omitempty"`
|
||||
Skill Skill `json:"skill"`
|
||||
Source string `json:"source,omitempty"`
|
||||
Hash string `json:"hash,omitempty"`
|
||||
HashDrift bool `json:"hashDrift"`
|
||||
}
|
||||
|
||||
// Install fetches the skill at options.Source and copies its SKILL.md into
|
||||
@@ -275,6 +276,7 @@ func InfoFromRoots(primaryDir string, roots []string, name string) (SkillInfo, b
|
||||
info.Hash = entry.Hash
|
||||
}
|
||||
}
|
||||
info.HashDrift = skillHashDrift(skill, info.Hash)
|
||||
return info, true
|
||||
}
|
||||
|
||||
@@ -481,3 +483,19 @@ func hashContent(data []byte) string {
|
||||
sum := sha256.Sum256(data)
|
||||
return "sha256:" + hex.EncodeToString(sum[:])
|
||||
}
|
||||
|
||||
// skillHashDrift reports whether the on-disk SKILL.md no longer matches the
|
||||
// lockfile hash recorded at install time. Missing lock hashes never count as
|
||||
// drift (agents-only / unlocked skills). A locked skill whose SKILL.md cannot
|
||||
// be read is treated as drifted — not as a clean match.
|
||||
func skillHashDrift(skill Skill, lockHash string) bool {
|
||||
lockHash = strings.TrimSpace(lockHash)
|
||||
if lockHash == "" {
|
||||
return false
|
||||
}
|
||||
data, err := os.ReadFile(skill.Path)
|
||||
if err != nil {
|
||||
return true
|
||||
}
|
||||
return hashContent(data) != lockHash
|
||||
}
|
||||
@@ -407,4 +407,28 @@ func TestInfoReturnsFrontmatterSourceAndHash(t *testing.T) {
|
||||
if info.Hash != installed.Hash {
|
||||
t.Fatalf("Info hash = %q, want %q", info.Hash, installed.Hash)
|
||||
}
|
||||
if info.HashDrift {
|
||||
t.Fatal("expected no hash drift immediately after install")
|
||||
}
|
||||
|
||||
if err := os.WriteFile(info.Skill.Path, []byte("---\nname: demo\ndescription: described\n---\nedited body\n"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
info, ok = Info(destDir, "demo")
|
||||
if !ok {
|
||||
t.Fatal("Info(demo) not found after edit")
|
||||
}
|
||||
if !info.HashDrift {
|
||||
t.Fatal("expected hash drift after SKILL.md edit")
|
||||
}
|
||||
}
|
||||
|
||||
func TestSkillHashDriftUnreadableLockedPath(t *testing.T) {
|
||||
missing := filepath.Join(t.TempDir(), "missing", "SKILL.md")
|
||||
if !skillHashDrift(Skill{Path: missing}, "sha256:deadbeef") {
|
||||
t.Fatal("expected drift when locked SKILL.md cannot be read")
|
||||
}
|
||||
if skillHashDrift(Skill{Path: missing}, "") {
|
||||
t.Fatal("missing lock hash must not count as drift")
|
||||
}
|
||||
}
|
||||
@@ -827,7 +827,7 @@ func runChildProcess(ctx context.Context, binaryPath string, args []string, prog
|
||||
// ExitCode() is -1 when the child was terminated by a signal rather
|
||||
// than exiting. ProcessState.String() is the portable description
|
||||
// (e.g. "signal: killed") — capture it so the failure isn't opaque.
|
||||
signalDesc = exitErr.ProcessState.String()
|
||||
signalDesc = exitErr.String()
|
||||
}
|
||||
} else {
|
||||
return ChildRunResult{Events: events, Stderr: stderr.String(), ExitCode: -1, Started: started}, fmt.Errorf("run specialist child: %w", err)
|
||||
|
||||
@@ -548,7 +548,7 @@ func (m model) switchProviderModel(providerName, modelID string) (model, string,
|
||||
// and a stored OAuth login (e.g. ChatGPT) is a credential too — the profile stays
|
||||
// keyless on purpose so newProvider attaches the bearer resolver + login key.
|
||||
if strings.TrimSpace(target.APIKey) == "" && strings.TrimSpace(target.AuthHeaderValue) == "" &&
|
||||
!(hasDescriptor && descriptor.Local) && !oauthLoginAvailable(target) {
|
||||
(!hasDescriptor || !descriptor.Local) && !oauthLoginAvailable(target) {
|
||||
return m, "Model\nprovider " + strconv.Quote(providerName) + " has no usable credential — run setup or `zero auth login " + providerName + "`.", false, nil
|
||||
}
|
||||
next, err := m.newProvider(target)
|
||||
|
||||
@@ -8,7 +8,6 @@ import (
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
tea "charm.land/bubbletea/v2"
|
||||
"charm.land/lipgloss/v2"
|
||||
"github.com/Gitlawb/zero/internal/dictation"
|
||||
)
|
||||
@@ -87,10 +86,6 @@ func (m model) scrollableTranscriptLayoutView(header string, body transcriptBody
|
||||
return m.renderScrollableTranscriptWindow(frame, bodyWindow, window, width, overlay)
|
||||
}
|
||||
|
||||
func (m model) scrollableTranscriptView(header string, body string, footer string, width int, overlay string) string {
|
||||
return m.scrollableTranscriptLayoutView(header, transcriptBodyLayout{lines: viewLines(body)}, footer, width, overlay)
|
||||
}
|
||||
|
||||
func (m model) overlayMouseTop(overlayHeight int, width int) int {
|
||||
return m.overlayMouseRect(overlayHeight, width).y
|
||||
}
|
||||
@@ -230,17 +225,6 @@ func (d *streamingDecoder) tailLines() []string {
|
||||
return out
|
||||
}
|
||||
|
||||
// ensureAgeTickReschedule is a small helper used after a fade-state change
|
||||
// to start the tick if it's not already running. The age-tick case
|
||||
// short-circuits when fadeActive is false, so calling this on a no-op
|
||||
// transition (e.g. a 0-byte delta) is safe.
|
||||
func (m model) ensureAgeTickReschedule() tea.Cmd {
|
||||
if !m.fadeActive {
|
||||
return nil
|
||||
}
|
||||
return streamingFadeTick()
|
||||
}
|
||||
|
||||
// newSTTDownloadPicker builds the model-download chooser, seeded with the
|
||||
// curated shortlist. The full model list from the release is fetched
|
||||
// asynchronously and merged in (see fetchSTTModelsCmd / handleSTTModelsFetched).
|
||||
@@ -288,11 +272,6 @@ func transcriptViewportStartForFrame(body string, frame transcriptFrameLayout, s
|
||||
return window.start, window.height, frame.bodyRect.y
|
||||
}
|
||||
|
||||
func transcriptViewportStartForLayout(layout transcriptBodyLayout, frame transcriptFrameLayout, scrollOffset int) (int, int, int) {
|
||||
window := transcriptViewportForLayout(layout, frame, scrollOffset).window()
|
||||
return window.start, window.height, frame.bodyRect.y
|
||||
}
|
||||
|
||||
func transcriptViewportForBody(body string, frame transcriptFrameLayout, offset int) transcriptViewport {
|
||||
return newTranscriptViewport(len(viewLines(body)), frame.bodyRect.height, offset)
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
package tui
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
@@ -192,7 +193,7 @@ func TestGitSweepCmdAgainstRealRepo(t *testing.T) {
|
||||
run("commit", "-q", "-m", "seed")
|
||||
write("pre-existing.txt", "dirt\n") // dirty BEFORE the TUI "opens"
|
||||
|
||||
baseline := gitSweepCmd(nil, dir, true)().(gitSweepMsg)
|
||||
baseline := gitSweepCmd(context.TODO(), dir, true)().(gitSweepMsg)
|
||||
if !baseline.ok || len(baseline.files) != 1 || baseline.files[0].path != "pre-existing.txt" {
|
||||
t.Fatalf("baseline should see only the pre-existing dirt: %+v", baseline)
|
||||
}
|
||||
@@ -201,7 +202,7 @@ func TestGitSweepCmdAgainstRealRepo(t *testing.T) {
|
||||
write("scaffolded.txt", "hello\n")
|
||||
write("tracked.txt", "one\ntwo\nthree\nfour\n")
|
||||
|
||||
sweep := gitSweepCmd(nil, dir, false)().(gitSweepMsg)
|
||||
sweep := gitSweepCmd(context.TODO(), dir, false)().(gitSweepMsg)
|
||||
if !sweep.ok {
|
||||
t.Fatal("sweep failed")
|
||||
}
|
||||
@@ -224,7 +225,7 @@ func TestGitSweepCmdAgainstRealRepo(t *testing.T) {
|
||||
run("commit", "-q", "-m", "second")
|
||||
run("mv", "tracked.txt", "renamed.txt")
|
||||
write("renamed.txt", "one\ntwo\nthree\nfour\nfive\n")
|
||||
renameSweep := gitSweepCmd(nil, dir, false)().(gitSweepMsg)
|
||||
renameSweep := gitSweepCmd(context.TODO(), dir, false)().(gitSweepMsg)
|
||||
if !renameSweep.ok {
|
||||
t.Fatal("rename sweep failed")
|
||||
}
|
||||
@@ -237,7 +238,7 @@ func TestGitSweepCmdAgainstRealRepo(t *testing.T) {
|
||||
}
|
||||
|
||||
// Not-a-repo → ok=false (the sweep latches off).
|
||||
if msg := gitSweepCmd(nil, t.TempDir(), false)().(gitSweepMsg); msg.ok {
|
||||
if msg := gitSweepCmd(context.TODO(), t.TempDir(), false)().(gitSweepMsg); msg.ok {
|
||||
t.Fatal("a non-repo should report ok=false")
|
||||
}
|
||||
}
|
||||
@@ -46,7 +46,7 @@ func (m model) settledRow(row transcriptRow, rc rowContext) bool {
|
||||
if row.id != "" && rc.resolved[rcKey(row.runID, row.id)] {
|
||||
return true
|
||||
}
|
||||
return !(m.pending && row.runID != 0 && row.runID == m.activeRunID)
|
||||
return !m.pending || row.runID == 0 || row.runID != m.activeRunID
|
||||
case rowPermission:
|
||||
event := row.permission
|
||||
if event == nil || event.ToolCallID == "" || event.Action != agent.PermissionActionPrompt {
|
||||
@@ -57,7 +57,7 @@ func (m model) settledRow(row transcriptRow, rc rowContext) bool {
|
||||
}
|
||||
// An undecided prompt renders live (with the modal card) until its
|
||||
// decision lands or its run ends.
|
||||
return !(m.pending && row.runID != 0 && row.runID == m.activeRunID)
|
||||
return !m.pending || row.runID == 0 || row.runID != m.activeRunID
|
||||
default:
|
||||
// User/system/error/assistant/ask-user rows are appended in final form;
|
||||
// welcome rows render nothing and settle trivially.
|
||||
|
||||
@@ -290,7 +290,7 @@ func writeTestPDF(t *testing.T, dir, name, text string) string {
|
||||
xrefStart := buf.Len()
|
||||
buf.WriteString("xref\n0 " + strconv.Itoa(len(offsets)+1) + "\n0000000000 65535 f \n")
|
||||
for _, off := range offsets {
|
||||
buf.WriteString(fmt.Sprintf("%010d 00000 n \n", off))
|
||||
fmt.Fprintf(&buf, "%010d 00000 n \n", off)
|
||||
}
|
||||
buf.WriteString("trailer\n<< /Size " + strconv.Itoa(len(offsets)+1) + " /Root 1 0 R >>\nstartxref\n" + strconv.Itoa(xrefStart) + "\n%%EOF\n")
|
||||
|
||||
|
||||
@@ -594,8 +594,8 @@ func (m model) loopListText() string {
|
||||
when = "due"
|
||||
}
|
||||
}
|
||||
b.WriteString(fmt.Sprintf(" %s · %s · iter %d · %s · %s\n",
|
||||
l.id, l.cadenceText(), l.iteration, when, truncateLoopPrompt(l.prompt)))
|
||||
fmt.Fprintf(&b, " %s · %s · iter %d · %s · %s\n",
|
||||
l.id, l.cadenceText(), l.iteration, when, truncateLoopPrompt(l.prompt))
|
||||
}
|
||||
b.WriteString("Stop with /loop stop <id> or /loop stop all.")
|
||||
return b.String()
|
||||
|
||||
+11
-1
@@ -2212,7 +2212,16 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
|
||||
}
|
||||
return m, nil
|
||||
}
|
||||
promptRow := permissionTranscriptRow(permissionEventFromRequest(msg.request))
|
||||
promptEvent := permissionEventFromRequest(msg.request)
|
||||
promptRow := permissionTranscriptRow(promptEvent)
|
||||
// The focused modal owns the actionable reason while the decision is
|
||||
// pending. Keep only structured block context in the transcript row so
|
||||
// the same explanation is not shown immediately above the card.
|
||||
if promptEvent.Block == nil {
|
||||
promptRow.detail = ""
|
||||
} else {
|
||||
promptRow.detail = permissionBlockDetail(promptEvent)
|
||||
}
|
||||
promptRow.runID = msg.runID
|
||||
m.transcript = appendTranscriptRow(m.transcript, promptRow)
|
||||
m.pendingPermission = &pendingPermissionPrompt{
|
||||
@@ -4807,6 +4816,7 @@ func (m model) beginRun(cancel context.CancelFunc) model {
|
||||
// suppressed by a stale preference.
|
||||
m.sidebarHidden = false
|
||||
m.turnStartedAt = m.now()
|
||||
m.lastStreamActivity = m.turnStartedAt
|
||||
m.turnStreamedRunes = 0
|
||||
m.spinnerTicking = true
|
||||
return m
|
||||
|
||||
@@ -1396,7 +1396,7 @@ func TestBeginRunPreparesRunCompletionWarningEachTurn(t *testing.T) {
|
||||
})
|
||||
|
||||
m = m.beginRun(nil)
|
||||
m = m.beginRun(nil)
|
||||
_ = m.beginRun(nil)
|
||||
|
||||
if prepares != 2 {
|
||||
t.Fatalf("PrepareRunCompletionWarning called %d times, want once per run", prepares)
|
||||
@@ -1606,6 +1606,9 @@ func TestPermissionRequestShowsFocusedPrompt(t *testing.T) {
|
||||
if strings.Contains(view, "risk:") || strings.Contains(view, "risk=") {
|
||||
t.Fatalf("focused permission prompt must not render risk labels, got %q", view)
|
||||
}
|
||||
if count := strings.Count(view, request.Reason); count != 1 {
|
||||
t.Fatalf("focused permission reason rendered %d times, want once:\n%s", count, view)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPermissionPromptChoicesResolveDecision(t *testing.T) {
|
||||
@@ -1720,6 +1723,34 @@ func TestPermissionRowRendersSandboxBlocks(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestPermissionRowRendersIdenticalEventAndBlockReasonOnce(t *testing.T) {
|
||||
const reason = "Reading /home/dev/.nvm requires access outside the workspace."
|
||||
event := agent.PermissionEvent{
|
||||
ToolCallID: "call_read",
|
||||
ToolName: "list_directory",
|
||||
Action: agent.PermissionActionDeny,
|
||||
DecisionAction: agent.PermissionDecisionDeny,
|
||||
SideEffect: "read",
|
||||
Reason: reason,
|
||||
Scope: "/home/dev/.nvm",
|
||||
Block: &sandbox.Block{
|
||||
Code: sandbox.BlockOutsideWorkspace,
|
||||
Path: "/home/dev/.nvm",
|
||||
Reason: reason,
|
||||
},
|
||||
}
|
||||
|
||||
row := permissionTranscriptRow(event)
|
||||
if count := strings.Count(event.Reason+"\n"+row.detail, reason); count != 1 {
|
||||
t.Fatalf("permission reason represented %d times, want once: detail=%q", count, row.detail)
|
||||
}
|
||||
for _, want := range []string{"denied by user", "outside workspace", "path: /home/dev/.nvm"} {
|
||||
if !strings.Contains(row.detail, want) {
|
||||
t.Fatalf("permission detail missing %q: %q", want, row.detail)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestAppendTranscriptRowDedupesRuntimeRowsByID(t *testing.T) {
|
||||
event := agent.PermissionEvent{
|
||||
ToolCallID: "call_1",
|
||||
|
||||
+20
-14
@@ -218,20 +218,22 @@ func (m model) handleSetupKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
|
||||
}
|
||||
return m, nil
|
||||
case keyIs(msg, tea.KeyUp) || keyCtrl(msg, 'p'):
|
||||
if m.setup.stage == setupStageMethod {
|
||||
switch m.setup.stage {
|
||||
case setupStageMethod:
|
||||
m.moveSetupMethod(-1)
|
||||
} else if m.setup.stage == setupStageProvider {
|
||||
case setupStageProvider:
|
||||
m.moveSetupProvider(-1)
|
||||
} else if m.setup.stage == setupStageModel {
|
||||
case setupStageModel:
|
||||
m.moveSetupModel(-1)
|
||||
}
|
||||
return m, nil
|
||||
case keyIs(msg, tea.KeyDown) || keyCtrl(msg, 'n'):
|
||||
if m.setup.stage == setupStageMethod {
|
||||
switch m.setup.stage {
|
||||
case setupStageMethod:
|
||||
m.moveSetupMethod(1)
|
||||
} else if m.setup.stage == setupStageProvider {
|
||||
case setupStageProvider:
|
||||
m.moveSetupProvider(1)
|
||||
} else if m.setup.stage == setupStageModel {
|
||||
case setupStageModel:
|
||||
m.moveSetupModel(1)
|
||||
}
|
||||
return m, nil
|
||||
@@ -279,15 +281,17 @@ func (m model) handleSetupKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
|
||||
case "q":
|
||||
return m, tea.Quit
|
||||
case "k":
|
||||
if m.setup.stage == setupStageProvider {
|
||||
switch m.setup.stage {
|
||||
case setupStageProvider:
|
||||
m.moveSetupProvider(-1)
|
||||
} else if m.setup.stage == setupStageModel {
|
||||
case setupStageModel:
|
||||
m.moveSetupModel(-1)
|
||||
}
|
||||
case "j":
|
||||
if m.setup.stage == setupStageProvider {
|
||||
switch m.setup.stage {
|
||||
case setupStageProvider:
|
||||
m.moveSetupProvider(1)
|
||||
} else if m.setup.stage == setupStageModel {
|
||||
case setupStageModel:
|
||||
m.moveSetupModel(1)
|
||||
}
|
||||
}
|
||||
@@ -360,16 +364,18 @@ func (m model) handleSetupMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) {
|
||||
switch {
|
||||
case mouseWheelUp(msg):
|
||||
m.clearMouseSelection()
|
||||
if m.setup.stage == setupStageProvider {
|
||||
switch m.setup.stage {
|
||||
case setupStageProvider:
|
||||
m.moveSetupProvider(-1)
|
||||
} else if m.setup.stage == setupStageModel {
|
||||
case setupStageModel:
|
||||
m.moveSetupModel(-1)
|
||||
}
|
||||
case mouseWheelDown(msg):
|
||||
m.clearMouseSelection()
|
||||
if m.setup.stage == setupStageProvider {
|
||||
switch m.setup.stage {
|
||||
case setupStageProvider:
|
||||
m.moveSetupProvider(1)
|
||||
} else if m.setup.stage == setupStageModel {
|
||||
case setupStageModel:
|
||||
m.moveSetupModel(1)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -126,6 +126,29 @@ func TestPermissionPromptMapsEscalatedSandboxReason(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestPermissionPromptWrapsLongReasonWithoutTruncating(t *testing.T) {
|
||||
request := agent.PermissionRequest{
|
||||
ToolName: "list_directory",
|
||||
SideEffect: "read",
|
||||
Reason: "Reading /home/dev/projects/a-very-long-directory-name/with/nested/configuration requires access outside the workspace.",
|
||||
AvailableDecisions: []agent.PermissionDecisionAction{
|
||||
agent.PermissionDecisionAllow,
|
||||
agent.PermissionDecisionDeny,
|
||||
},
|
||||
}
|
||||
|
||||
card, _ := renderFocusedPermissionPrompt(request, 0, false, "", 60)
|
||||
got := plainRender(t, card)
|
||||
for _, want := range []string{"Reading", "/home/dev/projects", "requires access outside the workspace."} {
|
||||
if !strings.Contains(got, want) {
|
||||
t.Fatalf("permission card truncated reason; missing %q:\n%s", want, got)
|
||||
}
|
||||
}
|
||||
if strings.Contains(got, "…") {
|
||||
t.Fatalf("permission card must wrap the decision reason instead of truncating it:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPermissionOptionsExposePersistentCommandPrefixApproval(t *testing.T) {
|
||||
request := agent.PermissionRequest{
|
||||
ToolName: "bash",
|
||||
|
||||
@@ -687,7 +687,7 @@ func TestProviderWizardPersistsPastedKeyToUserConfig(t *testing.T) {
|
||||
updated, _ = next.Update(testKey(tea.KeyEnter))
|
||||
next = updated.(model)
|
||||
updated, _ = next.Update(testKey(tea.KeyEnter))
|
||||
next = updated.(model)
|
||||
_ = updated.(model)
|
||||
|
||||
if captured.APIKey != secret {
|
||||
t.Fatalf("captured APIKey = %q, want pasted secret", captured.APIKey)
|
||||
|
||||
@@ -1131,7 +1131,13 @@ func renderFocusedPermissionPrompt(request agent.PermissionRequest, cursor int,
|
||||
}
|
||||
lines := []string{top, body}
|
||||
if reason := permissionDisplayReason(request.Reason); reason != "" {
|
||||
lines = append(lines, fill(zeroTheme.muted).Render(reason))
|
||||
reasonWidth := width
|
||||
if widthTier(width) != tierTiny {
|
||||
reasonWidth = maxInt(1, width-4)
|
||||
}
|
||||
for _, line := range wrapPlainText(reason, reasonWidth) {
|
||||
lines = append(lines, fill(zeroTheme.muted).Render(line))
|
||||
}
|
||||
}
|
||||
// Surface exactly what the grant covers (file/dir/host) so "always" is a
|
||||
// clear, bounded choice rather than a blind tool-wide yes.
|
||||
@@ -1467,7 +1473,7 @@ func renderToolResultCard(row transcriptRow, width int, rc rowContext, opts card
|
||||
// scrollback clean. Skipped for: the uncapped detailed view (opts.bodyCap==0),
|
||||
// diff tools whose body must stay reviewable, and short output.
|
||||
collapsedFooter := ""
|
||||
if opts.bodyCap > 0 && !toolCardAlwaysExpands(name) && !(!failed && (isExploreTool(name) || isLocalControlTool(name))) {
|
||||
if opts.bodyCap > 0 && !toolCardAlwaysExpands(name) && (failed || (!isExploreTool(name) && !isLocalControlTool(name))) {
|
||||
collapsedFooter = collapsedToolFooter(row.detail)
|
||||
}
|
||||
if collapsedFooter != "" && !row.expanded {
|
||||
|
||||
@@ -393,10 +393,10 @@ func toolCallSummary(event streamjson.Event) string {
|
||||
case "write_stdin":
|
||||
sessionID := toolCallIntArg(args, "session_id")
|
||||
chars, _ := args["chars"].(string)
|
||||
switch {
|
||||
case chars == "":
|
||||
switch chars {
|
||||
case "":
|
||||
return fmt.Sprintf("poll session %d", sessionID)
|
||||
case chars == "\x03":
|
||||
case "\x03":
|
||||
return fmt.Sprintf("interrupt session %d", sessionID)
|
||||
default:
|
||||
return fmt.Sprintf("send input to session %d", sessionID)
|
||||
|
||||
@@ -220,8 +220,3 @@ func (t tuiTheme) onPanel(style lipgloss.Style) lipgloss.Style {
|
||||
func (t tuiTheme) onSel(style lipgloss.Style) lipgloss.Style {
|
||||
return style.Background(t.bgSel)
|
||||
}
|
||||
|
||||
// onPerm paints on the permission-card tint.
|
||||
func (t tuiTheme) onPerm(style lipgloss.Style) lipgloss.Style {
|
||||
return style.Background(t.bgPerm)
|
||||
}
|
||||
@@ -398,8 +398,10 @@ func permissionDetailText(event agent.PermissionEvent) string {
|
||||
if event.GrantMatched {
|
||||
parts = append(parts, "approved by saved permission")
|
||||
}
|
||||
if event.Reason != "" {
|
||||
parts = append(parts, permissionDisplayReason(event.Reason))
|
||||
if event.Action == agent.PermissionActionPrompt {
|
||||
if reason := permissionDisplayReason(event.Reason); reason != "" {
|
||||
parts = append(parts, reason)
|
||||
}
|
||||
}
|
||||
if event.Block != nil {
|
||||
parts = append(parts, permissionBlockDetail(event))
|
||||
@@ -438,7 +440,9 @@ func permissionBlockDetail(event agent.PermissionEvent) string {
|
||||
if path := strings.TrimSpace(event.Block.Path); path != "" {
|
||||
parts = append(parts, "path: "+path)
|
||||
}
|
||||
if reason := permissionDisplayReason(event.Block.Reason); reason != "" {
|
||||
reason := permissionDisplayReason(event.Block.Reason)
|
||||
eventReason := permissionDisplayReason(event.Reason)
|
||||
if reason != "" && reason != eventReason {
|
||||
parts = append(parts, reason)
|
||||
}
|
||||
return strings.Join(parts, " ")
|
||||
|
||||
@@ -1360,7 +1360,7 @@ func (m model) handleTranscriptSelectionMouse(msg tea.MouseMsg) (model, tea.Cmd,
|
||||
}
|
||||
return m, nil, false
|
||||
}
|
||||
if line.permOption && !(m.pendingPermission != nil && m.pendingPermission.typing) {
|
||||
if line.permOption && (m.pendingPermission == nil || !m.pendingPermission.typing) {
|
||||
// A left-click on a permission-popup option resolves it directly. The
|
||||
// typing guard is defence-in-depth: renderFocusedPermissionPrompt already
|
||||
// returns nil offsets in feedback mode, so no option row is registered as
|
||||
|
||||
@@ -106,6 +106,20 @@ func TestQuietGenerationHint(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestBeginRunResetsQuietGenerationClock(t *testing.T) {
|
||||
now := time.Date(2026, 7, 24, 12, 0, 0, 0, time.UTC)
|
||||
m := model{
|
||||
now: func() time.Time { return now },
|
||||
lastStreamActivity: now.Add(-time.Minute),
|
||||
}
|
||||
|
||||
m = m.beginRun(nil)
|
||||
|
||||
if got := m.quietGenerationHint(); got != "" {
|
||||
t.Fatalf("new run inherited a stale quiet-generation warning: %q", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestQuietGenerationHintEscalatesPastHalfIdleTimeout: a heartbeating-but-
|
||||
// silent stream (chatgpt/gpt-5.x, ollama reasoning models — see
|
||||
// providerio.ErrStreamStalled) and a genuinely healthy-but-slow one look
|
||||
|
||||
@@ -162,10 +162,11 @@ func applyStandaloneUpdate(ctx context.Context, result Result, executablePath st
|
||||
|
||||
binaryName := "zero"
|
||||
optionalBinaries := linuxOptionalBinaries
|
||||
if runtime.GOOS == "windows" {
|
||||
switch runtime.GOOS {
|
||||
case "windows":
|
||||
binaryName = "zero.exe"
|
||||
optionalBinaries = windowsOptionalBinaries
|
||||
} else if runtime.GOOS == "darwin" {
|
||||
case "darwin":
|
||||
optionalBinaries = nil
|
||||
}
|
||||
|
||||
|
||||
@@ -151,10 +151,11 @@ func TestApplyStandaloneUpdateReplacesBinary(t *testing.T) {
|
||||
func TestApplyStandaloneUpdateWarnsWhenHelperRefreshFails(t *testing.T) {
|
||||
binaryName := "zero"
|
||||
optionalName := "zero-seccomp"
|
||||
if runtime.GOOS == "windows" {
|
||||
switch runtime.GOOS {
|
||||
case "windows":
|
||||
binaryName = "zero.exe"
|
||||
optionalName = "zero-windows-command-runner.exe"
|
||||
} else if runtime.GOOS == "darwin" {
|
||||
case "darwin":
|
||||
t.Skip("macOS ships no optional helper binaries to refresh")
|
||||
}
|
||||
|
||||
|
||||
@@ -344,7 +344,7 @@ func releasePlatform(goos string) (string, error) {
|
||||
// self-updating release asset. Point users at `npm update` rather
|
||||
// than a source rebuild, since that's the documented Termux
|
||||
// install/upgrade path and doesn't require a Go toolchain.
|
||||
return "", fmt.Errorf("no published release for %q (release assets: linux, macos, windows). Your build is the current version of record. Upgrade with `npm update -g @gitlawb/zero` to get the latest.", goos)
|
||||
return "", fmt.Errorf("no published release for %q (release assets: linux, macos, windows). Your build is the current version of record. Upgrade with `npm update -g @gitlawb/zero` to get the latest.", goos) //nolint:staticcheck // Preserve established user-facing error text.
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -109,7 +109,7 @@ func validCommandName(name string) bool {
|
||||
return false
|
||||
}
|
||||
for _, r := range name {
|
||||
if !(r >= 'a' && r <= 'z') && !(r >= '0' && r <= '9') && r != '-' {
|
||||
if (r < 'a' || r > 'z') && (r < '0' || r > '9') && r != '-' {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user