* refactor(go): adopt modern stdlib idioms across kernel and desktop
Mechanical sweep with golangci-lint's modernize/intrange/copyloopvar/
usestdlibvars fixers plus hand-fixes for what they could not rewrite:
slices.Contains, min/max, slices.Backward, WaitGroup.Go, reflect.TypeFor,
maps.Copy, strings.Cut/CutPrefix/SplitSeq, strings.Builder, range-over-int.
Net -549 lines with no behavior change.
Three loops in session.go stay index-based: slices.Backward yields element
copies, so the fixer's rewrite silently dropped writes to DecisionReceipts
and ToolCalls (caught by TestDelivery* and TestDependentSameBatch*). All 49
slices.Backward sites were audited for that shape; only these three write
through the index, and they carry a nolint with the reason.
CaptureAfter's scan loop had an empty body and never ran any logic, so the
fixer could not express it. Drop it and the accessor it was the only caller
of. omitzero is deliberately untouched: turning omitempty into omitzero
changes the JSON written for nested structs, which is a wire-format change
to session files and the extension protocol.
* build(lint): gate the modern-idiom linters
Nothing stopped the codebase from drifting back to pre-1.22 idioms. Both
modules are now clean for modernize, intrange, copyloopvar, and
usestdlibvars, so enable them rather than leaving the sweep as a one-off.
omitzero stays disabled with the reason inline; usetesting, perfsprint, and
errorlint still have residue and land separately.
Combine the provider translation, README and changelog corrections, extension-protocol and benchmark docs, locale cleanup, translation parity checks, and custom provider placeholder from the selected open PRs. Update benchmark guidance for the main-v2 base and SWE-bench mode as part of review.
Validated with: go test ./...; pnpm --dir desktop/frontend typecheck; pnpm --dir desktop/frontend test:typecheck.
The benchmark reported a cache-hit percentage but nothing about why the
prefix was invalidated, so a hit-rate regression in a PR gave no signal
about which operation caused it.
metricsSink now tallies event.Usage's CacheDiagnostics.PrefixChangeReasons
into RunMetrics.PrefixChangeReasonCounts, e2ebench mirrors the field in its
parallel runMetrics struct, and render() prints a "Cache resets by cause"
line next to the existing failures-by-class breakdown. The field is
omitempty and the report line is skipped entirely when no run reported any
reason, so older metrics files and runs without diagnostics render exactly
as before.
Verification: go build ./... and go vet clean; new
TestRenderShowsCacheResetsByCause and
TestRenderOmitsCacheResetsLineWhenNoReasonsReported pass, and the existing
TestRenderLeadsWithCostPerSolvedAndFailureClasses is unchanged.
Co-Authored-By: Claude Opus 5 <[email protected]>
* feat(bench): SWE-bench primitives verified against the real harness
Image naming, conda activation, prompt construction, predictions encoding and
grader-report parsing, each pinned by a test against output captured from
swebench 4.1.0 on a real gold run. The orchestration that drives containers
lands next; these are the pieces whose shape could only be learned by running
the harness, not by reading it.
* feat(bench): run SWE-bench inside the official instance images
The agent now runs where the harnesses it is compared against run: inside the
per-instance evaluation container, against /testbed, with the repo's own tests
available to it. Anything less measures a different task.
The container is the isolation boundary, so the in-container config turns the
bash sandbox off — Reasonix otherwise refuses to run bash at all when it cannot
find bubblewrap, which the official images do not ship, and an agent that
cannot run tests is not doing SWE-bench. Nothing else deviates from the shipped
defaults: the provider, model and context window are what a user gets on
install, because a hand-tuned config would make the published number
unreproducible.
Credentials stream into the container over stdin rather than through argv or an
image layer, and the issue text is fully shell-quoted before it becomes one
argv element — a problem statement is untrusted input that routinely contains
backticks and $().
Resolution is never decided here; predictions go to the official grader and its
report is parsed back into the published failure taxonomy.
* feat(bench): publish the SWE-bench Verified subset and how it was chosen
The subset is 50 of the 500 Verified instances, and the script that produced it
ships alongside it. Selection is deterministic — largest-remainder apportionment
across repos, then evenly spaced picks within each repo ordered by difficulty —
so re-running it against the same dataset revision reproduces the file byte for
byte. A sample nobody can regenerate is a sample that can be quietly retuned
after seeing results.
Composition tracks the real benchmark: django 23 of 50 against 46.2% of the
full set, sympy 8 against 15.0%, and difficulty lands at 18/28/4 against the
full set's 38.8/52.2/8.4%. Nothing is excluded, including the psf/requests
instance whose test suite makes live network calls; if it exhausts the grader
timeout it is published as eval_timeout, because dropping the instances a
harness handles badly is how a benchmark stops meaning anything.
* feat(metrics): keep accounting when the agent is killed, and say where it came from
A benchmark run that times out currently reports nothing: the metrics file is
written once, after the controller returns, so a killed agent leaves no trace
of the fifty tool calls it made. In the last SWE-bench run one solved instance
landed in the totals as a free solve, quietly making every per-task figure
cheaper than the truth.
The sink now publishes a throttled snapshot to a `.partial` sidecar as the run
proceeds, through the repository's existing atomic write so a reader only ever
sees a whole file. The final record is marked complete and retires the sidecar,
so the two can never both be counted. The harness prefers the final record and
falls back to the snapshot, reporting recovered numbers as lower bounds rather
than as fact.
Usage is also split by origin. Steps counts every billed model call — executor,
planner, sub-agents, compaction, classifier, router — which is why published
rows read above a max_steps budget the main loop never exhausted. An origin the
build does not recognise keeps its own key rather than being dropped, because a
total that cannot reconcile is worse than one with an unfamiliar row.
Snapshotting made an existing race consequential: Emit is serialized by the
session's event.Sync wrapper, but the run command read the same struct directly
while background jobs could still emit into it. The sink now owns a mutex and
hands out deep copies.
The race test needs `go test -race`, which cannot run on a host without cgo;
CI's race job is what covers it.
* feat(ablation): switch subsystems off behind one shared vocabulary
Nothing could actually be ablated. The only variable axis was the runtime
profile, a coarse bundle, so attributing a change in solve rate to evidence
gating, the planner, subagents, retrieval or cache-preserving compaction was
impossible and every claim about those subsystems rested on assertion.
Add internal/ablation as the single vocabulary: one Set flows from --ablate
through boot into the agent, the controller and spawned sub-agents, and names
the published arm. Each subsystem gets exactly one switch point instead of
scattered conditionals — the planner at effectivePlannerModel, subagents at
the task-tool registration, retrieval at the two BM25-backed tools plus
auto-recall, evidence at finalReadinessCheckFor, and compaction at a new
compactThresholds that collapses the cache-preserving deferral.
boot.Options.DisablePlanner folds into the same Set; ACP's --planner=off maps
onto it rather than carrying a second switch for one subsystem. The arm is
recorded in --metrics and heads the e2e report, and the control arm's command
line stays byte-identical to the pre-ablation one so old numbers remain
comparable.
* docs(cli): document the --ablate benchmark arm
The e2e report could say what a suite cost but not what one solved task
cost, and every failure rendered as a bare cross — a wrong patch and a
guard that deliberately stopped the run were indistinguishable. Neither
number survives a comparison against another harness.
Record wall time, tool-call accounting and the stopping guard in
--metrics, name the guards through agent.PauseClass so hosts stop
reaching into unexported pause types, and lead the report with
cost-per-solved plus a failure-class breakdown. Wall time is the
harness's own clock so the same suite stays comparable when it runs
against a harness that cannot emit our metrics file.
Problem:
The calibrated 400k-token cap was exhausted after three successful baseline tasks, so the final two scenarios were skipped and the release gate could never complete.
Root cause:
Current model and cache behavior uses roughly 425k tokens for the first three tasks while keeping provider cost low, but both the harness default and workflow override retained the older 400k limit.
Fix:
Raise the bounded fixed-suite budget to 800k in the harness and workflow, and add a regression contract for the five-task baseline. Failure or skipping still fails the workflow.
Verification:
go test ./cmd/e2ebench -count=5
go vet ./cmd/e2ebench
go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/e2e-bot.yml
bash scripts/release-workflows.test.sh
Problem:
The real-provider benchmark ran headlessly without an automatic writer posture and without a usable Linux sandbox backend, so valid file edits were denied and the suite exhausted its token budget.
Root cause:
The harness launched reasonix run in its default Ask mode, while the workflow did not install bubblewrap on the runner.
Fix:
Launch fixed-suite tasks with --auto and install plus verify bubblewrap before running the benchmark. Explicit ask/deny rules and the sandbox boundary remain enforced.
Verification:
go test ./cmd/e2ebench -count=5
go vet ./cmd/e2ebench
go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/e2e-bot.yml
bash scripts/release-workflows.test.sh
The recently merged e2ebench robustness fixes carried multi-paragraph
narrative comments the comment policy bans. Compress each to a single
why-line and drop two dead scaffolds that existed only to host a
comment: an unwired grading context and a diagnostic-only skip counter.
Co-authored-by: reasonix <[email protected]>
The bench's `-json` write path uses `json.MarshalIndent`
+ `os.WriteFile` and discards both errors via the
`_, _ = …` / `_ = …` shape. A marshal failure
(structurally invalid type) or a write failure (read-only
target dir, disk full, broken NFS mount) is silently
swallowed: the bench exits 0, the markdown report goes
out, and the JSON file is either missing (write failed)
or contains `null` (marshal produced an empty byte
slice).
A consumer of `-json` — the e2e-bot workflow, a CI
dashboard scraping the artifact, a downstream aggregator
— then either can't find the file or parses `null`,
with no stderr signal that something went wrong. The
markdown report says "all 0/0 passed" and the JSON
sidecar is empty, and the user has no breadcrumb back to
"the bench was run as root, target was a read-only
mount, the write silently failed".
This change surfaces both errors on stderr and exits 1,
matching the markdown-report write path (a few lines up)
which already does the same. `json.MarshalIndent` only
fails on a structurally invalid type (a channel or
function in the struct, an unencodable cycle) — the
`runMetrics` and `result` types are plain data, so
the marshal branch is unlikely to fire in practice, but
the check is cheap. `os.WriteFile` fails on every
filesystem-level problem the bench was vulnerable to
before, so the write branch is the one that actually
matters.
Behavior change: an `-json` write that previously
silently produced a broken artifact now exits 1 with
a clear stderr message.
Co-authored-by: HUQIANTAO <[email protected]>
When the suite directory has no tasks, the bench exits 1 with
"no tasks found under benchmarks/e2e/tasks". That message is
correct but unhelpful when the suite dir itself is missing
(a wrong `-suite` flag, a stale checkout that lost the
bench/) — the user has to look one level up the path to
realize the directory is gone, then dig further to find
out why.
This change splits the empty-tasks case into two:
* If `os.Stat` on the suite dir fails (most often ENOENT
for a stale or wrong path, or EACCES for a permissions
problem), the message includes the underlying error:
`no tasks found under /tmp/nope/tasks: stat
/tmp/nope/tasks: no such file or directory`.
* If the dir exists but is empty (the user pointed at a
fresh `benchmarks/e2e/` before any task was added, or
the bench is running against a stripped-down test
suite), the message says so explicitly: `no tasks found
under benchmarks/e2e/tasks (the directory exists but
contains no task.toml files)`.
Both paths still exit 1. The shape of the change is
diagnostic-only — a stat call and a slightly more verbose
error string — and the bench's happy path is unchanged.
Co-authored-by: HUQIANTAO <[email protected]>
The seed workdir the bench copies into a per-task temp dir
is a checked-in source tree, so a symlink is intentional
— a `node_modules` shim, a vendored link to a sibling repo,
a fixture pointing at `/dev/null`. The previous
`copyDir` walked the seed with `filepath.Walk` and
called `copyFile` for every entry, including symlinks —
which resolves the link target through `os.Open` and
copies the target's content into the workdir.
That's dangerous for a few reasons:
* A symlink to a secret file outside the seed (a test
fixture pointing at `~/.ssh/id_test`, a vendor
shim pointing at a sibling repo with credentials)
would be copied into the workdir and exposed to the
agent's tools.
* A symlink to a huge file (`/var/log/system.log`,
`/proc/self/exe`) would make the bench's per-task
temp dir balloon, and the `defer os.RemoveAll(work)`
would try to `rm` a multi-GB file at the end.
* A symlink to `/dev/null` or another special file
would fail `io.Copy` partway through, leaving a
half-written target in the workdir.
This change skips symlinks entirely — the loop early-
returns when `info.Mode()&os.ModeSymlink != 0`. The seed's
intent (a fixture, a vendor shim) is preserved in the
sense that the link itself just isn't carried over; the
workdir is a flat copy of the seed's regular files.
Anything a symlink would have provided needs to be set
up by the task's `verify.sh` or as a real file in the
seed.
No behavior change for seeds with no symlinks (the
common case) — the early-return is skipped on every
entry.
Co-authored-by: HUQIANTAO <[email protected]>
The seed workdir the bench copies into a per-task temp dir
sometimes carries intentional permissions — a read-only
fixture marker, an executable bit on a shell script, a 0600
secret the agent shouldn't read but the grader does. The
old `copyFile` shape used `os.Create` (which sets
`O_CREATE|O_WRONLY|O_TRUNC`, defaulting to mode 0666
umask'd) and then never touched the mode again.
The result: a seed 0400 file copied into the workdir would
become 0644, looking like a permission change to `git status`
even though we never wrote to it. A seed 0755 shell script
would copy as 0755 (matches by accident — both default to
the umask'd 0666 when the user's umask is 0022, so the
executable bit is set). A 0600 secret would silently widen
to 0644, which then leaks to any `go test` child
process inheriting the workdir.
This change uses `os.OpenFile` with `info.Mode().Perm()`
so the destination's mode is exact, and re-applies the mode
via `os.Chmod` as a belt-and-suspenders (the OpenFile
perm is the open-mode bit, not the on-disk perms, and
`os.Create` ignores it on some platforms). The seed's
read-only / executable / private bits now survive the
copy.
No behavior change for seeds with default mode (the
workdir-relative permissions look the same); the fix is
strictly a permission-preservation guarantee for non-default
modes.
Co-authored-by: HUQIANTAO <[email protected]>
The e2e bench is invoked by the `/e2e` and `/e2e diff` PR
comments and (less commonly) by a developer running it from
the command line. The current help output — `flag.PrintDefaults`
on its own — is correct but minimal: 13 flags across two
modes (suite / diff) with no mode marker on the diff-only
flags, no examples, and no idea what the binary is for unless
you already know.
A developer who types `go run ./cmd/e2ebench -h` gets a wall
of `-flag default meaning` lines with no header, no
description, and no idea which flags apply to which mode.
The flag names themselves (`-max-steps`, `-attempts`,
`-test-cmd`) are only meaningful if you already know the
bench is a multi-attempt diff-grader.
This change:
* Sets a `flag.Usage` that prints a one-line description, the
flag list, and a short "Examples" block showing the two
common invocations (run the committed suite; grade a PR
diff with a retry budget).
* Uses `flag.CommandLine.Name()` so the example renders
correctly regardless of how the binary was built (the
default `e2ebench` for the installed binary, `go run
./cmd/e2ebench` for an in-tree dev run, `./reasonix-e2e`
for a custom GoReleaser binary, etc.). The `strings.Replace`
rewrites the registered name to the `go run` form when
it looks like the in-tree dev path — so the user copy-paste
actually works in both modes.
* Flags themselves are unchanged; `-h` / `-help` now
produces a useful, copy-pasteable help block.
Co-authored-by: HUQIANTAO <[email protected]>
Both `runTask` (suite mode) and `runOnce` (diff mode) launch the
`reasonix` binary with `exec.CommandContext`. When the bench's
per-task ctx times out, Go's exec sends the cancel signal
(SIGKILL on Unix by default) and then `Wait()`s for the child to
exit. Without `WaitDelay`, that wait is unbounded: a stuck child
deep in a syscall, a wedged MCP stdio server blocked on a write to a
closed pipe, or an AV scanner holding the binary open on Windows
keeps `cmd.Run()` blocked forever, so the bench never reaches the
grade step and the run just hangs.
The fix: `cmd.WaitDelay = 10 * time.Second` on both call sites. After
WaitDelay, exec closes the child's I/O pipes, which unblocks any
stdio-bound wait in the child even when the signal was ignored
(Go's `os/exec` will then return `ErrWaitDelay` from `Wait`,
which surfaces as a non-nil runErr — handled identically to any other
non-zero exit by the rest of the bench). 10s is the same shape
`bash.go` uses for its 120s foreground timeout: long enough for a
graceful exit, short enough that the bench never wedges.
This affects only the e2e benchmark harness (`cmd/e2ebench`), not
the production chat path. The chat TUI and the desktop serve path
already have their own command-context wiring (and use
`setKillTree` / `proc.KillTree` for the actual child processes,
which is the right answer there).
Co-authored-by: HUQIANTAO <[email protected]>
* ci(e2e-bot): drive the PR head, not main-v2
The bot built reasonix from main-v2, so /e2e on a PR measured main-v2's agent, not the PR's code — worthless for pre-merge validation. Build the agent from the PR head (falling back to main-v2 only when the head predates run --metrics), shrink the e2e context_window to 20000 so the tiny suite actually crosses the compaction trigger, and add a compaction task whose six prose chapters force multi-file reads past the threshold while hiding the graded facts in the first and last chapter.
Harness and suite still come from main-v2 so a PR can't weaken its own grader or tests.
* feat(run): count compactions in run --metrics and the e2e report
The bot needs to show whether a task actually triggered compaction — that's the signal the cache/compaction PRs (#2405-#2407) are measured by. metricsSink counts CompactionStarted; e2ebench adds a Compactions total to the summary and a per-task Compact column.
* test(e2e): make the compaction task a sequential clue-chain
A single 'read all six files' prompt let the agent batch the reads into one turn, so the whole corpus landed in the kept tail with no foldable middle and compaction never fired. Chaining each chapter to the next forces one read per turn; history accumulates and folds, and a real run now triggers 3 auto-compactions. The final chapter restates the full deliverable so the task stays solvable across a degraded summary.
---------
Co-authored-by: reasonix <[email protected]>
- Changed-line coverage: report what % of the PR's changed source lines
the tests actually execute (parses a coverprofile against the diff's
new line ranges), turning "N tests added" into an adequacy signal.
- Regression gate: run `go build ./...` after the agent and fold it into
the pass criterion, so the agent can't green its tests while breaking
the build elsewhere.
- Best-of-N: the agent is stochastic, so `/e2e diff xN` retries up to N
times (≤5) until a run passes, resetting the tree between attempts and
keeping the best; a single run is labelled as one sample.
- Safety: pass the triggering login through an env var instead of
interpolating it into the shell; make provider pricing configurable via
repo variables (cost was a hardcoded placeholder).
Co-authored-by: reasonix <[email protected]>
- Raise the diff-mode agent cap to 80 steps / 1200s: a real run on the
full repo hit the 40-step cap before it could get its generated tests
green.
- Differential row is now tri-state: "n/a (tests not green)" when go test
failed (the check is gated on green), vs yes/no when it actually ran.
- List failing test names in the report when go test fails.
Co-authored-by: reasonix <[email protected]>
* feat(ci): e2e benchmark bot triggered by a PR comment
Comment "/e2e" on a pull request to run the committed task suite against
the real provider and get an accuracy / cache-hit / token / cost report
posted back. Gated to OWNER/MEMBER/COLLABORATOR because it checks out
PR-head code and runs it with the provider API key.
- reasonix run --metrics writes a JSON token/cache/cost summary of a run.
- cmd/e2ebench copies each task's seed workdir to a temp dir, runs the
agent there via the built binary, then grades with verify.sh (dropped
in only after the run so the answer key isn't readable), and aggregates
the per-task metrics into a markdown report.
- benchmarks/e2e seeds three tasks (fizzbuzz, fix-add-bug, palindrome),
each graded by a verify.sh that exits non-zero on failure.
- .github/workflows/e2e-bot.yml wires the comment trigger, writes the
provider config, builds, runs the suite under a token budget, and posts
the report.
Needs a DEEPSEEK_API_KEY repo secret; model/base URL are overridable via
repo variables.
* feat(ci): add diff-driven test-gen mode to the e2e bot
Comment "/e2e diff" on a PR to have the agent, on the PR branch, write
tests covering what the PR changed and run them — graded by the repo's
own tests, with the accuracy/cache/token/cost report posted back.
- e2ebench -mode diff: derives the changed Go source files and packages
from base...HEAD, prompts the agent to add tests for them (not touch
source), then grades: pass = the agent added >=1 new test function AND
`go test` on the affected packages is green. A no-op can't pass since
the suite was already green; non-test source edits by the agent are
surfaced as a warning.
- run-metrics is reused for the cost/token/cache numbers.
- the workflow branches on "/e2e diff", computes the base via merge-base,
and writes the provider config to the repo root too (project config
wins over user config, and diff mode runs the agent in the repo root).
Plain "/e2e" still runs the fixed suite.
* feat(ci): differential check for diff test-gen; match default model
The diff grader now reverts the PR's source to its pre-change state and
re-runs the generated tests: they must FAIL on the old code, proving they
capture the change rather than asserting behavior that already held. Pass
now requires added tests + green on HEAD + failing on base.
Also point the workflow's default model/pricing at deepseek-v4-flash to
match the project's own provider config.
---------
Co-authored-by: reasonix <[email protected]>
YHH