Skip to content

fix(runtime): bound the invariant-discovery git enumerations - #334

Merged
aviggiano merged 1 commit into
mainfrom
fix/ls-files-maxbuffer
Aug 7, 2026
Merged

aviggiano merged 1 commit into
mainfrom
fix/ls-files-maxbuffer

Conversation

@aviggiano

@aviggiano aviggiano commented Aug 7, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #323.

Five call sites — not the three originally filed — ran git ls-files through a bare execFileSync with no maxBuffer, so Node's 1 MB default applied. changedTestTreePaths and changedInvariantSourcePaths were missed by the original audit.

All now route through one bounded helper that rethrows ENOBUFS as a named artifact-contract failure: naming the subcommand, the bound, the bytes captured, and the largest contributing roots — following #311's pattern. #311's own helper could not be reused: it reads attribution from diff --git headers and is not re-exported from the runtime package index, which is the only surface a generated workflow can import.

Not the cause of #319 — that link was retracted.

Greptile Summary

The PR routes five invariant-discovery Git enumerations through a shared 16 MiB bounded capture helper and replaces anonymous ENOBUFS failures with actionable artifact-contract diagnostics.

  • Adds bounded Git path enumeration and contributor attribution for oversized output.
  • Preserves ordinary Git failures while stripping large captured payloads from overflow causes.
  • Adds regression coverage for overflow diagnostics, contributor ranking, cause durability, and listings beyond Node’s former default buffer.

Confidence Score: 5/5

The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issue identified.

The shared helper preserves successful enumeration behavior, explicitly bounds captured output, retains non-overflow Git failures, and has targeted coverage for the newly introduced overflow path.

Important Files Changed

Filename Overview
packages/runtime/src/templates/smithers/workflows/workflow.tsx Introduces the shared bounded Git enumeration helper, actionable ENOBUFS handling, and routes all five invariant-discovery captures through it without an accepted defect.
packages/runtime/test/generated-workflow-verifier.test.ts Extends source-level verification to ensure every relevant generated-workflow enumeration uses the bounded helper.
packages/runtime/test/invariant-suite-enumeration-overflow.test.ts Adds focused executable regression coverage for overflow messages, root ranking, cause stripping, normal Git failures, and listings larger than Node’s former default.
packages/runtime/test/invariant-suite-handoff-durability.test.ts Updates the extracted workflow harness with a compatible Git enumeration stub so existing handoff durability tests continue exercising the generated helper graph.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Invariant discovery caller] --> B[invariantSuiteGitPaths]
  B --> C[git ls-files or git diff]
  C -->|Within 16 MiB| D[Decode UTF-8 path listing]
  D --> E[Discover invariant sources and tests]
  C -->|ENOBUFS| F[Measure captured stdout and stderr]
  F --> G[Rank contributing roots]
  G --> H[Strip large capture fields]
  H --> I[Throw artifact-contract failure]
Loading

Reviews (1): Last reviewed commit: "fix(runtime): bound the invariant-discov..." | Re-trigger Greptile

Context used:

The three invariant-discovery call sites named in #323 shelled out to `git ls-files`
through a bare `execFileSync` with no `maxBuffer`, so Node's 1 MB default applied to a
listing of every tracked and untracked path under `src`, `contracts`, `test` and
`tests`. Past that the capture dies as `spawnSync git ENOBUFS`: no subcommand, no size,
no path, no hint that the workspace is at fault. #310 hardened `runGit` against exactly
that after six sandbox deaths across three Aave v4 runs turned out to be one uncaught
ENOBUFS; none of these sites goes through it.

Measured against the pre-change template, on 1,200 untracked paths of 1,024 bytes:
`invariantWorkspaceSourcePaths` threw `spawnSync git ENOBUFS`. After, the same fixture
returns all 1,200 paths.

Every capture now goes through one helper that states the bound, and an overflow is
rethrown naming what #311 established has to be named -- the subcommand, the bound, and
the largest contributing roots:

    artifact-contract failure: git ls-files listed more than the 8192-byte enumeration
    buffer of workspace paths. Largest contributors within the first 36416 bytes git
    wrote; git emits in path order, so anything past that cutoff is not visible here:
    test (>=35328 bytes in 512 paths), src (>=1088 bytes in 16 paths)

#311's helper is not reusable: it reads its attribution out of `diff --git` headers, and
it was deliberately moved out of the runtime package's public surface, which is the only
thing a generated workflow can import. What is reused is its shape, and the corrections
it paid for -- count bytes rather than UTF-16 code units, do not let Node decode the
capture before measuring it, blame stderr when stderr is the stream that overflowed, and
strip the payload before it becomes a `cause` (measured here: 193 bytes serialized).

