Compare commits

...
Author SHA1 Message Date
Vasanthdev2004 772419f8ba test(tools): cover the explicit session_id: nil case for write_stdin 2026-07-20 13:41:04 +05:30
Vasanthdev2004 5476250061 fix(tools): give write_stdin's invalid-session errors the same recovery guidance
Follow-up to #749 (write_stdin session-id probing) / #702.

write_stdin had three "no valid live session" error paths with three different
messages and signatures: a missing session_id returned "session_id is required",
a zero/negative/non-integer one returned "session_id must be at least 1", and
only a valid-but-unknown id got the rich UnknownExecSessionError recovery guidance
("do not guess or probe; start a session with exec_command, or use edit_file/
write_file"). So a model that called write_stdin with session_id 0 (the common
"I have no id" placeholder) got a terse error with no way forward, and naming the
minimum nudged it toward trying 1, 2, 3... the exact id-probing #749 suppresses.
The three signatures also meant a model moving 0 to 1 to 2 reset the
repeated-failure streak on each class of mistake.

Route the missing / non-integer / <1 cases through the same UnknownExecSessionError
message. Now every "no live session" mistake gives the same recovery guidance and,
because that message is id-invariant, shares one repeated-failure signature, so any
mix of missing/zero/probing accumulates toward the halt.

