mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-10 17:57:54 -10:00
Vasanth T
e1975c1b39
* 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.