Two more call sites than the issue lists. `changedTestTreePaths` and
`changedInvariantSourcePaths` enumerate the same roots through the same bare
`execFileSync`, so bounding three of five would have been arbitrary. Deliberately NOT
included: `removeStaleWorkspaceFiles`, which enumerates the whole tree plus every
untracked and ignored path, also unbounded. It is a larger overflow risk than any of
these, but it belongs to workspace-patch preparation rather than invariant discovery, and
its `-z` output needs a different attribution, so it wants its own issue.

Regression proven red against the unbounded template: the lifted helper does not exist
there, and the pre-change `invariantWorkspaceSourcePaths` fails the >1 MB fixture with
the raw ENOBUFS quoted above. The bound is injected in the tests, so real git really does
overflow it on a few hundred files instead of a fixture sized to 16 MB of path text; the
end-to-end case that must NOT overflow is the one that pays for its megabyte.

The ranking test pins that the largest root is named first rather than the one git
emitted first -- reversing the comparator fails it. runtime 390/390, 15 skipped.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@aviggiano
aviggiano merged commit 14a92a0 into main Aug 7, 2026
7 checks passed
@aviggiano
aviggiano deleted the fix/ls-files-maxbuffer branch August 7, 2026 15:53
aviggiano pushed a commit that referenced this pull request Aug 7, 2026
Four files conflicted; every resolution keeps both sides' behaviour.

workflow.tsx -- both destructuring imports were unions, not choices. Kept
#344's `checkInvariantSourcePinned` / `invariantPinnedSourceRefExists` and
#342's `validateWorkspacePatchCapture` alongside #188's cloud-selected-task,
finding-provenance and dynamic-topology symbols. The rest of the file merged
without overlap: #334's `invariantSuiteGitPaths` still bounds all five
invariant-discovery enumerations, #342's single ancestor-wide suite budget and
depth bound are intact, #343's per-path publisher resolution still fails closed
on unordered siblings, and #188's dynamic `taskSpecs` reassignment happens
before any of those helpers read it, so they see the expanded node set.

public-worker.ts -- kept #188's `REFERENCE_GITHUB_TOKEN_ENV` import and dropped
`redactSecretsInText`, whose only consumer (`sanitizePublicDiagnosticMessage`)
#335 deleted when it routed the stderr tail through
`sanitizeWorkerDiagnosticMessage`. The two compose: the reference token #188
adds to `publicBenchmarkWorkerSecretValues` is now redacted from the bounded
stderr tail as well.

node-provider.test.ts -- unioned `logicalDispatchFingerprint` and
`verificationMarker` into one options type, and moved #339's three new
verification-marker tests onto #188's two-argument `createResultArchive`
signature so their bundles carry a real `logical_dispatch_fingerprint` instead
of bypassing the provenance check.

topology-transform.test.ts -- kept both new tests. #342's invariant-only
exclusion test now reads the `REPOSITORY_ROOT` constant #188 hoisted rather than
recomputing the path, so the file keeps one spelling of the repository root.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
aviggiano pushed a commit that referenced this pull request Aug 24, 2026
…oots

