Repository navigation
fix(runtime): accept directory invariant scan probes - #290
Conversation
A scan probe records where the agent searched; an invariant source records bytes that become evidence. Both went through the same validation, so a probe naming a directory was rejected as "not a regular file" and failed project-discovery. R45's `project-discovery` died on `artifact-contract failure: invariant scan probe tests is unavailable: scan probe is not a regular file`, with `tests/` being an ordinary directory in the Aave v4 target. Nothing depended on the node yet, so the whole invariant pipeline stopped before a single property lens ran. This is an extension of behaviour the code already had rather than a new rule. The repository-root probe was already accepted, with the comment "A repository-wide probe names the workspace directory itself. It is valid evidence, but cannot be snapshotted as a regular UTF-8 file." A subdirectory probe is the same case and merely fell through to the file-only path. Any directory probe now takes the existing route. Fixed in both places the rule is implemented — the runtime gate in `artifact-gates.ts` and the generated workflow's `verifyInvariantLedgerSourceEvidence` — because R45 hit the template and fixing only one would still fail the node. The runtime diagnostic no longer claims a constraint it does not enforce. Invariant sources are deliberately unchanged: their bytes are the evidence, so a directory there must still fail, and a regression pins that alongside the probe case. Note the failure is intermittent, not deterministic: it depends on whether the model happens to name a directory among its probes. R44's project-discovery recorded only file probes and passed on the immediately preceding image. Fixes #289 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ly runs Two independent reviews of #290 agreed the behaviour change is safe, and both raised the same gap: every test added by the PR targeted `artifact-gates.ts`, while the copy of the rule that actually failed R45 lives in the generated workflow template. Reverting only the template hunk leaves `artifact-gates.test.ts` fully green at 42/42 — measured, not assumed — so a future refactor could reintroduce R45's failure with CI passing. That is the same split-brain class of bug this PR is fixing. - extract `verifyInvariantLedgerSourceEvidence` in `generated-workflow-verifier.test.ts`, following the existing extraction pattern for `restoreInvariantSuiteWorkspaceSnapshot`, and assert a directory probe never reaches the regular-file snapshot - assert a symlink-to-directory probe still does reach it and fails there, which pins the `!isSymbolicLink()` conjunct that nothing previously guarded - add the same symlink-probe rejection to the runtime gate's tests The first review also found that "still rejects a directory invariant source" passed for the wrong reason: the ledger omitted `kind`/`inventory_ids` and added a non-schema `statement`, so it failed schema validation before the source rule ever ran, leaving the PR's central safety claim unguarded. The ledger is now schema-valid with a complete markdown handoff, so the directory source is the only thing that can fail it, and the assertions name the real diagnostic (`not-file`) while explicitly ruling out `INVARIANT_LEDGER_SCHEMA_INVALID`. Also narrows the diagnostic wording to "a non-symlink directory": the only case that now reaches that message is a symlink to a directory, which `ls` calls a directory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two independent reviews, both actionable findings fixedBoth reviewers recommended merge and neither found a security regression. Full detail on what was measured, since the interesting part is what they found wrong with the tests rather than the change. Verified by both reviewers, independently
Fixed in c8923fbBoth reviewers flagged the same gap, and they were right: the template branch — the one that actually failed R45 — had zero coverage. Every test in the original push targeted
The first reviewer also found that my "still rejects a directory invariant source" test passed for the wrong reason. The ledger omitted Also added: symlink-probe rejection on the runtime-gate side, pinning the Wording nit taken. "regular file or directory" → "regular file or a non-symlink directory": the only case that reaches that message is a symlink to a directory, which Deliberately not fixed here
Validation
|
Second independent reproduction — the failure is not effectively intermittentR45's overseer retried Two independent attempts, same model, same target, same probe path. I had recorded this as intermittent because R44's Consequences worth stating plainly:
Nothing about the diff changes on this evidence; it just removes the "maybe it self-heals" reading. Full monorepo suite is green at 436 tests / 422 pass / 0 fail / 14 skipped. For the record on CI: |
Merging with
|
| job | queued | cancelled | steps |
|---|---|---|---|
| 92688882023 | 17:32:59Z | 17:48:02Z | 0 |
| 92689030888 | 17:33:57Z | ~17:48Z | 0 |
| 92692689305 | ~17:57Z | ~18:13Z | 0 |
Zero steps means it never reached Check out repository, so nothing in this branch can be the cause. Corroborating evidence that this is org/platform-level runner starvation rather than anything about #290: the scheduled Publish Eval History and Recover Incomplete Modal Benchmarks workflows on main (fdce587) failed in the same window, and Modal Eval Benchmarks had both launch and collect cancelled with zero steps too. There is no gh run cancel anywhere in .github/workflows/, and ci.yml's concurrency group is per-ref with no newer push to supersede these, so the cancellations are not self-inflicted. timeout-minutes: 30 cannot produce a 15m cancellation either.
Merging anyway, and stating the tradeoff rather than burying it:
For
- Both required independent reviews are complete; every actionable finding is fixed in
c8923fb, including the two the reviewers found in my tests rather than in the change. mainhas no branch protection, sorelease-gatesis not a required check, and the same deviation was recorded on fix(runtime): sanitize underscore-separated reference labels #284.- Every substantive check that did run is green: Greptile, semgrep, Socket Project Report, Socket PR Alerts.
release-gatesrunspnpm test. That exact command is green on this exact tree: 436 tests, 422 pass, 0 fail, 14 skipped, EXIT=0 (packages/runtime309/295/0/14). VerifiedHEAD == origin/fix/invariant-directory-scan-probes == c8923fbwith no uncommitted tracked changes, so the suite ran on the merged content, not an earlier tree.- R45 is dead-ended: two independent
project-discoveryattempts failed identically, so every hour without this fix is a wasted run.
Against, honestly
- I have no CI confirmation of the tests on
c8923fb, only local. The mitigation is that the command and tree are identical, and I will re-runrelease-gatesonmainafter merge once runners recover; if it fails there for a real reason, that is a revert, not a rationalisation.
Post-merge confirmation: the whole
|
| step | result |
|---|---|
pnpm -w format:check |
ok |
pnpm -w lint |
ok |
pnpm -w build |
ok |
pnpm -w benchmark:check |
ok |
pnpm -w validate:release |
ok |
pnpm test |
436 tests, 422 pass, 0 fail, 14 skipped |
The only steps not reproduced are the environment setup actions (checkout, pnpm/node/bun, pnpm install --frozen-lockfile) and the artifact upload. The lockfile is unchanged by this PR, so --frozen-lockfile has nothing new to resolve.
That is the confirmation the merge comment promised, and it is stronger than a green check mark on a re-run would have been, because it is the merged tree rather than the PR head. Script and full log kept at release-gates-local.sh / release-gates-local-23813cd.log in the ops directory.
R46 is live on this fix: image im-01KZC4NQPFJFHBK932P48YGQPM built from 23813cd, run aave-v4-v0012-main-issue289-r46-invariant-only, fresh durable volume, 9 nodes succeeded and project-discovery running. R45 was deliberately left running on the unfixed image as the control, which makes this the closest thing to a clean A/B this investigation has had — exactly one changed variable between the two.
The documentation half of two gate fixes that already landed (#290, #298). Each of these prompts told the model to do something a gate then rejected, and each cost a live Aave v4 run a full agentic node attempt before anyone noticed. **Fan-in `ledger_ids`.** Line 86 was already conditional -- "add a `ledger_ids` array to every canonical property THAT REPRESENTS one or more ledger entries" -- but two later sentences read unconditionally, so a model rendering the field on a property whose JSON has none is rejected with `INVARIANT_LEDGER_MARKDOWN_MAPPING_EXTRA`. The unconditional reading is unsatisfiable anyway: `ledger_ids` carries `minItems: 1`, so a property with no mapping has no list to render. Both sentences are now conditioned. **Project-discovery scan probes.** The prompt never said what kind of path a probe may name, while its own suggested `grep -rIlE ... test contracts` takes directory arguments. R45 named `tests` and died on it twice. It now states the real contract: a real directory or an absent path is fine and is not snapshotted; a symlink is not, and neither is a path reached through one; a probe naming an existing file IS snapshotted and must be UTF-8, tracked at the pinned commit and unmodified, so prefer the directory searched or the absent path over generated or untracked files. Getting that last part right took three passes. The first draft claimed "only ledger `entries` are checked byte-for-byte", which is FALSE -- a regular-file probe goes through the same `readInvariantSourceSnapshot` as an entry source, with `git ls-files --error-unmatch`, `git diff --quiet HEAD` and a byte comparison against the pinned ref. I found that by checking my own wording against the code; two independent reviews then confirmed it as their blocking finding. It was the exact error class this PR exists to remove, and it would have invited the model to cite build output. Reviews also caught that the fan-in file still contained the unconditional twin of the sentence being fixed four lines below it, that "a directory" was over-broad given both implementations reject symlinks, and that a pre-existing "use a line range or symbol" sentence now contradicted the new text while `scan_probes` has no line-range field at all. All fixed. Anchored in `semantic-anchors.test.ts` on whitespace-normalised text, so a reflow cannot masquerade as a semantic regression, with one anchor asserting the false sentence stays absent. Proven red against the unmodified prompts. Deferred until now on purpose: prompts change model output, so this had to land between runs. R46 and R47 both have their prompts baked into immutable images. Fixes #291 Fixes #299 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
Summary
project-discoveryThe failure
Run
aave-v4-v0012-main-issue281-r45-invariant-only, imageim-01KZBYNVDXXRJ7MNY4W6GMKMM3(mainfdce587). Nodeproject-discoveryfailed at2026-08-06T17:10Z:tests/is an ordinary directory in the Aave v4 target.project-discoveryis the first agentic node, so this stopped the whole invariant pipeline before a single property lens ran — run state9 succeeded / 1 failed / 22 pending.Why this is an extension of existing behaviour, not a new rule
verifyInvariantLedgerSourceEvidencealready accepted a directory probe — for the workspace root, with this comment:A subdirectory probe is precisely the same case; it just fell through to the file-only path in
readInvariantSourceSnapshot, which additionally demands UTF-8 decodability and a byte-identical match against the pinned commit. Those constraints are right for an invariant source and meaningless for a directory. Any directory probe now takes the route the root probe already took.Fixed in both implementations
The rule exists twice and R45 hit the second one:
packages/runtime/src/artifact-gates.ts— the runtime gate, which emitsINVARIANT_LEDGER_PROBE_PATH_INVALIDpackages/runtime/src/templates/smithers/workflows/workflow.tsx— the generated workflow'sverifyInvariantLedgerSourceEvidence, which produced R45's actual errorFixing only one would still fail the node. The runtime diagnostic message also no longer claims a constraint it does not enforce ("must be a regular file" → "must be a regular file or directory").
Invariant sources deliberately unchanged
Their bytes are the evidence, so a directory there must still fail. A second regression pins that, so this cannot be mistaken for a blanket relaxation.
Intermittent, which is worth knowing
This depends purely on whether the model names a directory among its probes. R44's
project-discoveryrecorded only file probes and passed on the immediately preceding image. So it is easy to miss and hard to attribute — R45 looked like a fresh regression until the error was read.Validation
packages/runtime/test/artifact-gates.test.ts— 41 pass / 0 failgit diff --checkcleanRelated
Draft PR #254 proposes "allow canonical directory scan probes as ledger search evidence while preserving strict file snapshots for source probes", bundled with eight unrelated changes. This is the third defect from that bundle to surface in a live run, after #281 and adjacently #283. The bundle is worth unpicking into landable pieces rather than left to predict failures nobody acts on.
Greptile Summary
The PR aligns runtime and generated-workflow validation so non-symlink directory scan probes are accepted while invariant sources remain restricted to regular files.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD P[Invariant ledger path] --> T{Evidence type} T -->|Scan probe| S{Path kind} S -->|Non-symlink directory| A[Accept without snapshot] S -->|Regular file| F[Validate and snapshot] S -->|Symlink or invalid type| R[Reject] T -->|Invariant source| F F -->|Snapshot succeeds| V[Accept evidence] F -->|Not a regular file| RReviews (2): Last reviewed commit: "test(runtime): pin the directory scan-pr..." | Re-trigger Greptile
Context used: