Commit graph

2017 commits

Author SHA1 Message Date
Gergo Magyar
dbb292d012 test(eval): run the offline sweep unstubbed in the job that can, and probe CLI identity
Items 5 and 6 turned out to be one change. The containment (ubuntu) job already
installs bubblewrap, the pinned Claude CLI, node_modules and a built GitNexus -
everything the sweep's two provisioning stubs stand in for. So the stubs are not
a property of the test, only of a machine that lacks those things.

GITNEXUS_REQUIRE_FULL_SWEEP=1 makes the sweep run with nothing stubbed: real
containment instead of --unsafe-no-bwrap, the real runtime mounts, the real
sanitized graph. Set in that job, following the GITNEXUS_REQUIRE_BWRAP_CANARY
pattern already there. The gate FAILS on a missing piece rather than degrading
to the stubbed path, which is the point - a green tick that silently tested less
is what the bubblewrap canary was written to prevent.

Verified both states here: default green, and gate-on fails on this machine
rather than skipping, since it cannot create user namespaces.

Item 7 is an experiment, not an answer. Per-cell attribution needs an
identifier that travels WITH the request, because one proxy serves the whole
sweep and anything read from its environment is identical for every call. What
the real CLI sends is not documented anywhere I can check, and guessing a wire
format is exactly how the last three accounting bugs happened. So the probe
drives the REAL pinned CLI against the mock and records the identity-bearing
headers and body keys that arrive. It asserts only that a request was made; the
recorded evidence is the deliverable, and the job log preserves it. Skips
without CLAUDE_CANARY_BIN.

Two guards caught this rather than review: the repo pins the containment job's
env and its exact test list, so both had to be updated deliberately - which is
the guard working, not friction.

682 eval tests pass, 17 skipped.
2026-09-09 06:16:17 +00:00
Gergo Magyar
a98ec461af fix(eval): an uninvoked skill must not move the arm's quality median
Found by the offline sweep: a skill-not-invoked row still carried its score
into the arm's quality median. aggregate()'s filter dropped
EXCLUDED_ERROR_KINDS and evidence_valid=False, and skill-not-invoked is
neither, so an arm could be credited for a review it never performed with the
skill under test - which is the one thing an arm exists to measure.

Excluded from the QUALITY metrics only. Cost and duration still count that row,
because the session really ran and really was billed, and the promotion gate
still sees it, because it has its own vocabulary for a candidate that never
loaded its skill.

Two wider fixes were tried and abandoned, both because the tests said so rather
than because I reasoned it out first. Reusing the health guard's evidence_failed
predicate also excluded transcript-missing rows, but
test_aggregate_excludes_session_error_rows_from_medians pins those as counting:
that session ran, only its transcript is unverifiable. Excluding the row from
`valid` outright turned a candidate whose skill never loaded from
keep_incumbent into insufficient_evidence - the safety property held either way,
but the decision vocabulary is promotion semantics and not mine to change on a
measurement fix.

Mutation-checked: putting the rows back into the quality median fails the new
test. Both directions asserted, since a filter that excludes everything would
also pass - a wrong-but-valid review still moves quality, because being wrong is
exactly what a quality median should reflect.

682 eval tests pass, 16 skipped.
2026-09-08 19:19:43 +00:00
Gergo Magyar
c7b61f118b test(eval): run a whole sweep offline, with negative controls
The layers between a model turn and a promotion decision had never been
exercised together. Unit tests covered each alone, and the paid runs that would
have covered the composition kept dying, so the contracts BETWEEN them went
unverified - which is where this harness has repeatedly shipped bugs.

Drives runner.main() the way the workflow does. Real task selection, hidden
oracle capture, sandbox, CLI subprocess, artifact capture, scoring against the
oracle, aggregation, health guard and promotion gate. Only the model is
scripted.

Getting to green meant satisfying nine real contracts nothing had exercised end
to end, and each failure was the harness correctly refusing bad evidence:
--unsafe-no-bwrap is restricted to the paired review arms; ce_* needs a plugin
carrying ce-plan, ce-work and ce-code-review; candidate_* needs an overlay; the
clone needs .gitnexus/meta.json with indexedAt and lastCommit; the evidence gate
needs a Skill request with a non-error result; review findings need exactly ten
fields with severity in critical/high/medium/low; and the hidden labels use a
DIFFERENT schema from the review output - line_start/line_end, six fields. That
last one only a real run surfaces.

Three negative controls, because a scorer that cannot be wrong measures nothing.
A finding in the wrong place is tp=0 fp=1 fn=1 and oracle-failed, while its
evidence stays VALID - being wrong is a quality result, not a broken
measurement. Approving defective code is a miss with no false positive, and
precision is None rather than 0, because it is undefined with no predictions.
One run cannot promote: the gate says it needs three valid paired runs.

A fourth control exists because a mutation demanded it. Forcing
skill_was_invoked_events to return True left every other test here passing, so
nothing pinned the gate that separates measuring a SKILL from measuring a model.

Writing it turned up behaviour worth recording rather than assuming: a
skill-not-invoked row still carries its score AND still counts toward the arm
median, because aggregate() drops EXCLUDED_ERROR_KINDS and evidence_valid=False
and skill-not-invoked is neither. The health guard stops the sweep, so a
single-run sweep cannot promote on it, but a mixed run's median would include a
cell whose skill never ran. Pinned as-is so it cannot change silently in either
direction; changing it is a promotion-semantics decision, not a test fix.

Two provisioning steps are stubbed and neither is harness logic: the pinned
runtime mounts (no node_modules in a worktree) and the sanitized graph build
(needs the gitnexus CLI at a mounted path). Containment is host-unsafe here;
bubblewrap stays with the real-sandbox canary.

681 eval tests pass, 16 skipped, none failing. Runs in ~18s.
2026-09-08 18:26:47 +00:00
Gergo Magyar
355175a7c6 fix(eval): the usage adapter read a shape the callback never receives
Running the gateway against the scripted provider proved the accounting merged
in #3220 does not work, and the same run showed why nothing had caught it.

LiteLLM does not hand a logger the upstream body. It normalises usage into its
own Chat-Completions-shaped object first, so an OpenAI Responses reply reaches
the callback as prompt_tokens / prompt_tokens_details.cached_tokens - never the
input_tokens / input_tokens_details the shipped adapter reads. Every field came
back unknown. The observed call_type is "anthropic_messages" as well, because
Claude Code calls the Anthropic-shaped endpoint, so canonical_provider returned
None and normalize_usage would have refused outright.

Both were assumptions about a boundary I had only read about. The unit tests
agreed with them because their fixture was written in the same wrong shape, so
producer and consumer were consistent and both wrong - the exact failure the
producer/consumer round trip exists to catch, one layer further out.

Adds a LITELLM_NORMALIZED adapter for the object that actually arrives. The
arithmetic is still OpenAI's - prompt_tokens is the whole, the details are
subsets - so ordinary input is recovered by subtraction. The Responses adapter
stays for a raw upstream body, which the mock still serves and tests directly.
An unrecognised provider is still refused rather than guessed.

The fixtures now carry the measured shape, and the end-to-end test asserts it
through a real proxy: 48k prompt tokens with 44k cached is read back as 3k
ordinary rather than as silence.

676 eval tests pass, 16 skipped, none failing.
2026-09-08 18:04:42 +00:00
Gergo Magyar
73ad083b69 test(eval): run a real session against the scripted provider
The mock only proves something once the harness runs against it. This adds the
stand-in CLI and the first integration tests that use it, so a session goes
through the real code with only the model faked.

tests/fixtures/fake_claude.py does what the CLI does at the two boundaries the
harness depends on: it calls ANTHROPIC_BASE_URL for a turn, EXECUTES the tool
blocks that come back, and prints the stream-json sequence the parent parses.
Everything between - the session runner, the event-stream parse, the usage
extraction, the artifact capture, the scorer - stays real.

Four tests, chosen for the layers that have actually broken here: the usage a
provider reported survives to the row, a scripted Write produces an artifact
parse_review_output accepts, the prompt the harness meant to send is what
arrived, and an upstream 529 lands as a failed session rather than a usable
measurement.

Writing the stand-in found two things worth keeping. The prompt arrives on
STDIN under "-p --input-format text"; scanning argv for a non-flag token picks
up a flag's value instead, and the prompt-fidelity test is what caught it. And
three of these tests had been holding a sandbox they never applied, since no
command_prefix is passed - that implied coverage which was not there, so the
sandbox is gone from them and stays only in the artifact test, which needs its
review directory.

What these do NOT cover, checked rather than assumed: making the stand-in write
in place instead of atomically still passes. On the host-unsafe backend there is
no read-only mount to refuse it, so the atomic-write requirement remains a
bubblewrap mount property that only the real-sandbox canary can prove. Dropping
cache_read from the recorded usage does fail, so that half is genuinely pinned.

672 eval tests pass, 17 skipped; the two test_model_gateway.py failures are the
environmental ones.
2026-09-08 17:49:03 +00:00
Gergo Magyar
6fd443a941 feat(eval): a scriptable stand-in for Anthropic and OpenAI
Every defect this harness shipped last round was invisible to its own tests for
one reason: the tests exercised a layer BELOW where the code runs. The usage log
was never written because the proxy is a subprocess with a constructed
environment. The callback could not be imported because LiteLLM loads it by
path, not as a package. Failures went unrecorded because only the async hook was
overridden. CI or review caught all three; no unit test could, because each
called the function directly instead of driving the path that calls it.

This closes that gap without spending money. It speaks the two wire protocols
the harness actually depends on - Anthropic Messages, streaming and not, and
OpenAI Responses - so a run can go through the real sandbox, the real CLI, the
real gateway and the real usage callback with only the model faked. The runner
already supports pointing at it: --base-url is the same path the free-model
proxy documentation uses.

Scripted rather than simulated. A test decides what the model says, which tools
it asks for, and exactly what usage it reports. That last part is what makes
provider-native accounting testable at all: real cache hits are not reproducible
on demand, but a declared cache_read of 44,000 is. One Reply served down both
protocols is also the cleanest demonstration that the same billed work is stated
as a sum on one side and as a whole on the other.

Tool blocks are the mechanism for artifact-producing cells. The CLI runs what it
is asked to run, so a scripted Write block makes it write that file inside the
sandbox for real - no model deciding anything.

The end-to-end test drives the real proxy against the mock and asserts the usage
log records the provider's own arithmetic through the Anthropic-shaped
translation. It SKIPS here, because litellm's console script is absent in this
environment, so it is unverified until CI runs it - the same footing the
bubblewrap canary started on, and that one found a real bug on its first CI run.

Not yet built: driving a whole sweep against this. That needs a scripted reply
sequence that carries a cell to a scored artifact, which is the next step and
the point of the exercise.