removeStaleWorkspaceFiles was the fourth call site of the class #323
closed: it captured `git ls-tree` and two `git ls-files` listings through
bare execFileSync with no maxBuffer, and it is the only site that passes
--ignored, which deliberately surfaces node_modules. Any isolated worktree
that had run a package install listed ~30k ignored paths (measured
1,716,025 bytes in #691) against Node's 1,048,576-byte default, so the
node died pre-agent with a bare `SystemError: spawnSync git ENOBUFS` --
and with [retry] same_agent_attempts=1 a node whose real work had already
succeeded became permanently unrunnable. Every one of those paths was then
discarded by isWorkspaceRuntimePath on the very next statement: the
overflow was incurred entirely for paths already decided-to-ignore.

The fix:
- Extract WORKSPACE_RUNTIME_ROOTS as a module-level const that both the
  enumeration (producer) and isWorkspaceRuntimePath (the final gate before
  rmSync -rf, deliberately retained) read, so the two cannot drift.
- Exclude the runtime roots in the pathspec itself: both ls-files listings
  now end with `--`, `.` and four `:(exclude)<root>` pathspecs. Bare
  `:(exclude)<root>` is component-exact -- it matches the root as a file
  or directory prefix, exactly the gate's first-segment test -- where the
  issue's `<root>/**` shape would rely on fnmatch across slashes and still
  enumerate a top-level file named like a root.
- Route the ls-tree and both ls-files captures through the #334 shared
  bounded helper invariantSuiteGitPaths (16 MiB bound, attributed ENOBUFS
  rethrow per #311).
- Name the cwd in the helper's overflow sentence (criterion 3): it now
  reads `artifact-contract failure: git <subcommand> in <workspaceRoot>
  <detail>...` on both the stdout and stderr branches.
- Teach rankInvariantSuiteEnumerationRoots to treat NUL as an entry
  separator alongside newline, so the -z listings still get contributor
  attribution on overflow.
- Add explicit `maxBuffer: 64 * 1024` to the remaining fixed-size git
  calls (read-tree, preservePinnedSourceProof's rev-parse/remote helper,
  the rev-parse --verify existence probe, and the two ledger rev-parse
  captures), completing criterion 2 exhaustively. A new verifier test
  scans every execFileSync("git", ...) span in the template and fails on
  any future bare site.

Regression tests (fail without the fix): stale cleanup completes at the
template's real bound with 4,300 ignored node_modules paths (>1 MiB of -z
listing) while deleting genuinely stale files and preserving expected
files and node_modules; an injected 8 KiB bound proves an overflowing
stale enumeration names the subcommand, bound, workspace and ranked
contributing roots with a stripped ENOBUFS cause; runtime-root-named
top-level files survive cleanup while a node_modules2 prefix collision is
still removed.

Closes #691

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
aviggiano added a commit that referenced this pull request Aug 24, 2026
…oots (#825)

* Bound removeStaleWorkspaceFiles git enumeration and exclude runtime roots

removeStaleWorkspaceFiles was the fourth call site of the class #323
closed: it captured `git ls-tree` and two `git ls-files` listings through
bare execFileSync with no maxBuffer, and it is the only site that passes
--ignored, which deliberately surfaces node_modules. Any isolated worktree
that had run a package install listed ~30k ignored paths (measured
1,716,025 bytes in #691) against Node's 1,048,576-byte default, so the
node died pre-agent with a bare `SystemError: spawnSync git ENOBUFS` --
and with [retry] same_agent_attempts=1 a node whose real work had already
succeeded became permanently unrunnable. Every one of those paths was then
discarded by isWorkspaceRuntimePath on the very next statement: the
overflow was incurred entirely for paths already decided-to-ignore.

The fix:
- Extract WORKSPACE_RUNTIME_ROOTS as a module-level const that both the
  enumeration (producer) and isWorkspaceRuntimePath (the final gate before
  rmSync -rf, deliberately retained) read, so the two cannot drift.
- Exclude the runtime roots in the pathspec itself: both ls-files listings
  now end with `--`, `.` and four `:(exclude)<root>` pathspecs. Bare
  `:(exclude)<root>` is component-exact -- it matches the root as a file
  or directory prefix, exactly the gate's first-segment test -- where the
  issue's `<root>/**` shape would rely on fnmatch across slashes and still
  enumerate a top-level file named like a root.
- Route the ls-tree and both ls-files captures through the #334 shared
  bounded helper invariantSuiteGitPaths (16 MiB bound, attributed ENOBUFS
  rethrow per #311).
- Name the cwd in the helper's overflow sentence (criterion 3): it now
  reads `artifact-contract failure: git <subcommand> in <workspaceRoot>
  <detail>...` on both the stdout and stderr branches.
- Teach rankInvariantSuiteEnumerationRoots to treat NUL as an entry
  separator alongside newline, so the -z listings still get contributor
  attribution on overflow.
- Add explicit `maxBuffer: 64 * 1024` to the remaining fixed-size git
  calls (read-tree, preservePinnedSourceProof's rev-parse/remote helper,
  the rev-parse --verify existence probe, and the two ledger rev-parse
  captures), completing criterion 2 exhaustively. A new verifier test
  scans every execFileSync("git", ...) span in the template and fails on
  any future bare site.

Regression tests (fail without the fix): stale cleanup completes at the
template's real bound with 4,300 ignored node_modules paths (>1 MiB of -z
listing) while deleting genuinely stale files and preserving expected
files and node_modules; an injected 8 KiB bound proves an overflowing
stale enumeration names the subcommand, bound, workspace and ranked
contributing roots with a stripped ENOBUFS cause; runtime-root-named
top-level files survive cleanup while a node_modules2 prefix collision is
still removed.

Closes #691

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

* Discover git capture sites whitespace-tolerantly in the #691 exhaustive scan

The criterion-2 scan anchored discovery on the single-line literal
`execFileSync("git"`, so a bare git call formatted multi-line --
`execFileSync(\n  "git", ...)`, the shape prettier already produces twice
in this template (the smithers and ultrafuzz sites) whenever arguments
exceed print width -- was never scanned and the suite stayed green with an
unbounded site present. Verified in this worktree: appending exactly such
a call left all three #691 tests passing, re-opening the recurrence
channel criterion 2 was filed to close (#323 missed a site because its
enumeration was shape-dependent).

The fix scans every `execFileSync(` occurrence with the same
balanced-paren walker, asserts every occurrence parses to one balanced
span (a site the scanner cannot delimit fails instead of silently
dropping out), and filters to git via /^execFileSync\(\s*"git"/u. The
adjacent enumeration-body doesNotMatch gains the same \s*-tolerance. The
>= 8 floor and the execSync/spawnSync exclusions stay. With the fix the
injected multi-line bare site fails the suite; the clean template passes
all 122 tests in the file.

Closes #691

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

---------

Co-authored-by: Claude Fable 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.

Three git ls-files call sites still use Node's 1 MB default maxBuffer

1 participant