[sunwrobert]

throw-to-result: src/datasets/cache-store.ts (vendored in openrouter-web as packages/bench-harness/src/datasets/cache-store.ts)

TL;DR

pipeTarGzTo and extractTarGzFrom return AsyncResult<void, TarGzError> instead of throwing; snapshotCheckout / tryHydrateCheckout branch on the Result with the same wLog fields and return values, and their generic try/catch is gone. The kill-and-await-exit cleanup is unchanged.

What changed?

• src/datasets/cache-store.ts

  • New TarGzError union next to the helpers (one kind per distinct failure, carrying the data the old message interpolated):

    type TarOperation = "create" | "extract";
    export type TarGzError =
      | { readonly kind: "tar_exited"; readonly operation: TarOperation; readonly code: number | null; readonly stderr: string }
      | { readonly kind: "spawn_failed"; readonly operation: TarOperation; readonly message: string }
      | { readonly kind: "stream_failed"; readonly operation: TarOperation; readonly message: string }
      | { readonly kind: "mkdir_failed"; readonly dir: string; readonly message: string };

  • waitForTar resolves to a Result (never rejects): close 0 → succeed, close ≠ 0 → tar_exited, child error event → spawn_failed.

  • streamStep wraps the node:stream/promises calls (pipeline, finished) with the package's tryPromiseResult + Result.mapError → stream_failed.

  • extractTarGzFrom wraps mkdirOwnerOnly(dir) in Result.try → mkdir_failed (the old caller catch covered this throw too; Devin Review caught that the first revision let it escape).

  • abortTar is the shared cleanup (proc.kill(); await exit) for both helpers; when the process never started it returns the spawn_failed Result instead of the pipeline's Premature close / EPIPE.

  • The two throw new Error("tar … did not provide stdout/stdin") sites are deleted: the stdio: ["ignore", "pipe", "pipe"] tuple selects the ChildProcessByStdio<null, Readable, Readable> overload of spawn, so proc.stdout / proc.stdin are non-nullable and the checks were unreachable.

  • describeTarGzError is the exhaustive switch (error.kind) (ending in satisfies never) that renders the log text; the callers do not branch per kind because every kind maps to the same outcome (warn + false / void).

  • Callers: tryHydrateCheckout wraps client.openObjectReadStream with tryPromiseResult (that call was inside the old try too), then branches on extractTarGzFrom's Result; snapshotCheckout wraps the synchronous client.openObjectWriteStream in Result.try (no await before pipeTarGzTo, so the pipeline attaches to the stream in the same tick as before) and branches on pipeTarGzTo's Result.

• src/datasets/cache-store.test.ts
  • aborts the upload when tar exits nonzero: .rejects.toThrow("tar create exited") → assertFailure + kind === "tar_exited", same inputs.
  • One test per kind for each helper (tar_exited, stream_failed, spawn_failed × create/extract, mkdir_failed for extract), a success case for pipeTarGzTo, and caller tests for the warn log on a corrupt snapshot, a throwing openObjectReadStream, an uncreatable target directory, a missing source dir, and a throwing openObjectWriteStream.
  • captureInfo generalised to captureConsole("info" | "warn") inside the GcsCacheStore describe.

Result library: this package already uses effect/Result (src/internal/result.ts, src/internal/gcs.ts), so the conversion uses Result.fail / Result.succeed / Result.isFailure / Result.mapError / Result.try from effect/Result and the package's AsyncResult alias, not @openrouter-monorepo/lib-result (which is not a dependency of this repo).

