feat: add defensible durability eval foundation - #566
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| const durationMs = Math.round((performance.now() - before) * 1000) / 1000; | ||
| trials.push({ | ||
| repetition, | ||
| passed: result.status === 0, |
There was a problem hiding this comment.
Empty cargo runs count as passes
High Severity
A trial is marked passed solely when the child exits 0. cargo test still exits 0 when --exact matches no tests, so a renamed or missing case can produce a publication-eligible durability artifact without exercising the claim.
Reviewed by Cursor Bugbot for commit 35106b7. Configure here.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 6 files
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
maintainability lens — FAILPR #566 — maintainability reviewBlocker
|
history lens — FAILBlocker
Concerns
Notes
REVIEW_FAILED |
structure lens — MISSING |
|
🎯 review-swarm: FAILED (M:fail H:fail S:missing) Lens transcripts posted as sibling comments above. |
| "fs.writeFileSync(process.argv[4], String(process.pid))", | ||
| 'const wait=new Int32Array(new SharedArrayBuffer(4))', | ||
| 'while(!fs.existsSync(process.argv[2])) Atomics.wait(wait,0,0,20)', | ||
| "fs.appendFileSync(process.argv[1], 'second\\n')", |
There was a problem hiding this comment.
Black-box eval uses wrong argv
High Severity
The -e scripts treat process.argv[1] as the first user argument, but Node puts [eval] there and starts user args at process.argv[2]. The fixture never writes effects.txt, attempts.txt, or step.pid, so every trial times out and a stray [eval] file is left in the suite working directory.
Reviewed by Cursor Bugbot for commit ec74641. Configure here.
| environmentFailurePatterns.some((pattern) => combined.includes(pattern)) | ||
| ) { | ||
| return 'inconclusive'; | ||
| } |
There was a problem hiding this comment.
Timeouts marked as environment failures
Medium Severity
classifyTrial treats any result.error as inconclusive, and spawnSync sets that field on timeout. A hung crash/resume trial is therefore recorded as an environment failure instead of a product failure, so a real durability hang is labeled as infrastructure noise.
Reviewed by Cursor Bugbot for commit ec74641. Configure here.
There was a problem hiding this comment.
2 issues found across 6 files (changes from recent commits).
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/run-evals.mjs">
<violation number="1" location="scripts/run-evals.mjs:107">
P2: This drops the previous `CARGO_TERM_COLOR=never` normalization; with `CARGO_TERM_COLOR=always`, Cargo can color test output and make successful trials fail the suite’s literal success-witness checks. Preserve the override in the child environment.</violation>
<violation number="2" location="scripts/run-evals.mjs:243">
P2: A `spawnSync` timeout sets `result.error.code` to `ETIMEDOUT`, so this classifies a hung durability test as an environment failure instead of a trial failure. Exclude timeout errors here so hangs are recorded as product failures.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (result.status === 0) return missingWitnesses.length === 0 ? 'passed' : 'invalid'; | ||
| const combined = `${stdout}\n${stderr}\n${result.error?.message ?? ''}`; | ||
| if ( | ||
| result.error || |
There was a problem hiding this comment.
P2: A spawnSync timeout sets result.error.code to ETIMEDOUT, so this classifies a hung durability test as an environment failure instead of a trial failure. Exclude timeout errors here so hangs are recorded as product failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/run-evals.mjs, line 243:
<comment>A `spawnSync` timeout sets `result.error.code` to `ETIMEDOUT`, so this classifies a hung durability test as an environment failure instead of a trial failure. Exclude timeout errors here so hangs are recorded as product failures.</comment>
<file context>
@@ -171,6 +230,25 @@ export function collectProvenance(rootDir, execute = spawnSync) {
+ if (result.status === 0) return missingWitnesses.length === 0 ? 'passed' : 'invalid';
+ const combined = `${stdout}\n${stderr}\n${result.error?.message ?? ''}`;
+ if (
+ result.error ||
+ result.status === 127 ||
+ environmentFailurePatterns.some((pattern) => combined.includes(pattern))
</file context>
| result.error || | |
| (result.error && result.error.code !== 'ETIMEDOUT') || |
| encoding: 'utf8', | ||
| timeout: testCase.command.timeoutMs, | ||
| maxBuffer: 64 * 1024 * 1024, | ||
| env: process.env, |
There was a problem hiding this comment.
P2: This drops the previous CARGO_TERM_COLOR=never normalization; with CARGO_TERM_COLOR=always, Cargo can color test output and make successful trials fail the suite’s literal success-witness checks. Preserve the override in the child environment.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/run-evals.mjs, line 107:
<comment>This drops the previous `CARGO_TERM_COLOR=never` normalization; with `CARGO_TERM_COLOR=always`, Cargo can color test output and make successful trials fail the suite’s literal success-witness checks. Preserve the override in the child environment.</comment>
<file context>
@@ -65,26 +81,48 @@ export function runEvalSuite(options) {
timeout: testCase.command.timeoutMs,
maxBuffer: 64 * 1024 * 1024,
- env: { ...process.env, CARGO_TERM_COLOR: 'never' },
+ env: process.env,
});
const durationMs = Math.round((performance.now() - before) * 1000) / 1000;
</file context>
| env: process.env, | |
| env: { ...process.env, CARGO_TERM_COLOR: 'never' }, |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c409218. Configure here.
There was a problem hiding this comment.
1 existing issue remains and 1 new issue found across 7 files (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/eval-provenance.mjs">
<violation number="1" location="scripts/eval-provenance.mjs:21">
P2: This can record `rustc: null` for an eligible run: the ambient lookup happens before `ops/cargo.sh` can bootstrap its private toolchain, and missing runtime metadata does not block publication. Capture the compiler version from the wrapper-selected toolchain and reject publication when it is unavailable.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| runtime: { | ||
| node: process.version, | ||
| nodePath: process.execPath, | ||
| rustc: commandText(execute, 'rustc', ['--version'], rootDir) || null, |
There was a problem hiding this comment.
P2: This can record rustc: null for an eligible run: the ambient lookup happens before ops/cargo.sh can bootstrap its private toolchain, and missing runtime metadata does not block publication. Capture the compiler version from the wrapper-selected toolchain and reject publication when it is unavailable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/eval-provenance.mjs, line 21:
<comment>This can record `rustc: null` for an eligible run: the ambient lookup happens before `ops/cargo.sh` can bootstrap its private toolchain, and missing runtime metadata does not block publication. Capture the compiler version from the wrapper-selected toolchain and reject publication when it is unavailable.</comment>
<file context>
@@ -0,0 +1,51 @@
+ runtime: {
+ node: process.version,
+ nodePath: process.execPath,
+ rustc: commandText(execute, 'rustc', ['--version'], rootDir) || null,
+ cargo: commandText(execute, resolve(rootDir, 'ops/cargo.sh'), ['--version'], rootDir) || null,
+ },
</file context>
Mutation verification: timeout classificationI reverted the exact timeout-classification branch from Mutated command: Captured mutated output: Restored command: Captured restored output:
|
Final-head mutation verificationAt exact head Command: Captured output with the mutation: I then restored that exact line byte-for-byte and ran: Captured output after restoration: The absent |
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…never outlives its lease (#561) (#576) * test(sdk): reproduce repeated authored CLI probes and expired leases * fix(sdk): reuse authored CLI probes for each run before worker admission * docs: capture lease regression mutation and live repro evidence * test(sdk): keep the artifact-gate cwd regression inside the run root The regression added by #517 declares `cwd` on a spec it hands to `preflight`, and used an `os.tmpdir()` directory — an absolute path. #566 landed one commit earlier and made an absolute `cwd` a compile refusal (`agent-cwd.ts`: a declared `cwd` is run-root-relative, the same rule `relayflowd_core::spec::is_run_root_relative_path` applies at the kernel boundary). Each PR was green alone; together they are not, and `main` at e30226c fails this test with `invalid_spec` where it expects `gate_path_unscanned`. The fixture now makes its directory inside the run root and declares the relative name. Nothing else moves: the warning, `ok`, the real `AgentWorker` dispatch, the empty scan snapshot, the journaled JSON output and the lowered gate command are asserted exactly as before. Reverting this file to its e30226c bytes fails the case and restoring it passes; both captures are in evidence/561/artifact-gates-{red,green}.txt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: classify every check failure against main and re-capture #561 evidence The repository check on this branch failed with 30 tests across five files. None came from #561. Each is classified by reverting this branch's three source files to origin/main (e30226c), re-running and restoring: * artifact-gates (1) — main is red; #566 and #517 conflict semantically. Fixed in the preceding commit. * live-kernel (7) — this sandbox's HOME declares "type": "commonjs" above the checkout, so testdata/preflight's extensionless ESM fixture CLIs load as CommonJS and emit nothing, silently. Local setup only. * authored-node-runtime — Bun 1.3.6 where every workflow pins 1.4.0. * hosted-extension (22) — unprivileged user namespaces denied to this container; bwrap cannot run even once installed. The mutation is re-run at this head. The mutated run reproduces the issue's exact signature — lease_expired on a first attempt that never heartbeated, retry, second attempt success — and the restore is byte-identical by SHA-256. The live-Claude repro could not be re-run: this environment's claude is no longer authenticated. The capture says so rather than the acceptance box claiming a pass it cannot show. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: re-run the #561 mutation at the branch head The committed mutation transcripts were captured before the artifact-gate fix (1ccaaed) landed. Re-run `evidence/561/mutation.patch` against the current head so the transcript matches the bytes a reviewer checks out, and record the restore with `git diff --exit-code` plus a sha256sum. The failing capture carries the issue's exact journal shape at both capacity 1 and the default: attempt 1 completes `lease_expired` with `wallclock_ms: 30011`, a `retry_backoff` sleep follows, and attempt 2 succeeds. Restored, all five cases pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Keep relayflow working files out of the change * fix(sdk): keep authored CLI probes from starving worker leases Session-Id: 01a0d409-e854-7540-a76e-a2f9cd136946 * fix(sdk): preserve authored probe compatibility and stdin isolation Session-Id: 01a0d409-e854-7540-a76e-a2f9cd136946 * fix(sdk): detect authored option bags without config key Session-Id: 01a0d409-e854-7540-a76e-a2f9cd136946 * fix(sdk): keep authored option bags from becoming config Session-Id: 01a0d409-e854-7540-a76e-a2f9cd136946 --------- Co-authored-by: Relayflow <noreply@agentrelay.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: khaliqgant <khaliqgant@gmail.com>
2 participants
Outcome
Adds a fail-closed, reproducible evidence foundation for Relayflows durability claims:
This PR does not claim competitive superiority. Relayflows versus DIY requires both adapters under the shared protocol; best-in-class additionally requires Temporal and Inngest.
Independent review repairs
The first review correctly rejected the original head. The current head now:
ops/cargo.shinstead of ambient Cargo;The remaining CI-wiring work is deliberately separate because
AGENTS.mdforbids changing a gate that judges the same work. It is tracked in #569.Verification at exact head
Exact head:
3f50d89c39dfe67c196138feb401987d0e79cdcdCommand:
Captured output:
Command:
Captured output:
Command:
Captured output:
Report summary:
{ "commit": "3f50d89c39dfe67c196138feb401987d0e79cdcd", "dirty": false, "suiteSha256": "d2972d947660474fba65a1a3f8e6d743379eb37d4025ad3bdadb24d82829918c", "passedTrials": 210, "failedTrials": 0, "timedOutTrials": 0, "invalidTrials": 0, "inconclusiveTrials": 0, "publicationStatus": "eligible", "blockers": [] }Per-case pass counts:
Negative evidence retained
The allowlist change initially omitted the active Rust toolchain selectors. The clean run failed closed instead of publishing:
Command:
Captured output:
That artifact records 180 failures and 30 inconclusive trials caused by missing Rust toolchain selection. The final head explicitly allows and records
CARGO_HOME,CARGO_TARGET_DIR,RUSTUP_HOME, andRUSTUP_TOOLCHAIN, then passed at a new exact commit.An earlier black-box workload also produced 207/210 and was correctly ineligible. It killed the daemon when a shell effect appeared without proving the step completion was durable, and on three trials observed the expected replay of uncommitted work. The corrected workload first obtains a CLI receipt after one durable completion, resumes, waits for the dependent step to be in flight, then kills the whole execution environment. The README records this boundary explicitly.
Note
Low Risk
Adds benchmark harnesses, Node scripts, and JSON fixtures only; no changes to relayflowd runtime or production gates.
Overview
Introduces a fail-closed durability evidence pipeline and separates product conformance from future competitive benchmarking.
Durability eval:
scripts/run-evals.mjsdrivesbenchmarks/durability/suite.json(30 repetitions × 7 cases). Reports pin git commit, suite hash, toolchain/host provenance, per-trial argv/cwd/allowlisted env, and full stdout/stderr. Publication is eligible only on a clean tree, sufficient repetitions, Rust/Cargo provenance, all trials passing with required success witnesses (so zero-test Cargo exits count as invalid), and no timeouts/failures/inconclusive environment outcomes. Output must live outside the repo; non-publishable runs exit nonzero.Black-box case:
benchmarks/durability/black-box-crash.mjsexercises public CLIrun/resumereceipts, SIGKILLs the resumed daemon and in-flight step child, then asserts 16 fixed outcomes (no journal inspection).Competitive protocol:
benchmarks/workflow-reliability/protocol.jsondefines vendor-neutral scenarios, metrics, and which participants are required for build-vs-buy vs best-in-class claims—without implementing adapters yet.scripts/benchmark-protocol.test.mjsguards protocol shape.Tests: Extensive
scripts/run-evals.test.mjscovers trial classification, symlink/cwd containment, env allowlisting, and black-box helpers.Reviewed by Cursor Bugbot for commit 3f50d89. Bugbot is set up for automated code reviews on this repo. Configure here.