Updates TestWriteStdinRequiresPositiveSessionID to cover missing/zero/negative/
non-integer and assert the recovery message. Guardrail signature/halt tests
unchanged.
2026-07-20 13:15:15 +05:30
Vasanth T e1975c1b39 fix(perfbench): grant write tools so mutating tasks measure real edits (#763)
* fix(perfbench): grant write tools so mutating tasks measure real edits

The turn benchmark invoked `zero exec` in its default read-only posture,
which exposes no write or shell tools (no edit_file/apply_patch/
write_file/exec_command/bash). So the mutating classes (edit/fix/
refactor) could never apply a change: a run either ground to the turn
ceiling hunting for an edit tool or reported a no-tool blocker, and the
only tasks that 'passed' were ones whose oracle grep matched the stamped
.zero-answer.txt narration rather than a real edit.

This invalidated every mutating-class number in both baselines. A live
trace of fix-07 (a one-line admin->user prefix change) showed the model
calling only read_file/grep/lsp_navigate/update_plan and then stating it
had no file-writing tool. Adding --skip-permissions-unsafe exposes the
full tool set; each task already runs in an isolated throwaway fixture
copy, so there is nothing to protect. With the flag, edit-01/fix-01/
fix-07/refactor-01 go from failing (or false-passing) to 4/4 real
passes, and tool_execution rises from ~0.4% to ~29% of wall as the write
tools are actually exercised.

Adds TestBuildTurnExecArgsGrantsWriteTools pinning the flag as a
correctness contract.

* fix(perfbench): use --auto member and let the oracle judge incomplete exits

Reviewer feedback on the write-tools grant: --skip-permissions-unsafe is
broader than the benchmark needs. It also drops the workspace, network,
and destructive-command guards and auto-retries sandbox-blocked commands
unsandboxed. Switch to --auto member, which grants exactly the write +
sandboxed-shell tools the mutating classes need while keeping those
safeguards.

member-auto's shell is sandboxed, though, so on a host without sandbox
setup the agent's own self-verification (go test/build) can't run and the
turn exits INCOMPLETE even after a correct edit. The runner used to treat
any nonzero exit as failure before consulting the oracle, which would
false-fail those correct edits. Make the oracle authoritative for
oracle-bearing tasks: the stamped test/grep against the actual fixture
files is ground truth, so a correct edit passes regardless of the exit
code. Latency-only tasks have no oracle, so a nonzero exit still fails
them.

Verified with a 4-task glm-5.2 smoke (edit-01/fix-01/fix-07/refactor-01):
4/4 real passes under --auto member. Updates the exec-args test to assert
--auto member is present and --skip-permissions-unsafe is absent, and adds
TestOracleAuthoritativeOnIncompleteExit and
TestNonzeroExitStillFailsLatencyOnly.

* fix(perfbench): reject fixtureless tasks so the write-tools grant can't escape isolation

CodeRabbit and gnanam both flagged the same structural gap: buildTurnExecArgs
grants the write + sandboxed-shell tools to every invocation, but the fixture
isolation was conditional (a task with no workspaceFixture ran in the caller's
cwd). So the grant and the guard could drift apart, and a fixtureless task would
run an agent with write/shell tools in the caller's real repo dir.

Make the isolation a checked precondition: NewTurnExecRunner now rejects a task
with no workspaceFixture before launching zero exec, failing loudly rather than
silently running in cwd. Every shipped task already declares a fixture, so this
only fires on a malformed or newly added one.

Adds TestTurnRunnerRejectsFixturelessTask (rejection is pre-launch: it returns
the fixture error even with a nonexistent binary).

* fix(perfbench): defer to the oracle only for INCOMPLETE (exit 4), not every nonzero exit

anand's follow-up: the previous commit cleared VerifyErr for any nonzero
run_end whenever a VerificationCommand was present, which would also convert
a crash (1), usage error (2), provider failure (3), or interruption (130)
into a passing result whenever a partial edit happened to satisfy the oracle.

Gate the oracle override on exitCode == exitIncomplete (4): the completion
gate's "work maybe done but not signaled" case, the only one where a correct
edit can legitimately exit nonzero (its sandboxed self-verify couldn't run
under --auto member). Every other nonzero exit stays authoritative, so a
partial edit can't launder a crashed or interrupted run into a pass.

The 4-task glm-5.2 smoke stays 4/4 (those tasks exit INCOMPLETE, not some
other code). Adds TestNonIncompleteExitStaysAuthoritative (a correct edit that
reports exit 1 must still fail) and mirrors internal/cli.exitIncomplete as a
local execExitIncomplete constant.

* test(perfbench): pin the exit-4 oracle deferral from both sides; drop dead fixture branch

Follow-ups from a self-review of the exit-4 gating:

- Add TestIncompleteExitStillFailsWhenOracleFails: an INCOMPLETE (exit 4) run
  that did NO work must still FAIL, because deferring to the oracle still means
  the oracle is consulted. The sibling TestOracleAuthoritativeOnIncompleteExit
  applies a correct edit and would pass either way, so without this a regression
  that turned exit 4 into a blanket pass would go unnoticed.
- Broaden TestNonIncompleteExitStaysAuthoritative to a table over exit 1/3/130
  (crash/provider/interrupt), not just 1, so every named launder-risk code is
  covered.
- Tighten the NewTurnExecRunner doc: the verdict gate is the VerificationCommand
  (which runs the stamped OracleTest via go test), not OracleTest on its own.
- Drop the now-dead "if WorkspaceFixture != empty" branch: the fixtureless guard
  added earlier already guarantees it is set.

No behavior change; the 4-task glm-5.2 smoke stays 4/4.
2026-07-20 11:34:20 +05:30
KRATOSandKRATOS 8c6d3022bd feat(tui): add /undo as an alias for /rewind (#698) (#747)
* feat(tui): add /undo as an alias for /rewind (#698)

/undo was unregistered and fell through to commandUnknown. Add it to the
/rewind command's aliases (the same mechanism /quit, /find, /mcp-status use), so
it dispatches to the existing rewind handler with args intact.

* test(tui): cover the /undo numeric-sequence form (#698 review)

Add a parseCommand("/undo 123") assertion so both the latest and numeric
<sequence> forms of the /rewind contract are exercised through the alias.

---------

Co-authored-by: KRATOS <[email protected]>
2026-07-20 11:32:44 +05:30
KRATOSandKRATOS 974fc036c2 fix(oauth): refuse redirects on credential POSTs (#729) (#741)
Token exchange/refresh and the device authorization/poll POSTs validated only
the initial endpoint, then let the http.Client follow a 307/308 that replays the
form body (code, PKCE verifier, refresh_token, client_secret) to an unvalidated
origin.

Add withoutRedirects(): a shallow client copy whose CheckRedirect refuses every
redirect (ErrUnsafeRedirect). Applied at PostToken (covers ExchangeCode, Refresh,
and MCP which delegate to it), RequestDeviceCode, and pollDeviceOnce. The caller's
client is never mutated; legitimate token endpoints that return 200 are unaffected.

Co-authored-by: KRATOS <[email protected]>
2026-07-20 08:19:37 +05:30
PierrunoYT 4cbd144d11 fix(providers): stop "provider not found" for env-derived profiles (#716)
* fix(providers): stop "provider not found" for env-derived profiles

* fix(providers): address env profile review feedback

* fix(providers): keep env JSON response schemas stable
2026-07-20 10:23:44 +08:00
euxaristiaandClaude Fable 5 c0b6b9e5c9 docs(readme): note govulncheck/golangci-lint install to GOPATH/bin (#711)
* docs(readme): note govulncheck/golangci-lint install to GOPATH/bin

The installed binaries land in $GOPATH/bin (default ~/go/bin), which must
be on PATH to run govulncheck directly. Document the export snippet so the
lint/security tooling from the contributor setup actually resolves.

* docs(readme): say "them" instead of singling out govulncheck

golangci-lint installs to the same $GOPATH/bin as govulncheck, so the
PATH note applies to both, not just the one named.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

* docs(readme): honor GOBIN and make the PATH snippet idempotent

go install writes to GOBIN when set, with GOPATH/bin as the fallback, and
rerunning the old snippet appended the same directory repeatedly.

Co-Authored-By: Claude Fable 5 <[email protected]>

---------

Co-authored-by: Claude Sonnet 5 <[email protected]>
2026-07-20 10:21:54 +08:00
PierrunoYT d74ceb1127 fix(tui): cache settled alt-screen transcript (#647)
* fix(tui): cache settled alt-screen transcript

* fix(tui): invalidate settled alt-screen cache on in-place status row updates

setDoctorStatusRow, setSandboxSetupStatusRow, and setCompactStatusRow all
mutate an already-flushed transcript row in place (m.transcript[i] = row)
without invalidating the alt-screen settled cache. Since none of frontier,
width, or row count change on such an update, settleTranscript's rebuild
guard never fires and the View fast path serves the stale cached snapshot -
e.g. the doctor connectivity spinner freezes on its first frame.

Zero altScreenSettledWidth on the in-place path, mirroring the existing
pattern in toggleTranscriptRow, so the next settle rebuilds the cache.

Addresses PR #647 review feedback from Vasanthdev2004.

* fix(tui): invalidate settled transcript cache

* fix(tui): invalidate settled file selection cards
2026-07-20 10:20:46 +08:00
Vasanth T fbf85984f6 fix(agent): stop write_stdin session_id probing thrash (#702) (#749)
A model that calls write_stdin with no live session probed sequential
session_ids (1, 2, 3, …), each failing, and the run ground on for a long
time before halting. The repeated-failure guard keys on a normalized,
80-char-truncated signature of the tool error, and the old message
"Unknown exec session_id %d" put the varying id inside that prefix, so
every probe produced a fresh signature and the 6x-same-error halt never
fired.

- Rewrite the unknown-session error so stable recovery guidance leads and
  the id trails past the signature window: probing now collapses to one
  signature and trips the halt, and the model is told how to recover
  (start an exec_command, or use the file-edit tools).
- Pin session_id's schema Minimum to 1 so the advertised schema matches
  the runtime floor (intArg min 1).
- system_prompt: state that session_id is only ever an id returned by a
  still-running exec_command and must never be guessed or probed.

Tests pin the id-invariance of the signature and that distinct-id probing
now halts at the guard's stop threshold, plus the schema floor and the
recovery text. The TUI "Sent input" label on a failed write_stdin
(issue criterion 4) is a separate cosmetic concern in a shared hot path
and is left as a follow-up.
2026-07-19 22:13:35 +05:30
Anandan 9242b9e037 Add transcript-grounded structured task state (#761)
* Add transcript-grounded structured task state

Project existing plan, tool-result, changed-file, verification, and completion events into a deterministic per-run snapshot.

Emit content-free aggregate snapshots to traces. Carry bounded objective context through compaction and ground completion checks only when the projection matches the transcript; mismatches retain transcript-derived behavior and drop stale carried state.

Tested: make fmt-check

Tested: go vet ./...

Tested: go test ./...

Tested: go test -race ./internal/agent ./internal/trace

Tested: go run ./cmd/zero-release build

Tested: go run ./cmd/zero-release smoke

Tested: govulncheck ./... (no vulnerabilities found)

Advisory: pinned golangci-lint reports only unrelated pre-existing findings

Tested: git diff HEAD --check

* Preserve task objective across repeated compaction

Keep the immutable run objective when plan calls have been elided, while admitting mutable task fields only when plan parity still matches the transcript.

Make plan parsing match update_plan rejection semantics, coalesce and cap task-state trace events, rename parity to its plan-only scope, use explicit nilable dependencies, and remove confirmable objective fingerprints from traces.

Tested: make fmt-check

Tested: go vet ./...

Tested: go test ./...

Tested: go test -race ./internal/agent ./internal/trace

Tested: go run ./cmd/zero-release build

Tested: go run ./cmd/zero-release smoke

Tested: govulncheck ./... (no vulnerabilities found)

Advisory: pinned golangci-lint reports only unrelated pre-existing findings

Tested: git diff HEAD --check
2026-07-19 21:38:58 +05:30
43 changed files with 2142 additions and 148 deletions

No files matched your search

+9
View File
@@ -384,6 +384,15 @@ go install github.com/golangci/golangci-lint/v2/cmd/[email protected]
go install golang.org/x/vuln/cmd/[email protected]
```
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
run them directly. If it isn't, add it:
```bash
gobin="$(go env GOBIN)"; [ -z "$gobin" ] && gobin="$(go env GOPATH)/bin"
case ":$PATH:" in *":$gobin:"*) ;; *) export PATH="$PATH:$gobin" ;; esac
```
### Cross-Compile Examples
```bash
+12 -3
View File
@@ -56,6 +56,10 @@ type CompactionOptions struct {
// injected so Compact stays pure and testable; the agent loop wires it to a
// real provider call.
Summarize func(toSummarize []zeroruntime.Message) (string, error)
// taskState is a snapshot supplied by the running agent. Its immutable
// objective is always preserved; mutable fields are admitted only when its
// plan projection still matches the transcript.
taskState *taskStateSnapshot
}
// CompactionResult is the metadata-bearing result returned by CompactMessages.
@@ -220,7 +224,7 @@ func CompactMessages(messages []zeroruntime.Message, opts CompactionOptions) (Co
// Preserve structured state (active plan + loaded skills) from the elided
// middle verbatim, so it is not lost or paraphrased away by the prose summary.
content := appendPreservedState(summaryLabel+"\n"+summary, middle)
content := appendPreservedState(summaryLabel+"\n"+summary, middle, opts.taskState)
compacted := make([]zeroruntime.Message, 0, systemEnd+1+(len(messages)-boundary))
compacted = append(compacted, messages[:systemEnd]...)
@@ -326,6 +330,7 @@ type compactionState struct {
// OnText is deliberately NOT forwarded (compaction stays invisible to the user),
// but its token COST must still be counted so usage reports and budgets include it.
onUsage func(Usage)
task *taskState
// calibrationRatio scales the raw byte/4 token estimate toward the provider's
// real prompt-token count. ApproxTextTokens over-counts code-heavy content by
@@ -365,13 +370,15 @@ func (state *compactionState) calibratedTokens(raw int) int {
return int(float64(raw) * state.calibrationRatio)
}
func newCompactionState(options Options) *compactionState {
return &compactionState{
func newCompactionState(options Options, task *taskState) *compactionState {
state := &compactionState{
enabled: options.ContextWindow > 0,
threshold: compactionThreshold(options.ContextWindow),
preserveLast: options.CompactionPreserveLast,
onUsage: options.OnUsage,
task: task,
}
return state
}
// maybeCompact runs proactive compaction at the top of a turn. It returns the
@@ -418,6 +425,7 @@ func (state *compactionState) maybeCompact(
compacted, err := Compact(messages, CompactionOptions{
PreserveLast: state.preserveLast,
Summarize: summarizeClosure(ctx, provider, state.onUsage),
taskState: state.task.snapshotForCompaction(messages),
})
if err != nil {
// Summarizer failed: keep the original history. The reactive path (or a
@@ -468,6 +476,7 @@ func (state *compactionState) recover(
result, compactErr := Compact(messages, CompactionOptions{
PreserveLast: state.preserveLast,
Summarize: summarizeClosure(ctx, provider, state.onUsage),
taskState: state.task.snapshotForCompaction(messages),
})
if compactErr != nil {
// A genuine compaction attempt was made (and failed): the budget is spent
+60 -29
View File
@@ -30,6 +30,10 @@ const (
const (
maxRecentEdits = 20
maxEditNoteBytes = 160
// maxTaskObjectiveBytes keeps the original objective recognizable after
// compaction without allowing an unusually large prompt to recreate the
// context pressure the compaction just removed.
maxTaskObjectiveBytes = 512
)
const (
@@ -76,32 +80,14 @@ func extractLatestPlan(messages []zeroruntime.Message) string {
// ({"plan":[{content,status,...}]}) as terse status-tagged bullet lines. Returns
// "" on malformed arguments or an empty plan.
func formatPlanArguments(arguments string) string {
var parsed struct {
Plan []struct {
Content string `json:"content"`
Step string `json:"step"`
Status string `json:"status"`
Notes string `json:"notes"`
} `json:"plan"`
}
if err := json.Unmarshal([]byte(strings.TrimSpace(arguments)), &parsed); err != nil {
plan, ok := parseTaskPlan(arguments)
if !ok {
return ""
}
lines := make([]string, 0, len(parsed.Plan))
for _, item := range parsed.Plan {
content := strings.TrimSpace(item.Content)
if content == "" {
content = strings.TrimSpace(item.Step)
}
if content == "" {
continue
}
status := strings.TrimSpace(item.Status)
if status == "" {
status = "pending"
}
line := "- [" + status + "] " + content
if notes := strings.TrimSpace(item.Notes); notes != "" {
lines := make([]string, 0, len(plan))
for _, item := range plan {
line := "- [" + item.Status + "] " + item.Content
if notes := item.Notes; notes != "" {
line += "\n Notes: " + notes
}
lines = append(lines, line)
@@ -431,12 +417,25 @@ func capBody(body string) string {
// preservedState is the JSON shape of the carried-across-compaction block.
type preservedState struct {
Plan string `json:"plan,omitempty"`
Task *preservedTaskState `json:"task,omitempty"`
RecentEdits []preservedEdit `json:"recent_edits,omitempty"`
Tools []preservedTool `json:"tools,omitempty"`
Skills []preservedSkill `json:"skills,omitempty"`
ProjectInstructions []preservedInstruction `json:"project_instructions,omitempty"`
}
type preservedTaskState struct {
Objective string `json:"objective"`
Status taskStatus `json:"status,omitempty"`
Pending int `json:"pending,omitempty"`
InProgress int `json:"in_progress,omitempty"`
Completed int `json:"completed,omitempty"`
Failed int `json:"failed,omitempty"`
VerificationPassed int `json:"verification_passed,omitempty"`
VerificationFailed int `json:"verification_failed,omitempty"`
VerificationOutcome Outcome `json:"verification_outcome,omitempty"`
}
type preservedEdit struct {
Path string `json:"path"`
Note string `json:"note,omitempty"`
@@ -464,8 +463,27 @@ type preservedInstruction struct {
// may live only inside the injected summary message, which on a later compaction
// lands in middle with no real tool calls left to extract. Fresh tool calls and
// instruction blocks override the carried-forward copy by name/source.
func appendPreservedState(summary string, middle []zeroruntime.Message) string {
func appendPreservedState(summary string, middle []zeroruntime.Message, taskSnapshot *taskStateSnapshot) string {
priorState := parsePreservedStateBlock(latestSummaryContent(middle))
task := priorState.Task
if taskSnapshot != nil {
task = &preservedTaskState{
Objective: capTaskObjective(taskSnapshot.Objective),
}
// Plan parity corroborates only the mutable task projection. The objective
// comes directly from the run prompt and is immutable, so it must survive
// even after compaction removes the plan tool call needed for comparison.
if taskSnapshot.PlanParity == taskPlanParityMatch {
task.Status = taskSnapshot.Status
task.Pending = taskSnapshot.Plan.Pending
task.InProgress = taskSnapshot.Plan.InProgress
task.Completed = taskSnapshot.Plan.Completed
task.Failed = taskSnapshot.Plan.Failed
task.VerificationPassed = taskSnapshot.Verification.Passed
task.VerificationFailed = taskSnapshot.Verification.Failed
task.VerificationOutcome = taskSnapshot.Verification.LastOutcome
}
}
// Plan: a fresh update_plan in middle is authoritative; otherwise carry the
// plan preserved by an earlier compaction.
@@ -493,12 +511,25 @@ func appendPreservedState(summary string, middle []zeroruntime.Message) string {
projectInstructionEntries(middle),
)
if block := formatPreservedState(plan, edits, tools, skills, instructions); block != "" {
if block := formatPreservedState(plan, task, edits, tools, skills, instructions); block != "" {
summary += "\n\n" + block
}
return summary
}
func capTaskObjective(objective string) string {
objective = strings.TrimSpace(objective)
if len(objective) <= maxTaskObjectiveBytes {
return objective
}
const suffix = "…"
limit := maxTaskObjectiveBytes - len(suffix)
for limit > 0 && !utf8.RuneStart(objective[limit]) {
limit--
}
return strings.TrimSpace(objective[:limit]) + suffix
}
// mergeRecentEdits overlays fresh edits onto edits preserved by an earlier
// compaction. Unlike mergeSkillEntries (which keeps refreshed entries in their
// original slot), a path touched again by a fresh edit MOVES to the newest
@@ -555,11 +586,11 @@ func preservedEditsToEntries(edits []preservedEdit) []skillEntry {
// formatPreservedState renders state as the labelled, single-line
// JSON block. Returns "" when there is nothing to preserve.
func formatPreservedState(plan string, edits, tools, skills, instructions []skillEntry) string {
if plan == "" && len(edits) == 0 && len(tools) == 0 && len(skills) == 0 && len(instructions) == 0 {
func formatPreservedState(plan string, task *preservedTaskState, edits, tools, skills, instructions []skillEntry) string {
if plan == "" && task == nil && len(edits) == 0 && len(tools) == 0 && len(skills) == 0 && len(instructions) == 0 {
return ""
}
state := preservedState{Plan: plan}
state := preservedState{Plan: plan, Task: task}
for _, e := range edits {
state.RecentEdits = append(state.RecentEdits, preservedEdit{Path: e.name, Note: e.body})
}
+101 -5
View File
@@ -1,6 +1,7 @@
package agent
import (
"encoding/json"
"strings"
"testing"
"unicode/utf8"
@@ -59,6 +60,103 @@ func TestCompactPreservesActivePlan(t *testing.T) {
}
}
func TestCompactPreservesBoundedTaskContext(t *testing.T) {
objective := strings.Repeat("世", maxTaskObjectiveBytes)
task := newTaskState(objective, nil)
task.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"write code","status":"in_progress"},{"content":"add tests","status":"pending"}]}`})
messages := stateConversation()
compacted, err := Compact(messages, CompactionOptions{
PreserveLast: 2,
Summarize: func([]zeroruntime.Message) (string, error) { return "SUMMARY", nil },
taskState: task.snapshotForCompaction(messages),
})
if err != nil {
t.Fatalf("Compact: %v", err)
}
state := parsePreservedStateBlock(compacted[1].Content)
if state.Task == nil || state.Task.Status != taskStatusActive || state.Task.InProgress != 1 || state.Task.Pending != 1 {
t.Fatalf("unexpected compact task state: %#v", state.Task)
}
if len(state.Task.Objective) > maxTaskObjectiveBytes || !utf8.ValidString(state.Task.Objective) {
t.Fatalf("objective was not safely bounded: %d bytes %q", len(state.Task.Objective), state.Task.Objective)
}
}
func TestCompactPreservesObjectiveAfterPlanParityMismatch(t *testing.T) {
prior := preservedState{Task: &preservedTaskState{Objective: "stale objective", Status: taskStatusActive, Pending: 1}}
encoded, err := json.Marshal(prior)
if err != nil {
t.Fatal(err)
}
messages := []zeroruntime.Message{
{Role: zeroruntime.MessageRoleSystem, Content: "system"},
{Role: zeroruntime.MessageRoleUser, Content: summaryLabel + "\nold\n\n" + preservedStateLabel + "\n" + string(encoded)},
{Role: zeroruntime.MessageRoleAssistant, Content: "continuing"},
{Role: zeroruntime.MessageRoleUser, Content: "more"},
{Role: zeroruntime.MessageRoleAssistant, Content: "working"},
{Role: zeroruntime.MessageRoleUser, Content: "again"},
{Role: zeroruntime.MessageRoleAssistant, Content: "done"},
}
compacted, err := Compact(messages, CompactionOptions{
PreserveLast: 2,
Summarize: func([]zeroruntime.Message) (string, error) { return "SUMMARY", nil },
taskState: &taskStateSnapshot{
Objective: "current objective",
PlanParity: taskPlanParityMismatch,
},
})
if err != nil {
t.Fatalf("Compact: %v", err)
}
state := parsePreservedStateBlock(compacted[1].Content)
if state.Task == nil || state.Task.Objective != "current objective" {
t.Fatalf("immutable objective was lost on plan mismatch: %#v", state.Task)
}
if state.Task.Status != "" || state.Task.Pending != 0 {
t.Fatalf("uncorroborated mutable task fields survived plan mismatch: %#v", state.Task)
}
}
func TestTaskObjectiveSurvivesRepeatedCompactionWithoutPlanRefresh(t *testing.T) {
task := newTaskState("keep this objective", nil)
task.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"write code","status":"in_progress"},{"content":"add tests","status":"pending"}]}`})
messages := stateConversation()
first, err := Compact(messages, CompactionOptions{
PreserveLast: 2,
Summarize: func([]zeroruntime.Message) (string, error) { return "FIRST", nil },
taskState: task.snapshotForCompaction(messages),
})
if err != nil {
t.Fatalf("first Compact: %v", err)
}
secondInput := append(append([]zeroruntime.Message{}, first...),
zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: "more"},
zeroruntime.Message{Role: zeroruntime.MessageRoleAssistant, Content: "working"},
zeroruntime.Message{Role: zeroruntime.MessageRoleUser, Content: "again"},
zeroruntime.Message{Role: zeroruntime.MessageRoleAssistant, Content: "done"},
)
snapshot := task.snapshotForCompaction(secondInput)
if snapshot.PlanParity != taskPlanParityMismatch {
t.Fatalf("plan call should be absent after first compaction, parity=%q", snapshot.PlanParity)
}
second, err := Compact(secondInput, CompactionOptions{
PreserveLast: 2,
Summarize: func([]zeroruntime.Message) (string, error) { return "SECOND", nil },
taskState: snapshot,
})
if err != nil {
t.Fatalf("second Compact: %v", err)
}
state := parsePreservedStateBlock(second[1].Content)
if state.Task == nil || state.Task.Objective != "keep this objective" {
t.Fatalf("objective lost after second compaction: %#v", state.Task)
}
if state.Task.Status != "" || state.Task.InProgress != 0 {
t.Fatalf("uncorroborated mutable fields survived second compaction: %#v", state.Task)
}
}
func TestCompactPreservesLoadedSkills(t *testing.T) {
summary := compactStateConversation(t, stateConversation())
if !strings.Contains(summary, preservedStateLabel) {
@@ -283,12 +381,10 @@ func TestExtractLatestPlanReturnsMostRecent(t *testing.T) {
}
}
func TestFormatPlanArgumentsAcceptsStepAlias(t *testing.T) {
func TestFormatPlanArgumentsRejectsUnsupportedStepAlias(t *testing.T) {
got := formatPlanArguments(`{"plan":[{"step":"write failing test","status":"in_progress"},{"content":"keep existing shape","status":"pending"}]}`)
for _, want := range []string{"- [in_progress] write failing test", "- [pending] keep existing shape"} {
if !strings.Contains(got, want) {
t.Fatalf("expected %q in formatted plan, got %q", want, got)
}
if got != "" {
t.Fatalf("unsupported alias should reject the whole plan like update_plan, got %q", got)
}
}
+2 -2
View File
@@ -581,7 +581,7 @@ func TestCompactNeverProducesConsecutiveUserMessages(t *testing.T) {
}
func TestRecoverNoopDoesNotConsumeReactiveBudget(t *testing.T) {
st := newCompactionState(Options{ContextWindow: 1000, CompactionPreserveLast: 2})
st := newCompactionState(Options{ContextWindow: 1000, CompactionPreserveLast: 2}, nil)
provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{{
{Type: zeroruntime.StreamEventText, Content: "SUMMARY"}, {Type: zeroruntime.StreamEventDone},
}}}
@@ -628,7 +628,7 @@ func TestRecoverNoopDoesNotConsumeReactiveBudget(t *testing.T) {
}
func TestRecoverDisabledIsNoop(t *testing.T) {
st := newCompactionState(Options{ContextWindow: 0})
st := newCompactionState(Options{ContextWindow: 0}, nil)
msgs := []zeroruntime.Message{{Role: zeroruntime.MessageRoleUser, Content: "x"}}
called := false
// recover must not invoke the provider/summarize when disabled, even on a
+2 -2
View File
@@ -39,7 +39,7 @@ func newCompletionPolicy(requireSemanticCheck bool) *completionPolicy {
return &completionPolicy{requireSemanticCheck: requireSemanticCheck}
}
func (policy *completionPolicy) evaluate(text string, planPending bool) completionEvaluation {
func (policy *completionPolicy) evaluate(text string, context completionContext) completionEvaluation {
// A direct admission is strong local evidence and takes precedence over all
// weaker signals, avoiding both continuation nudges and semantic checks.
if reason := selfReportedIncompletion(text); reason != "" {
@@ -50,7 +50,7 @@ func (policy *completionPolicy) evaluate(text string, planPending bool) completi
// weak evidence because bookkeeping can be stale. Both get a bounded chance
// to continue, but only a persisted cue is ultimately incomplete.
cue := endsWithContinuationCue(text)
if cue || planPending {
if cue || context.PlanPending {
if policy.continueNudges < maxContinueNudges {
policy.continueNudges++
reason := "your message ended mid-step"
+20 -9
View File
@@ -1,16 +1,19 @@
package agent
import "testing"
import (
"strings"
"testing"
)
func TestCompletionPolicyLocalEvidenceDecidesWithoutSemanticCheck(t *testing.T) {
policy := newCompletionPolicy(false)
complete := policy.evaluate("Done. All required checks pass.", false)
complete := policy.evaluate("Done. All required checks pass.", completionContext{})
if complete.Decision != CompletionComplete {
t.Fatalf("confident completion decision = %q, want %q", complete.Decision, CompletionComplete)
}
incomplete := policy.evaluate("I couldn't verify the result, so this is my best guess.", false)
incomplete := policy.evaluate("I couldn't verify the result, so this is my best guess.", completionContext{})
if incomplete.Decision != CompletionIncomplete {
t.Fatalf("admitted failure decision = %q, want %q", incomplete.Decision, CompletionIncomplete)
}
@@ -19,13 +22,13 @@ func TestCompletionPolicyLocalEvidenceDecidesWithoutSemanticCheck(t *testing.T)
func TestCompletionPolicyPreservesBoundedPlanStallProtection(t *testing.T) {
policy := newCompletionPolicy(false)
for attempt := 0; attempt < maxContinueNudges; attempt++ {
got := policy.evaluate("Let me inspect the remaining configuration:", true)
got := policy.evaluate("Let me inspect the remaining configuration:", completionContext{PlanPending: true})
if got.Decision != CompletionUncertain || got.Action != completionActionContinue {
t.Fatalf("attempt %d = (%q, %q), want uncertain continue", attempt+1, got.Decision, got.Action)
}
}
got := policy.evaluate("Let me inspect the remaining configuration:", true)
got := policy.evaluate("Let me inspect the remaining configuration:", completionContext{PlanPending: true})
if got.Decision != CompletionIncomplete {
t.Fatalf("decision after nudge budget = %q, want %q", got.Decision, CompletionIncomplete)
}
@@ -34,13 +37,13 @@ func TestCompletionPolicyPreservesBoundedPlanStallProtection(t *testing.T) {
func TestCompletionPolicyTreatsPendingPlanAsWeakEvidence(t *testing.T) {
policy := newCompletionPolicy(false)
for attempt := 0; attempt < maxContinueNudges; attempt++ {
got := policy.evaluate("All set.", true)
got := policy.evaluate("All set.", completionContext{PlanPending: true})
if got.Decision != CompletionUncertain || got.Action != completionActionContinue {
t.Fatalf("attempt %d = (%q, %q), want uncertain continue", attempt+1, got.Decision, got.Action)
}
}
got := policy.evaluate("All set.", true)
got := policy.evaluate("All set.", completionContext{PlanPending: true})
if got.Decision != CompletionComplete {
t.Fatalf("stale plan decision after nudge budget = %q, want %q", got.Decision, CompletionComplete)
}
@@ -49,12 +52,12 @@ func TestCompletionPolicyTreatsPendingPlanAsWeakEvidence(t *testing.T) {
func TestCompletionPolicyAllowsExactlyOneRequiredSemanticCheck(t *testing.T) {
policy := newCompletionPolicy(true)
first := policy.evaluate("Implemented and tested.", false)
first := policy.evaluate("Implemented and tested.", completionContext{})
if first.Decision != CompletionUncertain || first.Action != completionActionSemanticCheck {
t.Fatalf("first decision = (%q, %q), want uncertain semantic check", first.Decision, first.Action)
}
second := policy.evaluate("PASS. The result meets the task criterion.", false)
second := policy.evaluate("PASS. The result meets the task criterion.", completionContext{})
if second.Decision != CompletionComplete {
t.Fatalf("post-check decision = %q, want %q", second.Decision, CompletionComplete)
}
@@ -62,3 +65,11 @@ func TestCompletionPolicyAllowsExactlyOneRequiredSemanticCheck(t *testing.T) {
t.Fatal("semantic check was requested more than once")
}
}
func TestAcceptanceVerificationNudgeIncludesBoundedObjective(t *testing.T) {
objective := "make the requested behavior observable"
nudge := acceptanceVerificationNudge(objective)
if !strings.Contains(nudge, "Task objective: "+objective) {
t.Fatalf("semantic check was not grounded in the objective: %q", nudge)
}
}
+11 -13
View File
@@ -1,7 +1,6 @@
package agent
import (
"encoding/json"
"strconv"
"strings"
@@ -127,9 +126,8 @@ func endsWithContinuationCue(text string) bool {
// which the update_plan tool coerces to "pending") counts as remaining, matching
// that tool's own status normalization.
func planStatusRemaining(status string) bool {
switch strings.ToLower(strings.TrimSpace(status)) {
case "completed", "complete", "done", "finished", "resolved", "✓", "x", "[x]",
"failed", "fail", "error", "errored", "blocked", "cancelled", "canceled", "abandoned", "skipped":
switch normalizeTaskPlanStatus(status) {
case "completed", "failed":
return false
default:
return true
@@ -307,8 +305,8 @@ func hasAnyPrefix(s string, prefixes []string) bool {
// the bug-hunt surfaced: well-formed output treated as correct; pre-existing tests
// passing treated as the objective being met; and a result that merely matches a
// baseline the task asked to beat or improve. General — no task-specific content.
func acceptanceVerificationNudge() string {
return "Before this task can be marked complete, " + acceptanceNudgeMarker + " — " +
func acceptanceVerificationNudge(objective string) string {
nudge := "Before this task can be marked complete, " + acceptanceNudgeMarker + " — " +
"NOT the shape or format of your output, NOT that pre-existing tests pass, and NOT that your " +
"result merely matches a baseline you were asked to beat or improve. " +
"Re-read the original task, then run a concrete check that exercises the actual requirement: " +
@@ -316,6 +314,10 @@ func acceptanceVerificationNudge() string {
"thing the task asked you to produce, recover, fix, or optimize. " +
"If that check passes, reply PASS and cite the evidence. " +
"If it does not pass — or you cannot run such a check — say so plainly and keep working; do not claim success."
if objective = capTaskObjective(objective); objective != "" {
nudge += "\n\nTask objective: " + objective
}
return nudge
}
// toolFailureHintMarker is a stable substring for tests.
@@ -553,16 +555,12 @@ func (state *guardState) pendingPlanItems() bool {
// many items are still remaining. Malformed arguments leave the prior count
// unchanged (best-effort — the plan panel itself tolerates the same).
func (state *guardState) observePlanUpdate(arguments string) {
var parsed struct {
Plan []struct {
Status string `json:"status"`
} `json:"plan"`
}
if json.Unmarshal([]byte(arguments), &parsed) != nil {
plan, ok := parseTaskPlan(arguments)
if !ok {
return
}
pending := 0
for _, item := range parsed.Plan {
for _, item := range plan {
if planStatusRemaining(item.Status) {
pending++
}
+31
View File
@@ -169,6 +169,37 @@ func TestRunResetsEmptyTurnCounterOnToolCall(t *testing.T) {
}
}
// The whole point of #702's fix: the unknown-session error's normalized
// signature must not vary with the session id, so a model probing ids 1, 2,
// 3, … produces one repeated signature the failure guard can count. If the id
// leaked into the first 80 normalized chars, each probe would reset the streak
// and the halt would never fire.
func TestUnknownExecSessionErrorSignatureIsIDInvariant(t *testing.T) {
a := errorSignature(tools.UnknownExecSessionError(1))
b := errorSignature(tools.UnknownExecSessionError(999999))
if a != b {
t.Fatalf("unknown-session signature varies with id:\n %q\n %q", a, b)
}
}
// End to end: with an id-invariant signature, probing a different unknown id
// each turn now trips the repeated-failure halt at toolFailureStopAt, where
// before the fix it never would.
func TestUnknownExecSessionProbingTripsFailureHalt(t *testing.T) {
var state guardState
var stoppedAt int
for i := 1; i <= toolFailureStopAt; i++ {
out := state.observeToolResult(tools.WriteStdinToolName, true, tools.UnknownExecSessionError(i))
if out.Stop {
stoppedAt = i
break
}
}
if stoppedAt != toolFailureStopAt {
t.Fatalf("probing distinct unknown ids stopped at %d, want %d", stoppedAt, toolFailureStopAt)
}
}
func TestGuardStateResetsToolOnlyStreakOnEmptyNonToolTurn(t *testing.T) {
var state guardState
toolOnly := zeroruntime.CollectedStream{
+23 -3
View File
@@ -185,7 +185,16 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
messages := zeroruntime.SeedMessagesWithImages(promptParts.prompt, prompt, options.Images)
guards := newGuardState()
compactor := newCompactionState(options)
task := newTaskState(prompt, options.Trace)
compactor := newCompactionState(options, task)
defer func() {
// A final transcript comparison is observational. It records drift but
// never rewrites the result or changes execution after the fact.
task.observePlanParity(result.Messages)
// Tool-result observations are intentionally coalesced; this final event
// guarantees their aggregate counts are present even on an early return.
task.emit()
}()
// The execution-profile posture controller. nil Options.Profile (every
// existing caller) makes every observe/decide call a no-op, keeping the
// loop byte-identical for unprofiled runs.
@@ -570,7 +579,9 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
// model's final answer ONLY when the work is actually done. Default off
// (RequireCompletionSignal), so interactive runs stay byte-identical.
if options.RequireCompletionSignal {
evaluation := completionPolicy.evaluate(collected.Text, guards.pendingPlanItems())
completionContext := task.completionContext(messages, guards.pendingPlanItems())
evaluation := completionPolicy.evaluate(collected.Text, completionContext)
task.observe(taskStateEvent{kind: taskStateEventCompletion, completion: evaluation})
switch evaluation.Decision {
case CompletionIncomplete:
result.Incomplete = true
@@ -595,13 +606,15 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
options.Trace.Counter(trace.CounterAcceptanceChecks, 1)
messages = append(messages, zeroruntime.Message{
Role: zeroruntime.MessageRoleUser,
Content: acceptanceVerificationNudge(),
Content: acceptanceVerificationNudge(completionContext.Objective),
})
}
continue
case CompletionComplete:
// Local evidence is sufficient; proceed to final diagnostics.
}
} else {
task.observe(taskStateEvent{kind: taskStateEventCompletion, completion: completionEvaluation{Decision: CompletionComplete}})
}
// Finalization diagnostics gate: edits from this run may still have
// checks in flight — the per-turn drain waits only briefly and defers
@@ -625,6 +638,11 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
// executing so the empty-turn counter resets and plan-tracking signals
// stay current.
guards.observeTurn(collected)
for _, call := range collected.ToolCalls {
if call.Name == planToolName {
task.observe(taskStateEvent{kind: taskStateEventPlan, arguments: call.Arguments})
}
}
failureHint := ""
// turnRequestedModel records the FIRST mid-run escalation target requested
@@ -676,6 +694,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
}
options.Trace.Counter(trace.CounterToolCalls, 1)
recordOutputBudgetTrace(options.Trace, toolResult)
task.observe(taskStateEvent{kind: taskStateEventToolResult, toolResult: toolResult})
if options.OnToolResult != nil {
options.OnToolResult(toolResult)
}
@@ -757,6 +776,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
if options.SelfCorrect != nil && len(changedFilesThisBatch) > 0 {
feedback, selfCorrectOutcome := options.SelfCorrect.AfterEdit(ctx, dedupeStrings(changedFilesThisBatch))
posture.observeSelfCorrect(selfCorrectOutcome)
task.observe(taskStateEvent{kind: taskStateEventVerification, verification: selfCorrectOutcome})
if feedback != "" {
messages = append(messages, zeroruntime.Message{
Role: zeroruntime.MessageRoleUser,
+3
View File
@@ -100,6 +100,9 @@ work.
- Use exec_command with `tty: true` for interactive terminal-style commands that
need stdin beyond Ctrl-C. `/ps` and `/stop` are user-facing TUI commands; when
you need to clean up a running foreground command yourself, use write_stdin.
- write_stdin's session_id is only ever an id returned by a still-running
exec_command; never guess or probe ids. If you have no such session, start one
with exec_command, or use write_file/edit_file/apply_patch for file changes.
- write_stdin with empty input polls an existing exec_command session, and
`\u0003` interrupts it. Sending other stdin bytes may require approval because
it can drive the running process beyond the original command. Non-tty sessions
+359
View File
@@ -0,0 +1,359 @@
package agent
import (
"encoding/json"
"reflect"
"sort"
"strings"
"github.com/Gitlawb/zero/internal/tools"
"github.com/Gitlawb/zero/internal/trace"
"github.com/Gitlawb/zero/internal/zeroruntime"
)
type taskStatus string
const (
taskStatusActive taskStatus = "active"
taskStatusComplete taskStatus = "complete"
taskStatusIncomplete taskStatus = "incomplete"
)
type taskPlanParity string
const (
taskPlanParityUnknown taskPlanParity = "unknown"
taskPlanParityMatch taskPlanParity = "match"
taskPlanParityMismatch taskPlanParity = "mismatch"
)
type taskPlanItem struct {
Content string `json:"content"`
Status string `json:"status"`
Notes string `json:"notes,omitempty"`
}
type taskPlanState struct {
Items []taskPlanItem `json:"items,omitempty"`
Pending int `json:"pending"`
InProgress int `json:"in_progress"`
Completed int `json:"completed"`
Failed int `json:"failed"`
}
type taskToolState struct {
Succeeded int `json:"succeeded"`
Failed int `json:"failed"`
}
type taskVerificationState struct {
Passed int `json:"passed"`
Failed int `json:"failed"`
LastOutcome Outcome `json:"last_outcome,omitempty"`
}
type taskStateSnapshot struct {
Revision int `json:"revision"`
Objective string `json:"objective"`
Status taskStatus `json:"status"`
Plan taskPlanState `json:"plan"`
Tools taskToolState `json:"tools"`
Verification taskVerificationState `json:"verification"`
ChangedFiles []string `json:"changed_files,omitempty"`
CompletionDecision CompletionDecision `json:"completion_decision,omitempty"`
CompletionReason string `json:"completion_reason,omitempty"`
PlanParity taskPlanParity `json:"plan_parity"`
}
type taskStateEventKind int
const (
taskStateEventPlan taskStateEventKind = iota + 1
taskStateEventToolResult
taskStateEventVerification
taskStateEventCompletion
)
type taskStateEvent struct {
kind taskStateEventKind
arguments string
toolResult ToolResult
verification Outcome
completion completionEvaluation
}
// taskState is a deterministic projection of facts the loop already observes.
// It does not initiate work or replace the transcript; consumers must check
// transcript parity before using its compact representation.
type taskState struct {
snapshotValue taskStateSnapshot
changedFiles map[string]struct{}
recorder *trace.Recorder
planObserved bool
}
func newTaskState(objective string, recorder *trace.Recorder) *taskState {
state := &taskState{
snapshotValue: taskStateSnapshot{
Objective: strings.TrimSpace(objective),
Status: taskStatusActive,
PlanParity: taskPlanParityUnknown,
},
changedFiles: map[string]struct{}{},
}
state.recorder = recorder
return state
}
func (state *taskState) observe(event taskStateEvent) bool {
if state == nil {
return false
}
changed := false
switch event.kind {
case taskStateEventPlan:
plan, ok := parseTaskPlan(event.arguments)
if !ok {
return false
}
state.snapshotValue.Plan = summarizeTaskPlan(plan)
state.planObserved = true
state.markActive()
changed = true
case taskStateEventToolResult:
state.markActive()
if event.toolResult.Status == tools.StatusOK {
state.snapshotValue.Tools.Succeeded++
} else {
state.snapshotValue.Tools.Failed++
}
for _, path := range event.toolResult.ChangedFiles {
path = strings.TrimSpace(path)
if path != "" {
state.changedFiles[path] = struct{}{}
}
}
state.snapshotValue.ChangedFiles = sortedKeys(state.changedFiles)
changed = true
case taskStateEventVerification:
if event.verification == OutcomeDisabled {
return false
}
state.markActive()
state.snapshotValue.Verification.LastOutcome = event.verification
switch event.verification {
case OutcomePassed:
state.snapshotValue.Verification.Passed++
case OutcomeCorrecting, OutcomeReported, OutcomeAborted:
state.snapshotValue.Verification.Failed++
}
changed = true
case taskStateEventCompletion:
state.snapshotValue.CompletionDecision = event.completion.Decision
state.snapshotValue.CompletionReason = event.completion.Reason
switch event.completion.Decision {
case CompletionComplete:
state.snapshotValue.Status = taskStatusComplete
case CompletionIncomplete:
state.snapshotValue.Status = taskStatusIncomplete
default:
state.snapshotValue.Status = taskStatusActive
}
changed = true
}
if changed {
state.snapshotValue.Revision++
if event.kind != taskStateEventToolResult {
state.emit()
}
}
return changed
}
func (state *taskState) markActive() {
state.snapshotValue.Status = taskStatusActive
state.snapshotValue.CompletionDecision = ""
state.snapshotValue.CompletionReason = ""
}
func (state *taskState) snapshot() taskStateSnapshot {
if state == nil {
return taskStateSnapshot{}
}
snapshot := state.snapshotValue
snapshot.Plan.Items = append([]taskPlanItem(nil), state.snapshotValue.Plan.Items...)
snapshot.ChangedFiles = append([]string(nil), state.snapshotValue.ChangedFiles...)
return snapshot
}
// observePlanParity compares only the latest plan projection with the plan tool
// calls still present in messages. It mutates the snapshot and emits when the
// parity value changes; objective, tool, and verification fields are not part of
// this comparison.
func (state *taskState) observePlanParity(messages []zeroruntime.Message) taskPlanParity {
if state == nil {
return taskPlanParityUnknown
}
transcriptPlan, found, valid := latestTaskPlan(messages)
tracked := state.snapshotValue.Plan.Items
parity := taskPlanParityMatch
if !valid || found != state.planObserved || (found && !reflect.DeepEqual(transcriptPlan, tracked)) {
parity = taskPlanParityMismatch
}
if state.snapshotValue.PlanParity != parity {
state.snapshotValue.PlanParity = parity
state.snapshotValue.Revision++
state.emit()
}
return parity
}
type completionContext struct {
Objective string
PlanPending bool
PlanMatchesTranscript bool
}
func (state *taskState) completionContext(messages []zeroruntime.Message, transcriptPlanPending bool) completionContext {
context := completionContext{PlanPending: transcriptPlanPending}
if state == nil {
return context
}
context.Objective = state.snapshotValue.Objective
context.PlanMatchesTranscript = state.observePlanParity(messages) == taskPlanParityMatch
if context.PlanMatchesTranscript {
context.PlanPending = state.snapshotValue.Plan.Pending+state.snapshotValue.Plan.InProgress > 0
}
return context
}
func (state *taskState) snapshotForCompaction(messages []zeroruntime.Message) *taskStateSnapshot {
if state == nil {
return nil
}
state.observePlanParity(messages)
snapshot := state.snapshot()
return &snapshot
}
func (state *taskState) emit() {
if state == nil || state.recorder == nil {
return
}
snapshot := state.snapshotValue
state.recorder.EmitTaskState(trace.TaskStateEvent{
Revision: snapshot.Revision,
Status: string(snapshot.Status),
PlanPending: snapshot.Plan.Pending,
PlanInProgress: snapshot.Plan.InProgress,
PlanCompleted: snapshot.Plan.Completed,
PlanFailed: snapshot.Plan.Failed,
ToolsSucceeded: snapshot.Tools.Succeeded,
ToolsFailed: snapshot.Tools.Failed,
VerificationPassed: snapshot.Verification.Passed,
VerificationFailed: snapshot.Verification.Failed,
VerificationOutcome: string(snapshot.Verification.LastOutcome),
ChangedFileCount: len(snapshot.ChangedFiles),
CompletionDecision: string(snapshot.CompletionDecision),
PlanParity: string(snapshot.PlanParity),
})
}
func parseTaskPlan(arguments string) ([]taskPlanItem, bool) {
var object map[string]json.RawMessage
if json.Unmarshal([]byte(strings.TrimSpace(arguments)), &object) != nil {
return nil, false
}
rawPlan, ok := object["plan"]
if !ok || string(rawPlan) == "null" {
return nil, false
}
var parsed []struct {
ID *string `json:"id"`
Content string `json:"content"`
Status string `json:"status"`
Notes string `json:"notes"`
}
if json.Unmarshal(rawPlan, &parsed) != nil {
return nil, false
}
plan := make([]taskPlanItem, 0, len(parsed))
for _, raw := range parsed {
if raw.Content == "" {
return nil, false
}
plan = append(plan, taskPlanItem{
Content: raw.Content,
Status: normalizeTaskPlanStatus(raw.Status),
Notes: raw.Notes,
})
}
lastInProgress := -1
for index := range plan {
if plan[index].Status == "in_progress" {
if lastInProgress >= 0 {
plan[lastInProgress].Status = "completed"
}
lastInProgress = index
}
}
if len(plan) == 0 {
return nil, true
}
return plan, true
}
func normalizeTaskPlanStatus(status string) string {
switch strings.ToLower(strings.TrimSpace(status)) {
case "completed", "complete", "done", "finished", "resolved", "✓", "x", "[x]":
return "completed"
case "in_progress", "in-progress", "inprogress", "in progress", "active", "doing", "started", "current", "wip", "ongoing", "running":
return "in_progress"
case "failed", "fail", "error", "errored", "blocked", "cancelled", "canceled", "abandoned", "skipped":
return "failed"
default:
return "pending"
}
}
func summarizeTaskPlan(items []taskPlanItem) taskPlanState {
plan := taskPlanState{Items: append([]taskPlanItem(nil), items...)}
for _, item := range items {
switch item.Status {
case "completed":
plan.Completed++
case "in_progress":
plan.InProgress++
case "failed":
plan.Failed++
default:
plan.Pending++
}
}
return plan
}
func latestTaskPlan(messages []zeroruntime.Message) ([]taskPlanItem, bool, bool) {
for i := len(messages) - 1; i >= 0; i-- {
for j := len(messages[i].ToolCalls) - 1; j >= 0; j-- {
call := messages[i].ToolCalls[j]
if call.Name != planToolName {
continue
}
plan, ok := parseTaskPlan(call.Arguments)
return plan, true, ok
}
}
return nil, false, true
}
func sortedKeys(values map[string]struct{}) []string {
keys := make([]string, 0, len(values))
for value := range values {
keys = append(keys, value)
}
// Paths are evidence, not chronology. Stable ordering makes replay snapshots
// deterministic even if events originate from parallel read batches later.
sort.Strings(keys)
return keys
}
+204
View File
@@ -0,0 +1,204 @@
package agent
import (
"context"
"reflect"
"testing"
"github.com/Gitlawb/zero/internal/tools"
"github.com/Gitlawb/zero/internal/trace"
"github.com/Gitlawb/zero/internal/zeroruntime"
)
func TestTaskStateReplayIsDeterministic(t *testing.T) {
events := []taskStateEvent{
{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"inspect","status":"done"},{"content":"implement","status":"in progress"},{"content":"verify"}]}`},
{kind: taskStateEventToolResult, toolResult: ToolResult{Name: "apply_patch", Status: tools.StatusOK, ChangedFiles: []string{"a.go", "a.go", "b.go"}}},
{kind: taskStateEventToolResult, toolResult: ToolResult{Name: "go test", Status: tools.StatusError}},
{kind: taskStateEventVerification, verification: OutcomePassed},
{kind: taskStateEventCompletion, completion: completionEvaluation{Decision: CompletionUncertain, Reason: "pending work"}},
}
first := newTaskState("ship the change", nil)
second := newTaskState("ship the change", nil)
for _, event := range events {
first.observe(event)
second.observe(event)
}
if !reflect.DeepEqual(first.snapshot(), second.snapshot()) {
t.Fatalf("replaying the same events produced different snapshots:\nfirst: %#v\nsecond: %#v", first.snapshot(), second.snapshot())
}
got := first.snapshot()
if got.Status != taskStatusActive || got.Plan.Pending != 1 || got.Plan.InProgress != 1 || got.Plan.Completed != 1 {
t.Fatalf("unexpected plan snapshot: %#v", got)
}
if got.Tools.Succeeded != 1 || got.Tools.Failed != 1 || len(got.ChangedFiles) != 2 {
t.Fatalf("unexpected evidence snapshot: %#v", got)
}
if got.Verification.Passed != 1 || got.Verification.Failed != 0 || got.Verification.LastOutcome != OutcomePassed {
t.Fatalf("unexpected verification snapshot: %#v", got.Verification)
}
}
func TestTaskStateCoalescesToolResultsIntoNextTraceEvent(t *testing.T) {
recorder := trace.NewRecorder("session", "run", "")
state := newTaskState("objective", recorder)
for range 20 {
state.observe(taskStateEvent{kind: taskStateEventToolResult, toolResult: ToolResult{Status: tools.StatusOK}})
}
if events := recorder.Finish().TaskStates; len(events) != 0 {
t.Fatalf("tool results should be coalesced, got %d trace events", len(events))
}
recorder = trace.NewRecorder("session", "run-2", "")
state = newTaskState("objective", recorder)
for range 20 {
state.observe(taskStateEvent{kind: taskStateEventToolResult, toolResult: ToolResult{Status: tools.StatusOK}})
}
state.observe(taskStateEvent{kind: taskStateEventCompletion, completion: completionEvaluation{Decision: CompletionComplete}})
events := recorder.Finish().TaskStates
if len(events) != 1 || events[0].ToolsSucceeded != 20 {
t.Fatalf("coalesced tool total missing from completion snapshot: %#v", events)
}
}
func TestRunEmitsTaskStateFromExistingLoopEvents(t *testing.T) {
registry := tools.NewRegistry()
registry.Register(tools.NewUpdatePlanTool())
provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
{
{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "plan-1", ToolName: planToolName},
{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "plan-1", ArgumentsFragment: `{"plan":[{"content":"implement","status":"completed"}]}`},
{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "plan-1"},
{Type: zeroruntime.StreamEventDone},
},
{
{Type: zeroruntime.StreamEventText, Content: "done"},
{Type: zeroruntime.StreamEventDone},
},
}}
recorder := trace.NewRecorder("session", "run", "")
result, err := Run(context.Background(), "implement the change", provider, Options{
Registry: registry,
Trace: recorder,
})
if err != nil {
t.Fatalf("Run: %v", err)
}
if result.FinalAnswer != "done" {
t.Fatalf("final answer = %q, want done", result.FinalAnswer)
}
events := recorder.Finish().TaskStates
if len(events) < 3 {
t.Fatalf("expected plan, tool, completion, and parity snapshots, got %#v", events)
}
last := events[len(events)-1]
if last.Status != string(taskStatusComplete) || last.PlanCompleted != 1 || last.ToolsSucceeded != 1 || last.PlanParity != string(taskPlanParityMatch) {
t.Fatalf("unexpected final task snapshot: %#v", last)
}
}
func TestTaskStateSnapshotIsImmutable(t *testing.T) {
state := newTaskState("objective", nil)
state.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"one","status":"pending"}]}`})
state.observe(taskStateEvent{kind: taskStateEventToolResult, toolResult: ToolResult{Status: tools.StatusOK, ChangedFiles: []string{"one.go"}}})
snapshot := state.snapshot()
snapshot.Plan.Items[0].Content = "mutated"
snapshot.ChangedFiles[0] = "mutated.go"
fresh := state.snapshot()
if fresh.Plan.Items[0].Content != "one" || fresh.ChangedFiles[0] != "one.go" {
t.Fatalf("snapshot mutation leaked into task state: %#v", fresh)
}
}
func TestTaskStatePlanParityUsesLatestPlan(t *testing.T) {
state := newTaskState("objective", nil)
state.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"old","status":"completed"}]}`})
state.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"new","status":"in_progress"}]}`})
messages := []zeroruntime.Message{{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{
{Name: planToolName, Arguments: `{"plan":[{"content":"new","status":"in_progress"}]}`},
}}}
if parity := state.observePlanParity(messages); parity != taskPlanParityMatch {
t.Fatalf("parity = %q, want match", parity)
}
messages[0].ToolCalls[0].Arguments = `{"plan":[{"content":"different","status":"pending"}]}`
if parity := state.observePlanParity(messages); parity != taskPlanParityMismatch {
t.Fatalf("parity = %q, want mismatch", parity)
}
}
func TestTaskStateMatchesPlanToolNormalization(t *testing.T) {
state := newTaskState("objective", nil)
arguments := `{"plan":[{"content":"first","status":"in_progress"},{"content":"second","status":"in_progress"}]}`
state.observe(taskStateEvent{kind: taskStateEventPlan, arguments: arguments})
snapshot := state.snapshot()
if snapshot.Plan.Completed != 1 || snapshot.Plan.InProgress != 1 {
t.Fatalf("multiple active items were not normalized like the plan tool: %#v", snapshot.Plan)
}
messages := []zeroruntime.Message{{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{{Name: planToolName, Arguments: arguments}}}}
if parity := state.observePlanParity(messages); parity != taskPlanParityMatch {
t.Fatalf("normalized state should still match its transcript event, got %q", parity)
}
empty := newTaskState("objective", nil)
empty.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[]}`})
emptyMessages := []zeroruntime.Message{{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{{Name: planToolName, Arguments: `{"plan":[]}`}}}}
if parity := empty.observePlanParity(emptyMessages); parity != taskPlanParityMatch {
t.Fatalf("explicit empty plan should match transcript, got %q", parity)
}
}
func TestTaskStateContextFallsBackWhenTranscriptDiffers(t *testing.T) {
state := newTaskState("objective", nil)
state.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"tracked","status":"completed"}]}`})
messages := []zeroruntime.Message{{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{
{Name: planToolName, Arguments: `{"plan":[{"content":"transcript","status":"pending"}]}`},
}}}
context := state.completionContext(messages, true)
if !context.PlanPending || context.PlanMatchesTranscript {
t.Fatalf("completion context must retain transcript truth on mismatch: %#v", context)
}
if context.Objective != "objective" {
t.Fatalf("objective lost from completion context: %#v", context)
}
}
func TestTaskStateCompactionSnapshotRetainsObjectiveOnPlanMismatch(t *testing.T) {
state := newTaskState("objective", nil)
state.observe(taskStateEvent{kind: taskStateEventPlan, arguments: `{"plan":[{"content":"verify","status":"pending"}]}`})
matching := []zeroruntime.Message{{Role: zeroruntime.MessageRoleAssistant, ToolCalls: []zeroruntime.ToolCall{
{Name: planToolName, Arguments: `{"plan":[{"content":"verify","status":"pending"}]}`},
}}}
if snapshot := state.snapshotForCompaction(matching); snapshot == nil || snapshot.Objective != "objective" {
t.Fatalf("matching transcript should produce compact state, got %#v", snapshot)
}
mismatching := append([]zeroruntime.Message(nil), matching...)
mismatching[0].ToolCalls = []zeroruntime.ToolCall{{Name: planToolName, Arguments: `{"plan":[{"content":"other","status":"pending"}]}`}}
if snapshot := state.snapshotForCompaction(mismatching); snapshot == nil || snapshot.Objective != "objective" || snapshot.PlanParity != taskPlanParityMismatch {
t.Fatalf("plan mismatch must retain immutable objective and mark mutable state uncorroborated, got %#v", snapshot)
}
}
func TestParseTaskPlanRejectsArgumentsTheToolWouldReject(t *testing.T) {
for _, arguments := range []string{
`{}`,
`{"plan":null}`,
`{"plan":[{"step":"alias is not accepted"}]}`,
`{"plan":[{"content":""}]}`,
`{"plan":[{"content":"valid"},{"content":""}]}`,
`{"plan":[{"id":4,"content":"valid"}]}`,
} {
if plan, ok := parseTaskPlan(arguments); ok {
t.Fatalf("parseTaskPlan(%s) = %#v, true; want rejected", arguments, plan)
}
}
}
+159
View File
@@ -53,6 +53,22 @@ func runProvidersUse(args []string, stdout io.Writer, stderr io.Writer, deps app
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
}
// SetActiveProvider only ever matches profiles persisted in config.json
// (see config.ProviderPersisted), but a provider can be visible in
// `zero providers list`/the TUI picker purely because Resolve()
// synthesized it in-memory from an ambient env var (e.g. OPENAI_API_KEY)
// without ever writing a row to disk. Without this check, switching to
// that provider by name always fails with a confusing "not found" even
// though it is genuinely usable this session (issue #707).
persisted, err := config.ProviderPersisted(configPath, options.name)
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
}
if !persisted {
if exit, handled := reportUnpersistedProviderUse(stdout, stderr, deps, options, configPath); handled {
return exit
}
}
cfg, err := config.SetActiveProvider(configPath, options.name)
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
@@ -352,6 +368,22 @@ func runProvidersRemove(args []string, stdout io.Writer, stderr io.Writer, deps
return writeAppError(stderr, err.Error(), exitCrash)
}
name := options.names[0]
// RemoveProvider only ever matches profiles persisted in config.json (see
// config.ProviderPersisted), but a provider can be visible in
// `zero providers list`/the TUI picker purely because Resolve()
// synthesized it in-memory from an ambient env var (e.g. OPENAI_API_KEY)
// without ever writing a row to disk. Without this check, deleting that
// provider by name always fails with a confusing "not found" even though
// it is genuinely visible/usable this session (issue #707).
persisted, err := config.ProviderPersisted(configPath, name)
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
}
if !persisted {
if exit, handled := reportUnpersistedProviderRemove(stdout, stderr, deps, name, options.json, configPath); handled {
return exit
}
}
cfg, err := config.RemoveProvider(configPath, name)
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
@@ -428,6 +460,16 @@ func runProvidersRename(args []string, stdout io.Writer, stderr io.Writer, deps
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
}
oldName := options.names[0]
persisted, err := config.ProviderPersisted(configPath, oldName)
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
}
if !persisted {
if exit, handled := reportUnpersistedProviderRename(stdout, stderr, deps, oldName, options.json, configPath); handled {
return exit
}
}
cfg, err := config.RenameProvider(configPath, options.names[0], options.names[1])
if err != nil {
return writeAppError(stderr, err.Error(), exitCrash)
@@ -447,3 +489,120 @@ func runProvidersRename(args []string, stdout io.Writer, stderr io.Writer, deps
}
return exitSuccess
}
// providerResolvedByName reports whether name matches a provider in a
// resolved provider list — used to tell a genuinely unknown name (a typo)
// apart from a real, env-derived provider that just has no config.json row.
func providerResolvedByName(providers []config.ProviderProfile, name string) bool {
name = strings.TrimSpace(name)
for _, provider := range providers {
if strings.EqualFold(strings.TrimSpace(provider.Name), name) {
return true
}
}
return false
}
// reportUnpersistedProviderUse handles `zero providers use <name>` for a
// provider that is not persisted in config.json. If it's not resolvable at
// all (an unknown/misspelled name), it returns handled=false so the caller
// falls through to SetActiveProvider's real "not found" error. If it IS
// resolvable — an env-derived profile (e.g. ambient OPENAI_API_KEY) —
// SetActiveProvider would only ever fail "not found" against it, so this
// reports the situation plainly instead of that confusing error (issue
// #707).
func reportUnpersistedProviderUse(stdout, stderr io.Writer, deps appDeps, options providerUseOptions, configPath string) (int, bool) {
resolved, exitCode := resolveCommandCenterConfig(stderr, deps)
if exitCode != exitSuccess {
// resolveCommandCenterConfig already wrote its own error to stderr;
// stop here instead of letting the caller try SetActiveProvider too.
return exitCode, true
}
if !providerResolvedByName(resolved.Providers, options.name) {
return exitCode, false
}
message := fmt.Sprintf(
"Provider %q is not saved in config.json (likely set via an environment variable), so there is no saved profile to switch to.\nIt is available whenever its environment variable is set, but is only active when selected (for example via ZERO_PROVIDER); unset its environment variable to stop Zero from detecting it automatically.",
options.name,
)
if options.json {
if err := writePrettyJSON(stdout, map[string]any{
"activeProvider": resolved.ActiveProvider,
"configPath": configPath,
"persisted": false,
"message": message,
}); err != nil {
return exitCrash, true
}
return exitSuccess, true
}
if _, err := fmt.Fprintln(stdout, message); err != nil {
return exitCrash, true
}
return exitSuccess, true
}
// reportUnpersistedProviderRemove handles `zero providers remove <name>` for
// a provider that is not persisted in config.json, mirroring the TUI
// provider manager's delete handling (internal/tui/provider_manager.go). If
// name isn't resolvable at all, it returns handled=false so the caller falls
// through to RemoveProvider's real "not found" error.
func reportUnpersistedProviderRemove(stdout, stderr io.Writer, deps appDeps, name string, jsonOutput bool, configPath string) (int, bool) {
resolved, exitCode := resolveCommandCenterConfig(stderr, deps)
if exitCode != exitSuccess {
return exitCode, true
}
if !providerResolvedByName(resolved.Providers, name) {
return exitCode, false
}
message := fmt.Sprintf(
"Provider %q is not saved in config.json (likely set via an environment variable) — nothing to remove there.\nUnset its environment variable to stop Zero from detecting it automatically.",
name,
)
if jsonOutput {
if err := writePrettyJSON(stdout, map[string]any{
"removed": "",
"keyRemoved": false,
"activeProvider": resolved.ActiveProvider,
"configPath": configPath,
"persisted": false,
"message": message,
}); err != nil {
return exitCrash, true
}
return exitSuccess, true
}
if _, err := fmt.Fprintln(stdout, message); err != nil {
return exitCrash, true
}
return exitSuccess, true
}
func reportUnpersistedProviderRename(stdout, stderr io.Writer, deps appDeps, name string, jsonOutput bool, configPath string) (int, bool) {
resolved, exitCode := resolveCommandCenterConfig(stderr, deps)
if exitCode != exitSuccess {
return exitCode, true
}
if !providerResolvedByName(resolved.Providers, name) {
return exitCode, false
}
message := fmt.Sprintf(
"Provider %q is not saved in config.json (likely set via an environment variable), so there is no saved profile to rename.",
name,
)
if jsonOutput {
if err := writePrettyJSON(stdout, map[string]any{
"renamed": nil,
"configPath": configPath,
"persisted": false,
"message": message,
}); err != nil {
return exitCrash, true
}
return exitSuccess, true
}
if _, err := fmt.Fprintln(stdout, message); err != nil {
return exitCrash, true
}
return exitSuccess, true
}
+105
View File
@@ -75,6 +75,111 @@ func TestRunProvidersUseJSONIncludesActiveProviderAndConfigPath(t *testing.T) {
}
}
func TestRunProvidersUseSurfacesMalformedConfig(t *testing.T) {
var stdout, stderr bytes.Buffer
configPath := filepath.Join(t.TempDir(), "config.json")
if err := os.WriteFile(configPath, []byte("{"), 0o600); err != nil {
t.Fatalf("write malformed config: %v", err)
}
exitCode := runWithDeps([]string{"providers", "use", "openai"}, &stdout, &stderr, providerSetupDeps(configPath))
if exitCode != exitCrash {
t.Fatalf("exit code = %d, want %d", exitCode, exitCrash)
}
if !strings.Contains(stderr.String(), "invalid config JSON") {
t.Fatalf("stderr = %q, want malformed-config error", stderr.String())
}
}
func TestRunProvidersUseEnvDerivedJSONIncludesConfigPath(t *testing.T) {
t.Setenv("OPENAI_API_KEY", "sk-env")
var stdout, stderr bytes.Buffer
configPath := filepath.Join(t.TempDir(), "config.json")
writeProviderOnboardingConfig(t, configPath, config.FileConfig{})
exitCode := runWithDeps([]string{"providers", "use", "openai", "--json"}, &stdout, &stderr, providerSetupDeps(configPath))
if exitCode != exitSuccess {
t.Fatalf("exit code = %d, want %d: %s", exitCode, exitSuccess, stderr.String())
}
var payload struct {
ConfigPath string `json:"configPath"`
Persisted bool `json:"persisted"`
}
if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil {
t.Fatalf("decode JSON: %v\n%s", err, stdout.String())
}
if payload.ConfigPath != configPath || payload.Persisted {
t.Fatalf("unexpected payload: %+v", payload)
}
}
func TestRunProvidersRemoveEnvDerivedJSONKeepsSchema(t *testing.T) {
t.Setenv("OPENAI_API_KEY", "sk-env")
var stdout, stderr bytes.Buffer
configPath := filepath.Join(t.TempDir(), "config.json")
writeProviderOnboardingConfig(t, configPath, config.FileConfig{})
exitCode := runWithDeps([]string{"providers", "remove", "openai", "--json"}, &stdout, &stderr, providerSetupDeps(configPath))
if exitCode != exitSuccess {
t.Fatalf("exit code = %d, want %d: %s", exitCode, exitSuccess, stderr.String())
}
var payload struct {
Removed string `json:"removed"`
KeyRemoved bool `json:"keyRemoved"`
Persisted bool `json:"persisted"`
ConfigPath string `json:"configPath"`
}
if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil {
t.Fatalf("decode JSON: %v\n%s", err, stdout.String())
}
if payload.Removed != "" || payload.KeyRemoved || payload.Persisted || payload.ConfigPath != configPath {
t.Fatalf("unexpected payload: %+v", payload)
}
}
func TestRunProvidersRenameEnvDerivedExplainsNoSavedProfile(t *testing.T) {
t.Setenv("OPENAI_API_KEY", "sk-env")
var stdout, stderr bytes.Buffer
configPath := filepath.Join(t.TempDir(), "config.json")
writeProviderOnboardingConfig(t, configPath, config.FileConfig{})
exitCode := runWithDeps([]string{"providers", "rename", "openai", "renamed"}, &stdout, &stderr, providerSetupDeps(configPath))
if exitCode != exitSuccess {
t.Fatalf("exit code = %d, want %d: %s", exitCode, exitSuccess, stderr.String())
}
if !strings.Contains(stdout.String(), "no saved profile to rename") {
t.Fatalf("stdout = %q, want unpersisted explanation", stdout.String())
}
}
func TestRunProvidersRenameEnvDerivedJSONKeepsSchema(t *testing.T) {
t.Setenv("OPENAI_API_KEY", "sk-env")
var stdout, stderr bytes.Buffer
configPath := filepath.Join(t.TempDir(), "config.json")
writeProviderOnboardingConfig(t, configPath, config.FileConfig{})
exitCode := runWithDeps([]string{"providers", "rename", "openai", "renamed", "--json"}, &stdout, &stderr, providerSetupDeps(configPath))
if exitCode != exitSuccess {
t.Fatalf("exit code = %d, want %d: %s", exitCode, exitSuccess, stderr.String())
}
var payload struct {
Renamed *struct{} `json:"renamed"`
ConfigPath string `json:"configPath"`
Persisted bool `json:"persisted"`
}
if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil {
t.Fatalf("decode JSON: %v\n%s", err, stdout.String())
}
if payload.Renamed != nil || payload.ConfigPath != configPath || payload.Persisted {
t.Fatalf("unexpected payload: %+v", payload)
}
}
func TestRunProvidersUseRejectsUsageErrors(t *testing.T) {
cases := []struct {
name string
+27
View File
@@ -177,6 +177,33 @@ func SetActiveProvider(path string, name string) (FileConfig, error) {
return FileConfig{}, fmt.Errorf("provider %q not found", name)
}
// ProviderPersisted reports whether a provider profile named name actually has
// a row in the config file at path. A provider can appear in the resolved/
// in-memory provider list without ever being written to config.json — e.g.
// applyProviderEnv synthesizes an "openai" profile purely from an ambient
// OPENAI_API_KEY environment variable on every Resolve() call, without ever
// persisting it. RemoveProvider/SetActiveProvider/SetProviderModel only ever
// look at what's on disk, so a caller offering to mutate a provider by name
// should check this first: "not on disk" needs different handling (nothing to
// persist/remove there) than a name that doesn't exist anywhere at all.
func ProviderPersisted(path string, name string) (bool, error) {
path = strings.TrimSpace(path)
name = strings.TrimSpace(name)
if path == "" || name == "" {
return false, nil
}
cfg, err := loadConfigFile(path)
if err != nil {
return false, err
}
for _, provider := range cfg.Providers {
if strings.EqualFold(strings.TrimSpace(provider.Name), name) {
return true, nil
}
}
return false, nil
}
// RemoveProvider deletes the named provider profile from the config at path.
// When the removed profile was active, activeProvider hands off to the first
// remaining provider (or clears when none remain) so the config never points at
+6 -6
View File
@@ -49,9 +49,9 @@ func RequestDeviceCode(ctx context.Context, client *http.Client, cfg Config, now
if err := validateTokenEndpoint(endpoint); err != nil {
return DeviceAuth{}, err
}
if client == nil {
client = http.DefaultClient
}
// Refuse redirects so a device-authorization redirect can't replay the
// client_id/client_secret POST body to an unvalidated origin.
client = withoutRedirects(client)
if now == nil {
now = time.Now
}
@@ -157,9 +157,9 @@ func pollDeviceOnce(ctx context.Context, client *http.Client, cfg Config, device
if err := validateTokenEndpoint(cfg.TokenEndpoint); err != nil {
return Token{}, err
}
if client == nil {
client = http.DefaultClient
}
// Refuse redirects so a poll redirect can't replay the device_code/secret
// POST body to an unvalidated origin.
client = withoutRedirects(client)
form := url.Values{}
form.Set("grant_type", deviceGrantType)
form.Set("device_code", deviceCode)
+25
View File
@@ -2,6 +2,7 @@ package oauth
import (
"context"
"errors"
"net/http"
"net/http/httptest"
"strings"
@@ -28,6 +29,30 @@ func TestRequestDeviceCodeSendsClientSecret(t *testing.T) {
}
}
func TestRequestDeviceCodeRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusTemporaryRedirect) // 307
_, err := RequestDeviceCode(context.Background(), http.DefaultClient,
Config{ClientID: "c", ClientSecret: "shh", DeviceAuthorizationEndpoint: endpoint}, nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("RequestDeviceCode err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — client_secret replayed", n)
}
}
func TestPollDeviceOnceRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusPermanentRedirect) // 308
_, err := pollDeviceOnce(context.Background(), http.DefaultClient,
Config{ClientID: "c", ClientSecret: "shh", TokenEndpoint: endpoint}, "device-code", nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("pollDeviceOnce err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — device_code/secret replayed", n)
}
}
func TestRequestDeviceCode(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte(`{"device_code":"dc","user_code":"WXYZ-1234","verification_uri":"https://x/dev","verification_uri_complete":"https://x/dev?c=WXYZ","expires_in":600,"interval":3}`))
+20 -3
View File
@@ -116,6 +116,23 @@ func validateTokenEndpoint(endpoint string) error {
return ValidateEndpointURL(endpoint)
}
// withoutRedirects returns a shallow copy of client whose redirect policy
// refuses to follow ANY redirect. A credential-bearing OAuth POST (token
// exchange/refresh, device authorization, device-token poll) must never let a
// 307/308 replay its form body — codes, PKCE verifiers, refresh tokens, client
// secrets — to a target that was never endpoint-validated. The caller's client
// is left unmodified.
func withoutRedirects(client *http.Client) *http.Client {
if client == nil {
client = http.DefaultClient
}
copied := *client
copied.CheckRedirect = func(*http.Request, []*http.Request) error {
return ErrUnsafeRedirect
}
return &copied
}
func isLoopbackHost(host string) bool {
if host == "localhost" {
return true
@@ -176,9 +193,9 @@ func PostToken(ctx context.Context, client *http.Client, tokenEndpoint string, f
if err := validateTokenEndpoint(tokenEndpoint); err != nil {
return Token{}, err
}
if client == nil {
client = http.DefaultClient
}
// Refuse redirects so a 307/308 from the token endpoint can't replay this
// credential-bearing POST body to another (unvalidated) origin.
client = withoutRedirects(client)
if now == nil {
now = time.Now
}
+60
View File
@@ -5,10 +5,12 @@ import (
"crypto/sha256"
"encoding/base64"
"errors"
"io"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"sync/atomic"
"testing"
"time"
)
@@ -126,6 +128,64 @@ func TestBuildAuthorizationURLRejectsInsecureEndpoint(t *testing.T) {
}
}
// redirectTrap starts an "attacker" server that counts any hit, and a credential
// endpoint that redirects to it with the given status. It returns the endpoint
// URL and a func reporting how many times the attacker was contacted — a
// non-zero count means the credential-bearing POST body was replayed (#729).
func redirectTrap(t *testing.T, status int) (endpoint string, attackerHits func() int32) {
t.Helper()
var hits int32
attacker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
atomic.AddInt32(&hits, 1)
_, _ = io.WriteString(w, `{"access_token":"leaked","refresh_token":"leaked"}`)
}))
t.Cleanup(attacker.Close)
endp := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, attacker.URL, status)
}))
t.Cleanup(endp.Close)
return endp.URL, func() int32 { return atomic.LoadInt32(&hits) }
}
func TestPostTokenRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusTemporaryRedirect) // 307
form := url.Values{}
form.Set("grant_type", "authorization_code")
form.Set("code", "the-code")
form.Set("client_secret", "shh")
_, err := PostToken(context.Background(), http.DefaultClient, endpoint, form, Token{}, nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("PostToken err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — credential body was replayed", n)
}
}
func TestExchangeCodeRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusPermanentRedirect) // 308
_, err := ExchangeCode(context.Background(), http.DefaultClient,
Config{ClientID: "c", TokenEndpoint: endpoint}, "the-code", "verifier", "http://127.0.0.1/cb", nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("ExchangeCode err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — code/verifier replayed", n)
}
}
func TestRefreshRefusesRedirect(t *testing.T) {
endpoint, hits := redirectTrap(t, http.StatusTemporaryRedirect) // 307
_, err := Refresh(context.Background(), http.DefaultClient,
Config{ClientID: "c", TokenEndpoint: endpoint}, Token{RefreshToken: "rt"}, nil)
if !errors.Is(err, ErrUnsafeRedirect) {
t.Fatalf("Refresh err = %v, want ErrUnsafeRedirect", err)
}
if n := hits(); n != 0 {
t.Fatalf("attacker received %d request(s) — refresh_token replayed", n)
}
}
func TestValidateTokenEndpoint(t *testing.T) {
cases := map[string]bool{
"https://auth.example.com/token": true,
+5
View File
@@ -90,6 +90,11 @@ var (
// ErrInsecureTokenEndpoint is returned when a credential would be sent over a
// non-https, non-loopback endpoint.
ErrInsecureTokenEndpoint = errors.New("oauth: refusing to send credential to a non-https token endpoint")
// ErrUnsafeRedirect is returned when a credential-bearing OAuth POST (token
// exchange/refresh, device authorization, or device-token poll) is redirected.
// Following it would replay the form body — codes, PKCE verifiers, refresh
// tokens, client secrets — to a target that never passed endpoint validation.
ErrUnsafeRedirect = errors.New("oauth: refusing to follow a redirect from a credential endpoint")
// ErrNoRefreshToken is returned when a refresh is attempted without one.
ErrNoRefreshToken = errors.New("oauth: no refresh token available")
// ErrAuthorizationPending is the RFC 8628 "keep polling" signal.
+72 -17
View File
@@ -578,28 +578,54 @@ func WriteTurnBenchJSON(w io.Writer, result TurnBenchResult) error {
return encoder.Encode(result)
}
// execExitIncomplete mirrors internal/cli.exitIncomplete (the `zero exec` exit
// code, 4) for a headless run the completion gate marked INCOMPLETE: the run
// stopped without a completion signal (e.g. it couldn't self-verify because its
// sandboxed shell was unavailable under --auto member), which is distinct from a
// crash (1), usage error (2), provider failure (3), or interruption (130). It is
// the ONLY nonzero exit an oracle-bearing task may defer to the oracle for; every
// other nonzero exit stays authoritative. Kept as a local literal so perfbench
// doesn't import the cli package; must stay in sync with that constant.
const execExitIncomplete = 4
// NewTurnExecRunner builds the production turn-benchmark runner: it invokes
// headless `zero exec` with stream-json output AND `--trace <tmpfile>`, then
// parses the emitted NDJSON trace into a *trace.TurnTrace. binary is the path to
// the `zero` binary; extraArgs are appended to every invocation. Pass/fail is
// decided from the stream-json run_end exit code (and the task's
// VerificationCommand when present), exactly like NewExecRunner.
// decided by the task's oracle when it has one. The oracle that gates the verdict
// is the VerificationCommand (which compiles and runs the stamped OracleTest via
// `go test`, plus any greps against the fixture); it is ground truth, so an
// oracle-bearing task that applied the right edit but exited INCOMPLETE (exit 4,
// its sandboxed self-verification couldn't run) still passes. Any OTHER nonzero
// exit (crash/usage/provider/interrupt) stays authoritative and fails even an
// oracle-bearing task. A latency-only task has no VerificationCommand, so any
// nonzero exit is its only signal and fails it.
func NewTurnExecRunner(binary string, extraArgs ...string) TurnRunner {
return func(ctx context.Context, task BenchTask, rc RunContext) TurnTaskOutcome {
// buildTurnExecArgs grants the write + sandboxed-shell tool set to EVERY
// invocation, so a task MUST run inside an isolated fixture copy — never the
// caller's cwd. The grant is unconditional, so the isolation can't be
// optional: a fixtureless task would turn an agent with write/shell tools
// loose in the caller's working directory (their real repo). Enforce the
// invariant instead of trusting it — reject a fixtureless task up front,
// before anything is launched, so the grant and the guard can't drift apart.
// Every shipped task declares a fixture, so this only fires on a malformed or
// newly added task, and it fails loudly rather than silently.
if strings.TrimSpace(task.WorkspaceFixture) == "" {
return TurnTaskOutcome{Err: errors.New("benchmark task has no workspaceFixture: the run grants write/shell tools and must execute in an isolated fixture copy, not the caller's cwd")}
}
// Isolate the workspace: copy the fixture into a fresh temp dir so a
// mutating task (edit/fix/refactor) can't dirty the shared, checked-in
// fixture or bleed into a later iteration of the same task. When no
// fixture is configured the agent runs in the caller's cwd as before.
if fixture := strings.TrimSpace(task.WorkspaceFixture); fixture != "" {
copyDir, parent, cerr := copyFixture(fixture)
if cerr != nil {
return TurnTaskOutcome{Err: fmt.Errorf("isolate fixture: %w", cerr)}
}
// Clean the whole unique parent (which owns copyDir) so the
// per-invocation scratch dir never leaks.
defer os.RemoveAll(parent)
task.WorkspaceFixture = copyDir
// fixture or bleed into a later iteration of the same task. The guard above
// guarantees WorkspaceFixture is set, so this always runs.
copyDir, parent, cerr := copyFixture(strings.TrimSpace(task.WorkspaceFixture))
if cerr != nil {
return TurnTaskOutcome{Err: fmt.Errorf("isolate fixture: %w", cerr)}
}
// Clean the whole unique parent (which owns copyDir) so the per-invocation
// scratch dir never leaks.
defer os.RemoveAll(parent)
task.WorkspaceFixture = copyDir
traceFile, err := os.CreateTemp("", "zero-turn-trace-*.ndjson")
if err != nil {
@@ -656,10 +682,23 @@ func NewTurnExecRunner(binary string, extraArgs ...string) TurnRunner {
}
}
// A nonzero agent exit already decided failure; don't run verification or
// mark the task passed.
// Decide what a nonzero exit means. We defer to the oracle for exactly ONE
// case: an oracle-bearing task that exited INCOMPLETE (exit 4). That is the
// completion gate downgrading a run whose edits may be correct but that
// couldn't emit a completion signal (e.g. its sandboxed self-verify couldn't
// run under --auto member) — the oracle below (the compiled test / grep run
// against the actual fixture files) is ground truth, so if the edit really
// landed it passes, and if the run abandoned the task the oracle fails it.
// Every OTHER nonzero exit stays authoritative: a crash (1), usage error
// (2), provider failure (3), or interruption (130) is a genuine failure, so
// a partial edit that happens to satisfy the oracle can't launder it into a
// pass. A latency-only task has no oracle to defer to, so any nonzero exit
// fails it.
if outcome.VerifyErr != "" {
return outcome
if len(task.VerificationCommand) == 0 || exitCode != execExitIncomplete {
return outcome
}
outcome.VerifyErr = ""
}
// Stamp the compiler-backed oracle and capture the agent's final answer
// BEFORE running the verification command. Both write into the fixture
@@ -790,7 +829,23 @@ func stampOracleAndAnswer(task BenchTask, outBuf []byte) error {
}
func buildTurnExecArgs(task BenchTask, rc RunContext, tracePath string, extraArgs []string) []string {
args := []string{"exec", "--output-format", "stream-json", "--trace", tracePath}
// --auto member is REQUIRED, not optional: without a permission grant `zero
// exec` runs in its default read-only posture, which exposes no write or shell
// tools (no edit_file/apply_patch/write_file/exec_command/bash). The mutating
// task classes (edit/fix/refactor) then cannot apply any change, so every run
// grinds to the turn ceiling or reports a no-tool blocker, and the only tasks
// that "pass" are ones whose oracle grep matches the stamped answer text rather
// than a real edit. member-auto grants the write + sandboxed-shell tools the
// benchmark needs while keeping the workspace/network/destructive safeguards, so
// it is preferred over the broader --skip-permissions-unsafe (which also drops
// those guards and auto-retries blocked commands unsandboxed). Note the shell it
// grants is sandboxed: on a host without sandbox setup the agent's own
// self-verification (go test/build) can't run and the turn exits INCOMPLETE even
// after a correct edit — the runner treats the stamped oracle, not that exit
// code, as ground truth for oracle-bearing tasks (see NewTurnExecRunner), so a
// correct edit still passes. Each task also runs in an isolated, throwaway
// fixture copy, so the granted tools have nothing outside the task to harm.
args := []string{"exec", "--auto", "member", "--output-format", "stream-json", "--trace", tracePath}
if model := strings.TrimSpace(rc.Model); model != "" {
args = append(args, "--model", model)
}
+138
View File
@@ -5,6 +5,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
"strings"
@@ -750,6 +751,92 @@ echo '{"type":"run_end","exitCode":0}'
}
}
// TestOracleAuthoritativeOnIncompleteExit is the companion to the --auto member
// switch: under member-auto the agent's shell is sandboxed, so on a host without
// sandbox setup its self-verification (go test/build) can't run and the turn
// exits INCOMPLETE (exit 4) even after a correct edit. For an ORACLE-bearing task
// the runner must treat the stamped oracle — not that INCOMPLETE exit — as ground
// truth: here the stub applies the real edit-01 rename but reports exitCode 4, and
// the task must still pass because the fixture is correct. This defers ONLY for
// exit 4; TestNonIncompleteExitStaysAuthoritative pins the other side.
func TestOracleAuthoritativeOnIncompleteExit(t *testing.T) {
task := loadBaselineTask(t, "edit-01")
outcome := runTurnStub(t, task, `sed 's/const MaxRetries = 3/const RetryLimit = 3/' main.go > .zero-tmp && mv .zero-tmp main.go
echo '{"type":"run_end","exitCode":4}'
`)
if outcome.Err != nil {
t.Fatalf("incomplete-exit with a correct edit should pass, got harness error: %v", outcome.Err)
}
if !outcome.Passed {
t.Fatalf("a correct edit that exited INCOMPLETE (exit 4) must still pass its oracle: %+v", outcome)
}
if strings.TrimSpace(outcome.VerifyErr) != "" {
t.Fatalf("a passing oracle must clear the exit-code VerifyErr, got %q", outcome.VerifyErr)
}
}
// TestNonIncompleteExitStaysAuthoritative is the guard the oracle-authoritative
// change MUST NOT weaken: only INCOMPLETE (exit 4) defers to the oracle. A crash
// (1), provider failure (3), or interruption (130) is a genuine failure, so it
// stays authoritative even for an oracle-bearing task — otherwise a partial edit
// that happens to satisfy the oracle would launder a crashed or interrupted run
// into a pass. Each case applies the CORRECT edit-01 rename (so the oracle WOULD
// pass) but reports the failing exit code; the task must still fail, and the
// failure must be attributed to the exit code, not the oracle.
func TestNonIncompleteExitStaysAuthoritative(t *testing.T) {
for _, code := range []int{1, 3, 130} {
t.Run(fmt.Sprintf("exit%d", code), func(t *testing.T) {
task := loadBaselineTask(t, "edit-01")
outcome := runTurnStub(t, task, fmt.Sprintf(`sed 's/const MaxRetries = 3/const RetryLimit = 3/' main.go > .zero-tmp && mv .zero-tmp main.go
echo '{"type":"run_end","exitCode":%d}'
`, code))
if outcome.Err != nil {
t.Fatalf("a nonzero exit should be a task fail, not a harness error: %v", outcome.Err)
}
if outcome.Passed {
t.Fatalf("a run that exited %d must not pass even when the edit satisfies the oracle: %+v", code, outcome)
}
if want := fmt.Sprintf("exit code %d", code); !strings.Contains(outcome.VerifyErr, want) {
t.Fatalf("a non-INCOMPLETE nonzero exit must stay authoritative and surface its code, got VerifyErr=%q", outcome.VerifyErr)
}
})
}
}
// TestIncompleteExitStillFailsWhenOracleFails pins the OTHER half of the exit-4
// deferral: deferring to the oracle on INCOMPLETE must still MEAN the oracle is
// consulted, not a blanket pass. Here the stub does no real work and exits
// INCOMPLETE (4); the edit-01 rename never happened, so the oracle fails and the
// task fails. Without this, a regression that turned exit 4 into an unconditional
// pass would slip through, because its sibling TestOracleAuthoritativeOnIncomplete-
// Exit applies a CORRECT edit and would pass either way.
func TestIncompleteExitStillFailsWhenOracleFails(t *testing.T) {
task := loadBaselineTask(t, "edit-01")
outcome := runTurnStub(t, task, `echo '{"type":"run_end","exitCode":4}'
`)
assertVerifyFailed(t, "incomplete exit with no edit applied", outcome)
}
// TestNonzeroExitStillFailsLatencyOnly is the guard on the other side of that
// change: a latency-only task has no oracle to appeal to, so the exit code is
// the ONLY correctness signal and a nonzero exit must still fail it. Without
// this, dropping the exit-code gate for oracle tasks could be misread as
// dropping it everywhere.
func TestNonzeroExitStillFailsLatencyOnly(t *testing.T) {
task := loadBaselineTask(t, "longproc-01")
outcome := runTurnStub(t, task, `echo '{"type":"run_end","exitCode":4}'
`)
if outcome.Err != nil {
t.Fatalf("latency-only nonzero exit should be a verify fail, not a harness error: %v", outcome.Err)
}
if outcome.Passed {
t.Fatalf("a latency-only task that exited nonzero must not pass: %+v", outcome)
}
if strings.TrimSpace(outcome.VerifyErr) == "" {
t.Fatalf("a latency-only nonzero exit must surface a VerifyErr, got none: %+v", outcome)
}
}
// --- Satisfiable tests: the oracle PASSES the right thing (real fix applied) ---
// TestStampedOraclePassesWhenRefactorHappened proves the refactor-01 oracle is
@@ -1192,6 +1279,57 @@ func TestBuildTurnExecArgsIncludesExecProfile(t *testing.T) {
}
}
// Every benchmark invocation MUST grant the write/shell tool set via --auto
// member. Without a permission grant the agent runs read-only and cannot apply
// any edit, so the mutating classes measure nothing but oracle/answer
// contamination. member-auto is the RIGHT grant: it exposes the write +
// sandboxed-shell tools the benchmark needs while keeping the workspace/network/
// destructive safeguards, so the args must NOT reach for the broader
// --skip-permissions-unsafe (which drops those guards). This is a correctness
// contract, not a preference.
func TestBuildTurnExecArgsGrantsWriteTools(t *testing.T) {
args := buildTurnExecArgs(BenchTask{ID: "t", Prompt: "edit the file"}, RunContext{Model: "m"}, "trace.ndjson", nil)
autoMember := false
for i := 0; i < len(args)-1; i++ {
if args[i] == "--auto" && args[i+1] == "member" {
autoMember = true
break
}
}
if !autoMember {
t.Fatalf("benchmark exec args must include --auto member so the agent can apply edits, got %v", args)
}
for _, arg := range args {
if arg == "--skip-permissions-unsafe" {
t.Fatalf("benchmark exec args must NOT use --skip-permissions-unsafe (member-auto keeps the sandbox/network guards), got %v", args)
}
}
if args[len(args)-1] != "edit the file" {
t.Fatalf("prompt must stay the last argument, got %v", args)
}
}
// TestTurnRunnerRejectsFixturelessTask is the other half of the write-tools
// contract: because buildTurnExecArgs grants the write + sandboxed-shell tool
// set to every invocation, a task with no fixture would run an agent with those
// tools in the caller's cwd. The runner must reject such a task BEFORE launching
// zero exec. Passing a binary path that does not exist proves the rejection is
// pre-launch — the outcome carries the fixture error, not a spawn error — so this
// runs on every OS (it never reaches the POSIX-only exec stub).
func TestTurnRunnerRejectsFixturelessTask(t *testing.T) {
task := BenchTask{ID: "no-fixture", Prompt: "do a thing"} // no WorkspaceFixture set
outcome := NewTurnExecRunner(filepath.Join(t.TempDir(), "does-not-exist-zero"))(context.Background(), task, RunContext{Model: "m"})
if outcome.Err == nil {
t.Fatalf("a fixtureless task must be rejected before launch, got %+v", outcome)
}
if !strings.Contains(outcome.Err.Error(), "workspaceFixture") {
t.Fatalf("rejection must name the missing workspaceFixture, got: %v", outcome.Err)
}
if outcome.Passed {
t.Fatalf("a rejected task must not pass, got %+v", outcome)
}
}
// The configured profile must reach the runner's RunContext and be stamped
// into the result, so a profile A/B report is self-describing. The boundary
// canonicalizes (case/whitespace) so equivalent postures always carry the same
+28 -8
View File
@@ -707,6 +707,18 @@ type writeStdinTool struct {
manager *execSessionManager
}
// UnknownExecSessionError is the result returned when write_stdin targets a
// session_id with no live exec session. The stable recovery guidance leads and
// the numeric id trails on purpose: the agent's repeated-failure guard keys on
// a normalized, truncated prefix of the error string, so keeping the id out of
// that prefix makes a model probing ids 1, 2, 3, … produce ONE signature that
// finally trips the halt instead of a fresh signature per id — while also
// telling the model how to actually recover. See TestUnknownExecSessionErrorSignatureIsIDInvariant
// in internal/agent, which pins this invariant against the real guard.
func UnknownExecSessionError(sessionID int) string {
return fmt.Sprintf("Error: write_stdin needs a session_id returned by a still-running exec_command; do not guess or probe session ids. Start a process with exec_command, or use write_file/edit_file/apply_patch for file changes. (no live session %d)", sessionID)
}
func (writeStdinTool) outputCategory(map[string]any) outputCategory {
return outputCategoryProcess
}
@@ -722,7 +734,7 @@ func NewWriteStdinTool(manager *execSessionManager) Tool {
parameters: Schema{
Type: "object",
Properties: map[string]PropertySchema{
"session_id": {Type: "integer", Description: "Identifier of the running unified exec session."},
"session_id": {Type: "integer", Description: "Identifier of a running unified exec session, as returned by exec_command. Never guess or probe ids.", Minimum: intPtr(1)},
"chars": {Type: "string", Description: "Bytes to write to stdin. Defaults to empty, which polls without writing.", Default: ""},
"yield_time_ms": {Type: "integer", Description: "Wait before yielding output. Non-empty writes default to 250 ms and cap at 30000 ms; empty polls wait 5000-300000 ms by default.", Default: defaultPollYieldTimeMS, Minimum: intPtr(1), Maximum: intPtr(maxPollYieldTimeMS)},
"max_output_tokens": {Type: "integer", Description: "Output token budget. Defaults to 10000 tokens; larger requests may be capped by policy.", Default: defaultMaxOutputTokens, Minimum: intPtr(1), Maximum: intPtr(maxExecOutputTokenRequest)},
@@ -763,12 +775,20 @@ func (tool writeStdinTool) Run(ctx context.Context, args map[string]any) Result
}
func (tool writeStdinTool) RunWithOptions(ctx context.Context, args map[string]any, _ RunOptions) Result {
if value, ok := args["session_id"]; !ok || value == nil {
return errorResult("Error: Invalid arguments for write_stdin: session_id is required")
}
sessionID, err := intArg(args, "session_id", 0, 1, 0)
if err != nil {
return errorResult("Error: Invalid arguments for write_stdin: " + err.Error())
// A missing, non-integer, or < 1 session_id all mean the same thing: the model
// has no live session to write to. Route them to the SAME recovery guidance the
// no-live-session case uses (UnknownExecSessionError) instead of a terse
// "session_id must be at least 1". The terse error gives no way forward and, by
// naming the minimum, nudges the model to try 1, 2, 3... — the exact id-probing
// #749 works to suppress. The recovery message instead tells it to start a
// session or edit files directly, and because that message is id-invariant, the
// missing/zero/non-integer entry points and id-probing collapse to ONE
// repeated-failure signature, so any mix of them accumulates toward the halt
// rather than resetting the streak on each class of mistake.
value, present := args["session_id"]
sessionID, sessionErr := intArg(args, "session_id", 0, 1, 0)
if !present || value == nil || sessionErr != nil {
return errorResult(UnknownExecSessionError(sessionID))
}
chars, err := stringArgWithEmpty(args, "chars", "", false, true)
if err != nil {
@@ -784,7 +804,7 @@ func (tool writeStdinTool) RunWithOptions(ctx context.Context, args map[string]a
}
session, ok := tool.manager.get(sessionID)
if !ok {
return errorResult(fmt.Sprintf("Error: Unknown exec session_id %d.", sessionID))
return errorResult(UnknownExecSessionError(sessionID))
}
session.touch()
interrupted := false
+46 -13
View File
@@ -881,7 +881,7 @@ func TestRegistryHonorsWriteStdinArgumentPermission(t *testing.T) {
registry.Register(NewWriteStdinTool(newExecSessionManager()))
poll := registry.Run(context.Background(), WriteStdinToolName, map[string]any{"session_id": 9999})
if poll.Status != StatusError || !strings.Contains(poll.Output, "Unknown exec session_id") {
if poll.Status != StatusError || !strings.Contains(poll.Output, "still-running exec_command") {
t.Fatalf("empty poll should reach tool without permission prompt, got status=%s output=%q", poll.Status, poll.Output)
}
@@ -901,24 +901,57 @@ func TestWriteStdinReportsUnknownSession(t *testing.T) {
if result.Status != StatusError {
t.Fatalf("status = %s, want error", result.Status)
}
if !strings.Contains(result.Output, "Unknown exec session_id 1234") {
t.Fatalf("unexpected output: %q", result.Output)
// The message must lead with recovery guidance and still identify the id.
if !strings.Contains(result.Output, "still-running exec_command") {
t.Fatalf("unknown-session error must carry recovery guidance, got: %q", result.Output)
}
if !strings.Contains(result.Output, "1234") {
t.Fatalf("unknown-session error must name the offending id, got: %q", result.Output)
}
}
// The runtime rejects session_id < 1 (intArg min 1); the advertised schema must
// say the same so a model sees the constraint before it probes id 0.
func TestWriteStdinSchemaPinsSessionIDMinimum(t *testing.T) {
schema := NewWriteStdinTool(nil).(writeStdinTool).parameters
prop, ok := schema.Properties["session_id"]
if !ok {
t.Fatal("session_id property missing from write_stdin schema")
}
if prop.Minimum == nil || *prop.Minimum != 1 {
t.Fatalf("session_id schema Minimum = %v, want 1 to match the runtime floor", prop.Minimum)
}
}
// A missing, zero, negative, or non-integer session_id all mean the model has no
// live session, so write_stdin returns the SAME recovery guidance the no-live-
// session case uses (start a session with exec_command, or edit files directly),
// not a terse "session_id must be at least 1" that gives no way forward and names
// a minimum that nudges the model to probe ids 1, 2, 3... Sharing one id-invariant
// message also keeps a single repeated-failure signature, so any mix of these
// mistakes accumulates toward the halt instead of resetting the streak (#749).
func TestWriteStdinRequiresPositiveSessionID(t *testing.T) {
tool := NewWriteStdinTool(newExecSessionManager())
for _, args := range []map[string]any{
{},
{"session_id": 0},
want := UnknownExecSessionError(0)
for name, args := range map[string]map[string]any{
"missing": {},
"nil": {"session_id": nil},
"zero": {"session_id": 0},
"negative": {"session_id": -3},
"non-integer": {"session_id": "abc"},
} {
result := tool.Run(context.Background(), args)
if result.Status != StatusError {
t.Fatalf("Run(%#v) status = %s, want error", args, result.Status)
}
if !strings.Contains(result.Output, "Invalid arguments for write_stdin") {
t.Fatalf("Run(%#v) output = %q, want invalid arguments", args, result.Output)
}
t.Run(name, func(t *testing.T) {
result := tool.Run(context.Background(), args)
if result.Status != StatusError {
t.Fatalf("status = %s, want error", result.Status)
}
if result.Output != want {
t.Fatalf("output = %q,\n want the recovery message %q", result.Output, want)
}
if strings.Contains(result.Output, "must be at least 1") {
t.Fatalf("still returns the terse minimum error: %q", result.Output)
}
})
}
}
+30
View File
@@ -137,6 +137,27 @@ func WriteNDJSON(w io.Writer, t *TurnTrace) error {
return err
}
}
for _, event := range t.TaskStates {
if err := enc.Encode(map[string]any{
"type": "task_state",
"revision": event.Revision,
"status": event.Status,
"plan_pending": event.PlanPending,
"plan_in_progress": event.PlanInProgress,
"plan_completed": event.PlanCompleted,
"plan_failed": event.PlanFailed,
"tools_succeeded": event.ToolsSucceeded,
"tools_failed": event.ToolsFailed,
"verification_passed": event.VerificationPassed,
"verification_failed": event.VerificationFailed,
"verification_outcome": event.VerificationOutcome,
"changed_file_count": event.ChangedFileCount,
"completion_decision": event.CompletionDecision,
"plan_parity": event.PlanParity,
}); err != nil {
return err
}
}
return nil
}
@@ -206,6 +227,15 @@ func WriteText(w io.Writer, t *TurnTrace) error {
event.Truncated, event.Reason, event.SpillCreated)
}
}
if len(t.TaskStates) > 0 {
latest := t.TaskStates[len(t.TaskStates)-1]
write("task state: revision=%d status=%s plan=%d/%d/%d/%d tools=%d/%d verification=%d/%d(%s) files=%d plan_parity=%s completion=%s\n",
latest.Revision, latest.Status, latest.PlanPending, latest.PlanInProgress,
latest.PlanCompleted, latest.PlanFailed, latest.ToolsSucceeded,
latest.ToolsFailed, latest.VerificationPassed, latest.VerificationFailed,
latest.VerificationOutcome, latest.ChangedFileCount, latest.PlanParity,
latest.CompletionDecision)
}
return firstErr
}
+22 -2
View File
@@ -132,6 +132,26 @@ func ReadNDJSON(r io.Reader) (*TurnTrace, error) {
Reason: stringField(obj, "reason"),
SpillCreated: boolField(obj, "spill_created"),
})
case "task_state":
if !sawTraceHeader {
return nil, errors.New("parse trace: not a valid NDJSON trace (no type:trace header)")
}
t.TaskStates = append(t.TaskStates, TaskStateEvent{
Revision: int(parseInt64(obj["revision"])),
Status: stringField(obj, "status"),
PlanPending: int(parseInt64(obj["plan_pending"])),
PlanInProgress: int(parseInt64(obj["plan_in_progress"])),
PlanCompleted: int(parseInt64(obj["plan_completed"])),
PlanFailed: int(parseInt64(obj["plan_failed"])),
ToolsSucceeded: int(parseInt64(obj["tools_succeeded"])),
ToolsFailed: int(parseInt64(obj["tools_failed"])),
VerificationPassed: int(parseInt64(obj["verification_passed"])),
VerificationFailed: int(parseInt64(obj["verification_failed"])),
VerificationOutcome: stringField(obj, "verification_outcome"),
ChangedFileCount: int(parseInt64(obj["changed_file_count"])),
CompletionDecision: stringField(obj, "completion_decision"),
PlanParity: stringField(obj, "plan_parity"),
})
default:
// Unknown event type: tolerate (forward-compat) but only after a
// header has been seen.
@@ -152,8 +172,8 @@ func ReadNDJSON(r io.Reader) (*TurnTrace, error) {
if !sawTraceHeader {
return nil, errors.New("parse trace: non-empty input had no type:trace header")
}
if len(t.Spans) == 0 && len(t.Counters) == 0 && len(t.PrefixHashes) == 0 && len(t.OutputBudgets) == 0 {
return nil, errors.New("parse trace: header present but no spans, counters, or prefix hashes recovered (corrupt or truncated)")
if len(t.Spans) == 0 && len(t.Counters) == 0 && len(t.PrefixHashes) == 0 && len(t.OutputBudgets) == 0 && len(t.TaskStates) == 0 {
return nil, errors.New("parse trace: header present but no events recovered (corrupt or truncated)")
}
return t, nil
}
+23
View File
@@ -29,6 +29,10 @@ type Recorder struct {
firstActionStamped bool
}
// maxTaskStateEvents bounds trace growth in pathological long runs. Once full,
// the latest aggregate replaces the tail so final state is never lost.
const maxTaskStateEvents = 128
// NewRecorder returns a ready recorder. sessionID correlates with the agent
// session (Options.SessionID); runID is a per-Run sequence; profile is an
// optional label (e.g. "cold", "warm") for benchmark runs.
@@ -203,6 +207,24 @@ func (r *Recorder) EmitOutputBudget(event OutputBudgetEvent) {
r.tr.OutputBudgets = append(r.tr.OutputBudgets, event)
}
// EmitTaskState records one content-free structured task snapshot. Calls retain
// event order so revisions can be replayed alongside the tool and turn stream.
func (r *Recorder) EmitTaskState(event TaskStateEvent) {
if r == nil {
return
}
r.mu.Lock()
defer r.mu.Unlock()
if r.finished {
return
}
if len(r.tr.TaskStates) < maxTaskStateEvents {
r.tr.TaskStates = append(r.tr.TaskStates, event)
return
}
r.tr.TaskStates[len(r.tr.TaskStates)-1] = event
}
// Finish stamps CompletedAt, derives each span's parent (by interval
// containment) and exclusive time, and returns a snapshot of the trace. Calling
// Finish more than once returns the same snapshot.
@@ -223,6 +245,7 @@ func (r *Recorder) Finish() *TurnTrace {
snap.Counters = append([]Counter(nil), r.tr.Counters...)
snap.PrefixHashes = append([]PrefixHash(nil), r.tr.PrefixHashes...)
snap.OutputBudgets = append([]OutputBudgetEvent(nil), r.tr.OutputBudgets...)
snap.TaskStates = append([]TaskStateEvent(nil), r.tr.TaskStates...)
return &snap
}
+66
View File
@@ -0,0 +1,66 @@
package trace
import (
"bytes"
"strings"
"testing"
)
func TestTaskStateTraceRoundTripContainsNoTaskContent(t *testing.T) {
recorder := NewRecorder("session", "run", "")
recorder.Start()
event := TaskStateEvent{
Revision: 4,
Status: "active",
PlanPending: 2,
PlanCompleted: 1,
ToolsSucceeded: 3,
ToolsFailed: 1,
VerificationPassed: 1,
VerificationOutcome: "passed",
ChangedFileCount: 2,
PlanParity: "match",
}
recorder.EmitTaskState(event)
var output bytes.Buffer
if err := WriteNDJSON(&output, recorder.Finish()); err != nil {
t.Fatalf("WriteNDJSON: %v", err)
}
if strings.Contains(output.String(), "secret objective") || strings.Contains(output.String(), "private.go") {
t.Fatalf("task content leaked into trace: %s", output.String())
}
if strings.Contains(output.String(), "objective_hash") {
t.Fatalf("trace contains a confirmable objective fingerprint: %s", output.String())
}
parsed, err := ReadNDJSON(strings.NewReader(output.String()))
if err != nil {
t.Fatalf("ReadNDJSON: %v", err)
}
if len(parsed.TaskStates) != 1 || parsed.TaskStates[0] != event {
t.Fatalf("unexpected round trip: %#v", parsed.TaskStates)
}
}
func TestTaskStateIsDocumentedAsOptionalTraceEvent(t *testing.T) {
for _, key := range OptionalEventKeys() {
if key == "task_state" {
return
}
}
t.Fatal("task_state missing from OptionalEventKeys")
}
func TestTaskStateTraceIsBoundedAndRetainsLatestSnapshot(t *testing.T) {
recorder := NewRecorder("session", "run", "")
for revision := 1; revision <= maxTaskStateEvents+20; revision++ {
recorder.EmitTaskState(TaskStateEvent{Revision: revision})
}
events := recorder.Finish().TaskStates
if len(events) != maxTaskStateEvents {
t.Fatalf("task state events = %d, want cap %d", len(events), maxTaskStateEvents)
}
if got := events[len(events)-1].Revision; got != maxTaskStateEvents+20 {
t.Fatalf("latest revision = %d, want %d", got, maxTaskStateEvents+20)
}
}
+23
View File
@@ -103,6 +103,27 @@ type OutputBudgetEvent struct {
SpillCreated bool `json:"spill_created"`
}
// TaskStateEvent is a content-free snapshot of the run's structured task
// projection. Objective text, plan content, tool output, file paths, and
// confirmable content fingerprints are deliberately excluded so tracing cannot
// become a secret-bearing side channel.
type TaskStateEvent struct {
Revision int `json:"revision"`
Status string `json:"status"`
PlanPending int `json:"plan_pending"`
PlanInProgress int `json:"plan_in_progress"`
PlanCompleted int `json:"plan_completed"`
PlanFailed int `json:"plan_failed"`
ToolsSucceeded int `json:"tools_succeeded"`
ToolsFailed int `json:"tools_failed"`
VerificationPassed int `json:"verification_passed"`
VerificationFailed int `json:"verification_failed"`
VerificationOutcome string `json:"verification_outcome,omitempty"`
ChangedFileCount int `json:"changed_file_count"`
CompletionDecision string `json:"completion_decision,omitempty"`
PlanParity string `json:"plan_parity"`
}
// TurnTrace is the finished record for one agent.Run. It is the value
// emitters serialize; it is not mutated after Finish returns a snapshot.
type TurnTrace struct {
@@ -118,6 +139,7 @@ type TurnTrace struct {
Counters []Counter `json:"counters"`
PrefixHashes []PrefixHash `json:"prefix_hashes,omitempty"`
OutputBudgets []OutputBudgetEvent `json:"output_budgets,omitempty"`
TaskStates []TaskStateEvent `json:"task_states,omitempty"`
}
// PrefixHash is one fingerprint of the prompt prefix emitted by an agent run.
@@ -280,5 +302,6 @@ func OptionalEventKeys() []string {
"counter:" + CounterPrefixDrift,
"counter:" + CounterPostureEscalations,
"event:prefix_hash",
"task_state",
}
}
+14
View File
@@ -214,6 +214,12 @@ func (m model) setDoctorStatusRow(text string) model {
for i := len(m.transcript) - 1; i >= 0; i-- {
if m.transcript[i].id == doctorStatusRowID {
m.transcript[i] = row
// Settled alt-screen rows are cached across frames; force a rebuild
// so an in-place update (e.g. the spinner tick) is reflected
// immediately instead of serving the stale cached snapshot.
if i < m.flushed {
m.altScreenSettledWidth = 0
}
return m
}
}
@@ -691,6 +697,14 @@ func (m model) persistSelectedModel(profile config.ProviderProfile) (bool, error
if model == "" {
return false, nil
}
persisted, err := config.ProviderPersisted(path, name)
if err != nil {
return false, err
}
if !persisted {
// Env-derived providers have no config.json row to update.
return false, nil
}
if _, err := config.SetProviderModel(path, name, model); err != nil {
return false, err
}
+1
View File
@@ -261,6 +261,7 @@ var commandDefinitions = []commandDefinition{
},
{
name: "/rewind",
aliases: []string{"/undo"},
usage: "/rewind [latest|<sequence>]",
group: commandGroupSession,
description: "Restore workspace files to a checkpoint and truncate the session.",
+25
View File
@@ -985,6 +985,31 @@ func TestParseBackgroundTerminalCommands(t *testing.T) {
}
}
// /undo is an alias for /rewind (issue #698): previously it fell through to
// commandUnknown.
func TestUndoAliasesRewind(t *testing.T) {
rewind, ok := resolveCommand("/rewind")
if !ok {
t.Fatal("expected /rewind to resolve")
}
alias, ok := resolveCommand("/undo")
if !ok || alias.kind != rewind.kind {
t.Fatalf("expected /undo to alias /rewind, got ok=%v command=%#v", ok, alias)
}
// End-to-end: a typed /undo dispatches to the rewind handler with args intact,
// for both the "latest" and numeric-<sequence> forms of the /rewind contract.
if parsed := parseCommand("/undo latest"); parsed.kind != commandRewind || parsed.text != "latest" {
t.Fatalf("parseCommand(%q) = %#v, want commandRewind with text=latest", "/undo latest", parsed)
}
if parsed := parseCommand("/undo 123"); parsed.kind != commandRewind || parsed.text != "123" {
t.Fatalf("parseCommand(%q) = %#v, want commandRewind with text=123", "/undo 123", parsed)
}
// Guard: without the alias it would be commandUnknown.
if parsed := parseCommand("/undo"); parsed.kind == commandUnknown {
t.Fatal("/undo resolved to commandUnknown — the alias is not wired")
}
}
func TestCommandSelectionRequiresInputFromUsage(t *testing.T) {
cases := []struct {
name string
+19 -2
View File
@@ -348,12 +348,29 @@ func (m model) lastRowIndexForFile(path string) int {
return -1
}
func (m *model) setSelectedFile(path string) {
if m.selectedFile == path {
return
}
oldPath := m.selectedFile
m.selectedFile = path
for i := 0; i < m.flushed && i < len(m.transcript); i++ {
for _, changedPath := range m.transcript[i].changedFiles {
if changedPath == oldPath || changedPath == path {
m.altScreenSettledWidth = 0
return
}
}
}
}
// selectFile marks path as the selected file and scrolls the transcript so its
// most recent edit card is in view; the card tint comes from the renderers
// reading selectedFile (rowTouchesSelectedFile).
func (m model) selectFile(path string) model {
m.selectedFile = path
if offset, ok := m.scrollOffsetForTranscriptRow(m.lastRowIndexForFile(path)); ok {
rowIndex := m.lastRowIndexForFile(path)
m.setSelectedFile(path)
if offset, ok := m.scrollOffsetForTranscriptRow(rowIndex); ok {
m.chatScrollOffset = offset
if offset == 0 {
m.chatBodyLines = 0
+61 -9
View File
@@ -69,13 +69,6 @@ func (m model) settledRow(row transcriptRow, rc rowContext) bool {
// of the unflushed tail is rendered once and queued for scrollback. It returns
// the (possibly nil) print command for the queued batch.
func (m model) settleTranscript() (model, tea.Cmd) {
// In alt-screen mode there is no native scrollback surface for tea.Println:
// Bubble Tea drops that output by design. Keep rows in the managed view so
// the chat behaves like a fullscreen app and cannot reveal prior shell
// history by scrolling.
if m.altScreen {
return m, nil
}
// Never freeze history at the pre-WindowSizeMsg default width: the first
// real WindowSizeMsg arrives before any user-visible content settles, and
// flushing earlier would hard-wrap startup rows at 96 cols forever.
@@ -87,10 +80,14 @@ func (m model) settleTranscript() (model, tea.Cmd) {
// frontier themselves; this is a safety net).
m.flushed = len(m.transcript)
}
rc := buildRowContext(m.transcript)
oldFlushed := m.flushed
// An unsettled row blocks the frontier, so every cross-row dependency needed
// to decide or render the tail (call/result and prompt/decision pairs) also
// lives in the tail. Avoid rescanning the settled prefix on every tick.
rc := buildRowContext(m.transcript[m.flushed:])
width := chatWidth(m.width)
batch := []string{}
previousKind, havePreviousKind := previousVisibleTranscriptKind(m.transcript, m.flushed, rc)
previousKind, havePreviousKind := m.flushedPreviousKind, m.flushedHavePreviousKind
for m.flushed < len(m.transcript) {
row := m.transcript[m.flushed]
if !m.settledRow(row, rc) {
@@ -100,6 +97,15 @@ func (m model) settleTranscript() (model, tea.Cmd) {
if row.kind == rowWelcome || rc.skip(row) {
continue
}
// Bubble Tea intentionally discards tea.Println in alt-screen mode. The
// bookkeeping still advances; the settled prefix is retained in the body
// item cache rebuilt below.
if m.altScreen {
m.flushedAny = true
previousKind = row.kind
havePreviousKind = true
continue
}
rendered := m.renderRowMode(row, width, rc, true)
if rendered == "" {
continue
@@ -115,6 +121,15 @@ func (m model) settleTranscript() (model, tea.Cmd) {
previousKind = row.kind
havePreviousKind = true
}
m.flushedPreviousKind = previousKind
m.flushedHavePreviousKind = havePreviousKind
if m.altScreen {
bodyWidth := m.chatColumnWidth()
if m.flushed != oldFlushed || m.altScreenSettledWidth != bodyWidth || m.altScreenSettledFrontier != m.flushed {
m.rebuildAltScreenSettledItems(bodyWidth)
}
return m, nil
}
if len(batch) > 0 {
m.flushQueue = append(m.flushQueue, strings.Join(batch, "\n"))
}
@@ -139,6 +154,11 @@ func (m model) drainFlushQueue() (model, tea.Cmd) {
// surface ended.
func (m *model) resetFlushFrontier(divider string) {
m.flushed = 0
m.flushedPreviousKind = rowWelcome
m.flushedHavePreviousKind = false
m.altScreenSettledItems = nil
m.altScreenSettledWidth = 0
m.altScreenSettledFrontier = 0
// Renumbers every row's bodyY from the top (used by /clear, /resume, /rewind,
// /compact); a transcript-hover target's bodyY would otherwise risk
// coincidentally matching an unrelated row in the rebuilt transcript.
@@ -147,3 +167,35 @@ func (m *model) resetFlushFrontier(divider string) {
m.flushQueue = append(m.flushQueue, zeroTheme.faint.Render(divider))
}
}
// rebuildAltScreenSettledItems snapshots the stable prefix once when the
// frontier (or width) changes. View then reuses these item descriptors and only
// constructs descriptors/context for the live tail.
func (m *model) rebuildAltScreenSettledItems(width int) {
if !m.altScreen || m.flushed == 0 {
m.altScreenSettledItems = nil
m.altScreenSettledWidth = width
m.altScreenSettledFrontier = m.flushed
return
}
snapshot := *m
snapshot.transcript = m.transcript[:m.flushed]
snapshot.flushed = 0
snapshot.flushedAny = false
snapshot.flushedHavePreviousKind = false
snapshot.altScreenSettledItems = nil
snapshot.altScreenSettledWidth = 0
snapshot.altScreenSettledFrontier = 0
snapshot.pending = false
snapshot.pendingSpecReview = nil
snapshot.fileView = fileViewState{}
built := snapshot.transcriptBodyItems(width, "", false)
// Keep scratch capacity for the live tail. transcriptBodyItems can then
// append transient rows without copying the entire settled prefix. The cache
// length never includes those scratch entries, and the next frame overwrites
// them on the single Bubble Tea render goroutine.
m.altScreenSettledItems = make([]transcriptBodyItem, len(built), len(built)+256)
copy(m.altScreenSettledItems, built)
m.altScreenSettledWidth = width
m.altScreenSettledFrontier = m.flushed
}
+147 -4
View File
@@ -36,7 +36,7 @@ func TestSettledRowsAdvanceFrontierAndLeaveLiveView(t *testing.T) {
}
}
func TestAltScreenKeepsSettledRowsInManagedView(t *testing.T) {
func TestAltScreenAdvancesFrontierAndCachesSettledRows(t *testing.T) {
m := newModel(context.Background(), Options{AltScreen: true})
updated, _ := m.Update(tea.WindowSizeMsg{Width: 80, Height: 30})
m = updated.(model)
@@ -47,12 +47,155 @@ func TestAltScreenKeepsSettledRowsInManagedView(t *testing.T) {
if cmd != nil {
t.Fatal("alt-screen mode should not print rows into native scrollback")
}
if next.flushed != 0 {
t.Fatalf("alt-screen mode should keep the flush frontier unchanged, got %d", next.flushed)
if next.flushed != len(next.transcript) {
t.Fatalf("alt-screen mode should advance the frontier to %d, got %d", len(next.transcript), next.flushed)
}
view := viewString(next.View())
if !strings.Contains(view, "hello there") || !strings.Contains(view, "noted") {
t.Fatalf("settled rows should remain in the managed alt-screen view, got %q", view)
t.Fatalf("cached settled rows should remain in the managed alt-screen view, got %q", view)
}
}
func TestAltScreenSettledCacheRebuildsAfterRowToggle(t *testing.T) {
m := newModel(context.Background(), Options{AltScreen: true})
updated, _ := m.Update(tea.WindowSizeMsg{Width: 80, Height: 30})
m = updated.(model)
m.transcript = appendTranscriptRow(m.transcript, transcriptRow{kind: rowReasoning, text: "private thought"})
m, _ = m.settleTranscript()
m = m.toggleTranscriptRow(len(m.transcript) - 1)
m, _ = m.settleTranscript()
if !m.transcript[len(m.transcript)-1].expanded {
t.Fatal("expected reasoning row to be expanded")
}
if m.altScreenSettledWidth == 0 || len(m.altScreenSettledItems) == 0 {
t.Fatal("expected invalidated settled cache to be rebuilt")
}
}
func TestAltScreenSettledCacheRebuildsAfterDoctorStatusUpdate(t *testing.T) {
m := newModel(context.Background(), Options{AltScreen: true})
updated, _ := m.Update(tea.WindowSizeMsg{Width: 80, Height: 30})
m = updated.(model)
m = m.setDoctorStatusRow("frame 0")
m, _ = m.settleTranscript()
if m.altScreenSettledWidth == 0 {
t.Fatal("precondition: settled cache should be populated")
}
// An in-place update to an already-settled doctor row (e.g. a spinner tick)
// must invalidate the settled cache, just like toggleTranscriptRow does for
// expand/collapse, otherwise the alt-screen view keeps serving the stale
// snapshot forever.
m = m.setDoctorStatusRow("frame 1")
if m.altScreenSettledWidth != 0 {
t.Fatal("expected in-place doctor status update to invalidate the settled cache")
}
m, _ = m.settleTranscript()
view := viewString(m.View())
if !strings.Contains(view, "frame 1") {
t.Fatalf("expected updated doctor status text in view, got %q", view)
}
if strings.Contains(view, "frame 0") {
t.Fatalf("stale doctor status text must not remain in view, got %q", view)
}
}
func TestAltScreenSettledCacheInvalidatesWithFileSelection(t *testing.T) {
m := filesPanelTestModel()
m.altScreen = true
m.width = 80
m.height = 30
m, _ = m.settleTranscript()
if m.altScreenSettledWidth == 0 {
t.Fatal("precondition: settled cache should be populated")
}
m = m.selectFile("internal/tui/sidebar.go")
if m.altScreenSettledWidth != 0 {
t.Fatal("selecting a file touched by a settled row must invalidate the cache")
}
m, _ = m.settleTranscript()
selectedView := viewString(m.View())
updated, _ := m.Update(tea.KeyPressMsg{Code: tea.KeyEsc})
m = updated.(model)
if m.selectedFile != "" {
t.Fatalf("Esc should clear the selected file, got %q", m.selectedFile)
}
if m.altScreenSettledWidth == 0 || m.altScreenSettledFrontier != m.flushed {
t.Fatal("clearing a selected file should rebuild the invalidated cache")
}
if clearedView := viewString(m.View()); clearedView == selectedView {
t.Fatal("clearing a selected file should remove the settled row's selection tint")
}
}
func TestAltScreenSettledCacheInvalidatesForOlderFileCard(t *testing.T) {
const path = "internal/tui/sidebar.go"
m := model{
transcript: []transcriptRow{
{kind: rowToolResult, changedFiles: []string{path}},
{kind: rowToolResult, changedFiles: []string{path}},
},
flushed: 1,
altScreenSettledWidth: 80,
}
m.setSelectedFile(path)
if m.altScreenSettledWidth != 0 {
t.Fatal("selecting a file with an older settled card must invalidate the cache")
}
m.altScreenSettledWidth = 80
m.setSelectedFile("")
if m.altScreenSettledWidth != 0 {
t.Fatal("clearing a file with an older settled card must invalidate the cache")
}
}
func TestAltScreenRepeatedStatusCollapseRewindsSettledFrontier(t *testing.T) {
m := newModel(context.Background(), Options{AltScreen: true})
updated, _ := m.Update(tea.WindowSizeMsg{Width: 80, Height: 30})
m = updated.(model)
m.pending = true
m.activeRunID = 7
m.transcript = appendTranscriptRow(m.transcript,
transcriptRow{kind: rowToolCall, id: "old", tool: "swarm_status", runID: 7},
)
m.transcript = appendTranscriptRow(m.transcript,
transcriptRow{kind: rowToolResult, id: "old", tool: "swarm_status", status: tools.StatusOK, detail: "1 running", runID: 7},
)
m.transcript = appendTranscriptRow(m.transcript,
transcriptRow{kind: rowToolCall, id: "new", tool: "swarm_status", runID: 7},
)
m, _ = m.settleTranscript()
if m.flushed != len(m.transcript)-1 {
t.Fatalf("precondition: frontier=%d, want %d", m.flushed, len(m.transcript)-1)
}
updated, _ = m.Update(agentRowMsg{
runID: 7,
row: transcriptRow{
kind: rowToolResult, id: "new", tool: "swarm_status", status: tools.StatusError, detail: "1 running", runID: 7,
},
})
m = updated.(model)
if len(m.transcript) != 3 {
t.Fatalf("repeated status should collapse to welcome plus the new call/result pair, got %d rows", len(m.transcript))
}
if m.flushed != len(m.transcript) {
t.Fatalf("collapsed replacement should settle through %d rows, got frontier %d", len(m.transcript), m.flushed)
}
if m.altScreenSettledWidth == 0 || m.altScreenSettledFrontier != m.flushed {
t.Fatal("collapsed replacement should rebuild the invalidated settled cache")
}
cachedView := viewString(m.View())
rebuilt := m
rebuilt.altScreenSettledWidth = 0
rebuilt, _ = rebuilt.settleTranscript()
if rebuiltView := viewString(rebuilt.View()); rebuiltView != cachedView {
t.Fatal("collapsed replacement cache is stale compared with a forced rebuild")
}
}
+24 -10
View File
@@ -312,16 +312,23 @@ type model struct {
chatBodyLines int
// Flush-frontier state (see flush.go). In inline mode, transcript[:flushed]
// is already in native scrollback; in alt-screen mode this frontier stays
// idle so history cannot reveal prior shell output.
// is already in native scrollback. Alt-screen mode advances the same
// frontier, but keeps the settled prefix as cached body items so fullscreen
// scrolling still exposes the complete transcript without rebuilding it on
// every frame.
// flushedAny gates the first turn-separator blank line; flushQueue/
// printInFlight serialize ordered scrollback prints; headerPrinted records
// the one-time inline title-bar print at startup.
flushed int
flushedAny bool
flushQueue []string
printInFlight bool
headerPrinted bool
flushed int
flushedAny bool
flushedPreviousKind rowKind
flushedHavePreviousKind bool
flushQueue []string
printInFlight bool
headerPrinted bool
altScreenSettledItems []transcriptBodyItem
altScreenSettledWidth int
altScreenSettledFrontier int
// Composer input history (shell-style ↑/↓ recall of submitted inputs).
// lastPrompt is the verbatim text of the most recent submitted prompt, so
@@ -401,8 +408,9 @@ type model struct {
// mouseReleased, when true, forces terminal mouse capture OFF so the user can
// drag-select and copy text natively (Ctrl+E toggles it). App mouse features
// (clickable suggestions, right-click paste, transcript select) pause while on.
mouseReleased bool
transcriptSelection transcriptSelectionState
mouseReleased bool
transcriptSelection transcriptSelectionState
transcriptInteraction *transcriptRenderInteraction
// hover identifies the single clickable row (if any) currently under the
// mouse cursor with no button pressed, so it renders in a distinct style —
// the visual cue that it's clickable. Requires AllMotion mouse reporting
@@ -841,6 +849,7 @@ func newModel(ctx context.Context, options Options) model {
usageTracker: usageTracker,
transcript: initialTranscript(),
transcriptBodyHeights: newTranscriptBodyHeightCache(defaultTranscriptBodyHeightCacheMaxEntries),
transcriptInteraction: &transcriptRenderInteraction{},
prService: prService,
prState: prService.GetState(),
input: input,
@@ -1472,7 +1481,7 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
// selection is a passive highlight, so Esc dropping it is cheap and
// expected (mirrors how editors clear selection on Esc).
if m.selectedFile != "" {
m.selectedFile = ""
m.setSelectedFile("")
return m, nil
}
if m.hasQueuedMessage() {
@@ -2469,7 +2478,12 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
}
// Collapse a repeated swarm status/collect card so re-checks don't flood
// the chat with identical blocks.
beforeCollapse := len(m.transcript)
m.transcript = collapseRepeatedStatusCard(m.transcript, msg.row)
if removed := beforeCollapse - len(m.transcript); removed > 0 {
m.flushed = max(0, m.flushed-removed)
m.altScreenSettledWidth = 0
}
m.transcript = appendTranscriptRow(m.transcript, msg.row)
m = m.captureStepWork(msg.row)
// A finished command tool may have mutated files git can see but no
+34 -5
View File
@@ -347,22 +347,42 @@ func (m model) deleteManagerSelection() (model, tea.Cmd) {
wizard.manageStatus = "No user config path — cannot delete."
return m, nil
}
cfg, err := config.RemoveProvider(m.userConfigPath, name)
persisted, err := config.ProviderPersisted(m.userConfigPath, name)
if err != nil {
wizard.manageStatus = "Delete failed: " + err.Error()
return m, nil
}
var notes []string
var activeAfter string
var cleanup tea.Cmd
if persisted {
cfg, err := config.RemoveProvider(m.userConfigPath, name)
if err != nil {
wizard.manageStatus = "Delete failed: " + err.Error()
return m, nil
}
activeAfter = cfg.ActiveProvider
notes = []string{"Deleted " + name + "."}
cleanup = providerManagerCleanupCmd(m.userConfigPath, row.profile)
} else {
// Env-derived providers have no persisted profile or credential to
// delete. Keep this path session-only.
notes = []string{
"Removed " + name + " from this session.",
"It wasn't saved in config.json (likely set via an environment variable) — unset it to stop Zero from detecting it automatically.",
}
}
// Surgical removal — see saveManagerEdit for why the raw cfg.Providers list
// must not replace the resolved/filtered savedProviders wholesale.
m.savedProviders = removeSavedProvider(m.savedProviders, name)
notes := []string{"Deleted " + name + "."}
if strings.EqualFold(strings.TrimSpace(m.providerName), strings.TrimSpace(name)) {
notes = append(notes, "This session keeps running on it until you switch.")
} else if active := strings.TrimSpace(cfg.ActiveProvider); active != "" && !strings.EqualFold(active, name) {
notes = append(notes, "Active provider: "+active+".")
} else if activeAfter != "" && !strings.EqualFold(activeAfter, name) {
notes = append(notes, "Active provider: "+activeAfter+".")
}
cleanup := providerManagerCleanupCmd(m.userConfigPath, row.profile)
if len(m.savedProviders) == 0 {
m.providerWizard = nil
@@ -570,6 +590,15 @@ func (m model) saveManagerEdit() (model, tea.Cmd) {
return m, nil
}
oldName := strings.TrimSpace(wizard.editOriginal.Name)
persisted, err := config.ProviderPersisted(m.userConfigPath, oldName)
if err != nil {
wizard.err = err.Error()
return m, nil
}
if !persisted {
wizard.err = "provider " + oldName + " is not saved in config.json, so there is no saved profile to edit"
return m, nil
}
newName := strings.TrimSpace(wizard.editDraft.Name)
if newName == "" {
wizard.err = "name cannot be empty"
+5
View File
@@ -66,6 +66,11 @@ func rcKey(runID int, id string) string {
}
func buildRowContext(rows []transcriptRow) rowContext {
if len(rows) == 0 {
// Nil maps are safe for all rowContext lookups. This is the steady-state
// frontier-at-tail path, so avoid allocating maps on every View.
return rowContext{}
}
rc := rowContext{
resolved: map[string]bool{},
hints: map[string]string{},
+6
View File
@@ -111,6 +111,12 @@ func (m model) setSandboxSetupStatusRow(text string) model {
for i := len(m.transcript) - 1; i >= 0; i-- {
if m.transcript[i].id == sandboxSetupStatusRowID {
m.transcript[i] = row
// Settled alt-screen rows are cached across frames; force a rebuild
// so an in-place update is reflected immediately instead of serving
// the stale cached snapshot.
if i < m.flushed {
m.altScreenSettledWidth = 0
}
return m
}
}
+6
View File
@@ -976,6 +976,12 @@ func (m model) setCompactStatusRow(text string) model {
for i := len(m.transcript) - 1; i >= 0; i-- {
if m.transcript[i].id == compactStatusRowID {
m.transcript[i] = row
// Settled alt-screen rows are cached across frames; force a rebuild
// so an in-place update (e.g. the compact spinner tick) is reflected
// immediately instead of serving the stale cached snapshot.
if i < m.flushed {
m.altScreenSettledWidth = 0
}
return m
}
}
@@ -0,0 +1,38 @@
package tui
import (
"fmt"
"testing"
)
func benchmarkIssue561Model(turns int) model {
m := transcriptViewTestModel()
m.altScreen = true
for i := 0; i < turns; i++ {
m.transcript = append(m.transcript,
transcriptRow{kind: rowUser, text: fmt.Sprintf("question %d", i)},
transcriptRow{kind: rowAssistant, text: fmt.Sprintf("answer %d", i), final: true},
)
}
m, _ = m.settleTranscript()
return m
}
func BenchmarkIssue561SettledAltScreen(b *testing.B) {
m := benchmarkIssue561Model(5000)
width := m.chatColumnWidth()
if m.altScreenSettledFrontier != m.flushed || m.altScreenSettledWidth != width {
b.Fatalf(
"settled cache miss before benchmark: frontier=%d flushed=%d width=%d wantWidth=%d",
m.altScreenSettledFrontier,
m.flushed,
m.altScreenSettledWidth,
width,
)
}
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
_ = m.transcriptBodyItems(width, "", false)
}
}
+70 -3
View File
@@ -4,6 +4,7 @@ import (
"os"
"strconv"
"strings"
"sync"
"time"
"unicode/utf8"
@@ -38,6 +39,29 @@ type transcriptSelectionState struct {
cursor transcriptSelectionPoint
}
// transcriptRenderInteraction lets settled-item render closures read the
// current selection and hover state without rebuilding the cached descriptors.
// The mutex keeps the shared cache state safe even if a renderer inspects a
// model copy concurrently with an update.
type transcriptRenderInteraction struct {
mu sync.Mutex
selection transcriptSelectionState
hover hoverTarget
}
func (s *transcriptRenderInteraction) set(selection transcriptSelectionState, hover hoverTarget) {
s.mu.Lock()
s.selection = selection
s.hover = hover
s.mu.Unlock()
}
func (s *transcriptRenderInteraction) get() (transcriptSelectionState, hoverTarget) {
s.mu.Lock()
defer s.mu.Unlock()
return s.selection, s.hover
}
type transcriptSelectableLine struct {
bodyY int
rowIndex int
@@ -167,6 +191,9 @@ func shiftSelectableX(lines []transcriptSelectableLine, gutter int) []transcript
// so the highlight is computed in the same shifted coordinate the mouse maps to
// and lands exactly where the user selected (instead of gutter cells off).
func (m model) finalizeTranscriptBodyRow(rendered string, selectable []transcriptSelectableLine, gutter int, startBodyY int) transcriptBodyRenderedItem {
if m.transcriptInteraction != nil {
m.transcriptSelection, m.hover = m.transcriptInteraction.get()
}
lines := padTranscriptBodyLines(viewLines(rendered), gutter)
shifted := shiftSelectableX(selectable, gutter)
if m.transcriptSelection.active {
@@ -211,6 +238,21 @@ func (m model) renderHoverHighlight(rendered string, selectable []transcriptSele
}
func (m model) transcriptBodyItems(width int, emptyOverlay string, detailed bool) []transcriptBodyItem {
if m.transcriptInteraction != nil {
m.transcriptInteraction.set(m.transcriptSelection, m.hover)
}
// Steady-state alt-screen frames (notably the idle cursor blink) can reuse
// the settled list directly. Keeping this fast path outside the closure-heavy
// builder also prevents the large model receiver from escaping to the heap.
if m.altScreen && !detailed && !m.fileView.active && !m.pending && m.pendingSpecReview == nil &&
m.flushedAny && m.flushed == len(m.transcript) &&
m.altScreenSettledWidth == width && m.altScreenSettledFrontier == m.flushed {
return m.altScreenSettledItems
}
return m.buildTranscriptBodyItems(width, emptyOverlay, detailed)
}
func (m model) buildTranscriptBodyItems(width int, emptyOverlay string, detailed bool) []transcriptBodyItem {
// File drill-in: the chat column's body swaps to the viewed file's
// diff/content. Swapping HERE (the single source every consumer reads) keeps
// the viewport, scroll engine, renderer, and mouse hit-tests consistent.
@@ -239,7 +281,6 @@ func (m model) transcriptBodyItems(width int, emptyOverlay string, detailed bool
items = append(items, transcriptBlockBodyItem(transcriptBodyItemEmpty, -1, m.emptyState(width)))
}
} else {
rc := buildRowContext(m.transcript)
shownAny := false
// The detailed view shows the full transcript from index 0, not
// the managed region after m.flushed.
@@ -247,11 +288,32 @@ func (m model) transcriptBodyItems(width int, emptyOverlay string, detailed bool
if detailed {
startIdx = 0
}
useSettledCache := m.altScreen && !detailed &&
m.altScreenSettledWidth == width &&
m.altScreenSettledFrontier == m.flushed
if useSettledCache {
// The cache reserves scratch capacity for live-tail descriptors, avoiding
// an O(history) slice copy on every streaming/spinner frame.
items = m.altScreenSettledItems[:len(m.altScreenSettledItems)]
} else if m.altScreen && !detailed {
// A directly-constructed model (not yet passed through Update), or an
// explicitly invalidated cache, must still render complete history.
// The next settle rebuilds the cache for subsequent frames.
startIdx = 0
}
// Dependencies for an unflushed row cannot live wholly before the
// frontier: such a row would have blocked the frontier. Build context only
// for the live tail instead of rescanning settled history each frame.
rc := buildRowContext(m.transcript[startIdx:])
renderRowFn := transcriptRowDispatchFn(m.renderTranscriptRow)
if detailed {
renderRowFn = transcriptRowDispatchFn(m.renderTranscriptDetailedRow)
}
previousKind, havePreviousKind = previousVisibleTranscriptKind(m.transcript, startIdx, rc)
if startIdx > 0 {
previousKind, havePreviousKind = m.flushedPreviousKind, m.flushedHavePreviousKind
} else {
previousKind, havePreviousKind = previousVisibleTranscriptKind(m.transcript, startIdx, rc)
}
specialistSummaryEmitted := false
for index := startIdx; index < len(m.transcript); index++ {
row := m.transcript[index]
@@ -1326,7 +1388,7 @@ func (m model) handleTranscriptSelectionMouse(msg tea.MouseMsg) (model, tea.Cmd,
// opens/switches the file view.
if path, ok := m.fileRowAtMouse(msg); ok {
if m.fileView.active || m.selectedFile == path {
m.selectedFile = path
m.setSelectedFile(path)
return m.openFileView(path), nil, true
}
return m.selectFile(path), nil, true
@@ -1442,6 +1504,11 @@ func (m model) toggleTranscriptRow(rowIndex int) model {
switch m.transcript[rowIndex].kind {
case rowReasoning, rowToolResult:
m.transcript[rowIndex].expanded = !m.transcript[rowIndex].expanded
// Settled alt-screen rows are cached across frames; force a rebuild so an
// expand/collapse interaction is reflected immediately.
if rowIndex < m.flushed {
m.altScreenSettledWidth = 0
}
}
return m
}