remaining (this repo, git grep -E "^\s*throw new Error\(" -- 'src/**/*.ts' ':!*.test.ts' ':!*.integration.test.ts' | grep -vE "// invariant:" | wc -l): 60 → 58
remaining (openrouter-web inventory command over packages/** + services/**, after the subtree pull lands this file): 1298 → 1296

Callers (all in src/datasets/cache-store.ts): makeGcsCacheStore().tryHydrateCheckout, makeGcsCacheStore().snapshotCheckout. No caller outside the module (git grep -n "pipeTarGzTo\|extractTarGzFrom" → only this file and its test).

// invariant: markers added: none.

purely mechanical: no

• The error field of GCS checkout hydrate failed / GCS checkout snapshot failed no longer carries the Error: prefix that String(error) added ("tar create exited 2: …" instead of "Error: tar create exited 2: …"; "bucket unreachable" instead of "Error: bucket unreachable"). It now goes through the package's unknownErrorToString, which is what src/internal/gcs.ts already logs. Field names, log messages, levels and return values are unchanged.
• When tar is not on PATH, the logged error is the spawn error (Executable not found in $PATH: "tar") instead of the downstream pipeline error (Premature close), because abortTar prefers spawn_failed.
• The unreachable did not provide stdout/stdin throws are removed rather than converted (see above).

Prod impact: the bench worker's checkout cache (BENCH_DATASET_CACHE_BACKEND=gcs). Same hit/miss/snapshot behaviour, same return values, same cleanup; only the two warn-log texts above differ. Worst case: a tar failure that previously surfaced as a rejected promise inside the caller's try now surfaces as a Result.Failure on the same path, so the blast radius is the two log lines. No per-request allocation concern (not an inference path).

Why?

Code-scan finding sfind-64accc3de1154543891546001134aef4 (Throw → Result burn-down): the helpers threw for missing stdio and rethrew pipeline failures, and their only callers caught generically and logged String(error).

How to test

bun test src/datasets/cache-store.test.ts   # 32 pass
bun run format:check && bun run check && bun run typecheck && bun test && bun run build

Integration against a real GCS wire (fake-gcs-server 1.54.0 in Docker, real @google-cloud/storage streams, real tar):

docker run -d --name fake-gcs -p 4443:4443 fsouza/fake-gcs-server:1.54.0 -scheme http -port 4443 -public-host localhost:4443
# the Node client sends JSON-API calls un-prefixed (/b/...), so a 10-line Bun proxy on :4444 rewrites /b/ → /storage/v1/b/ and forwards the rest
OR_ENV=development LOG_LEVEL=1 STORAGE_EMULATOR_HOST=http://localhost:4444 GOOGLE_CLOUD_PROJECT=test bun run gcs-integration.ts

Expected: checkout cache snapshot written → a 213-byte application/gzip object; checkout cache hydrate … result: "hit" with task.toml and nested/data.txt extracted; result: "miss" → false; missing source dir → GCS checkout snapshot failed { key, error: "tar create exited 2: …" } and no object written.

Show me

 makeGcsCacheStore().snapshotCheckout(localDir, scope, commit)
-  try
-    dest = client.openObjectWriteStream(full)
-    await pipeTarGzTo(localDir, dest)          // throws on stdio / pipeline / tar exit
-    iLog "checkout cache snapshot written"
-  catch (error)
-    wLog "GCS checkout snapshot failed" { key, error: String(error) }
+  opened = await tryPromiseResult(() => client.openObjectWriteStream(full))
+  if isFailure(opened) → wLog "GCS checkout snapshot failed" { key, error: unknownErrorToString(...) }; return
+  written = await pipeTarGzTo(localDir, opened.success)   // AsyncResult<void, TarGzError>
+    waitForTar      → succeed | tar_exited | spawn_failed
+    streamTarCreate → pipeline → await exit → dest.end(); finished(dest)   (each step a Result)
+    on failure      → dest.destroy(err); abortTar(proc, exit)   (kill + await exit, unchanged)
+  if isFailure(written) → wLog "GCS checkout snapshot failed" { key, error: describeTarGzError(...) }; return
+  iLog "checkout cache snapshot written"

Proof matrix

Base = worktree at origin/main (c6a05aa) with the head's src/datasets/cache-store.test.ts checked out; head = a6e5095 (same test file as 8258776). Command for both: bun test src/datasets/cache-store.test.ts. Head: 32 pass / 0 fail. Base: 22 pass / 10 fail, every base failure an assertion or an unhandled rejection from the helper (no import/setup failure).

Behaviour	Unit	Integration	E2E
pipeTarGzTo returns tar_exited on nonzero exit and aborts the upload	aborts the upload when tar exits nonzero — base: rejects error: tar create exited 2: …/missing: Cannot open; head: pass	fake-gcs-server run: missing dir → GCS checkout snapshot failed … tar create exited 2, bucket unchanged (same on base, Error: prefix)	n/a (no HTTP/UI surface; the harness CLI reaches this only through snapshotCheckout, covered by the integration run)
pipeTarGzTo returns stream_failed when the destination rejects writes	pipeTarGzTo reports a stream failure when the destination rejects writes — base: rejects error: disk full; head: pass	n/a (needs a failing GCS write stream; covered by unit Writable)	n/a
pipeTarGzTo returns spawn_failed when tar is not on PATH	pipeTarGzTo reports a spawn failure when tar is not on PATH — base: rejects error: Premature close; head: pass	n/a	n/a
pipeTarGzTo succeeds and ends the destination	pipeTarGzTo succeeds and ends the destination for a real directory — base: fails (assertSuccess on a Promise<void> → result.success undefined); head: pass	fake-gcs-server run: checkout cache snapshot written, 213-byte application/gzip object (same on base)	n/a
extractTarGzFrom returns tar_exited on a corrupt archive	extractTarGzFrom reports tar_exited for a corrupt archive — base: rejects error: tar extract exited 2:; head: pass	n/a	n/a
extractTarGzFrom returns stream_failed when the source errors	extractTarGzFrom reports a stream failure when the source errors — base: rejects error: connection reset; head: pass	n/a	n/a
extractTarGzFrom returns spawn_failed when tar is not on PATH	extractTarGzFrom reports a spawn failure when tar is not on PATH — base: rejects error: Cannot pipe to a closed or destroyed stream; head: pass	n/a	n/a
extractTarGzFrom returns mkdir_failed when the target cannot be created	extractTarGzFrom reports mkdir_failed when the target cannot be created — base: rejects ENOTDIR; head: pass	n/a	n/a
tryHydrateCheckout → false + warn when the target directory cannot be created (unchanged input)	tryHydrateCheckout returns false and warns when the target directory cannot be created — passes on base and head; failed on the first revision 0292bb9 (resolves.toBe(false) → rejected)	n/a	n/a
tryHydrateCheckout → false + GCS checkout hydrate failed { key, error } on a corrupt snapshot (unchanged input)	tryHydrateCheckout returns false and warns when the snapshot is corrupt — passes on base and head	fake-gcs-server run: hit → true + files extracted, miss → false (same on base)	n/a
tryHydrateCheckout → false + warn when openObjectReadStream throws	tryHydrateCheckout returns false and warns when opening the object throws — base: toEqual fails, received error: "Error: bucket unreachable"; head: pass ("bucket unreachable")	n/a	n/a
snapshotCheckout → warn + no object on tar failure (unchanged input)	snapshotCheckout warns and writes nothing when tar fails — passes on base and head	fake-gcs-server run (above)	n/a
snapshotCheckout → warn when openObjectWriteStream throws	snapshotCheckout warns when opening the write stream throws — base: toEqual fails, received error: "Error: bucket unreachable"; head: pass	n/a	n/a
Round-trip, hit/miss logging, JSON cache (equivalence)	existing 19 GcsCacheStore/DiskCacheStore/resolveCacheStore tests — pass on base and head	fake-gcs-server run — identical output on base and head except the Error: prefix	n/a

Thermo-nuclear fix loop

Three passes. Pass 1 findings, all fixed before the first push: describeTarGzError was exported with no external reader → module-private; the stdio null checks were dead under the typed spawn overload → deleted instead of converted; the spawn_failed kind was unreachable from the pipeline path → abortTar consults the settled exit Result; captureInfo/captureWarn duplicated → captureConsole(level); toHaveProperty("message") was a weak assertion → toContain("tar"). Pass 2 on 0292bb9: no findings; Devin Review then found the escaped mkdirOwnerOnly throw → mkdir_failed kind + two tests (8258776). Pass 3 on 8258776: no findings. Perry's approval flagged the tryPromiseResult(async …) await before pipeTarGzTo as an unproven timing concern → Result.try (a6e5095), tests unchanged. Deslop-micro (fix mode) on the final diff: helper inventory (streamStep, abortTar, streamTarCreate, describeTarGzError, withoutTarOnPath, captureConsole) — no existing equivalent in src/ (rg for abort|kill.*exit|mapError.*stream), all keep; name inventory (TarGzError, TarOperation, tar_exited, spawn_failed, stream_failed, mkdir_failed) — kind discriminants match the package's existing unions (verifier_log, judge_runs, rejected), all keep; literal inventory — "create" | "extract" are the two tar invocations in this file, no shared vocabulary elsewhere, keep; no new comments. No cleanup commit was needed.

Visualization

Artifact: tar-helpers-result-2026-11-01.html (attached in the session; check-artifact.py passes). Not committed. The artifact predates the mkdir_failed kind; the three kinds it shows are unchanged.

No ## Before / after: this change has no user-facing surface (package helper + warn logs), so the API-only evidence above replaces the recording and screenshots.

Reviewer focus

• abortTar preferring spawn_failed over the pipeline error is the one place the returned kind depends on ordering; waitForTar settles on the child error event before the pipeline rejects in both Bun and Node (probe in the session), and abortTar awaits exit either way.
• streamTarCreate keeps the three old steps in the same order (pipeline → await exit → dest.end(); finished(dest)) and pipeTarGzTo still destroys dest with an Error before cleanup, so a tar failure still aborts the GCS upload.

Checklist

• Tests cover changed behavior
• Public API or configuration changes are backward compatible, or the break is documented — pipeTarGzTo / extractTarGzFrom now resolve to a Result instead of rejecting; both are module-internal apart from the test (documented above)
• Benchmark changes document dataset provenance and licensing — n/a
• No credentials, private results, or restricted dataset contents are included
• Documentation is updated where needed — n/a

Link to Devin session: https://openrouter.devinenterprise.com/sessions/e506bddacbe34b6b9f3af4359f36e491
Open in Devin Desktop: https://openrouter.devinenterprise.com/desktop/session/e506bddacbe34b6b9f3af4359f36e491?variant=devin
Requested by: @sunwrobert

---

[devin-ai-integration]

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

• Disable automatic comment, CI, and merge conflict monitoring

---

Original prompt from Devin Bot

Please remediate these findings together. --code-scan-step fix:sfind-64accc3de1154543891546001134aef4
Shared root cause: in packages/bench-harness/src/datasets/cache-store.ts (OpenRouterTeam/openrouter-web), pipeTarGzTo and extractTarGzFrom throw when the spawned tar process has no stdout/stdin and rethrow pipeline failures; their only callers snapshotCheckout / tryHydrateCheckout in the same file catch generically, wLog String(error) and return void/false. Make both helpers return a Result (keep the kill/cleanup logic), have the callers branch on the failure with the same wLog fields and return values, and remove the now-redundant try/catch. Update cache-store.test.ts. The fix is contained to this one file plus its test. Finding guidance notes this package uses effect/Result; the profile names lib-result — follow the package's existing convention and state the choice in the PR body.

@playbook:playbook-063fbf6c0b3945bf948a331c5e4f011d

[devin-ai-integration]

Evidence

API-only change (package helper + warn logs), so no recording / before-after screenshots. Integration run against fake-gcs-server 1.54.0 with the real @google-cloud/storage client and real tar:

--- snapshotCheckout (real tar -> real @google-cloud/storage stream -> fake-gcs-server)
checkout cache snapshot written { key: "bench/repos/harbor-deadbeefdead.tar.gz", scope: "harbor", commit: "deadbeefdeadbeef" }
--- tryHydrateCheckout (fake-gcs-server -> real stream -> real tar)
checkout cache hydrate { key: "bench/repos/harbor-deadbeefdead.tar.gz", scope: "harbor", commit: "deadbeefdeadbeef", result: "hit" }
hydrated: true task.toml: "name = 'a'\n" nested/data.txt: "hello\n"
--- tryHydrateCheckout miss
checkout cache hydrate { key: "bench/repos/harbor-000000000000.tar.gz", scope: "harbor", commit: "0000000000000000", result: "miss" }
hydrated(miss): false
--- snapshotCheckout failure path (missing dir -> tar exits 2 -> wLog, upload aborted)
GCS checkout snapshot failed { key: "bench/repos/harbor-cafebabecafe.tar.gz", error: "tar create exited 2: tar: /tmp/gcs-int-src-0JJO96/missing: Cannot open: No such file or directory\ntar: Error is not recoverable: exiting now\n" }
objects in bucket: [{"name":"bench/repos/harbor-deadbeefdead.tar.gz","size":"213","contentType":"application/gzip"}]

Same script against origin/main (c6a05aa): identical lines except error: "Error: tar create exited 2: …" (the Error: prefix from String(error)).

Unit proof: bun test src/datasets/cache-store.test.ts on head vs the same test file in a worktree at origin/main: every new Result assertion fails on base (the helpers still reject) and passes on head; the unchanged-behaviour log checks pass on both. Full gate on head: bun run format:check && bun run check && bun run typecheck && bun test && bun run build → green.

Visualization artifact tar-helpers-result-2026-11-01.html is attached in the Devin session (not committed).

[perry-v2]

✅ Approved. The tar create/extract helpers now return typed Results, and the cache callers translate failures into their existing warning and fallback outcomes. No confirmed blocking regression found.

Scope and evidence

Reviewed both changed files against actual base c6a05aa324948d4fd66b89c2c7e8b9a3065ee01a, at head 8258776e5a10047ecd890b1d17464ec334f282b1. Two independent correctness passes covered both files; additional passes examined error paths, security, reuse, coupling and callers. Checked process completion/error conversion, upload finalization after successful tar exit, kill-and-await cleanup, mkdir failure conversion, stream-opening handling and cache callers. Candidate defects were independently verified; none established a blocking regression.

This was a static review: no PR code, tests, installs or builds were run. The author's unit/integration results were not independently reproduced.

Worth a look: src/datasets/cache-store.ts:442-452 introduces an await between creating the writable and attaching pipeline listeners; a next-tick error would be unhandled, but whether the default GCS SDK emits errors that early could not be established because its implementation is absent from the checkout. This is an unproven timing concern, not a confirmed production defect.

Risk

Dimension	Severity	Risk	Reasoning
Implementation risk	🟩	Low	Result branches preserve the checked tar-exit-before-upload-finalization ordering and kill/await cleanup (cache-store.ts:145-201); no confirmed regression.
Premise risk	🟩	Low	Existing Result utilities match the conversion; callers still warn and return false/void on represented failures (cache-store.ts:405-461). Early SDK stream timing remains unchecked.
Estimated impact	🟨🟨	Medium	A hypothetical stream-error regression could interrupt a benchmark worker, rather than only lose a cache hit; no such trigger was confirmed (cache-store.ts:442-452).

Risk factor	Severity	Risk	Reasoning
Reversibility	🟩	Low	No schema or archive-format change; prior helper/caller implementation can be restored.
Detectability	🟩	Low	Represented failures retain warning paths and keys (cache-store.ts:424-461); unit tests inspect failure Results and caller warnings.
Blast radius	🟨🟨	Medium	Changes affect the GCS checkout cache and its benchmark-worker callers, not the disk/JSON cache paths. A worker failure remains conditional on an unconfirmed error path.
Data integrity	🟩	Low	Creation still waits for successful tar exit before ending the upload (cache-store.ts:170-182); extraction format and arguments are unchanged.
Financial exposure	—	Unknown	Failed caches could repeat checkout work; workload frequency and resulting spend were not checked.
Security and privacy exposure	🟩	Low	No changed credentials, object-key policy or tar arguments; owner-only directory creation remains in place (cache-store.ts:206-219).
Propagation	🟩	Low	Failure Results are consumed by the two module-local cache callers (cache-store.ts:424,452), retaining their fallback outcomes.
Availability	🟨🟨	Medium	Conditional early writable errors could affect a worker; SDK timing is unverified, while represented tar/stream failures return through warning paths.
Recovery cost	🟩	Low	No new persisted format or migration requires reconciliation; source rollback restores previous behavior.
Time to correct	—	Unknown	Deployment and rollback cadence were not inspected.

Verification and CI

Inspected tests covering tar exit, stream failure, missing executable, mkdir failure, upload success and caller warnings. These support the intended boundaries but do not establish the SDK timing noted above. At review time, all four check runs on this exact head succeeded (validate, CodeQL, and both Analyze jobs); the Devin Review commit status also succeeded. These are fetched CI results, not locally reproduced checks.

[devin-ai-integration]

Re Perry's "worth a look" (snapshotCheckout, await between creating the writable and attaching pipeline listeners): removed in a6e5095. client.openObjectWriteStream is synchronous, so it is now wrapped in Result.try instead of tryPromiseResult(async …); pipeTarGzTo runs in the same tick the stream is created, exactly as before this PR (pipeline attaches its listeners synchronously inside tryPromiseResult's await thunk()). No behaviour change otherwise; 32/32 tests, format/check/typecheck/build green locally.

[perry-v2]

✅ Approved. The tar-helper Result conversion retains the checked cache fallback and cleanup paths. The latest revision also removes the await between opening the upload stream and attaching pipeline listeners, addressing the prior timing note. No confirmed blocking regression found.

Scope and evidence

Re-reviewed the full actual-base diff, both src/datasets/cache-store.ts and src/datasets/cache-store.test.ts, at head a6e5095999c1f1efabe55733e3e2b7d24e69140e against base c6a05aa324948d4fd66b89c2c7e8b9a3065ee01a. Two independent correctness passes covered both files, with additional error-path, blast-radius, security, reuse and coupling passes. Checked Result branching, tar exit/error handling, pipeline finalization, kill-and-await cleanup, mkdir handling and the benchmark callers. Independently verified the candidate synchronous-spawn concerns; no meaningful supported trigger was established.

Static review only: no PR code, tests, installs or builds were run. Author-reported integration results were not independently reproduced. Third-party GCS internals were not available in the checkout.

Prior review

The previous Peary review approved with no blockers. Its non-blocking note is addressed: snapshotCheckout now uses synchronous Result.try (cache-store.ts:442-453), so opening the writable no longer adds an await before the helper attaches pipeline listeners. The earlier directory-creation failure conversion remains covered by mkdir_failed and caller tests. No new inline findings.

Risk

Dimension	Severity	Risk	Reasoning
Implementation risk	🟩	Low	Checked tar/stream failures return Results, retain kill/await cleanup and preserve successful-exit-before-upload-finalization (cache-store.ts:145-225).
Premise risk	🟩	Low	Conversion uses existing Result conventions; the new synchronous write-opening wrapper preserves the previous listener-attachment timing (cache-store.ts:442-453).
Estimated impact	🟩	Low	Represented checkout-cache failures remain warn plus false/void, permitting existing fallback behavior (cache-store.ts:403-461); no confirmed escaping failure on supported paths.

Risk factor	Severity	Risk	Reasoning
Reversibility	🟩	Low	No new persisted format or migration; the previous helper implementation can be restored.
Detectability	🟩	Low	Existing keyed warnings remain on both caller failure paths (cache-store.ts:424-461); tests inspect returned kinds and warnings.
Blast radius	🟩	Low	Changed consumers are the GCS checkout hydrate/snapshot paths; disk and JSON-cache behavior is not refactored.
Data integrity	🟩	Low	Upload completion still follows successful tar exit (cache-store.ts:170-182); extraction flags and archive format are unchanged.
Financial exposure	—	Unknown	Repeated cache misses could add checkout work, but workload frequency and spend were not checked.
Security and privacy exposure	🟩	Low	No changes to credential handling, object-key policy or tar arguments; owner-only directory creation is retained (cache-store.ts:206-219).
Propagation	🟩	Low	Helpers' Results are consumed by the two module-local store methods (cache-store.ts:424,453) rather than exposed through a changed CacheStore interface.
Availability	🟩	Low	Checked failure paths continue to warn and return fallback values; synchronous upload opening no longer adds the earlier timing gap (cache-store.ts:442-461).
Recovery cost	🟩	Low	No format conversion, migration or new durable metadata requires reconciliation after a source rollback.
Time to correct	—	Unknown	Deployment and rollback timing were not inspected.

Verification and CI

Inspected coverage for tar nonzero exit, missing executable, source/destination errors, mkdir failure, successful upload and caller warnings. At review time, all four current-head check runs succeeded: validate, CodeQL and both Analyze jobs. The current-head Devin Review commit status also succeeded. These CI results were fetched, not reproduced locally.