668 eval tests pass, 17 skipped; the two test_model_gateway.py failures are the
pre-existing environmental ones.
2026-09-08 17:33:59 +00:00
Gergő Magyar
18cbeb907c
feat(eval): record provider-native usage at the gateway instead of inferring it after translation (#3220)
* feat(eval): record provider-native usage at the gateway, not after translation

The benchmark reads token counts out of Claude Code's session output, which is
Anthropic-shaped whatever actually served the request. That holds until the
upstream is OpenAI, because the two providers do not merely name their fields
differently - they mean opposite things by them:

    Anthropic:  total_input = input_tokens + cache_creation + cache_read
                (input_tokens is the UNCACHED remainder; cache fields ADD)

    OpenAI:     total_input = input_tokens
                ordinary    = input_tokens - cached - cache_write
                (input_tokens is the WHOLE; cache fields are SUBSETS)

Adding OpenAI's three double-counts; subtracting Anthropic's under-counts. One
shared struct cannot be right for both, so the seam goes at the gateway, on the
far side of the translation: a LiteLLM callback appends each upstream request's
usage verbatim, along with the model that actually answered, the response id and
the cell it belongs to. Normalization is derived offline from that record, so the
derivation can be revisited without re-running a paid sweep.

Two rules the tests encode literally.

The native object is authoritative. The callback stores it unflattened,
unrenamed and unsummed. Reasoning tokens are kept as the decomposition of output
tokens they are, not added to them a second time.

A field nobody reported is unknown, never zero. A stored cache_read of 0 used to
mean either "the provider said zero" or "our adapter never looked" - the first
says caching is not working, the second says we cannot tell. NormalizedUsage
therefore uses None, and refuses to compute the ordinary portion when a term is
missing rather than subtracting an invented zero.

Mutation-checked three ways. Giving OpenAI Anthropic's arithmetic fails four
tests. Making unknown fall back to zero fails the unknown test. Dropping
input_tokens_details in the callback fails the end-to-end accounting test with
"assert None == 3000" - it goes unknown rather than passing with zeros, which
was the point of the exercise.

The actual model is recorded separately from the requested role because several
Claude role names map onto one upstream model here; pricing must follow what
answered. Cost is deliberately NOT stored: prices change, and tokens plus a
versioned pricing table can answer both what a past run cost and what the same
usage would cost today, without rewriting historical evidence.

The callback never raises. A cell that fails still spent money upstream, and
losing the accounting because a log write failed is the worse outcome. Failed
requests are recorded too.

No caching configuration, model, skill or promotion change: this installs the
thermometer without altering the experiment. 538 eval tests pass plus 27 gateway
tests; ruff clean. The two test_model_gateway.py failures are environmental -
litellm[proxy]'s console script is absent in this venv - and predate this branch.

* fix(eval): drop the accidentally committed .venv symlink

I symlinked eval/.venv at a sibling worktree's virtualenv to avoid rebuilding
it, and git add -A committed the symlink. .gitignore lists ".venv/" with a
trailing slash, which matches a directory and not a symlink, so nothing stopped
it.

That broke eval / containment (windows), where uv then refused to create the
environment: "failed to create directory eval\\.venv: Cannot create a file when
that file already exists". A machine-specific absolute path had no business in
the tree in the first place.

Removed, and .gitignore now also lists the bare name so the same slip cannot
repeat.

* Address PR review feedback (#3220)

Forward the usage environment into the proxy. This is the one that mattered:
the callback returns immediately when GITNEXUS_BENCH_PROVIDER_USAGE is absent,
the proxy runs as its own process, and Popen(env=...) REPLACES the parent
environment rather than extending it. The gateway's allowlist carried the
OpenAI and master keys and nothing else, so the callback loaded, found no
destination, and silently recorded nothing on every request. The accounting
looked configured and measured nothing at all.

My tests could not see it. They set the variable in-process and called the
logger directly, so none of them ever crossed the subprocess boundary the
feature actually runs behind. The new test drives OpenAIGateway.__enter__ with
Popen captured and asserts each variable reaches the child - and that the
result is still an allowlist rather than the inherited parent environment,
since forwarding by name is what keeps the credential boundary explicit.

Resolve the provider label into an adapter key. The callback recorded
LiteLLM's custom_llm_provider, which is "openai", while the adapter table is
keyed "openai-responses" - so nothing the logger wrote could have been
normalized. The end-to-end test hid this by passing OPENAI_RESPONSES by hand
instead of using the provider the log recorded; it now uses the logged value,
which is what makes the mismatch visible.

The label alone cannot pick an adapter: LiteLLM reports "openai" for Chat
Completions as well, and the two report usage differently. canonical_provider
combines the label with the call type and returns None when it cannot resolve
one, so normalize_usage refuses rather than guessing token semantics. Both are
stored - provider_label is what LiteLLM said, provider is the adapter key.

The shared env-var names moved into provider_usage.py so model_gateway can
import them without importing litellm, which only the in-proxy callback needs.

Mutation-checked. Removing the forwarding loop fails the gateway test; using
the raw label as the adapter key fails two.

656 eval tests pass, ruff clean. The two test_model_gateway.py failures are the
environmental ones - litellm[proxy]'s console script is absent here, which is
also why the new test patches the argv builder to reach Popen at all.

* fix(eval): stop recording a cell id the proxy cannot know

Setting out to build the correlation this PR was missing - cell usage as the
sum of its upstream requests - turned up that the field it would have been
built on cannot hold what its name claims.

attach_openai_gateway wraps the whole sweep (runner.py:2122), so ONE proxy
serves every cell, and its environment is fixed for that process's lifetime.
Cells run concurrently under --workers and interleave requests through it. A
cell id forwarded at launch is therefore the same constant on every event the
callback ever writes - not an attribution, just a label that looks like one.
Worse than absent, because a reader would trust it.

So GITNEXUS_BENCH_CELL_ID is gone rather than left to be wired up later. What
remains is honest about its scope: sweep_id is genuinely sweep-wide, and
session_id is the per-request half - the only thing that can attribute a
request to a cell, since anything read from the environment is shared by all of
them. It is recorded even when the provider supplies nothing, because knowing
attribution is unavailable is itself a fact about the run.

Pinned by a test asserting the forwarded set contains no per-cell variable, so
a later change does not reintroduce one and quietly stamp a single value across
concurrent cells.

What this leaves open, stated plainly: per-cell attribution is NOT built, and
cannot be until a per-request identifier is available. Whether Claude Code
propagates a session identifier through the proxy is unverified - determining
it needs a real session against the gateway, which is a paid run. Sweep-level
totals and per-request cache ratios do not need it, and those are what the
caching question actually turns on.

658 eval tests pass, ruff clean; the two test_model_gateway.py failures remain
environmental.

* fix(eval): keep the usage callback importable the way LiteLLM loads it

CI caught a regression I introduced: "ImportError: Could not import handler
from provider_usage_callback", and the proxy exited before becoming ready.

Moving the shared constants into provider_usage.py, I imported them from the
callback with "from .provider_usage import ...". But LiteLLM resolves a dotted
callback through spec_from_file_location against the config directory, so the
copied file runs as a top-level module with no parent package and no sys.path
entry - the relative import raises and the gateway never starts. The module's
own docstring says it is deliberately self-contained for exactly this reason,
and I broke that invariant while tidying.

The in-package tests could not see it. They import
workflow_bench.litellm_usage_callback, where the relative import resolves
fine; the failure only exists on the path where the file is copied and loaded
standalone.

The callback carries its own literals again. Two tests keep that honest: one
loads the copied file the way LiteLLM does - by path, as a top-level module -
so an import that only works in-package fails there, and one asserts the
copied constants and the provider resolver still agree with the canonical
copies in provider_usage.py, so the deliberate duplication cannot drift
silently.

Mutation-checked: restoring the relative import reproduces CI's exact error.

660 eval tests pass locally; the two remaining test_model_gateway.py failures
are the environmental ones (litellm[proxy]'s console script is absent here,
which is also why this never reproduced locally).

* test(eval): import the installed callback instead of grepping it

Two review findings on the same weakness, both correct.

The install test asserted "class ProviderUsageLogger" appeared in the copied
file's text. That passes whenever the string is present, including when the
module cannot load at all - which is precisely how a package-relative import
got through review here and took the proxy down. It now loads the copy the way
LiteLLM does, by path as a top-level module, and checks the handler instance
the config actually names.

The gateway-forwarding test built its work directory with tempfile.mkdtemp(),
which nothing removed, so every run left the generated config and the copied
callback behind in the system temp directory. It uses the pytest-managed
tmp_path fixture like its neighbours.

660 eval tests pass; the two test_model_gateway.py failures are the
environmental ones.

* fix(eval): record failures on the synchronous callback path too

ProviderUsageLogger overrode both async hooks and the sync SUCCESS hook, but
not the sync failure hook. On that path failures fell through to CustomLogger's
base implementation and were never appended - so a sweep recorded its
successes and quietly understated what it spent, since a failed request is
billed all the same. That contradicts the module's own stated reason for
handling failures at all.

The failure test could not have caught it: it called _append directly, which
exercises neither public hook. Both failure tests now drive the hooks LiteLLM
actually calls, and a new one walks all four - sync and async, success and
failure - asserting each records in order. Removing the sync failure hook fails
both.

661 eval tests pass; the two test_model_gateway.py failures remain
environmental.

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
2026-09-08 18:22:04 +01:00
Ankit Verma
8ddab9aed5
feat(api): honor branch on POST /api/analyze (#3199)
* feat(api): honor branch on POST /api/analyze

The serve route accepted a `branch` field in the request body, returned
202 and reported the job `complete` — while indexing the remote's default
branch. Express drops unknown body fields, so the caller got no error and
no warning; the only way to notice was to inspect the checked-out clone.

Both ends of the plumbing already existed: CloneOrPullOptions.branch is
honored by cloneOrPull, and AnalyzeOptions.branch already drives
resolveBranchPlacement. Only the HTTP layer was missing, so this wires
`branch` from the route through cloneOrPull and LaunchOptions into the
worker's AnalyzeOptions. StartMessage.options is already typed as
AnalyzeOptions, so the IPC protocol is unchanged.

Validation reuses validateBranchName — the same function backing the
CLI's `--branch` — so both entry points accept exactly the same refs and
a malformed value is rejected with 400 before it can reach git.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(api): make branch part of job identity and complete ref validation

Addresses the review on #3199.

1. Job dedup ignored `branch`, so a request for branch B while branch A was
   in flight was answered with A's job and a 202. The caller would read that
   as "B is indexed" — the same silent wrong-branch outcome honoring `branch`
   was meant to remove. Branch is now part of the dedup identity; a
   different-branch request falls through to the single-slot guard and gets a
   truthful 409 instead.

2. validateBranchName implemented only a subset of git's ref rules, so
   `feature.lock`, `/feature`, `feature/`, `feature//next`, `@`, `@{` and
   dot-prefixed components passed validation and failed later in the git
   subprocess — a 202 plus a background failure rather than the advertised
   400. The remaining `git check-ref-format` rules are now enforced at the
   same chokepoint, which fixes the CLI and `.gitnexusrc` paths too. No
   branch git can create is affected.

3. The LaunchOptions doc claimed an explicit branch always pins
   `branches/<slug>/`. resolveBranchPlacement keeps the run on the flat slot
   when that slot has no owner, or when its owner is already this label.
   Comment and CHANGELOG corrected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(server): describe cloneOrPull's branch path in its contract comment

The header comment predated `options.branch` and still said an existing
clone is only ever `git pull --ff-only`. The implementation has a second
path: with a branch it fetches that ref and runs
`checkout -B <branch> origin/<branch>`, so the requested branch — not the
one already checked out — ends up in the working tree.

The stale comment is actively misleading: a reviewer reading it concludes
that requesting a branch on an existing clone silently analyzes the
default branch, which is not what happens.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(server): scope the origin check to the existing-clone path

My previous comment said remote.origin is verified "in both cases", which
is wrong: assertRemoteMatchesRequestedUrl runs inside `if (exists)`, so a
fresh clone has no origin to check. Restructured around whether targetDir
exists, which is what actually selects the behavior.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(server): cover branch job identity in JobManager's own suite

The branch dedup tests were sitting in analyze-api.test.ts, but they
exercise JobManager directly, so they belong beside the existing
"returns existing job for same repoUrl when active" case in
analyze-job.test.ts. Moved, and extended to cover the callers that omit
branch entirely — the upload route, the embed manager and the existing
tests — which compare undefined === undefined and are unaffected.

Also pins that branch survives the whole clone -> analyze -> terminal
update sequence, since it is now part of dedup identity and must not
drift mid-flight.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(api): give a pinned branch its own clone and settle the slot it wrote

Addresses the review on #3199.

Per-branch clone directories (review option 2). One checkout per repo made
`branch` one-shot: after any analyze the tree is dirty with generated
AGENTS.md / CLAUDE.md / .claude/, so a pinned request 202'd and then died on
cloneOrPull's porcelain refusal. Worse, a later request that OMITTED `branch`
pulled whatever branch the last pin left checked out and indexed it as the
default — silent wrong content, the same class as #3198 one request later.
A pinned run now clones into `<repo>__<branchSlug>`, so the two requests no
longer share a tree. The pinned clone registers under its directory name,
because both dirs share an origin and the inferred name would otherwise
collide; that name re-derives through getCloneDir, so DELETE still finds it.

Finalization gate. registerRepo always records the flat `.gitnexus`, but a
pinned run whose label differs from the flat slot's owner writes
`branches/<slug>/`. The gate probed the flat path regardless, so it never
settled, and the worker's normal exit 0 — sent ~500ms after `complete`, while
the job is deliberately still non-terminal — was classified as a crash and a
successful analysis was retried three times and failed. The gate now follows
the placement the worker reports (isPrimaryBranch, added to the IPC allowlist
under the rule that module already documents), and an exit after a terminal
IPC counts as winding down, not dying. Reported by the maintainer and
reproduced independently by @azizur100389.

analyzeCloneOptions extracted so the token/branch combination is asserted.
Inline, the branch-only case — a public URL with no token — was untested, and
dropping it there would silently reindex the default branch while every other
test stayed green.

CHANGELOG: the previous entry claimed the newly-400'd payloads "would have
failed at git", which is true of CLI --branch but wrong for HTTP, where they
succeeded on the default branch. Documented as an explicit behavior change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(server): bound the branch clone-dir name to one path component

`validateBranchName` allows a 255-character ref and `branchSlug` appends a
dash plus 8 hash characters, so `<repo>__<slug>` reached 267 — past the
255-byte component limit on ext4/APFS/NTFS. The clone would then fail to
create its target directory, which the character-only regex could not catch.

Only the readable half is trimmed. The hash is a digest of the full ref and
is always kept, so two long branches sharing a prefix still resolve to
different directories rather than silently sharing an index. `branchSlug`
itself is untouched: the per-branch index slots already use those names on
disk, and shortening them there would orphan existing indexes.

Also corrects two comments: the forwarding test claimed a branch selector
always pins `branches/<slug>/` (it keeps the flat slot when that slot has no
owner or already owns the label), and the web client's `branch` doc said
omitting it means the remote default — true for a `url` request, but a `path`
request is never cloned and indexes whatever that tree has checked out.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(server): bound the branch clone-dir name and unblock same-branch re-index

Two defects found by testing this branch end to end, plus the review nits.

1. Path length. `validateBranchName` allows a 255-character ref and
   `branchSlug` appends a dash plus 8 hash characters, so `<repo>__<slug>`
   reached 267 — past the 255-byte component limit on ext4/APFS/NTFS, and the
   clone could not create its directory. Only the readable half is trimmed;
   the hash is a digest of the full ref and is always kept, so two long
   branches sharing a prefix still get separate directories. `branchSlug`
   itself is untouched — the per-branch index slots already use those names on
   disk and shortening them there would orphan existing indexes.

2. Same-branch re-index. Per-branch clone dirs stopped branches from
   contaminating each other, but a REPEAT pin still failed: analyze writes
   AGENTS.md / CLAUDE.md / .claude/ into the clone, so the second pinned run
   met its own dirt at the porcelain check and asked for
   `overwrite_local_changes`. When HEAD already matches the requested branch
   there is nothing to switch, so the run now takes the same `pull --ff-only`
   path an unpinned request takes — review option (1), alongside (2). The
   refusal is untouched where it matters: a real switch, or a detached HEAD,
   still goes through the checkout path and can still refuse.

Also: repositions getCloneDir's JSDoc, which an inserted constant had
orphaned; gates the pinned `registryName` on the same condition as the clone,
so supplying both `url` and `path` no longer renames the operator's local
repo; corrects a test comment that claimed a branch selector always pins
`branches/<slug>/`; and corrects the web client's `branch` doc, which said
omitting it means the remote default — true for `url`, but a `path` request
is never cloned.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: drop the CHANGELOG edits from this PR

Requested in review. Checking the history, no feat/fix PR here touches
gitnexus/CHANGELOG.md — the only recent commit on it is `chore: release
v1.6.11`, and CONTRIBUTING says release notes are generated from the merged
PR title via .github/release.yml. Hand-editing an [Unreleased] section from a
feature branch was my mistake, not the project's convention.

The behavior change it documented (branch: null / "" / non-string now 400
where they were previously dropped and the default branch indexed) is stated
in the PR description instead, which is what feeds the release notes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(server): align the clone/pull contract with the same-branch fast path

Two comments I wrote went stale against my own later change.

`cloneOrPull`'s header still said an existing clone with `options.branch`
always fetches and runs `checkout -B`. Since the same-branch fast path landed
that is only true when the branch actually differs; when it is already checked
out the run takes `pull --ff-only` and no dirty-tree check applies. The header
now splits on whether the branch differs, which is what the code branches on.

The web client's `branch` doc said omitting it on a `url` request clones the
remote default. That holds only when there is no clone yet — an existing
unpinned clone is pulled on whatever branch it already has checked out.

Comments only; no behavior change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Address PR review feedback (#3199)

Pin a same-branch re-index to `git pull --ff-only origin <branch>` so the job cannot follow an unverified `branch.<name>.merge` while still skipping the dirty-tree refuse.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(server): close the #3199 holes on pinned analyze re-index

Same-ref updates were a raw pull dest (force-fetch via +) or a shallow ff-merge that could not move, and a tag pin compared the tag-object SHA so re-index refused a dirty tree. Fetch the mapped remote-tracking ref, stay put on a peeled SHA match, restore only GitNexus overlays, skip the 60s settle on alreadyUpToDate, and keep branch validation in core so the HTTP route does not import the CLI.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore(autofix): apply prettier + eslint fixes via /autofix command

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-09-08 15:15:45 +01:00
Gergő Magyar
96132bd13a
perf(scope-resolution): stop re-scanning the ParsedFile store once per language (#3211)
* perf(scope-resolution): stop re-scanning the ParsedFile store once per language

Scope resolution calls `loadParsedFilesForPaths` once per language, and every
call walks every shard in the store. The skip decision needs the envelope's
path listing, and that listing is only trustworthy after the payload digest
has been checked -- so a pass that wants 50 Python files still opens and
SHA-256s all 413 shards / 301MB of a TypeScript-dominated store to prove it
can skip them. A pass wanting a SINGLE file costs 335ms. The cost scales with
language count, not with the files that language has, so a polyglot repo pays
it worst.

`tryLoadV8Cache` now returns the listing it already parsed for that skip
decision, and the store memoizes it per run. Later passes skip on the
memoized listing without reopening the file.

Measured on a 2234-file, 3-language repo, min-of-5:

  before   python 411ms   typescript 2872ms   javascript 507ms  = 3834ms
  after    python 415ms   typescript 2805ms   javascript 248ms  = 3484ms

-350ms here, roughly -250ms per additional language elsewhere. The first pass
is unchanged by construction -- it is what populates the memo. End-to-end the
graph is byte-identical: 51,288 nodes / 163,094 edges / 2106 clusters /
759 flows on a true incremental run.

Keyed on size+mtime as well as name. Shard names are content-addressed, so a
name collision across different content should be impossible, but that
invariant lives in the parse-cache keying rather than here and one stat per
shard is a few ms against the hundreds this saves. The memo holds one store
directory at a time, so a new repo in a long-lived MCP process drops the
previous set instead of accumulating.

The failure mode a listing memo introduces is a FALSE SKIP: a pass concludes a
shard holds nothing it wants and those files silently never reach the graph --
an exit-0 wrong answer, not a crash. The new test walks four passes with
disjoint wants over one store, plus a shard written after the memo is warm;
it fails when the skip is forced.

Also records the full scopeResolution breakdown in bench/. The headline is
that `emit` is 7161ms of the 14.7s phase and ~21% of the edit loop, spread
across a fan of passes with no hot inner loop -- so the win there is not
running them for unchanged files, which is a design rather than a patch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Address PR review feedback (#3211)

Strengthen the shard-listing memo test so a later miss asserts fs.open and
v8.deserialize never run for the skipped shard. Key-set checks alone still
passed if the memo never skipped.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore(autofix): apply prettier + eslint fixes via /autofix command

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-09-08 13:48:30 +01:00
dependabot[bot]
4757c0d3cb
chore(deps)(deps-dev): bump @types/node in /gitnexus (#3215)
Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 26.4.0 to 26.4.1.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

---
updated-dependencies:
- dependency-name: "@types/node"
  dependency-version: 26.4.1
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-09-08 10:58:41 +01:00
Gergő Magyar
b1d87c1f33
fix(eval): sweep evidence handling and measurement health, with guarded comparator reuse (#3207)
* fix(eval): cut skill-evolution wall clock without shrinking the gate

Reuse matching incumbent/CE cells, sanitize each SHA once, and default
dispatch workers to 3 so weekly review generations finish inside the
EventBridge window. Cap the sweep from leftover instance uptime so a
Friday dispatch still uploads evidence.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf(eval): pipeline graph setup and correct the wall-clock cost model

The evolution sweep paid `sanitize` + `analyze --pdg --index-only` for every
unique task SHA on the critical path, one at a time, with nothing overlapping.
`_run_sweep` now starts the next unpaid SHA's clone template and graph snapshot
on a prefetch thread as soon as the current task's cells are dispatched, so
every SHA but the first hides behind a paid session wave. The thread is joined
before that SHA is used and before the trees tempdir is torn down, and a
prefetch failure is recorded against the SHA exactly as an inline failure is.

Tasks whose cells are all reusable comparator rows are not prefetched: they
never build a graph, so priming one would be pure cost.

Adds `measure_evolution_cost.py`, the cost model behind these numbers. It reads
the review corpus, the evolve defaults, and the workflow's workers default —
it does not start a session. Its first version charged `copy_isolated_tree`
once per paid cell, serially. `run_cell` clones inside its own pool worker, so
the clones in a wave overlap and only one is on the critical path per wave;
the model now charges `ceil(cells / workers)` waves.

Estimated review generation at workers=3: cold 21570s, weekly 7710s.

Wall clock is quantised by `ceil(cells_per_task / workers)`. A cold review task
is 9 cells, so workers=4 buys the wall clock of workers=3 and pays host
contention for it. Documented in the workflow's rollout checklist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf(eval): price the benchmark against measured cell durations

The cost model assumed every cell runs the 1140s mean. Cells are not uniform:
the 41 rows in Actions run 33912693948's artifact are 826s at the median,
1262s at the mean, 2976s at p90, with two pinned at the 5400s session ceiling.
A wave waits for its slowest cell, so a mean understates every concurrent
schedule — the previous model called workers=3 cold 5.99h when the same
schedule against real durations is 10.33h.

session_durations.json carries the sample in submission order with its
provenance and its caveat: every cell in that run returned unusable evidence,
so the durations are real but a clean run may sit lower. It is the only live
artifact; the 2026-07-22 green run's has expired.

The model now simulates the schedule cell by cell rather than multiplying a
mean by a wave count, averaged over all 41 rotations of the sample so no
single alignment between sample order and cell index decides the answer. It
prices today's barrier (wave_makespan) against a continuously fed pool
(fed_makespan) and reports both, and it charges the proposer session — one
per generation, measured at 344.7s — which it had been omitting entirely.

Measurement only; no runtime behaviour changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf(eval): price arms separately and stop inventing setup constants

Two errors in the model, both found by auditing it against the artifact it
claims to describe.

The arms are not interchangeable. `candidate_review` runs 1416s at the mean
against `review`'s 1204s and `ce_review`'s 1176s, and the weekly lane pays the
candidate arm and nothing else — reuse skips both incumbents. Pricing weekly
from a pooled sample charged it for arms it never runs: weekly is 4.59h, not
the 3.65h a pooled sample reported. Cells are also submitted run-major and
arm-minor, so at workers=3 every wave holds one cell of each arm and the
slowest arm sets the wave; the model now builds cells in that order.

The setup constants were invented. GRAPH_ANALYZE_SECONDS=600 and
TEMPLATE_SANITIZE_SECONDS=180 charged 3900s of per-SHA setup for a cold run —
more than the entire non-session time of the source run, which was 2541s for
41 cells and 5 SHAs. `duration_s` is the sum of a cell's Claude sessions
(runner_sessions.py), so that 2541s residual is every clone, graph build,
sandbox and teardown the sweep paid. The model now charges the measured
residual per cell, 62.0s, and no longer credits clone templates or graph
prefetch: both landed after that run and there is no measurement of them yet.
The residual bounds what they can be worth.

Cold 37452s (10.40h), weekly 16541s (4.59h), against a fed pool at 31683s and
16541s. Measurement only; no runtime behaviour changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf(eval): charge sweep overhead where more workers cannot dissolve it

Three defects, found by auditing the model against the artifact again.

The overhead was charged inside the schedule. session_durations.json claimed
the residual was charged "per cell and serially - the pessimistic reading",
but task_cells folded it into each cell's duration, where the pool then
divided it by the worker count. The residual mixes per-cell work the pool
really does divide with per-SHA graph setup it cannot, and the artifact cannot
separate them, so it now sits outside the schedule: cold 11.09h, not 10.40h.

Alignment averaging weighted the shortest sample twice. The arm samples are 13,
14 and 14 long and the average ran over max()=14 offsets, so candidate_review's
first cell was counted twice and its last never. Averaging over lcm()=182
offsets weights every arm's sample evenly.

The wall assumed all 54 cells run. Replaying the sample's own error_kind
sequence through today's systemic_outage_streak trips the outage breaker at
cell 5 of 41. The source run executed all 41, so its runner did not break on
that sequence, but the current one would: these numbers price a HEALTHY sweep,
and a sweep with the sample's failure profile never reaches them. Stated on
generation_seconds and recorded next to the sample it qualifies.

Measurement only; no runtime behaviour changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(eval): give the review agent somewhere it can actually write

Every review cell in the last recorded generation returned unusable evidence.
Not some — all 41, across all three arms and all six tasks, at $3653 for the
run. The transcripts say why, 127 times across 35 of 35 sessions:

    EROFS: read-only file system,
    open '/workspace/review-output.json.tmp.2.90a76e583b0c'

The review arm mounted the artifact as a writable FILE at
/workspace/review-output.json while binding /workspace read-only. The Write
tool writes atomically: it creates `<target>.tmp.<n>.<hex>` beside the target
and renames it. The parent was read-only, so the temp create failed and the
artifact was never written. A writable file inside a read-only directory is
not writable to anything that writes atomically. Agents tried
/proc/self/root/workspace/... and /proc/1/root/workspace/... to get around it;
all 41 artifacts came back 0 bytes.

The artifact now lives in its own writable directory bound at /review-output,
outside the workspace. That is what a rename needs, and it lets the workspace
get stricter rather than looser: the review phase may now change nothing there
at all (enforce_phase_workspace gained allowed_artifact=None), where before it
was entitled to one path inside it. The file is no longer pre-created — the
agent writes it, and absence is now meaningful evidence.

parse_review_output reported every one of these as "review output is not valid
UTF-8 JSON". The file was empty, and its except folded OSError, UnicodeError
and JSONDecodeError into that one string, so a sandbox that made writing
impossible was indistinguishable from an encoding fault. That is why this read
as an agent-quality problem for fifteen consecutive non-green runs. Each cause
now names itself: never written, empty, not valid UTF-8, not valid JSON with
the decoder's position. run_arm also keeps the FIRST error_detail, as it
already did for error_kind, so a phase-boundary violation is no longer buried
under the parse failure it causes.

The test double conflated sandbox.private_root with the clone, which put the
artifact directory inside the workspace and would have hidden the stricter
check. Regression tests pin the mount shape in the generated bwrap argv, the
contract path in the prompt, the four parse diagnostics, and the
untouched-workspace contract.

Verified by unit tests only: this container has unprivileged user namespaces
disabled, so bwrap cannot run here and the mount was not exercised end to end.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(review): close the artifact path in every layer that gates it

Code review of this branch found the relocated review artifact was fixed in the
bwrap mount and nowhere else. Four independent layers decide whether the agent
can write it, and three still named the old location.

Claude Code applies its own filesystem policy to its own tools, and
build_claude_settings listed only /workspace, /tmp and /home/agent under
allowWrite with denyRead ["/"]. The artifact used to live under /workspace, so
this list was correct until it moved. SANDBOX_REVIEW_OUTPUT is now in allowWrite
and allowRead; without it the bwrap bind grants a write the CLI then refuses.

The task corpus still ran `test -s review-output.json` from the workspace, in a
separate sandbox invocation that never sees the artifact mount. Every review
cell would have been stamped verify-failed with resolved=False no matter how
good the review was, which also made those rows permanently unreusable and so
silently disabled this branch's own comparator reuse for review arms. The verify
and hidden-oracle commands now read the location from
GITNEXUS_BENCH_REVIEW_OUTPUT and get the directory bound read-only, mirroring
the mount-plus-env-var shape _run_hidden_oracle already used.

host_text and host_path did not translate the new path, so the host-unsafe
backend told the agent to write somewhere that exists on neither backend.
Adding the mapping exposed a second defect: host_text substituted every
occurrence of a target, and "/review-output" appears twice in
"/review-output/review-output.json" - once as the directory and once inside the
filename. Matching is now anchored to a path boundary.

Comparator reuse had three ways to accept evidence it should have rejected. A
row with no runtime_digest passed the drift lock because the guard only compared
when both sides were bound, and the branch's own test asserted that as correct;
absence is now a mismatch and the test states the rule. materialize_reused_row
overwrote recorded_at with the copy time while the age check read that field, so
a row copied forward each generation refreshed its own clock and never aged out;
the first measurement time is now preserved and aged against. A future-dated
stamp passed a one-sided bound and is now rejected as corrupt.

RUNTIME_DIGEST never reached the runner at all: runner_environment builds a
fixed dict and process_control replaces the child environment wholesale, so the
digest the workflow exports was dropped and the lock it feeds was inert. The
instance-window deadline was also checked only after run_proposer returned,
buying a proposal the generation had no room to benchmark.

The graph prefetch thread was started without copy_context, so it never saw the
cancellation ContextVar the rest of the sweep shares, and the outage breaker
returned without setting cancel_event - together, a tripped breaker would block
on joining a prefetch that was never told to stop. Both fixed, with outage
checked before cancellation at the two exits so an outage keeps exit 1 instead
of becoming a Ctrl-C's 130.

Both bwrap canaries that actually execute a write still bound the pre-fix shape
against a file this branch no longer creates, so they would have errored rather
than caught anything. They now bind the directory and write atomically - temp
file beside the target, then rename - which is the exact operation that failed
with EROFS. A source-text assertion over inspect.getsource(run_arm) was replaced
with one that inspects the real mount, and a wall-clock assertion was pinned to
a fixed monotonic clock.

Not applied, and why: binding task-asset and dependency digests into comparator
reuse needs asset snapshots prepared before the reuse decision rather than
inside the per-task loop, and shipping the comparison without that would add a
guard that silently never fires. Forcing a paid canary cell per incumbent arm
and folding reused rows into the outage streak are behaviour decisions, not
fixes. Clone-template reuse still has no test. The cost model's per-cell
residual still shrinks with arm count, overstating weekly savings by at most the
2541s residual; the docstring now says so rather than inventing a split.

585 eval tests pass, ruff clean, 29 workflow contract tests pass. The two
test_model_gateway.py failures are pre-existing and fail on main.

* fix(review): bind reuse to its environment and keep the health canary real

Applies the five findings the previous review round left open.

Comparator reuse ignored the environment a row was measured in. TaskReuseBinding
carried the task and oracle identity but not the task-asset or sandbox-dependency
digests, and this branch itself changes sandbox_dependencies in the review
corpus - so a reused comparator could be measured against one dependency set and
compared against a candidate built on another, handing the gate a false
comparison. Closing it needed the digests to exist before the reuse decision, so
asset snapshots are now prepared for every task up front instead of lazily
inside the per-task loop. That also removes the concurrent TaskAssetCache.prepare
the prefetch thread could otherwise race, which the file's own "plain dict,
read-then-write race" comment warned about. Both digests fail closed on either
side, matching the runtime digest.

The broken-incumbent canary could not fire when reuse was working. It read
`resolved`, which counts reused rows, so an arm whose cells were all reused
always looked healthy - in precisely the run where a broken environment would go
unnoticed. aggregate now also reports `resolved_fresh` and the canary reads it.
That count would be vacuous if an arm were reused end to end, so the sweep keeps
one paid cell per incumbent arm and says which one it kept.

Reused rows did not participate in the outage streak, so a run of failures could
carry across them and trip on stale history. A reused success now resets the
streak the way a paid success does.

The cost model charged sweep overhead per cell, which credited a weekly
generation for shrinking work it still performs: it pays one arm instead of
three but builds exactly the same graphs. Overhead is charged per SHA now.
Weekly is 5.20h rather than the 4.80h the per-cell rate reported; cold is
10.86h. The residual still cannot be split between per-SHA and per-cell work
from one artifact, so session_durations.json records that assumption and the
direction it errs in, rather than leaving a number nobody can trace.

Clone-template reuse - the branch's core speedup, taken on essentially every
multi-cell sweep - now has a test that builds a real sanitized template, asserts
the cell runs against the copy with the template's HEAD, and fails if run_cell
re-clones. A second test asserting only on a namespace built inside the test was
written and deleted: it exercised nothing, which is the failure this review
round penalised elsewhere.

589 eval tests pass, ruff clean, 29 workflow contract tests pass. The two
test_model_gateway.py failures are pre-existing and fail on main.

* refactor(eval): consolidate duplicated harness logic after the review round

Simplification pass over the branch. Behavior-preserving throughout; three
reviewers, nine findings applied, two skipped.

The review-artifact block in _run_hidden_oracle was unreachable. That function
runs only in run_arm's non-review branch, while the directory it probes for is
created only in the review branch, and each sandbox serves exactly one arm - so
`review_artifact.parent.is_dir()` could never be true. It was added an hour
earlier to make the hidden oracle resolve the moved artifact; the oracle never
runs for review tasks, so the guard was dead on arrival. Deleting it also
removes the duplication it had with the verify-command wiring.

EXCLUDED_ERROR_KINDS is now one definition. runner.py and comparator_reuse.py
each carried the same six-member frozenset, kept in sync by a comment. Only one
direction is possible: runner already imports from comparator_reuse, so the
reverse import fails at module-init with a circular-import error. That is now
stated where the alias lives, so nobody tries it the other way.

ensure_task_graph and prefetch_next_graph shared ten keyword parameters, passed
through two call sites and forwarded whole between them. They now take a
GraphBuildEnv, mirroring TaskCellContext, which already bundles per-cell state
in this file. Its ready_keys() replaces an inline four-set union at the call
site.

Smaller consolidations: _sha256_file's hand-rolled chunk loop becomes
hashlib.file_digest (3.11+, already used in runner_artifacts); _copy_owner_only
reuses task_assets._write_all and COPY_CHUNK_BYTES instead of repeating the
short-write retry; its stat-then-open existence check becomes the O_EXCL failure
it was already relying on, which is atomic rather than merely narrow; and
runner_environment reads the digest through comparator_reuse.current_runtime_digest
instead of re-parsing the environment variable.

Three test docstrings summarised the branch's own history ("the branch's core
speedup", "the regression that produced fifteen runs") rather than the invariant
under test. Rewritten to state the constraint, which is what survives the merge.
Repaired the indentation left behind by the outage-streak edit and flattened the
prefetch dispatch from three nested conditionals to one.

Skipped: consolidating comparator_reuse._real_directory onto proposer_sandbox's
same-named helper - they differ, the sandbox one rejects any symlink in the
resolved path while this one checks only the leaf, so sharing it would tighten
behavior rather than preserve it. That needs a decision about which policy the
reuse path wants, not a simplification.

589 eval tests pass, ruff clean, 29 workflow contract tests pass. Unrelated and
pre-existing: two test_model_gateway.py failures, and
test_process_control.py::test_timeout_kills_term_ignoring_descendants_before_they_write,
which is a TERM-to-KILL timing flake (passes 2 of 3 in isolation) in a file this
branch does not touch.

* refactor(eval): name the reuse directory check for the promise it makes

The simplification pass left one finding open: comparator_reuse and
proposer_sandbox both defined `_real_directory`, same name and same shape, with
different guarantees. The sandbox one rejects every symlink hop in the path; the
reuse one checks only the leaf and resolves through parents. Sharing the name
invites a consolidation that would silently tighten one of them.

They should not be merged, so the name stops claiming they could be.
proposer_sandbox guards a mount root, where a symlink hop changes what an
untrusted session is handed. comparator_reuse guards a data directory whose
contents are already validated one file at a time - reads go through
_regular_file, which lstats and rejects symlinks, and writes through O_NOFOLLOW.
A symlinked parent therefore grants nothing those guards do not already cover,
while refusing one would reject a symlinked artifacts directory or macOS's /var
for no gain.

Renamed to _resolved_directory, with the reasoning recorded at the definition,
and a test that pins both halves: a symlinked parent is accepted and resolved, a
symlinked leaf is still refused. Behavior is unchanged.

591 eval tests pass, ruff clean. The two test_model_gateway.py failures are
pre-existing and fail on main.

* test(eval): measure the sweep scheduler instead of modelling it

measure_evolution_cost predicts wall clock from a model of what
sweep_task_cells does. This runs the real thing - real threads, the real wave
barrier, the real outage breaker - with only the paid agent session replaced by
a sleep, and times it.

Durations are the measured per-arm samples divided by 5000, so a 1416s cell
takes ~0.28s. The shape is kept on purpose: the median cell is 826s against a
5400s ceiling, and that spread is the entire reason a barrier costs anything.
Uniform random sleeps would erase the effect under test. All schedulers consume
one identical seeded plan, so a comparison cannot be an artifact of one of them
drawing luckier cells.

The model survives contact: it tracks real execution within about 10%, and
workers=1 - which runs without a pool at all - sits at 0.95, so the residual
above 1.0 at higher worker counts is per-wave thread overhead rather than a
modelling error. Two structural claims that were arithmetic are now observed.
Weekly is flat from workers=3: 3.59, 3.59, 3.59, 3.60, 3.59, 3.59 across w=3..8.
workers=4 buys nothing over workers=3 on cold, 7.68 against 7.78.

Two prototype schedulers are measured beside it, deliberately before any
production code exists. A continuously fed pool per task is worth more than the
model claimed on cold, -27.3% against a predicted -17.9%, and exactly nothing on
weekly, +0.0%, because a weekly task is one wave with nothing to feed. One pool
across all tasks beats both: -40.7% weekly and -42.9% cold at workers=3, rising
to -65.7% and -63.9% at workers=8. It also subsumes the fed pool, since packing
across tasks is a fed pool.

That reorders the backlog. Cross-task packing moves from second to first: it
dominates on both profiles, and it is the only thing that moves weekly at all.
Raising the worker count is worth nothing until it lands - under the barrier
weekly does not improve from w=3 to w=8, and speedup against serial is 1.58x for
three workers and only 2.40x for eight.

The bound on all of it: sleeping threads do not contend. Real sandboxed sessions
compete for CPU, page cache and disk, and the duration sample was itself
measured at workers=1, so it carries no contention either. These speedups are
upper bounds. The ordering is trustworthy because the schedulers were compared
under identical conditions; the magnitudes are not. The packed prototype is also
a bare ThreadPoolExecutor with no breaker folding, no per-task graph lifecycle
and no reuse binding - which is the actual cost of building it, and is not
measured here.

* test(eval): carry the sweep invariants into the packed prototype

The first packed prototype was a bare ThreadPoolExecutor. It reported -43% and
none of the invariants the shipped scheduler holds, so it priced an idea nobody
could ship. This one carries them: a global submission order continued across
task boundaries, in-order folding, the real outage breaker, and per-task graph
readiness gating behind a serial builder.

The fidelity check first reported the two schedulers tripping on different
cells, 17 against 16. That was my instrumentation, not a divergence -
sweep_task_cells folds an entire wave before it evaluates the breaker, so the
last cell folded is not the cell that tripped. With the harness mirroring the
breaker's own evaluation the two agree exactly, across failures starting at
cell 0, 4 and 12, with overrun inside the workers-1 bound the wave docstring
promises.

Two results worth the exercise.

Head-of-line blocking, not the barrier, is what a naive in-order design pays.
Holding submission to `workers` cells beyond the fold pointer leaves the
faithful scheduler at -8.1% cold and -2.7% weekly: one slow cell stalls the
pointer, the window cannot slide, and it reproduces the wave almost exactly.
That is the number to quote if anyone proposes the obvious implementation.

But the overrun bound turns out to be set by the worker count, not the window.
Only `workers` cells can be running when the breaker trips; everything queued
behind them short-circuits on the halt flag. Overrun is 3 at an unbounded
window exactly as at 6, and the trip cell never moves off 16. So H2 does not
have to trade breaker fidelity for speed - a wide window takes -42% with the
semantics intact. The tension I assumed was there is not, and window=12 already
captures 97% of it.

Still an upper bound: sleeping threads do not contend, and the sample was
measured at workers=1. What this establishes is that the invariants are
affordable, which was the thing blocking H2. Not built here: the trees tempdir
lifecycle, reuse-row binding, and the cancel_event path.

591 eval tests pass, ruff clean.

* test(eval): put the scheduler comparison under real CPU contention

Every Phase 2 number so far came from sleeping threads, which contend for
nothing, against a duration sample measured at workers=1, which contains no
contention either. That was the standing caveat on the whole result, so this
measures it.

A cell now waits for its API share and then burns a fixed number of sha256
rounds in a subprocess. Work-bounded rather than wall-clock bounded, so it takes
longer when cores are busy - that is the effect under test. A subprocess because
Python threads burning Python would measure the GIL rather than the machine.
Calibrated at 519k rounds/s, stable within 2% across three probes.

The first run of this was worthless and is recorded as such: on a 24-core host
with 3 to 6 workers nothing ever contends, since cpu_fraction 0.5 at 6 workers
is about 3 cores of demand out of 24. It measured an absence. Re-run pinned with
taskset to 4 and 2 cores.

The packing advantage survives. It holds between -40% and -47% across every host
size and CPU fraction tested, including a genuinely oversubscribed 2-core box at
cpu_fraction 0.5 with 6 workers.

But contention erodes packing more than it erodes waves, for a structural
reason: packing is what creates the concurrency. Moving from 24 cores to 2 at
cpu 0.5 and 6 workers, the faithful scheduler slows 13% while the wave slows
3.7%, and the gain narrows from 45.0% to 39.8%. Packing and a higher worker
count are therefore not independent wins - packing spends the contention
headroom first, so raising workers has to be re-argued after it lands rather
than added to it.

Three things this still does not measure, and they bound the result. The real
CPU fraction of a benchmark cell is a guess informed by roughly 180 tool calls
per session; nobody has profiled one. The evolution runner's core count decides
which column applies and is unknown here. And the burn is sha256, pure CPU,
while real cells run vitest and analyze, which are memory and IO heavy - so this
is a floor on contention, not a ceiling.

591 eval tests pass, ruff clean.

* perf(eval): add a packed sweep scheduler, and correct the bound I claimed for it

sweep_task_cells finishes one task before starting the next and drains a wave
before refilling it, so a task with fewer cells than workers leaves workers
idle and one slow cell stalls its whole wave. sweep_packed_cells feeds every
task's cells through a single pool instead. Measured against the review corpus
it is worth about 40% of a cold sweep, and it is the only change that moves a
seeded weekly run at all - there a task is three cells and a wave is never full.

The breaker keeps its exact meaning. Cells carry a total submission order
continued across task boundaries, a folder walks results in that order, and
consecutive systemic failures are counted there, so a doomed run aborts on the
same cell it would have under waves. Verified at three failure positions.

This commit also corrects a finding from the Phase 2 prototype. I claimed the
overrun bound was set by the worker count rather than the submission window,
and that packing therefore cost nothing in breaker fidelity. That was derived
from a window sweep that only ever injected failures at one position. Driving
the real function at other positions shows the halt flag does not bound overrun
at all: the folder walks in order, so a slow early cell lets workers race ahead
and the trip is detected after those cells have already paid. An unbounded
queue overran by 11 cells where waves overrun by 2.

So the window is load-bearing and the trade is real, measured at workers=3 with
failures injected at four positions:

    window 3  ->  -8% wall,  overrun 2   (the wave scheduler's own bound)
    window 6  -> -27% wall,  overrun 4
    window 12 -> -42% wall,  overrun 9
    window 54 -> -44% wall,  overrun 11

Overrun is wasted paid sessions at roughly $70 each. The default multiplier is
2, keeping the worst case within twice the wave bound while taking most of the
gain; the curve is in the constant's comment so raising it is an informed
decision rather than a guess.

Not wired in yet: _run_sweep still calls sweep_task_cells per task. Moving the
per-task graph, trees tempdir and reuse binding out of that loop behind
await_ready is the larger and riskier half, and it belongs in its own change.

595 eval tests pass, ruff clean.

* fix(eval): judge harness health on execution, not on how many tasks resolved

broken_incumbent_arms infers "the environment is broken" from an arm resolving
zero tasks. That inference does not hold: a reviewer can be wrong about every
task in a hard corpus while every process, mount and capture worked perfectly.
Actions run 33962002890 is exactly that shape - 51 cells, all resolved=False
with error_kind=oracle-failed, median score 0.212, and a healthy harness.

Someone already knew this, and patched it by excluding review arms at the call
site. That leaves the unsound inference in place for workflow and
workflow_direct, and leaves review arms with no health check at all - so the
run that genuinely was broken, 33912693948, where the mount made an atomic
write impossible and all 41 artifacts came back empty, could not have been
caught here either.

So this replaces the inference rather than adding another exemption. aggregate
now classifies fresh rows into execution failures (the process or its tooling
did not complete), evidence failures (it completed but produced nothing
trustworthy or scoreable), and admissible measurements. An arm is unhealthy
only when it has fresh attempts, zero admissible measurements, and at least one
execution or evidence failure. Resolution count is no longer consulted. Arms
with only reused rows report current health as UNKNOWN rather than good.

With the inference corrected, review arms are checked again, which is what lets
the empty-artifact case be caught at all.

Deliberately unchanged: comparator reuse eligibility, quality denominators,
promotion thresholds, model settings, skill prompts and scheduler behaviour.
Failures that stop being called infrastructure failures still surface in the
counts and reasons - an agent-originated failure must not vanish from reporting
because it was reclassified. broken_incumbent_arms and its tests are left in
place; deleting behaviour belongs in its own change.

Seven regression tests, built from both runs' shapes and labelled as
reconstructed from logged observations, since 33962002890's results.jsonl did
not survive the instance shutdown. They pin: a badly-scoring reviewer is
healthy; an all-zero score is still a valid negative; empty artifacts are
unhealthy; one admissible cell keeps an arm healthy while its failures stay
visible; reused rows alone leave health unknown; reused successes do not mask
fresh failures; and a parseable artifact does not excuse a failed session.

602 eval tests pass, ruff clean.

* fix(eval): pin the health guard below the breaker, and stop calling mixed runs healthy

Two corrections to the health-classification patch.

The regression I wrote could not have proved what it claimed. A fixture of 41
empty artifacts aborts through the outage breaker long before finalization:
review-evidence-invalid is in SYSTEMIC_ERROR_KINDS and the limit is 5, so it
trips at cell 5 through the pre-existing path. It demonstrated failure
detection, not the new guard. The decisive test now uses ONE fresh unusable
cell, asserts the streak stays under the breaker threshold, and only then
requires finalization to abort - leaving the new check as the only thing that
can catch it. Removing the call makes that test fail; restoring it passes.

The accurate defect statement is narrower than the last message claimed. Review
arms were excluded from the final incumbent-health check while the consecutive-
failure breaker gave them separate, partial coverage. They were not unguarded.

Second: "one admissible cell plus two execution failures" was asserted as
healthy. That converts "not wholly unusable" into "ran reliably", which is how
a partly-broken sweep passes review. Arms now report UNKNOWN, OBSERVED_OK,
DEGRADED or UNUSABLE. Only UNUSABLE is fatal, so eligibility and promotion are
untouched - this changes what is reported, not what is allowed.

The guard is extracted as enforce_measurement_health so it can be driven
directly, and it now reports a status line per arm. It names no cause: an empty
artifact establishes that evidence is unusable, not that a mount rejected the
write, so it prints cause=undetermined rather than guessing EROFS. It still
runs after report.md and promotion.json are written, so a failing sweep leaves
its evidence behind.

ce_review is named explicitly at the call site. It is a comparator rather than
a candidate, so it is absent from CANDIDATE_ARMS.values(), and dropping the
review exclusion alone would have left it unclassified.

The wiring test reads _run_sweep's compiled code object for the referenced
global rather than matching source text. It is honest about its limit: it
proves the call exists and would catch its removal, but no test here drives
_run_sweep end to end, which needs bwrap and a sandbox.

broken_incumbent_arms is marked LEGACY and NON-AUTHORITATIVE with removal
tracked. It has no production caller.

608 eval tests pass, ruff clean. The two test_model_gateway.py failures are
test_locked_litellm_translates_messages_to_offline_responses and
test_openai_gateway_never_leaves_proxy_output_on_an_undrained_pipe; both fail
identically on origin/main in this environment, checked directly rather than
carried forward as an inherited label.

* fix(eval): review artifact path, evidence classification, comparator reuse

Extracted from the combined skill-evolution branch. This is the runtime change
set: everything that alters how a sweep executes and what it records. The
packed scheduler and its measurement harness were separated onto
perf/skill-evolution-packed-scheduler, which is purely additive.

Correctness. The review artifact was mounted as a writable FILE inside a
read-only workspace while the agent's Write tool writes atomically - temp file
beside the target, then rename - so the temp create failed EROFS and the
artifact was never written. Four layers gate that path and three named the old
location: the CLI's own allowWrite/allowRead policy, the task corpus verify
command run in its own sandbox invocation, and host_text/host_path for the
host-unsafe backend. Fixing the translator exposed a second defect, since
"/review-output" appears twice in "/review-output/review-output.json"; matching
is now anchored to a path boundary. parse_review_output folded OSError,
UnicodeError and JSONDecodeError into one message, so an artifact that was
never written looked like an encoding fault; each cause now names itself.

Health classification. broken_incumbent_arms inferred a broken environment from
an arm resolving zero tasks, which a reviewer facing a hard corpus falsifies -
Actions run 33962002890 is exactly that shape. Arms are now classified from
fresh execution and evidence outcomes as UNKNOWN, OBSERVED_OK, DEGRADED or
UNUSABLE, and only UNUSABLE aborts. Resolution count is not consulted. The
guard names no cause: an empty artifact establishes unusable evidence, not that
a mount rejected the write.

Comparator reuse. Reuse accepted evidence it should have rejected: a row
without a runtime_digest passed the drift lock, recorded_at was overwritten with
the copy time so a row could outlive its own max_age, and the binding ignored
task-asset and dependency digests although this change alters
sandbox_dependencies in the review corpus. Closing the last one required
preparing asset snapshots before the reuse decision, which also removes the
concurrent TaskAssetCache.prepare the prefetch thread could race.

These three concerns share aggregate() and _run_sweep, which is why they ship
together: separating them further would mean hunk-level surgery on a function
all three modify, and the risk of a silent omission outweighs the reviewability
gain.

592 eval tests pass at this base. The two test_model_gateway.py failures,
test_locked_litellm_translates_messages_to_offline_responses and
test_openai_gateway_never_leaves_proxy_output_on_an_undrained_pipe, fail
identically on origin/main in this environment.

Known gap, and the reason this is not ready to merge: no test drives _run_sweep
end to end. enforce_measurement_health is unit-tested including the
below-breaker unusable case, and the caller wiring is pinned structurally by
reading _run_sweep's compiled code object, but interruption semantics, exit
precedence and persisted artifacts are not exercised through the real path.

* fix(eval): address PR review feedback (#3207)

- aggregate: count admissible rows directly instead of subtracting the
  execution and evidence counters, which double-charged a row that is both
  a session error and invalid review evidence and could report UNUSABLE for
  an arm holding real measurements.
- run_proposer: bound the session timeout by what is left of
  --max-runtime-seconds, so clearing the sweep minimum cannot start a
  full-length session past the instance window.
- comparator reuse: hold one O_NOFOLLOW descriptor for the size check,
  digest and copy, and prove it is the inode that was checked, closing the
  swap window a concurrent writer of the reuse directory had.
- Drive the review-artifact mount assertion through run_arm and the
  clone-template assertion through run_cell, instead of rebuilding the
  expected values in the tests (also removes the CodeQL unnecessary lambda).
- Assert the workflow invokes run-evolution.sh rather than that its YAML
  mentions --max-runtime-seconds, which only appears in a comment.
- Correct the parse_review_output failure-mode claim: the fold was empty
  artifacts reported as "not valid UTF-8 JSON"; a never-created file raised
  FileNotFoundError.
- prettier: wrap the over-long readFileSync call flagged by PR autofix.

Note: pre-existing failure in tests/test_model_gateway.py::test_locked_litellm_translates_messages_to_offline_responses (local LiteLLM proxy never becomes ready in this environment) not addressed by this PR.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(eval): address the second round of PR review feedback (#3207)

- Refuse a symlinked `transcripts` component on both sides of comparator
  reuse. O_NOFOLLOW guards the leaf only, so a link there redirected the
  read or the copy out of the results directory; checked per component as
  evolution._require_directory_chain does.
- Base the paid incumbent canary on the cells this sweep PLANS. Reuse
  selection accepts any prior run index, so a results directory produced
  with more runs left extra keys, the equality never held, and the canary
  stopped firing. Extracted as drop_canary_reuse_key and unit-tested.
- Start the runtime clock in main(). --max-runtime-seconds is measured from
  /proc/uptime before exec, so parsing, task I/O, preflight and gateway
  setup were being handed back to the sweep out of the upload reserve.
- Do not fall back to shutil.copytree when the managed clone copy was
  cancelled or timed out; that fallback is for a filesystem that cannot
  reflink, and copytree cannot be cancelled.
- Assert the review session's writable mount, not only the verifier's
  read-only one: the EROFS bug is about the agent's write.
- Exercise ref isolation in the copy_isolated_tree test rather than
  comparing an initial HEAD a shared namespace would also match.
- Point the stale-symlink fixture at the sentinel via os.path.relpath, and
  skip the reuse symlink tests where symlink creation needs privilege.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(eval): close the runtime-cap gap and pin the reuse directory

Both were left open on #3207 as approach decisions rather than nits.

Runtime cap: run-evolution.sh computed the budget in its own
`uv run python -c` and passed a number, so the script's remaining
provenance work and the CLI's own startup were spent by nobody and charged
to the sweep — out of the upload reserve the cap exists to protect. The
script now passes --max-runtime-from-instance-window and evolve reads
/proc/uptime itself, on the line after it starts the clock the budget is
measured against, so no interval exists to lose. Also removes an
interpreter start from the script and lets --dry-run print the real argv.

Reuse directory: _real_child_directory lstat-checked `transcripts` and
returned its pathname, so a concurrent writer could rename the directory
and leave a symlink before the name was used again — O_NOFOLLOW guards
only the leaf. Every artifact is now resolved against a held descriptor:
_open_real_directory opens with O_DIRECTORY|O_NOFOLLOW (check and open in
one syscall), and _open_regular / _copy_owner_only take dir_fd. The reuse
path is therefore POSIX-only; _require_openat says so and fails closed,
which the runner already treats as "run a paid cell". _resolved_directory
still tolerates a symlinked reuse root, unchanged and still tested.

evolution._require_directory_chain is still lstat-per-component. It guards
a different surface (candidate overlay reads) that neither review raised,
so it is left alone rather than widened into here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(eval): sample the proposer budget where it is spent, digest what is copied

Four findings against f0cdc9e7, all of them mine.

- run_proposer took a precomputed remaining_seconds, but it clones,
  sanitizes and builds a sandbox before the session starts, so the caller's
  reading was already stale. It takes started_monotonic now and samples the
  budget on the last line before run_claude. The caller's earlier reading
  still decides whether to start at all — it just no longer decides how
  long to allow.
- _copy_transcript_artifact hashed the source and then read it again to
  copy it. A held descriptor stops the pathname being substituted, not the
  inode being rewritten, so the row could record the expected digest while
  the destination held other bytes. _copy_owner_only now digests the same
  buffers it writes and returns (digest, bytes); a mismatch unlinks the
  destination and raises. One read instead of two.
- Explain the empty except in _open_real_directory: an existing directory
  is the ordinary case, and the O_DIRECTORY|O_NOFOLLOW open below is what
  proves what it is (CodeQL).
- Drop the unused uptime fixture from the runtime-cap test, and correct the
  cross-file contract described in the workflow-contract test and the
  workflow YAML: neither names --max-runtime-from-instance-window, the
  script does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(eval): close the reuse producer/consumer contract through the real run_cell

Every comparator-reuse test built its rows by hand. That proves the predicate's
logic and nothing about the producer: a fixture can satisfy eligibility while a
row the runner actually emits never does, and a key-name inventory cannot tell
the difference. I checked that inventory first - all 25 keys the predicate reads
have a producer - which is exactly why it was not sufficient evidence.

This carries one record through the production path instead:

    real run_cell -> production JSONL writer -> load_result_rows
        -> row_is_reusable_comparator

Only the expensive dependencies are replaced: the model session, sandbox launch,
repository acquisition, graph preparation, git plumbing. The digest fields the
reuse binding compares are assembled by run_cell itself from its
TaskCellContext, so they stay real - they are the subject, not scaffolding.
Expectations are built from the sweep's own configuration rather than copied out
of the emitted row, since copying them back would make producer and consumer
agree because the test arranged it.

Two cases: a production-emitted row with matching bindings is eligible, and the
same evidence with one dependency binding changed is rejected. A test-controlled
digest set keeps both deterministic.

Mutation-checked. Replacing run_cell's task_asset_manifest_digest assignment
with None makes the positive case fail; restoring it passes. That is evidence
the inventory audit could not produce.

Building it also documented what a real review row must carry, which no fixture
had recorded: a scored review with review_weighted_f1 present, and at least one
transcript artifact whose source is PARENT_EVENT_STREAM_SOURCE. An empty
artifact list is not reusable. Those are production contracts my first double
got wrong, and the reader rejected it each time.

No production code changed. 607 eval tests pass, ruff clean.

* test(eval): drive the real sweep to its finalization decision

enforce_measurement_health was unit-tested and its call site pinned by reading
_run_sweep's compiled code object. Neither showed the guard running inside a
sweep. These drive the real _run_sweep with cell execution scripted and
everything downstream left alone: folding, aggregation, the artifact writers,
the health guard and the exit selection.

The below-breaker case is the decisive one. A fixture of many unusable cells
aborts through the pre-existing outage breaker instead - review-evidence-invalid
is systemic with a limit of five - and would pass whether or not the
finalization guard exists. One fresh unusable cell stays under that threshold,
so only the guard can catch it. The test asserts the arm and status the guard
reports, and that no systemic-outage line was printed, rather than accepting any
SystemExit: an unrelated setup failure must not satisfy it.

Mutation-checked at the runtime path. Removing the guard invocation fails the
below-breaker test because the expected finalization behaviour disappears, not
because a name went missing from a code object.

A zero score is covered separately. review_weighted_f1 = 0.0 must stay a present
valid negative measurement; a truthiness check would read it as absent and turn
a quality result into an execution-health failure. The third test asserts the
evidence survives - results.jsonl carries the scored row and report.md exists -
so the persisted artifacts tell the same story as the exit.

Only expensive setup is replaced: cell execution, graph preparation, asset
snapshots, and task-binding resolution, which clones the repository and verifies
the ref. Building the fixture also documented that the report renders the whole
review metric set, so an incomplete row fails in formatting rather than logic.

Still open on this track: interruption semantics and exit-reason precedence
(outage 1 before cancellation 130) are not yet exercised, and fold order and the
real sandbox mount contract remain on separate tracks.

No production code changed. 610 eval tests pass, ruff clean. The two
test_model_gateway.py failures reproduce on origin/main in this environment.

* fix(eval): an interrupted sweep is interrupted, not aborted

Driving the real _run_sweep to its exit revealed that cancellation without an
outage exits 1 and writes "Sweep aborted: partial evidence", where the contract
is 130 and "Sweep cancelled".

sweep_task_cells returns (streak, tripped), and its caller assigns that flag to
outage_tripped and turns it into exit 1. Both cancellation paths returned True
for it. The breaker's own return is also True, so the two became
indistinguishable one frame up and cancellation inherited the outage's exit and
wording. The fix is to return False from the cancellation paths: the flag means
the breaker tripped, and the caller already tests cancel_event itself for the
stop decision, so nothing stops running any later than before.

Reproduced before the fix, through the real caller, not from reading. The
failing assertion was the report line; the exit code was 1.

Two runtime tests cover it. Each begins with one admissible cell so the arm
classifies DEGRADED rather than UNUSABLE - otherwise enforce_measurement_health
supplies exit 1 first and a precedence test passes without ever reaching exit
selection. Cancellation is set from a completed cell rather than a sleep or a
real signal, so the interruption point is deterministic.

The precedence test is mutation-checked: moving the cancel_event check ahead of
the outage check fails it with "assert 130 == 1" - it observes the wrong exit
code, not merely some failure. My first attempt at that mutation silently
matched nothing and the suite passed; a no-op mutation proves nothing, so the
edit now asserts its own anchor.

test_process_control read the same flag as "stopped" and asserted it after a
cancellation. It now reports the event and the flag separately, which is what it
was really asserting: cancelled, and not an outage.

The scored-row shape moved into tests/bench_fixtures.py with zero values
written out, so the finalization and interruption tests share one definition
of what a real review row carries.

Impact analysis returned risk UNKNOWN - the index predates this branch and does
not carry the eval harness - so the callers were confirmed by text search as the
rules require for UNKNOWN: one production caller and three test sites, all
updated or verified. detect_changes reports zero for the same reason; that zero
is unseen, not unaffected.

612 eval tests pass, ruff clean. The two test_model_gateway.py failures
reproduce on origin/main in this environment.

* test(eval): prove the review artifact mount under a real sandbox

The orchestration tests replace the sandbox, so they say nothing about
isolation. This covers the filesystem contract the EROFS defect actually broke,
through the production configuration: the same command_prefix_for call run_arm
makes for a review cell, with read_only_workspace=True and the review-output
directory as the writable mount - not a hand-built mount tuple that merely looks
right.

A deterministic writer stands in for the agent. It writes a temp file beside the
destination and renames it into place, which is the operation that failed: an
atomic write needs a WRITABLE PARENT DIRECTORY, and binding the file itself left
nowhere to put the temp file. It then attempts a workspace write and must be
refused, and the production parse_review_output reads the bytes the sandbox left
behind. No model session and no credentials.

Placed in test_proposer_sandbox.py, which the "eval / containment (ubuntu)" job
already runs with GITNEXUS_REQUIRE_BWRAP_CANARY=1. That gate is the point: where
the variable is set, a missing namespace capability FAILS the job instead of
skipping into a green tick. Verified here - forcing the variable on this machine
fails with "bwrap: No permissions to create new namespace" rather than skipping.

What this does not establish: the assertions have never executed. This machine
cannot create user namespaces, so the test stops at the preflight. Imports,
signatures and the payload were checked statically instead - parse_review_output
returns a tuple rather than an object, and it rejects a body without "verdict",
so both of my first attempts were wrong and are fixed. The first CI run on a
namespace-capable runner is what will actually confirm it.

Scope is the filesystem and process contract only. A stand-in writer does not
establish that a particular agent CLI's own file-access policy permits the same
operation; that is a second, independent gate.

40 passed, 10 skipped locally; ruff clean.

* fix(eval): read the review artifact before the sandbox deletes it

The containment (ubuntu) job has been red since before the mount canary was
added, and both failures have the same cause.

This branch moved the review artifact out of the clone and into the session's
private root, which is the point of the change: the agent writes atomically, so
the artifact needs a writable parent DIRECTORY outside the read-only workspace.
prepare_sandbox removes that private root in a finally on scope exit. Two tests
read the artifact AFTER the with block, so they were asserting against a
directory the sandbox had already deleted - FileNotFoundError, reported as
"review output was never written".

Production is not affected, and this is the reason: run_arm holds the live
session and reads the artifact inside that scope, both to score it and to mount
it read-only into the verifier's separate sandbox invocation. The tests were the
only readers outside it.

Both now read inside the scope. The pre-existing canary
(test_read_only_review_workspace_exposes_only_one_writable_artifact) predates
this PR and passes on main, where the artifact still lived in the clone and
survived teardown; the relocation is what broke it, so the fix belongs here.

The new canary found this independently and agreed on the cause, which is what
it was written for - though only after CI ran it, since this machine cannot
create user namespaces.

40 passed, 10 skipped locally; ruff clean. The bwrap-gated tests still skip here
and remain unverified until the ubuntu job runs them.

* test(eval): pin what an interrupted sweep persists and refuses to promote

The completed-run persistence test could not show either of these: it never
interrupts, so it would pass even if the writers only ran on the clean path.

Evidence already paid for survives cancellation. The cell that completed before
the interruption is still in results.jsonl with its measurement intact, and
report.md is still written. Losing those rows would mean paying for evidence the
sweep then discards.

An interrupted run emits nothing that authorizes promotion. Asserted as the
semantic condition rather than the absence of a file: promotion.json IS still
written for an aborted run - it is the record of why nothing was promoted - so
the test requires run_status "aborted" and every decision reduced to
insufficient_evidence with a partial-evidence reason, rather than requiring the
artifact to disappear.

Mutation-checked. Forcing complete=True at the promotion_evidence call site
fails the test with "assert 'complete' == 'aborted'" - it observes an aborted
run claiming completeness, not merely some failure.

These were the last two unasserted items on this PR's finalization checklist.

614 eval tests pass, ruff clean. The two test_model_gateway.py failures are
environmental (litellm[proxy] console script absent) and predate this branch.

* Address PR review feedback (#3207)

An exhausted runtime cap no longer buys a second. remaining_runtime_seconds
floors at 0 and the proposer call site wrapped it in max(1, ...), so a cap fully
spent by the clone, the sanitize pass and the sandbox build started a paid
session with a one-second allowance instead of stopping before the upload
reserve the cap exists to protect. It now returns a not-ok record, which the
caller already treats as end-of-run. Pinned by a test driving the real
run_proposer with setup that consumes the whole budget; reverting to max(1, ...)
fails it.

Reuse copies are bounded before their size is validated, not after. The source
is a prior sweep directory this module already treats as concurrently writable,
so a transcript appended to after its metadata was recorded was streamed to EOF
and only then compared against its declared size - filling the destination, or
never reaching EOF, long before the drift check could reject it. _copy_owner_only
now takes max_bytes and stops one byte past the ceiling, which keeps the drift
comparison meaningful. Review and patch artifacts carry no recorded size, but
"no recorded size" is not "no limit": they get MAX_TRANSCRIPT_BYTES, the ceiling
the capture path already enforces.

A reused review row must carry its artifact. The predicate accepted a row on its
score and transcript metadata while materialize_reused_row copied the review
artifact only when the row named one, so a scored review could be carried
forward with nothing for a proposer to read. The shared row fixture was the
thing out of step here, not the requirement - production sets review_artifact
whenever the review source exists - so it now carries one too.

Test fixes, all against code this PR added:

unusable_review_row could not be overridden at all. It passed explicit keywords
beside **overrides, and Python rejects the duplicate in the call expression
before scored_review_row can apply its update, so unusable_review_row(error_kind=...)
raised TypeError. Merged into one mapping.

The finalization helper monkeypatched runner.prepare_ce_plugin_snapshot with
raising=False. No such symbol exists - the real one is staged_ce_plugin_snapshot
- so it silently added an attribute nothing reads. Removed.

The promotion test named candidate_review in candidate_arms but _run_sweep builds
cells only from args.arms, so no candidate cell ran and insufficient_evidence
could hold because nothing executed rather than because partial evidence is
barred. The candidate arm now runs, cancellation fires after its cell, and the
test asserts the candidate actually produced a row so it cannot silently return
to being vacuous.

The reuse round trip serialized with json.dumps and write_text while claiming to
cover the production writer, which applies redact_text over the row's own bytes.
It now writes the way the sweep does. Its fixture also writes the review
artifact, because run_cell records review_artifact only when the review source
exists and the emitted row was otherwise one production never emits.

Not addressing the duplicate-match nit on proposer_sandbox.py: the claim is that
"/review-output" occurs once in "/review-output/review-output.json". It occurs
twice, at offsets 0 and 14 - the separator before the basename forms it again -
so the boundary the comment describes is real. Verified with re.finditer.

The workspace-snapshot finding is parked for a human: excluding bootstrap noise
is a documented deliberate choice, and tightening it is a genuine tradeoff.

634 eval tests pass, ruff clean. The two test_model_gateway.py failures are
environmental (litellm[proxy] console script absent) and predate this branch.

* Address PR review feedback (#3207), round 2

Carry review_f1 in the scored-row fixture. score_review emits "f1"
(review_scoring.py:317), which the runner folds in as review_f1, so a real
scored row has it and the fixture did not - the same fidelity gap as the metrics
already added there. aggregate now reports 0.5 for it instead of None.

The reported failure mode was not real, and the distinction matters for anyone
reading the thread later. aggregate filters on record.get(metric) is not None
BEFORE indexing record[metric], so a missing key is skipped rather than raising
KeyError; the finalization tests were passing throughout. Verified by running
aggregate against the old fixture. Fixed because the fixture should match what
production emits, not because anything was crashing.

Drop the unused row parameter from the round-trip _expectation helper. It never
read the argument - deliberately, since the docstring says the bindings must be
derived from sweep configuration rather than copied out of the emitted row - but
passing the row anyway was dead plumbing that suggested the opposite.

The reuse-root TOCTOU finding is parked for a human rather than fixed: the
lstat-then-resolve window is real, but _resolved_directory documents the weaker
promise as deliberate and says not to merge it with proposer_sandbox's helper
"without first deciding which promise the reuse path should make". That decision
is the fix, and it is the same class as the reuse TOCTOU already parked on this
PR.

634 eval tests pass, ruff clean. test_process_control's TERM-ignoring-descendant
test flaked once under full-suite load and passes 3/3 in isolation; it is a
timing test and neither file changed here touches it. The two
test_model_gateway.py failures remain environmental.

* fix(eval): pin the reuse roots to the directory that was checked

Settles the reuse TOCTOU that has been parked twice on this PR. The docstring
asked to decide which promise the reuse path makes before merging its helper
with proposer_sandbox's, and the answer is that two separate things were being
conflated.

The symlink POLICY stays exactly as it was: parent hops remain allowed, so a
symlinked artifacts directory or macOS's /var still works, and a symlinked leaf
is still refused. Rejecting hops would break ordinary setups for no gain, which
is what the docstring argued and it is still right.

What is closed is the other thing - the gap between checking a name and using
it. _resolved_directory lstats a name and the later open re-walks that same
name, so a prior sweep that renames its results root and leaves something else
behind is opened somewhere else entirely, and O_NOFOLLOW cannot see a link that
resolve() already followed. _resolved_directory now returns the checked
directory's identity alongside its path, and _open_pinned_root fstats the
descriptor it opened and refuses a mismatch.

The two are separable, so there was no trade to make: comparing identity
rejects nothing that holds still, since a stable directory always matches
itself. Both cases are pinned - a replacement by a different REAL directory is
refused (the leaf-symlink rule does not cover that one), and an ordinary
unchanged root opens normally.

Why it matters here rather than as a general hardening: the failure is silent.
Rows would be copied out of some other directory and folded into a comparator
baseline as though they were this sweep's own evidence, which is the one thing
the reuse path exists to get right - a corrupted baseline decides promotions.

636 eval tests pass, ruff clean. The two test_model_gateway.py failures remain
environmental.

* refactor(eval): one prompt digest, and drop two pieces of dead scaffolding

Compute task_prompt_digest once. row_is_reusable_comparator compares the value a
prior row stored against the value this sweep derives, as exact strings, and the
two inline copies of that hash had already drifted apart - the expectation side
had picked up a str() cast the row side lacked. They agree today only because
tasks validate "prompt" as a string (runner_tasks.required_strings), so nothing
was broken; a later divergence on one side would have silently stopped rows
matching, with no test to catch it. Verified the extracted helper is
byte-identical to what it replaced.

Two comments named broken_incumbent_arms as the thing that would read stale
health. This branch replaced that call with the measurement-health path, so the
reasoning still holds but the name no longer does; they now name arm_health.
The function itself stays: origin/main still calls it at runner.py:2098, and
retiring it is its own change rather than a cleanup pass.

Removed instance_window_budget_from_proc and its one test. It composes two
helpers main() deliberately calls separately - there is a comment there
explaining why the read and the budget calculation stay decoupled - and nothing
outside its own test ever called it. Introduced on this branch, absent from
main, so it was never deployed, public, or consumed elsewhere.

Dropped an unused tmp_path parameter from a comparator-reuse test that does no
filesystem work.

Skipped three findings. Hoisting _assert_self_contained_git_objects to a
once-per-template check is a real saving - measured at 5.66s over 1551 objects -
but it verifies the OUTPUT of each copy operation, and git clone from a local
path hardlinks objects by default, which is exactly what its st_nlink check
catches. Checking a predecessor instead of the clone each cell runs against
thins an isolation guarantee for 0.7% of a median cell. Deleting
broken_incumbent_arms outright would remove a symbol main still calls. And a
fourth copy of the test-only _git helper extends a pattern that already exists
in three other test modules; centralising it means editing conftest.py, outside
this scope.

635 eval tests pass, ruff clean. The two test_model_gateway.py failures are the
environmental ones.

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-08 09:25:45 +00:00
cosark
a4e70ec3b4
docs: add RepoCloud one-click deploy button (#3212)
Co-authored-by: cosark <cosark@users.noreply.github.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-09-08 09:23:52 +01:00
Gergő Magyar
bddbb0ff9f
test(cli): prove detached refresh by ordering, not by wall clock (#3221)
`lets the parent exit without waiting for a detached refresh child` bet
twice on absolute wall-clock budgets and lost both bets on loaded CI
runners:

- `expect(elapsed).toBeLessThan(1_800)` bounded the *parent's* cold
  `node --import tsx` boot, inferring "did not wait" from a 1050ms margin
  over the mock fetch's 750ms sleep. Reproduced failing on Linux under
  40-way load at 1904ms.
- The 15s poll for the cache file had to cover the whole detached child:
  node boot, a tsx transpile of 22 source files, an `acquireFileLock`
  that shells out to `ps` (POSIX) or `powershell.exe -Command
  Get-CimInstance Win32_Process` (Windows), the mocked fetch, and the
  atomic write. That chain measures ~1.0s locally but has no bounded
  upper limit on a contended runner, and it is what timed out in CI.

Replace both budgets with an ordering proof. The preloaded mock fetch now
parks the refresh child until the test releases it, so the sequence
asserted is: parent exited -> child provably still mid-refresh (started
marker present, cache absent) -> release -> cache written. That is
strictly stronger than the old elapsed-time inference, and it holds at any
machine speed. The remaining `expect.poll` timeouts no longer carry the
assertion's meaning; they are only "is the child dead" safety nets.

The mock's wait is bounded at 60s so an abandoned child (test failed
before releasing, temp home already removed) still exits instead of
spinning forever.

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-08 09:14:47 +01:00
Gergő Magyar
d1463977c8
feat(eval): Add bounded packed-scheduler primitives and offline replay benchmarks (#3206)
* perf(eval): packed sweep scheduler and the harness that measured it

Extracted from the combined skill-evolution branch so it can be reviewed on its
own. Purely additive against main: no existing function changes behaviour, and
sweep_packed_cells has no production caller yet.

sweep_task_cells finishes one task before starting the next and drains a wave
before refilling it, so a task with fewer cells than workers leaves workers
idle and one slow cell stalls its whole wave. sweep_packed_cells feeds every
task's cells through a single pool instead, keeping the breaker's meaning: a
total submission order continued across task boundaries, a folder walking
results in that order, and consecutive systemic failures counted there, so a
doomed run aborts on the same cell it would have under waves.

simulate_sweep.py is what produced the numbers. It drives the real schedulers
with only the paid agent session stubbed, using the measured per-arm durations
in session_durations.json divided by a scale factor. The distribution's shape
is kept deliberately - median 826s against a 5400s ceiling - because that
spread is the entire reason a barrier costs anything, and uniform sleeps would
erase the effect under test. All schedulers consume one identical seeded plan.

Measured at workers=3 against the review corpus, packing is worth about 40% of
a cold sweep, and it is the only change that moves a seeded weekly run at all -
there a task is three cells and a wave is never full. The submission window is
a real trade, measured with failures injected at four positions:

    window 3  ->  -8% wall,  overrun 2   (the wave scheduler's own bound)
    window 6  -> -27% wall,  overrun 4
    window 12 -> -42% wall,  overrun 9
    window 54 -> -44% wall,  overrun 11

Overrun is wasted paid sessions on an aborted sweep. The default multiplier is
2; the curve lives in the constant's comment so raising it is an informed
decision. Contention was measured separately by burning real CPU in
subprocesses under taskset: the advantage holds between -40% and -47% from 24
cores down to an oversubscribed 2, though packing erodes faster than waves do
because packing is what creates the concurrency.

measure_evolution_cost.py is the offline cost model, with no runtime caller. It
reports workers from the workflow's current default, which on this base is 1.

Limits worth stating: sleeping threads do not contend and the duration sample
was itself recorded at workers=1, so the speedups are upper bounds; the ordering
of the schedulers is trustworthy because they were compared under identical
conditions, the magnitudes are not.

562 eval tests pass at this base. The two test_model_gateway.py failures,
test_locked_litellm_translates_messages_to_offline_responses and
test_openai_gateway_never_leaves_proxy_output_on_an_undrained_pipe, fail
identically on origin/main in this environment.

* fix(eval): compare the shipped window and bound the overrun by it

Address PR review feedback (#3206).

run_faithful defaulted its submission window to `workers` while
runner.sweep_packed_cells defaults to `max(workers * PACKED_WINDOW_MULTIPLIER,
workers)`, so every run that named no window compared a prototype queued twice
as tightly as the shipped scheduler and presented it as the production
invariant. The faithful default now reads the same constant. Measured at
workers=3, faithful and production agreed on nothing before and agree exactly
now: breaker overrun 2/1/2 vs 2/4/3 becomes 2/4/3 vs 2/4/3 across the three
failure positions.

The contention sweep hard-coded `window=12` for faithful only, which the
production run never saw - masked at workers=6 where both are 12. Removed, and
the production measurement it was already paying for is now reported as
`production_s` instead of being discarded.

breaker_fidelity checked the overrun against `args.workers`. The bound the
producer actually enforces is `window - 1` cells past the fold pointer, which
is the wave scheduler's own `workers - 1` when window == workers; against the
shipped default of 6 the old predicate reported a failure for an in-bound run.
The window is now passed explicitly, reported in each row, and checked against
its own bound.

--window was parsed and never read. Wired into the schedulers that hold one.
Dropped two unused plan constructions CodeQL flagged, and the `skipped` set in
sweep_packed_cells that nothing reads - the None appended to `submitted` is the
skip representation the fold loop consumes.

Verification: 562 passed, 15 skipped, 2 failed (the two test_model_gateway.py
failures the PR description documents as reproducing on origin/main), ruff
clean.

* fix(eval): carry the cancellation scope into packed cells, reject the args that hang

Address PR review feedback (#3206).

sweep_packed_cells submits from a producer THREAD, and a new thread starts with
an empty context, so `copy_context()` there copied the producer's context rather
than the one cancellation_scope had just bound _CANCELLATION in. Every packed
cell therefore ran with no cancellation event, and run_managed falls back to
_CANCELLATION when none is passed - so a cancelled run's subprocesses would
never have learned about it. sweep_task_cells gets this right for free by
submitting from the thread that entered the scope. Reproduced directly: packed
workers observed [False, False], wave workers [True, True]. The caller's context
is now captured before the producer starts and copied per submission; the new
test fails without the fix.

Three CLI arguments were accepted and then wedged the run:

  --scale 0                       ZeroDivisionError before any scheduler starts
  --graph-seconds -1              hangs: the builder thread dies on a negative
                                  sleep, every scheduler waits on a readiness
                                  event nobody sets
  --window 0 (faithful)           hangs: submitted - fold_pointer >= 0 holds
                                  before the first submission, so the producer
                                  and the consumer wait on each other

The first two are rejected at the parser, which is the only layer that runs
before a thread exists. run_faithful now enforces the same window >= workers
rule sweep_packed_cells already had, so the prototype rejects exactly what the
shipped function rejects. All three were confirmed to crash or hang first.

Verification: 563 passed, 15 skipped, 2 failed (the two test_model_gateway.py
failures the PR description documents as reproducing on origin/main), ruff
clean.

* chore(autofix): apply prettier + eslint fixes via /autofix command

* Address PR review feedback (#3206)

Preserve settled sibling rows when a packed cell raises. run_cell deliberately
lets unexpected harness exceptions propagate, and sweep_task_cells answers that
by folding every non-failing sibling before it re-raises - the cells already ran
and already spent their budget, so dropping their rows means paying for evidence
the sweep then discards. sweep_packed_cells called future.result() bare, so the
fold stopped at the failing index and every later cell that had already
completed was silently lost. It now folds forward over the settled futures
before re-raising. The failing index itself has no row, since execute() assigns
only on success, so folding forward cannot duplicate it.

Pinned by a regression test that fails without the fix: the later cell is made
to finish first, so there is real settled evidence to lose at the moment cell 0
raises.

Reject arguments that cannot produce a run, at the boundary rather than deep
inside a thread. NaN defeats every comparison it appears in, so the existing
"> 0" and ">= 0" checks admitted --scale nan and --graph-seconds nan; the NaN
then reached time.sleep in a worker or the graph thread, raised there, and left
every scheduler waiting forever on a readiness event nobody would set. Infinity
was worse than a crash: it scaled all durations to zero and the run reported a
sweep that took no time. Both flags now require a finite value.

The count flags are indexed or handed straight to a thread pool, so a zero
surfaced as an IndexError on plans[0], a median over an empty sequence, or
ThreadPoolExecutor's own error - none naming the flag responsible. --workers,
--repeat and --runs now require at least 1.

Two flags were not in the review but carry the same invariant and the same
one-line treatment, so they are fixed with the class rather than left to
resurface: --runs (same empty-plan path as --repeat) and --window, where zero
admits no cell at all because the producer waits for a fold pointer to move past
a cell it was never allowed to submit.

Verified each guard fires with its own message rather than a stack trace.

563 eval tests pass. Note: pre-existing failures in test_model_gateway.py not
addressed by this PR - litellm[proxy]'s console script is absent in this
environment, and neither test touches the files changed here.

* Address PR review feedback (#3206), round 2

Stop charging the fed baseline for overlap the wave scheduler gets free.
run_fed is documented as pricing the barrier alone, but it slept graph_seconds
serially before every task, while run_wave starts one background builder that
prepares task N+1 while task N's cells run. The fed-versus-wave delta therefore
mixed the loss of that overlap into what was reported as the price of the
barrier. run_fed now uses the same builder, started before the clock, so the
barrier is the only remaining difference.

This moved the numbers. On the weekly profile fed was 4.203s and is now 3.694s,
exactly equal to wave - which is the answer that profile should give. On cold,
fed was 5.995s and is now 5.487s, so the measured price of the barrier widens
from 1.844s to 2.352s: the old arrangement understated it by about a quarter.
No committed results file or PR-body figure quotes these, so there is nothing
stale to regenerate.

Enforce the window bound the schedulers actually hold. Last round's guard
required only >= 1, but run_faithful and sweep_packed_cells both refuse a window
below the worker count, so --scheduler faithful --workers 3 --window 1 passed
validation and then died on an uncaught ValueError. The check now uses the
worker count.

It also uses the LARGEST worker count the invocation will really use.
--contention-sweep runs its own counts irrespective of --workers, so validating
against --workers alone let the three-worker measurements finish and then raised
on the six-worker one, losing the run partway through. Those counts are now a
named constant the validator can see.

Verified: --scheduler faithful --workers 3 --window 1 is rejected naming 3, and
--workers 3 --window 3 --contention-sweep is rejected naming 6.

No regression test for the graph-overlap fix. Discriminating it from the old
behaviour requires cell work to overlap graph work, which makes the assertion a
timing comparison, and this project does not take non-deterministic tests. It is
verified by the before/after measurement above instead.

564 eval tests pass. Note: pre-existing failures in test_model_gateway.py not
addressed by this PR - litellm[proxy]'s console script is absent here.

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-09-08 08:22:12 +01:00
Subham Kundu
95858e7549
Update README.md (#3217)
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-09-08 07:25:22 +01:00
dependabot[bot]
a7d9229326
chore(deps)(deps): bump express-rate-limit in /gitnexus (#3214) 2026-09-08 06:23:22 +01:00
Gergő Magyar
0d1aed942f
docs(bench): close FTS as an optimization target with measured evidence (#3209)
PR #3208 landed two claims that further measurement disproved.

The narrowing is not inconsistent. A true incremental leaf edit rebuilds
exactly 8 of the 20 configured indexes -- the tables the writeback DMLs --
and the 20-index run I compared it against was a forced full rebuild
(runner-identity trap). Per-index costs for those 8 sum to 7769ms against
the 7525ms measured in-analyze. There is nothing to fix in `touchedFts`.

The 845ms was not `import('./platform/capabilities.js')`. The CLI already
imports that module statically; a cached dynamic import measures 0.035ms.
The `await` is the first yield after the native FTS build and absorbs the
libuv work still queued behind it. Recorded as a fourth measurement trap,
since it invalidates any mark placed on an await that follows native work.

What replaces them is a floor, established by probing a copy of the corpus
index directly:

  - narrowing further: nothing left, the 8 tables are exactly the DML'd set
  - concurrent builds: hard error, one write transaction at a time
  - connection thread count: flat at 4/8/16/24 (7298/7133/7109/7345ms
    min-of-3), though the default burns ~60% more CPU for it
  - dropping `content` from File: 3541ms -> 241ms, but that deletes
    full-file keyword search (#2317/#2323); capping is a bad trade because
    the size distribution is flat

The one lever left is overlap: the build runs on a libuv thread and hides
behind main-thread JS (3337ms for the index plus a 3000ms JS burn, against
~6859ms serial). File rows are `{ name, filePath }` from `processStructure`
with content lazy-read at CSV time, so they are known before parsing. What
blocks it is that the DB is closed for the whole pipeline and that an early
write moves `liveIndexMutationStarted` ahead of it.

The `ponytail:` comment in `createSearchFTSIndexes` invited exactly the fix
that cannot work -- every caller has already dropped the indexes it passes,
so a presence gate would never fire. Replaced with the measured reason.

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-07 19:22:24 +01:00
Gergő Magyar
eba42d2994
docs(bench): correct the edit-loop numbers and add the FTS per-index breakdown (#3208)
The numbers this file shipped with were full rebuilds labelled as incremental.
Rebuilding or re-copying `dist` between runs changes the analyzer runner
identity, and the tool then forces a full rebuild — every measurement taken
that way is a full run wearing an incremental label. That is the third
"silently fall back to full work" guard in this pipeline, after the non-git
corpus and the escalation gate, so the method section now says to read the
banner on every run.

Corrected, measured on a leaf file with a stable runner identity so the
incremental path is genuinely taken: 31.7s, not 36.5s. The graph write is a
3,980-node subgraph rather than the full 51,288, and parse is ~9% of the loop.

Adds the per-index FTS breakdown, which is the actionable finding: 10.2s across
20 indexes, of which File.file_fts alone is 3.5s because File nodes carry file
content and that index re-tokenizes ~30MB of source to reflect one changed row.
Also records that a two-importer leaf edit rebuilt 20 indexes while another run
rebuilt 8 — `touchedFts` narrowing is at least inconsistent, and it has a
withdrawal path when the index catalog cannot be read. That needs pinning
before anyone optimizes against the narrowed set.

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-07 17:35:31 +01:00
Gergő Magyar
f48bf81256
perf(parse): tighten the dispatch-round memory bound and unclamp the worker-pool override (#3200)
* docs(parse): record why dispatchGroups is a required interface member

Review finding #10 argued dispatchGroups should be optional to match
`getQuarantinedPaths?` / `getStats?`. Those are compatibility accommodation
for WorkerPool shapes that predate them, not a convention for new members;
optional here would force a `?.` plus an unreachable fallback at the single
production call site. Documenting the decision so the next reader does not
re-litigate it from the neighbouring optional markers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit addaab647377f3c4553f752fa3ca1388bcb9ca81)

* refactor(parse): simplify round accounting and dispatch setup

Simplification pass over the dispatch-rounds change. Behavior preserved:
identical graph on a full analyze (51,286 nodes / 163,092 edges).

- Drop `roundMissBytes`. `roundBufferedBytes` counts the same bytes plus the
  cache hits, so it is always the greater of the two and the first disjunct of
  the close condition could never fire on its own. One counter, one reset, one
  check.
- Measure round bytes with `Buffer.byteLength(content, 'utf8')` instead of
  `String.length`. UTF-16 code units undercount non-ASCII source by up to 3x,
  so the cap meant to bound main-thread retention was letting a CJK-heavy repo
  hold well past its nominal budget. Matches `estimateItemBytes` in the pool.
- Reset the durable ParsedFile directories for a round's chunks concurrently.
  Each targets its own chunk-hash directory, and running them serially put N
  round trips of fs work on the critical path the round exists to shorten.
  The try/catch stays inside the mapped callback, so one failure still
  degrades that chunk alone.
- Skip the quarantine filter entirely when nothing is quarantined, which is
  every run without a worker death. It was an identity copy of every group.
- `dispatchChunkParseRound` takes `DispatchGroup<...>` rather than re-declaring
  that shape inline; the type was already imported and used in its body.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 527d5b6e0ca8ae7bbc6a414c5ac7e27fd85e9995)

* refactor(parse): count round misses with the same idiom startRound uses

`drainRound` hand-rolled a reduce to count 'miss' entries while `startRound`,
one function above, filters the same predicate over the same union. Same
integer, one idiom.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 7eaa193b0cb5fa515844f36ae1401d6fb2fed7b8)

* fix(parse): honor GITNEXUS_WORKER_POOL_SIZE above the auto sizing cap

The auto pool size is bounded by source bytes so a tiny repo does not spawn a
full idle pool. That bound was also clamping the operator's env override,
because the env value is read inside `resolveAutoPoolSize()` and the result
went through `Math.min(..., workProportionalCap)`.

`DEFAULT_POOL_SIZE_CAP`'s own comment offers `GITNEXUS_WORKER_POOL_SIZE` and
`--workers <N>` as equivalent escape hatches for operators on bigger machines.
They were not. Measured on a 30MB corpus, where the byte-derived cap is 16:

  --workers 24                  -> pool: 24/24 active
  GITNEXUS_WORKER_POOL_SIZE=24  -> pool: 16/16 active   (silently ignored)

Both are deliberate operator input, so both now bypass the work-proportional
cap, which goes back to bounding only the auto default. After the fix, on the
same corpus, with identical graph output (51,286 nodes / 163,092 edges):

  GITNEXUS_WORKER_POOL_SIZE=24  -> pool: 24/24 active
  GITNEXUS_WORKER_POOL_SIZE=4   -> pool: 4/4 active
  unset                         -> pool: 16/16 active

Verified by hand against the pool's own throughput log; not covered by an
automated regression test, since the pool size is only observable through
that log line and not through the progress stream a test can read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 17ed08608c878079b2927da25cfd39c1608a02a2)

* fix(parse): bound the durable-reset fan-out and pin the pool-size override

Review follow-ups on #3200.

The round's durable ParsedFile directory resets went out as one unbounded
`Promise.all` — one recursive rm + mkdir per miss chunk, all at once. A round
can hold hundreds of small packs, and those resets compete for descriptors with
the chunk prefetch this loop already has in flight. `readFileContents` degrades
a losing read SILENTLY by documented contract, so a dropped file would vanish
from the chunk, from the graph, and from the chunk hash — shipping a narrowed
index with exit 0. Now routed through `mapConcurrent` at the same width the file
reads use, which keeps the pipelining win and caps in-flight descriptors.

An operator's pool size is now also bounded by the number of files there are to
parse, so `GITNEXUS_WORKER_POOL_SIZE=100000` on a five-file repo cannot become
the literal thread count. This applies to `--workers` and the env var alike, so
the parity the previous commit established is intact. It does NOT shrink an
incremental re-analyze: `totalParseable` counts every parseable file in the
scan, not the changed ones.

Adds the regression test a reviewer asked for. The existing coverage
(`worker-pool-resilience` calling `resolveAutoPoolSize` directly,
`analyze-worker-pool-size` mocking `runFullAnalysis`) never reaches
`runChunkedParseAndResolve`'s `effectivePoolSize`, so both stayed green through
a revert of the fix. The new test drives the real parse phase with a worker
double that writes a per-`threadId` marker, and counts them: verified it fails
on the reverted line with `expected [ 'worker-1' ] to have a length of 3 but
got 1`, and passes on HEAD.

Also corrects the `GITNEXUS_PARSE_ROUND_BYTES` docstring, which still described
the cache-miss counter deleted two commits ago.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(parse): skip caching a chunk with a stale durable generation; warn on over-subscription

Closes the two findings left open by the review of #3200.

When `prepareDurableParsedFileChunk` fails, the previous generation's shards
are still on disk, so a later warm hit would union them with the new ones. The
chunk is now recorded and its parse-cache write skipped -- the same posture
`finalizeWorkerChunk` already takes for a quarantined chunk, and for the same
reason: do not cache what we cannot vouch for. The next run re-dispatches into
a directory it can actually clear. Bounding the reset fan-out removed the
correlated trigger; this closes the individual case.

Pool size over-subscription now warns rather than caps. Silently capping is
precisely what the override exists to prevent, so an operator's number is still
honored -- but an exported GITNEXUS_WORKER_POOL_SIZE applies to every analyze
in a long-lived caller (watch auto-sync, the MCP server), including small
incremental ones, and that is easy to set once and forget. The warning names
the host's usable core count, so it is a hardware fact rather than an invented
threshold. `resolveHostParallelism` is extracted from `resolveAutoPoolSize`
rather than re-deriving the cgroup-aware fallback at the new call site.

Tests: the stale-generation skip is pinned by a new case asserting nothing is
written under any key; verified it fails without the guard with
`expected 1 to be +0`. 60 unit and 49 integration tests pass across the
affected suites.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(parse): guard dispatch-round cadence with a bench, not a wall-clock budget

Round boundaries are deliberately invisible to graph output — batching that
changed output would be a bug — so nothing in the repo could see the #3196 win
regress. It would have come back as a silent ~1.5x on every cold analyze. Two
earlier attempts to pin it as a unit test failed for that exact reason: one
scraped a logger line the progress stream does not carry, the other asserted
graph content that is identical either way.

Extracts the round-close fold into `createRoundBudget`, so the decision is a
shared unit the bench measures rather than a copy that drifts. The parse loop
is streaming and cannot know chunk sizes up front, so an accumulator is the
honest shape — not a planner.

Four deterministic arms, one ratio, no millisecond gate:
- layout_fingerprint — pack membership. Every cache key derives from it, so
  drift needs a SCHEMA_BUMP, never a lone re-baseline.
- packs / single_file_packs — the FLOOR. `rounds` only asserts something while
  the corpus over-splits (774 packs where the byte budget needs 5). This is
  bench/import-target's lesson, where four heap arms read 0 B and passed every
  ceiling: a ceiling says "not too big", nothing said "still measuring".
- rounds — the regression signal, both directions.
- cjk_rounds vs ascii_rounds — pins UTF-8 byte accounting. The two corpora
  share a UTF-16 length and differ only in encoded size, so String.length
  collapses them to equal. This is the arm no unit test could be.
- pack_scaling_ratio — (t_4n/t_n)/4, min-of-15. A ratio because wall-clock is
  runner-speed-dependent and this repo has the scar: callable-value-flow's ms
  gate failed twice at 2.07 and 1.975 against 1.9 with correct code, on a
  sub-11ms measurement.

Every arm verified to fail before being recorded: close-every-chunk reads 774
rounds, disabling the close reads 1, reverting roundFileBytes to String.length
takes cjk_rounds 8 -> 3, and shrinking the corpus trips the shape floor.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(bench): record the analyze phase breakdown and the rejected optimizations

Where analyze time actually goes, measured while landing #3194/#3196/#3200,
plus the two optimizations that looked compelling and were measured away.

The headline is that the parse work is done: a one-file-edit re-analyze is
36.5s, of which parse is 2.8s (8%). scopeResolution is 40% and the unlogged
graph emit + FTS rebuild is 49% — neither is incremental, and the ~18s sits
outside the phase runner so every phase log is blind to it.

Also records the trap that invalidated an earlier measurement: a non-git
corpus never records a schema fingerprint, so every run is a forced rebuild
and any "warm" number taken that way is fiction.

Rejected, with numbers: more workers (16/20/24 land inside run-to-run spread)
and bundling the worker entry (~250ms on a normal filesystem; the 8.6s that
motivated it was a 9p-mount artifact).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-07 13:53:31 +01:00
Parafee41
1c1cbf111e
fix(zig): resolve cross-file static gates (#3185)
Some checks failed
CodeQL / Analyze (javascript-typescript) (push) Has been cancelled
CodeQL / Analyze (python) (push) Has been cancelled
Gitleaks / gitleaks (push) Has been cancelled
Publish / Classify release event (push) Has been cancelled
Scorecard / Scorecard analysis (push) Has been cancelled
Trivy Image Scan / Trivy (gitnexus-cli) (push) Has been cancelled
Trivy Image Scan / Trivy (gitnexus-web) (push) Has been cancelled
Publish / Build & Push RC Docker images (push) Has been cancelled
Publish / RC guard (marker + release-PR skip) (push) Has been cancelled
Publish / ci (push) Has been cancelled
Publish / Publish to npm (push) Has been cancelled
* fix(zig): resolve cross-file static gates

* Fix Zig workspace import alias enrichment

* Handle extensionless Zig workspace imports

* Reuse Zig import resolution for static gates

* Document Zig workspace static gating

* Harden Zig workspace reference enrichment

* Benchmark Zig cross-file static gating

* Enforce linear Zig benchmark scaling

---------

Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-09-07 07:19:30 +01:00
Gergő Magyar
8f006bd759
perf(parse): batch cache packs into one dispatch round (#3196)
* perf(parse): batch cache packs into one dispatch round

`WorkerPool.dispatch` is a barrier, so dispatching one parse-cache pack at a
time leaves most slots idle for every round-trip. Packs are keyed by
`(language, hash(path) % 128)`, so the byte budget rarely binds: this repo
produces 1285 packs where the budget alone needs 16, and 549 of those hold a
single file. In a real analyze, 76 of 221 dispatched chunks carried one file
and cost 15.3s — 20% of the parse phase for 3.4% of the files.

Chunks now accumulate into a round bounded by `GITNEXUS_PARSE_ROUND_BYTES` of
cache-missing source (default: the chunk byte budget) and go out through a new
`WorkerPool.dispatchGroups`. Jobs are still cut at pack boundaries, so each job
carries exactly one `chunkHash` and every result stays attributable to the pack
whose cache key owns it. Cache hits ride along as round entries, and rounds
drain in `chunkIdx` order, so deferred aggregation stays deterministic.

Cold `analyze --index-only` on this repo (2234 parseable files, 16 workers):
110.3s -> 70.5s total, parse phase 74.0s -> 40.5s, 221 dispatches -> 15.
Graph output is unchanged: 51,286 nodes / 163,092 edges / 2106 clusters /
759 flows in both arms. Peak main-thread RSS 3372MB -> 3487MB (+3.4%).

`dispatchGroups` also claims the pool synchronously and rejects a concurrent
call. Two overlapping dispatches hand the same slots out twice and both stall;
the first version of this change did exactly that, and the only symptom was
every worker idle-timing out ~10s later with no indication of the cause.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(parse): bound what an open round holds, not just what it dispatches

Follow-up to the review of #3196. Three reviewers independently found the
same defect: `roundMissBytes` was the only in-loop close condition, but cache
HITS were queued into the same round without contributing to it. A warm
re-analyze misses nothing, so no round ever closed and every chunk's source
plus its cached worker output stayed resident until the tail drain — the
#2649 heap failure shape on a large repo.

- Hit entries now carry a file COUNT, not the file array, so a replayed chunk
  never pins its source text. `applyChunkResults` only ever read `.length`.
- Track `roundBufferedBytes` across hits and misses and close on either cap.
  Verified on a warm run: with the cap, draining starts as soon as 2MB is
  buffered; without it all 221 merges land in the final 10% of the phase.
- Warm progress no longer freezes at the phase floor. `filesParsedSoFar` only
  advances at drain, so a new `queuedFilesSoFar` feeds the progress events
  while `filesParsedSoFar` stays the merge-accurate throughput number.
- A throw from `drainRound` used to unwind straight to `terminate()` while the
  next round's workers were still busy — the #2432 mid-N-API abort hazard.
  Settle the in-flight round first, then propagate.
- `dispatchGroups` returns one array per group; assert that length instead of
  `?? []`, which turned a contract break into a silently empty chunk.
- Collapse `PendingWorkerChunk` into the `miss` RoundEntry it duplicated.
- Repair two stale doc comments: `dispatch`'s JSDoc had been orphaned onto
  `dispatchGroups`, and `dispatchChunkParse` still described chunk overlap
  that now lives in parse-impl's round machinery.
- New test: a round mixing a cache hit and a cache miss. `drainRound` walks
  entries in chunkIdx order but pulls results on a separate cursor, and no
  existing test put both kinds in one round with content assertions.

Cold analyze unchanged: 71.3s, 15 rounds, 51,286 nodes / 163,092 edges.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 18:27:09 +01:00
Gergő Magyar
780cac7885
fix(parse): keep stable cache packs parallel (#3194)
Stable cache packs introduced by 9718e1247 often fit one worker job,
leaving most workers idle. Size jobs against live pool capacity and
reuse extraction queries per native grammar instead of recompiling
on every pack. Preserve recovery readiness across dispatches and
bound failed-thread termination acknowledgment.

Bisect confirms the scheduling regression. Controlled parsing of 951
TypeScript files falls from 56.39s to 26.66s with identical graph output;
peak RSS increases roughly 10%. Add scheduling and query reuse coverage
and repair recovery fixtures that assumed single-job dispatch.

Validation: build, typecheck, formatting and 229 focused tests pass.
Full suite was interrupted during lengthy native DB testing; full
release CI remains outstanding.

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
2026-09-06 10:37:05 +01:00
Gergő Magyar
a049b2dac6
Merge pull request #2785 from magyargergo/fix/skill-evolution-gate
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
fix(eval): stop discarding completed benchmark sessions as unverifiable
2026-09-05 12:25:20 +01:00
Claude
de3131fed8 fix(eval): require finite gateway startup budgets 2026-09-05 11:03:23 +00:00
Claude
fc61507da5 fix(eval): repair native containment checks 2026-09-05 10:48:39 +00:00
Claude
3598a69188 fix(eval): close CI and remaining review gaps 2026-09-05 10:29:58 +00:00
Gergő Magyar
a957c5d757
Merge branch 'main' into fix/skill-evolution-gate 2026-09-05 11:11:45 +01:00
Gergo Magyar
1054e3e038 fix(eval): make evolution evidence valid and bounded 2026-09-05 10:08:35 +00:00
Gergő Magyar
c5c4fbe43c
fix(zig): vendor tree-sitter-zig so npm i -g no longer warns on peers (#3180)
* fix(zig): vendor tree-sitter-zig so npm i -g no longer warns on peers

Published overrides do not apply to dependents, so the Zig optionalDependency
kept warning that tree-sitter@0.21.1 does not satisfy peerOptional ^0.22.1.
Load it from vendor/ like Dart/Kotlin/Swift instead.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(ci): add zig parse snippet to prebuild validate

The six zig prebuild jobs failed at "Validate the .node loads and parses"
because snippets[GRAMMAR] was undefined and tree-sitter threw
"Input must be a function".

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore: drop Unreleased changelog note from the Zig vendor PR

CHANGELOG.md is owned by the release process, not individual PRs.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33949409205

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33949616521

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33949829377

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33950025077

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33950220655

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33950400275

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33950607933

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33950882912

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33951170483

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33951386477

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33951624305

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33951813452

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33951998309

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33952225172

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33952524393

* fix(ci): stop native prebuild rebuild loops

PR path filters and source checks see the cumulative diff, so generated binaries kept rebuilding the original source change. Skip output-only synchronize events using their exact before/head range, failing closed when Git cannot compare it. Exercise the workflow against real commit histories, including multi-commit source pushes and merge-ref drift.

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33953226606

* fix: address Zig PR review feedback (#3180)

Check for the vendored package without loading its native binding so a
broken installed Zig grammar fails the parsing test instead of skipping.
Match the optional child descriptor in the Zig metadata declaration and
include Zig in the two optional/vendored grammar comments.

Validation: 159 targeted tests, TypeScript, metadata type fixture, and
formatting passed. Injected native-load failure now fails instead of
skipping; explicit Zig opt-out still skips.

* chore(vendor): rebuild native prebuilds (tree-sitter-zig)

Built by https://github.com/abhigyanpatwari/GitNexus/actions/runs/33956342308

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: gitnexus-release-bot[bot] <gitnexus-release-bot[bot]@users.noreply.github.com>
2026-09-05 10:35:33 +01:00
dependabot[bot]
bf4fa2bf99
chore(deps)(deps-dev): bump @types/node in /gitnexus (#3164)
Some checks failed
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Skill copy sync / shipped skills drift guard (push) Has been cancelled
Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 26.3.0 to 26.4.0.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

---
updated-dependencies:
- dependency-name: "@types/node"
  dependency-version: 26.4.0
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-09-05 07:41:50 +01:00
dependabot[bot]
ebde34a899
chore(deps)(deps): bump ignore from 7.0.7 to 7.0.8 in /gitnexus (#3165)
Bumps [ignore](https://github.com/kaelzhang/node-ignore) from 7.0.7 to 7.0.8.
- [Release notes](https://github.com/kaelzhang/node-ignore/releases)
- [Commits](https://github.com/kaelzhang/node-ignore/compare/7.0.7...7.0.8)

---
updated-dependencies:
- dependency-name: ignore
  dependency-version: 7.0.8
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
2026-09-04 22:08:25 +01:00
Gergo Magyar
7fabbb044a fix(eval): stop hiding review patches from sandboxed git apply
The oracle-mask overlay covered the same path review setup reads, so every historical cell died with can't-open-patch. Leave the staged copy visible for apply, then fail closed if it is still there when the model starts.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-04 19:43:47 +00:00
Gergo Magyar
951e272557 Merge branch 'fix/skill-evolution-gate' into pr-2785-feedback 2026-09-04 19:11:43 +00:00
Gergo Magyar
7421b7813f Address PR review feedback (#2785)
Close follow-up holes in host write locks, preview redaction, runtime mounts, and review matching.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-04 19:10:55 +00:00
Gergő Magyar
8f08261d05
Merge branch 'main' into fix/skill-evolution-gate 2026-09-04 20:02:51 +01:00
Gergő Magyar
c0c3fa18a9
chore: release v1.6.11 (#3177)
* chore: release v1.6.11

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(test): read /api/info version from package.json

server-info unit tests hardcoded 1.6.10 while buildServerInfo reads
package.json, so the 1.6.11 bump failed ubuntu coverage 3/3.

Co-authored-by: Cursor <cursoragent@cursor.com>

* refactor: read the published version from one helper

Release bumps kept breaking tests that each re-required package.json.
packageVersion() is now the single read for CLI, MCP, serve, and those tests.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-04 20:02:37 +01:00
Gergo Magyar
ac7ae6a8ce Address PR review feedback (#2785)
Tighten review-evolution scoring, sandbox lock, and gateway cleanup so historical cells score instead of aborting or leaking host state.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-04 18:59:32 +00:00
Gergo Magyar
7c68905aac Merge branch 'main' into pr-2785-feedback 2026-09-04 18:38:07 +00:00
Gergo Magyar
cc2b5df296 Merge branch 'fix/skill-evolution-gate' into pr-2785-feedback 2026-09-04 18:37:48 +00:00
Gergo Magyar
8491cf4203 fix(eval): make historical review evolution score instead of aborting
Seed the current gitnexus-review skill into older PR checkouts, force-add
historically gitignored skill paths, accept plugin-qualified Skill ids,
and lock host-unsafe workspaces to review-output.json so a generation can
finish and score. Sandbox cleanup restores owner write bits before delete
because a session that copytrees the locked clone otherwise leaves 0555
trees that rmtree cannot remove.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-04 18:37:39 +00:00
Gergő Magyar
9bf307123e
feat: notify users when a newer gitnexus version is available (#3175)
* feat(core): add cache-first npm update-check service

Shared fail-open checker: validated 24h cache under GITNEXUS_HOME,
acquireFileLock-guarded refresh, monotonic publication, hardened
registry fetch (no credentials, private-address redirects refused,
body-capped), strict x.y.z comparator, install-eligibility
classification, and an unref'd refresh scheduler for long-lived
processes. Extracts getGlobalDir into storage/global-dir.ts with a
repo-manager re-export (no caller changes).

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(cli): notify on available updates via stderr and doctor

One i18n'd stderr line on interactive invocations when the validated
cache holds a newer version (TTY-gated, CI/opt-out/eligibility-gated,
hook and help/version command identities excluded). Stale cache spawns
a detached hidden __update-check refresh child so command exit latency
is unchanged. doctor prints the cached latest version when known.
Dockerfile.cli sets GITNEXUS_NO_UPDATE_NOTIFIER=1.

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(mcp): emit one stderr update notice per process per version

Process-scoped adapter in mcpCommand (stdio and --http), dynamically
imported after the stdout sentinel, started only after connect, fully
catch-isolated. Arms the shared refresh scheduler with cleanup on
process exit. Protocol payloads stay free of update state (R15).

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(serve): expose update state on /api/info

Serve-scoped controller owns an in-memory update snapshot: one
staleness evaluation after listen, then the shared unref'd scheduler,
stopped on close/shutdown. /api/info reads only the snapshot and gains
optional latestVersion/updateAvailable fields for eligible installs;
the existing three fields are byte-compatible.

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(web): dismissible update-available banner from /api/info

Fetches server info after backend connect and on reconnect, renders a
fixed banner in the exploring view only when updateAvailable is true
and the version is undismissed, hides while the reconnect banner is
active, and fails open on fetch errors. role=status + aria-live with a
keyboard-focusable dismiss; dismissal persists per version in
localStorage. Copy in en/zh-CN common.json with version interpolation.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(cli): document update notifications and opt-outs

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(review): apply review findings and simplify pass

Review: gate the detached refresh spawn on a live lock-owner probe so
parallel CLI invocations coalesce to one refresh child (validated P2,
three-reviewer agreement); poll /api/info on a slow cadence while
exploring so post-load server-side discoveries surface (validated P1);
add a monotonic sequence guard so overlapping server-info fetches
commit in order.

Simplify (behavior-preserving): shared truthy-env/opt-out/freshness
helpers in update-cache.ts, shared cachedUpdateNoticeLine for CLI and
doctor, extracted install-eligibility core with per-process memo,
memoized registry parsing, single evaluation per scheduler cycle,
cache-only startup evaluate in serve, flattened MCP exit handler,
shared bottom-banner shell, storage keys in ui-constants.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(update-notifier): address residual review tickets on this PR

Stop the lock-busy 1ms scheduler spin, replace clock-skewed cache
entries, move the outbound URL guard into core, and extract the serve
update controller. Pin the startup/guard/single-flight/MCP/CLI contracts
those tickets called out.

Fixes #3167 #3168 #3169 #3170 #3171 #3172 #3173 #3174

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address PR review feedback (#3175)

Fetch the npm /latest document instead of the full packument so the 64KiB cap can succeed, and treat reused lock PIDs as stale so refresh is not suppressed.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address PR review feedback (#3175)

Register the CLI spawn suite on the OS matrix, pin MCP opt-out env, and compare versions without IEEE-754 rounding.

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(cli): add gitnexus update install and versioned command banners

Give an explicit Claude/Codex-style upgrade (`npm i -g gitnexus@version`) and print `GitNexus <Name> (version)` on every command so the running build is obvious without silent self-update.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address PR review feedback (#3175)

- Document the pinned install as npm i -g gitnexus@<x.y.z>, not a copyable @version tag
- Wait for wall-clock-future cache repair to publish before asserting
- Restore the stdout spy if the TTY notice assertions fail

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(update-notifier): keep last known latestVersion on a failed refresh

A later offline check was wiping the pin and hiding a known update for 24h.
gitnexus update still treats a failed live fetch as checkFailed.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address PR review feedback (#3175)

- Word update.current so a newer-than-latest install is not called the latest stable version.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(ci): hide the detached update-check spawn on Windows

The refresh child was spawned without windowsHide, so Windows CI could stall
before writing the cache and then fail cleanup with EBUSY.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-04 18:50:03 +01:00
Gergő Magyar
a348bc3957
fix(build): build the web UI from prepack, not from every npm ci (#3166)
* fix(build): build the web UI from prepack, not from every npm ci

gitnexus-web is a separate ~650-package tree (React, Vite, LangChain,
Mermaid). Because `prepare` built it, every `npm ci` in gitnexus/ also
installed and Vite-built a second product. On CI that install ran
uncached inside an execSync timeout, so a healthy-but-slow install was
SIGTERM'd mid-flight and surfaced as `spawnSync /bin/sh ETIMEDOUT` --
repeatedly killing node floor compat, a job that only import-links the
CLI dist and never needs the UI.

The UI is only needed inside the published tarball, so build it from
prepack instead. `npm run build` and `prepare` are now CLI-only; pass
--web (or npm run build:web) to include it. Jobs that pack or publish
install gitnexus-web in their own visible step, and the in-script
fallback install is untimed so a slow install can no longer be killed
halfway and reported as a build failure. The tsc/vite timeout default
goes 300s -> 600s so the remaining bounded steps have headroom.

Default build on this machine: 30s, no gitnexus-web work.

* fix(build): enforce web package artifact integrity

Co-authored-by: Cursor <cursoragent@cursor.com>

* refactor(build): clarify web packaging helpers without changing behavior

Keep the same opt-in, fail-closed, and pack/publish preserve rules while
trimming comments, sharing the test harness, and reading index.html
directly instead of probing it first.

Co-authored-by: Cursor <cursoragent@cursor.com>

* ci: skip prepare on typecheck so a cold shared install cannot cancel the job

quality/typecheck's 10-minute budget was spent on an uncached gitnexus-shared
npm install plus a full prepare tsc that tsc --noEmit does not need.

Co-authored-by: Cursor <cursoragent@cursor.com>

* ci: stop typecheck-web from canceling before the npm cache can save

Hashing gitnexus-shared into the web cache key forced a cold 650-package
install; the 10-minute job then canceled and never wrote a warm cache.

Co-authored-by: Cursor <cursoragent@cursor.com>

* ci: give format the same 10-minute budget as lint

A cold root npm ci already took 4m19s and canceled prettier at the 5-minute
cap. Lint does the same install and needed 7m41s on that run.

Co-authored-by: Cursor <cursoragent@cursor.com>

* ci: stop installing TypeScript 7 just to compile gitnexus-shared

A dedicated npm ci in gitnexus-shared took 7 minutes to add two packages
(TypeScript 7's optional per-platform binaries) and cancelled typecheck,
Windows pack, and coverage shard 1. Compile shared with gitnexus's tsc.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address PR review feedback (#3166)

- Run tsc via execFileSync so the compiler path is never interpolated into a shell.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Address PR review feedback (#3166)

- Run tsc as node typescript/bin/tsc so Windows never has to execFile a .cmd shim.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Launch tsc via node and lib/tsc.js on every OS.

The npm .bin/tsc shim is tsc.cmd on Windows, which execFileSync cannot spawn.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(ci): lock eval containment against a dedicated shared npm ci

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-04 13:28:11 +01:00
Gergo Magyar
9047bf00a5 fix(eval): align review metrics and corpus evidence
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-04 05:38:25 +00:00
Gergo Magyar
6925fb344d feat(eval): evolve review skills against historical PRs
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-04 05:32:31 +00:00
Parafee41
47c2799ba7
fix(analyze): add cold parser rebuild option (#3153) 2026-09-04 05:14:00 +01:00
Gergo Magyar
167642ec9d feat(ci): let a dispatched evolution run start from a blank slate
Seeding is unconditional today, so the next run would inherit the rejected
proposal from a generation whose proposer could still read the hidden
oracles. That taint propagates: each generation stages the previous
proposal, so one contaminated proposal survives until the artifact expires.
Scheduled runs still always seed — memoryless weekly runs would re-propose
the same rejected candidate forever.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-03 20:14:05 +00:00
Gergo Magyar
a2fdf9e93c fix(eval): hide the hidden harness from the proposer; drop inert hooks
The proposer authors the overlay the benchmark arms are scored with, but its
clone was never sanitized: it could read eval/workflow_bench, i.e. the task
prompts and the hidden oracles it was about to be graded against. The last
diagnostic run did exactly that, reading
inv-feature-list-repos-filter.oracle.test.ts directly, so a proposal could
win the gate by encoding expected behavior into a skill instead of being a
better skill. Sanitize the proposer clone exactly as run_cell already does.

Also remove the PreToolUse tool-input normalizer. It never ran: headless
`claude -p` (2.1.247) dispatches no hooks from inline --settings, a settings
file, project/user/local --setting-sources, or a trusted ~/.claude.json
project entry. Keeping it would read as a control in review while enforcing
nothing, and it was the sole reason the proposer stopped using --bare —
which stays off on its own merits, since bare ignores --tools and would cost
the proposer Grep and Glob.

Blank optional arguments from the OpenAI adapter remain handled where the
code is ours: MCP aliases in local-backend normalizeToolParams. Built-in
Read still rejects pages:"" and the model self-corrects on the next turn.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-03 20:03:09 +00:00
Gergo Magyar
aea20ccf72 fix(workflow-bench): keep proposer hooks and JSONL evidence intact
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-03 19:44:58 +00:00
Gergo Magyar
bc7f4a907e Merge branch 'main' into pr-2785-feedback 2026-09-03 19:01:52 +00:00
Gergo Magyar
d541105340 fix(workflow-bench): repair the harness defects the verbose proposer logs exposed
The first fully-logged skill-evolution run failed for five deterministic
reasons that had nothing to do with the candidate under test. Each is fixed
at the layer that actually owns the contract:

- Strict provider adapters materialize omitted optional string arguments as
  "". The MCP alias normalizer now treats a blank optional alias as absent
  (a blank REQUIRED target is still rejected), and a trusted PreToolUse hook
  strips blank strings before Read/GitNexus tool calls.
- MCP semantic errors rode home in a successful envelope and logged as
  result=ok. SessionProgress now inspects the payload and reports them as
  semantic-error.
- Claude Code's nested sandbox overlays absent root dotfiles with device
  nodes, which the provenance snapshot read as unauthorized workspace
  changes. Those names are excluded at the workspace root and hidden from
  git via an immutable excludes file.
- The proposer could not read /evidence from Bash (missing allowRead entry)
  and had no offline gitnexus runner, so it fell back to npx and hit the
  network. Both are now mounted; ripgrep is installed in CI.
- selected-rows.json advertised host artifact names that do not exist in the
  mount. Rows now name their staged patch_file/transcript_files, the prompt
  describes the real layout, and oversized bundles compact artifacts before
  dropping evidence rows so no row is silently lost.

Also replaces two benchmark scenarios that main already satisfies
(trivial-version-alias, inv-bug-pdg-note) with non-vacuous ones, verified to
fail against a pristine checkout.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-09-03 19:01:45 +00:00