Skip to content

fix(runtime): stage workspace handoff without exclude pathspecs - #282

Merged
aviggiano merged 5 commits into
mainfrom
fix/workspace-handoff-ignored-runtime-roots
Aug 6, 2026
Merged

aviggiano merged 5 commits into
mainfrom
fix/workspace-handoff-ignored-runtime-roots

Conversation

@aviggiano

@aviggiano aviggiano commented Aug 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The failure

Found live in Aave v4 run R44 (aave-v4-v0012-main-issue272-r44-invariant-only, main 77da78a). Node property-specification-a16z failed at 2026-08-06T14:14Z:

Command failed: git add -A -- . :(exclude).ultrafuzz/** :(exclude).smithers/** :(exclude)node_modules/** :(exclude)artifacts/**
The following paths are ignored by one of your .gitignore files:
node_modules
hint: Use -f if you really want to add them.

Why the obvious fix is wrong

My first attempt added :(exclude)node_modules alongside :(exclude)node_modules/**, on the theory that the glob covered the contents but not the directory entry. The test stayed red. So I measured git's actual behaviour instead of reasoning further:

invocation result
git add -A -- . OK — ignored paths skipped silently
git add -A -- . :(exclude)<root>/** FAIL
git add -A -- . :(exclude)<root> :(exclude)<root>/** FAIL
git add -A -- . :(exclude)<root> FAIL
git add -A --ignore-errors -- . :(exclude)<root> FAIL

Any negative pathspec makes git add report an ignored path as an error rather than skipping it. So the exclusions themselves had to go.

The fix

Stage plainly with git add -A -- ., then git reset --quiet <treeish> -- <runtime roots> so those roots contribute nothing to the captured tree without ever being named to add.

Two details that matter:

  • Reset to the tree-ish, not rm --cached. If a file under one of those roots is tracked, resetting restores its baseline content; dropping the index entry would record a deletion and silently corrupt the captured tree.
  • The target is threaded through, not assumed to be HEAD. captureWorkspaceTree reads HEAD but captureWorkspacePatch reads the caller's baselineTree; resetting to HEAD in the latter would produce a patch against the wrong base.

Impact

The node fails visibly, so it is retryable — less severe than #275's invisible prepare:-wrapper strand. But property-specification-fanin depends on all eight lens nodes, so a lens that fails deterministically in its own worktree blocks the invariant pipeline from reaching fan-in, invariant setup, Recon, findings or report.md.

It reproduces only when that node's agent actually installed dependencies in its worktree, which is why most nodes survive and this went unnoticed until R44.

Validation

  • new regression proven red without the source change and green with it; it plants all four runtime roots (node_modules ignored, the others not) plus a real setup change, then asserts the change is captured and no runtime root leaks into the patch
  • pnpm --filter @ultrafuzz/runtime test — 278 pass / 0 fail
  • typecheck, eslint, prettier, git diff --check — clean

Relationship to #254

Draft PR #254 proposes the same direction ("stage from an explicit Git path list instead of broad add pathspecs") bundled with eight unrelated changes. This is the narrow fix for the observed abort only; #254 can be rebased or closed on top of it.

Greptile Summary

The PR replaces exclusion-based workspace staging with byte-exact enumeration of tracked and non-ignored paths, omitting runtime roots before staging.

  • Uses NUL-delimited literal pathspecs to preserve unusual filenames.
  • Retries staging when an enumerated path disappears.
  • Adds regression coverage for ignored and unreadable roots, tracked deletions, nested repositories, unusual filenames, and baseline preservation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/runtime/src/workspace-handoff.ts Reworks workspace staging around byte-exact path enumeration, runtime-root filtering, and bounded retries for disappearing paths.
packages/runtime/test/workspace-handoff.test.ts Adds focused regression tests covering ignored roots, concurrency, tracked-path semantics, nested repositories, and unusual filenames.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Load baseline tree into temporary index] --> B[Enumerate tracked and non-ignored paths]
  B --> C[Exclude runtime roots and directory entries]
  C --> D{Any paths remain?}
  D -- No --> E[Write captured tree]
  D -- Yes --> F[Stage NUL-delimited literal paths]
  F --> G{Path vanished?}
  G -- Yes, attempts remain --> B
  G -- No --> E
  G -- Other failure --> H[Abort capture]
Loading

Reviews (5): Last reviewed commit: "fix(runtime): retry staging when a liste..." | Re-trigger Greptile

Context used:

aviggiano and others added 2 commits August 6, 2026 14:26
Workspace capture aborted whenever a gitignored runtime root existed in the
worktree. R44's `property-specification-a16z` node died on it:

  Command failed: git add -A -- . :(exclude).ultrafuzz/** :(exclude).smithers/**
    :(exclude)node_modules/** :(exclude)artifacts/**
  The following paths are ignored by one of your .gitignore files:
  node_modules

The exclusions were the cause, not an incomplete form of it. Measured against
git directly: a plain `git add -A -- .` skips ignored paths quietly, while every
exclude variant fails — `:(exclude)<root>/**`, `:(exclude)<root>`, both together,
and even with `--ignore-errors`. Any negative pathspec makes `git add` report an
ignored path as an error instead of skipping it.

Stage plainly and then reset the runtime roots back to the tree that was read in,
so those roots contribute nothing to the captured tree without ever being named
to `add`. Resetting to the tree-ish rather than dropping the index entries keeps a
tracked file under one of those roots at its baseline content instead of
recording a deletion, and the two callers read different trees, so the target is
threaded through rather than assumed to be HEAD.

Reproduces only when the node's agent installed dependencies in its own worktree,
which is why most nodes survive and this went unnoticed.

Fixes #281

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review found the first commit traded #281's abort for the same failure class, and
that a `core.excludesFile` variant did not fix it either.

The old exclude pathspecs did two jobs, and the first commit only accounted for
one. `:(exclude)artifacts/**` kept entries out of the index AND kept `git add`
from descending into the directory. A bare `git add -A -- .` avoids the
ignored-path error but walks the runtime roots, so a transient ENOENT or an
unreadable file under `artifacts/` aborts capture with exit 128 — the runtime
writes `artifacts/<attemptId>/` and `.ultrafuzz/schemas/` into the workspace while
agent subprocesses are still running, and a target repo has no reason to gitignore
them. Measured on a 157 MB non-ignored tree: 8.62s and +158 MB of unreachable
objects per capture, and capture runs once per dependency plus twice per patch
application.

`core.excludesFile` is the lowest-precedence ignore source, so a repository
`.gitignore` negation such as `!artifacts/` re-admits the path, and the unreadable
file still aborted. Verified, not assumed.

Name only the other top-level entries as positive pathspecs instead. No negative
pathspec means no ignored-path error, and never naming the roots means never
descending into them. The entry list is the union of worktree and tree entries so
deleting a tracked top-level path is still captured, and the empty case is skipped
because `git add -A --` with no pathspec would stage everything.

Regressions added for the three cases that broke the earlier attempts: an
unreadable file under a non-ignored runtime root, a repository that un-ignores the
runtime roots, and deletion of a tracked top-level path.

Refs #281

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

Copy link
Copy Markdown
Collaborator Author

Redesigned after adversarial review — 793757d

Two independent reviews. One recommended merge with a P2; the other found a P1 that invalidated the approach, and separately disproved the core.excludesFile follow-up I had started. Both were right, and the second attempt was wrong for a different reason than the first.

What the first commit missed

The old :(exclude) pathspecs did two jobs. :(exclude)artifacts/** kept entries out of the index and kept git add from descending into the directory. 5f946b1 only accounted for the first.

So a bare git add -A -- . avoids the ignored-path error but walks the runtime roots. Measured by review on a 157 MB non-ignored artifacts/ tree:

wall .git growth
git add -A -- . 8.62 s +158 MB unreachable objects
:(exclude)artifacts/** 0.00 s 0

And worse than the cost, it reintroduces the abort:

# unreadable file under a non-ignored runtime root
error: unable to index file 'artifacts/att/x.json'
fatal: adding files failed        exit=128

That matters because the runtime writes artifacts/<attemptId>/ (workflow.tsx:248) and .ultrafuzz/schemas/ (workflow.tsx:371) into the workspace, and a target repo like Aave v4 has no reason to gitignore either. Capture runs once per dependency (workflow.tsx:470) plus twice inside applyWorkspacePatch, so the cost is O(N) per fan-in node.

Why core.excludesFile also failed

It is the lowest-precedence ignore source. A repository .gitignore containing !artifacts/ re-admits the path, and the unreadable-file case still aborted. Verified in a scratch repo rather than assumed.

The design now

Name only the other top-level entries as positive pathspecs. No negative pathspec means no ignored-path error; never naming the roots means never descending into them. The entry list is the union of worktree entries and git ls-tree entries so deleting a tracked top-level path is still captured, and the empty case is skipped because git add -A -- with no pathspec stages everything.

This is what #254 proposed ("stage from an explicit Git path list instead of broad add pathspecs"); both reviewers converged on it independently.

Verified against the cases that broke the earlier attempts

PASS  ignored node_modules does not abort            (the original #281 repro)
PASS  unreadable file under artifacts does not abort (broke commit 1)
PASS  gitignore negation of artifacts still excluded (broke core.excludesFile)
PASS  deleted tracked top-level path captured
PASS  only runtime roots present does not stage them (empty pathspec guard)
PASS  tracked artifacts file restored to baseline

Three are now pinned as regression tests; the unreadable-file one skips as root, which can read a 0000-mode file regardless.

Also fixed from review

  • My test comment stated the cause incorrectly — it claimed /** excluded contents but not the directory entry. Disproved: :(exclude)<root>, :!<root> and the /** form all fail identically. Corrected in the test and the source doc comment, because a maintainer trusting it could have "fixed" it by dropping /** and reintroduced Workspace handoff staging aborts when an ignored runtime root exists: exclude pathspec misses the directory entry #281.
  • Added the missing tracked-runtime-root test. Swapping the reset for git rm --cached would have passed every other test while silently converting tracked files into spurious deletions in result_tree.
  • Replaced a brittle regex assertion with a git ls-tree check on the captured tree.

Review also independently confirmed the risk I was most worried about is a non-issue: the HEAD vs baselineTree reset skew is byte-for-byte unchanged by this PR, with matching tree SHAs on both implementations.

Validation

pnpm --filter @ultrafuzz/runtime test — 282 pass / 0 fail. typecheck, eslint, prettier, git diff --check clean.

aviggiano and others added 3 commits August 6, 2026 14:56
Third design for this fix; the previous two were each rejected by review after
being measured, and this one addresses what both reviewers found.

Naming top-level entry names as positive pathspecs aborts `git add` on ANY
ignored top-level entry, not just an ignored runtime root. Foundry's own
.gitignore lists `cache/` and `out/`, which `forge build` creates, so that shape
would have failed on essentially every real target — a wider blast radius than
the bug it fixed. Measured against the same fixture: main exits 0, the previous
commit exits 1.

Name the exact paths git itself reports instead. `--exclude-standard` drops
ignored untracked paths so none is ever named; `--cached` keeps tracked paths,
including ones deleted from the worktree, so deletions are still captured;
filtering on the first path segment means the runtime roots are never named and
therefore never descended into, which is what the original exclude pathspecs
were really buying.

`--force` is needed and is safe because of `--exclude-standard`: every ignored
untracked path is already gone from the list, so the only names passed are
tracked paths or stageable untracked paths, each an exact file rather than a
directory. Without it, naming a tracked file whose parent directory is ignored
still trips the ignored-path error.

Paths stay as bytes: a non-UTF-8 filename decoded through a string returns with
replacement characters and matches nothing, aborting capture. They are fed over
stdin so a wide repository cannot hit E2BIG, and `--literal-pathspecs` stops a
name containing glob or `:` magic from being reinterpreted.

The previous commit's own test passed only because its fixture never created the
ignored `cache/` directory it declared. It now creates `cache/` and `out/` and
asserts neither is captured, so the regression cannot come back silently.

Refs #281

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review findings on the previous commit, both measured.

`ls-files` was enumerating the entire contents of every runtime root before the
JS filter dropped them. The runtime creates those roots inside a target repo
whose .gitignore does not mention them, so that measured 17 MB of path text over
170k entries against a 32 MB maxBuffer — a growing `.smithers/` would eventually
abort capture with an opaque `spawnSync git ENOBUFS`, the same failure class this
PR exists to remove. Unlike `git add`, `ls-files` tolerates negative pathspecs
without erroring on ignored paths and prunes the traversal, so the roots are now
excluded there: the same call drops to 2k entries and is roughly 800x faster.
`--literal-pathspecs` is deliberately omitted from that call so the exclude
pathspecs are honoured; they are ours, not caller-supplied, and the `add` keeps
it.

`ls-files --others` reports an untracked nested repository as a directory, which
falsified the doc comment's claim that every name passed to `add` is a file — the
claim `--force` safety was resting on. With no commit checked out `git add` fails
hard, which an interrupted `forge install` or clone can produce. Directory
entries are now skipped, which makes the invariant true by construction; the
gitlink that would otherwise be recorded points at an object living only in the
nested repository, so it was unresolvable downstream anyway. The `--force`
rationale is restated in terms of what `--exclude-standard` enumerates.

Regressions added for the untracked nested repository, for filenames that look
like pathspec magic (pins `--literal-pathspecs` on the `add`), and for a
non-UTF-8 filename (pins the byte handling).

Refs #281

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Remaining adversarial review findings.

A listed path can disappear before it is staged, because agent subprocesses are
still running while capture happens. `git add --pathspec-from-file` fails the
entire invocation when any name matches nothing, and `--ignore-errors` does not
suppress it — so the previous shape introduced a brand new abort in code whose
whole purpose is to stop aborting mid-run. Review reproduced it deterministically
by removing a listed file between the two git calls. Capture now re-lists and
retries a bounded number of times, and a test drives it through a git shim that
deletes a listed path on the first `add`.

The trailing `reset` of the runtime roots is removed as dead code. Review proved
it by replacing the line with `void treeish` and watching every test still pass,
including the one whose comment claimed to guard exactly that substitution.
Nothing here ever names a root path, so `read-tree <treeish>` already leaves their
index entries at `treeish` by construction. That test's comment now states what it
actually pins rather than advertising a guarantee it never provided.

Also verified: the directory-entry skip landed in the previous commit covers an
untracked nested repository *with* a commit as well as without one. Without it git
records a gitlink pointing at an object that exists only inside the nested
repository, which `assertPatchPathsMatchManifest` later refuses — a captured
baseline that could never produce an applicable patch.

Refs #281

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@aviggiano
aviggiano merged commit ec8594e into main Aug 6, 2026
9 checks passed
@aviggiano
aviggiano deleted the fix/workspace-handoff-ignored-runtime-roots branch August 6, 2026 15:33
aviggiano added a commit that referenced this pull request Aug 7, 2026
Integrates origin/main (fa8be64): PR #287's invariant-suite handoff deletion
channel and durable provenance, plus #282, #284, #290, #294, #296, #298, #300,
#303, #305 and #308. CHANGELOG keeps both Unreleased sections; the modal runner
test import list keeps both new symbols.

Fixes the review finding on #165 (issue #212). Compiled task specs can only name
singular `test/...` output roots, because they are built before the worktree
exists. Targets are not consistent: the Aave v4 target at 6959e321 has no `test/`
at all, only plural `tests/`. The generated workflow created the singular tree
regardless, so such a repository grew empty `test/recon`, `test/chimera` and
`test/invariants` directories, which setup.md tells the model to read as the
repository's convention, and retry cleanup then scrubbed that invented tree while
leaving the real `tests/` output behind.

Workspace output roots are now re-anchored at execution time onto the test root
the repository actually uses, restoring the semantics of main's `invariantTestRoots`
helper that this branch dropped in merge 2bbecba, at the layer this branch's retry
preparation uses.

Three review findings on the first cut of that fix are addressed here:

- Exactly ONE root is selected, never the union of those present. Anchoring under
  every existing root would have recreated the same defect from the other side: once
  an ancestor stage introduces the other root, each invariant stage would create an
  empty duplicate tree beside the real one. Preference follows
  INVARIANT_TEST_ROOT_NAMES, so a repository that has `test/` behaves exactly as it
  did before this helper existed and only a repository without it is redirected.
- The resolution is pinned per attempt at preparation time. Rediscovering per call was
  unsafe: prepareTaskWorkspaceOutputRoots finishes by replaying ancestor workspace
  patches, and an ancestor stage can introduce the other test root as a side effect, so
  a second independent discovery could select a root whose subdirectories were never
  created. taskWorkspaceOutputRoots would then lstat a missing path and throw a bare
  ENOENT instead of the typed artifact-contract failure, killing a retries:0 node on
  exactly the restart and resume paths this machinery exists to survive.
- Only ENOENT and ENOTDIR mean "this root is absent". Swallowing EACCES or ELOOP would
  silently fall back to the singular default against a root that does exist.

Test changes, also from review:

- The non-directory case was vacuous: with only a regular file named `test` present,
  "rejected it" and "fell back to the default" both read `test`. It now runs against a
  repository that also has a real `tests/`, so the outputs differ; verified by mutation
  (dropping the isDirectory() guard fails the test).
- taskWorkspaceOutputRoots' use of the helper is now pinned in the generated-source
  assertions. Reverting only that call site previously left every test green while
  every task on a `tests/`-only target died on a bare ENOENT.
- Adds coverage for the pinning behaviour, for per-attempt isolation of the pin, and
  for surfacing an unreadable candidate root.

Also repairs a merge-integration gap the new #217 harness could not have anticipated:
it lifts resetTaskArtifactsForRetry, which on this branch calls prepareAnchoredDirectory
(branch-only, absent from main), so the harness now supplies that collaborator and drops
the dead invariantTestRoots stub.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workspace handoff staging aborts when an ignored runtime root exists: exclude pathspec misses the directory entry

1 participant