* refactor(go): adopt modern stdlib idioms across kernel and desktop
Mechanical sweep with golangci-lint's modernize/intrange/copyloopvar/
usestdlibvars fixers plus hand-fixes for what they could not rewrite:
slices.Contains, min/max, slices.Backward, WaitGroup.Go, reflect.TypeFor,
maps.Copy, strings.Cut/CutPrefix/SplitSeq, strings.Builder, range-over-int.
Net -549 lines with no behavior change.
Three loops in session.go stay index-based: slices.Backward yields element
copies, so the fixer's rewrite silently dropped writes to DecisionReceipts
and ToolCalls (caught by TestDelivery* and TestDependentSameBatch*). All 49
slices.Backward sites were audited for that shape; only these three write
through the index, and they carry a nolint with the reason.
CaptureAfter's scan loop had an empty body and never ran any logic, so the
fixer could not express it. Drop it and the accessor it was the only caller
of. omitzero is deliberately untouched: turning omitempty into omitzero
changes the JSON written for nested structs, which is a wire-format change
to session files and the extension protocol.
* build(lint): gate the modern-idiom linters
Nothing stopped the codebase from drifting back to pre-1.22 idioms. Both
modules are now clean for modernize, intrange, copyloopvar, and
usestdlibvars, so enable them rather than leaving the sweep as a one-off.
omitzero stays disabled with the reason inline; usetesting, perfsprint, and
errorlint still have residue and land separately.
Problem
-------
Two regressions rode along with the execution contract.
1. Every shell call lost stdout/stderr interleaving. A command printing
progress on stdout and errors on stderr handed the model two separate
blocks instead of the order the child actually wrote, so an error no
longer sat next to the line that caused it.
2. Ordinary (non-Delivery) mode rejected the most common shell shape in
real projects: `go build ./... && go test ./...`, `npm install && npm
test`, `cargo build && cargo test`, `make build && make test`,
`git pull && go test ./...`. Promoting the Delivery-only mixed and
non-terminal-inline classifiers to global made every short-circuit
chain a preflight block.
Root cause
----------
1. shellrun assigned two different io.MultiWriter values to cmd.Stdout and
cmd.Stderr so stderr could be teed into its own tail. os/exec reuses a
single pipe only while the two hold the same writer value; distinct
values mean two pipes and two copy goroutines, and the merge order
becomes a scheduling artifact. The pre-contract code passed one writer
to both fields, which is why ordering held before.
2. The mixed/inline classifiers ask whether a call combines mutation with
verification, not whether the exit status can lie. With `&&` bash
short-circuits and reports the failing step, so those chains never had
the failure-swallowing problem the contract exists to prevent.
Fix
---
- shellrun shares one writer between Stdout and Stderr, restoring
child-write ordering and a single Progress caller. The bounded tail now
covers combined output and is dropped on success, so a successful run
no longer persists up to 16 KiB of ordinary stdout per tool card.
- Add shellparse.CanMaskEarlierFailure: `&&` chains are exempt, while `;`,
newline, `||`, `|`, `|&`, and `&` can all hide an earlier failure. It
reports ok=false for input it cannot analyze rather than guessing.
- Ordinary mode uses evidence.BashToolCallMixesMutationAndMaskableVerification
and a maskability gate on the non-terminal inline interpreter rule.
Delivery keeps the broad classifier: there a mutation invalidates the
verification receipt even when the exit status is honest.
- Reword both recovery messages to name the real defect and offer '&&' as
the fix, instead of telling the model never to combine the two.
Verification
------------
- Both regressions have failing-first guards: reverting either fix fails
TestRunForegroundSharesOnePipeForStdoutAndStderr,
TestRunForegroundPreservesInterleaving (10 runs), and the evidence and
agent allow-list tests.
- go test ./internal/agent/... ./internal/evidence/... ./internal/shellrun/...
./internal/shellparse/... ./internal/tool/... ./internal/provider/...
./internal/control/... ./internal/acp/... — pass, except a pre-existing
root-only failure (TestMissingReasoningWarnStateReadFailureDoesNotOverwrite-
ExistingIncidents) that fails identically on main-v2.
- go build ./... and cd desktop && go build ./... pass; git diff --check clean.
* fix(permission): harden dynamic Bash authorization
Problem:
Broad Bash allow rules could silently authorize nested command and process substitutions, while treating every dynamic shell form as human-required would make Auto unnecessarily disruptive.
Root cause:
Bash rule matching did not distinguish reusable static commands, exact-only dynamic syntax, and nested or indirect execution before applying scoped allows and controller bypasses.
Fix:
Classify Bash ASTs into reusable, exact-only, and human-required tiers. Restrict all dynamic approvals to exact Bash=<literal> grants, require a human for nested or indirect execution outside YOLO, preserve Auto fallback for expansions, assignments, redirects, heredocs, and globs, and apply the same contract across interactive, headless, persistence, CLI, and ACP paths.
Verification:
- go test ./internal/shellparse ./internal/permission ./internal/control ./internal/boot ./internal/cli ./internal/acp
- go test -race ./internal/control
- go vet ./...
- env -u DEEPSEEK_API_KEY go test ./...
- gofmt -l .
- git diff --check
Fixes#6963
* fix(control): remove unused approval wrapper
Problem:
The CI lint job rejected PR #6985 because requestApprovalDecision was left unused after approval paths moved to the option-aware helper.
Root cause:
The dynamic Bash approval refactor replaced the wrapper call sites but did not remove the obsolete wrapper itself.
Fix:
Remove the unused wrapper without changing approval behavior.
Verification:
- go run github.com/golangci/golangci-lint/v2/cmd/[email protected].2 run --timeout=5m
- go test ./internal/shellparse ./internal/permission ./internal/control ./internal/boot ./internal/cli ./internal/acp
- go test -race ./internal/control
- gofmt -l .
- git diff --check
* fix(permission): preserve dynamic compound rule precedence
Problem:
Dynamic compound Bash commands could reach fallback before per-segment Deny or Ask rules were evaluated. Raw rule matching also lost the established :* prefix semantics, and brace expansion or extglob arguments could be treated as reusable static commands.
Root cause:
The approval class short-circuited compound decomposition, the raw matcher interpreted Bash prefix markers as ordinary glob text, and the AST feature pass did not normalize brace expansion before testing whether words were static.
Fix:
Run compound segments through the same complete policy recursively, preserve Bash prefix semantics with AST-aware raw matching, prioritize wildcard tokens in glob matching, and classify brace/extglob or other non-static arguments as exact-only expansions using mvdan syntax nodes.
Verification:
- go run github.com/golangci/golangci-lint/v2/cmd/[email protected].2 run --timeout=5m
- go test ./internal/shellparse ./internal/permission ./internal/control ./internal/boot ./internal/cli ./internal/acp
- go test -race ./internal/control
- go vet ./...
- env -u DEEPSEEK_API_KEY go test ./...
- gofmt -l .
- git diff --check
* fix(permission): detect indirect builtin execution
Problem:
Bash builtin eval and builtin source could inherit broad Bash authorization even though their direct forms require human approval.
Root cause:
The indirect-execution classifier recursively handled command, exec, env, and other wrappers, but omitted Bash's builtin wrapper.
Fix:
Treat builtin as a recursive command wrapper and add positive and negative regression cases so eval/source require a human while ordinary builtins stay reusable.
Verification:
- go test ./internal/shellparse ./internal/permission ./internal/control ./internal/boot ./internal/cli ./internal/acp
- go test -race ./internal/control
- go vet ./...
- gofmt -l .
- git diff --check
YHH