[sunwrobert]

TL;DR

readOptions in scripts/mirror-vgi-bench-media.ts returns Result.Result<MirrorOptions, ReadOptionsError> instead of throwing on an invalid --concurrency or --limit; the import.meta.main boundary writes the same message to stderr and exits 1, so the CLI behaves exactly as before.

throw-to-result: packages/bench-harness/scripts/mirror-vgi-bench-media.ts (vendored copy of scripts/mirror-vgi-bench-media.ts in this repo)

remaining: 1298 → 1296 in OpenRouterTeam/openrouter-web, measured with git grep -E "^\s*throw new Error\(" -- 'packages/**/*.ts' 'services/**/*.ts' ':!*.test.ts' ':!*.integration.test.ts' | grep -vE "// invariant:" | wc -l on main (1298) and with this change applied to packages/bench-harness (1296). The count drops in openrouter-web only when the follow-up subtree pull lands. In this repo the same inventory (-- 'src/**/*.ts' 'scripts/*.ts' 'scripts/**/*.ts') goes 71 → 69.

This repo uses effect/Result (Result.succeed / Result.fail / Result.isFailure, assertSuccess / assertFailure from src/internal/testing.ts), so this change uses it rather than @openrouter-monorepo/lib-result, which the remediation profile names and which is not a dependency here.

Why this repo and not openrouter-web: packages/bench-harness in openrouter-web is a read-only vendored subtree pinned by scripts/check-subtree-integrity.ts (an in-place edit fails bun run verify on "Vendored subtree integrity", confirmed locally). The fix lands here first; a scripts/subtree-pull-bench-harness.sh PR then brings it into openrouter-web and closes the scan finding there.

What changed?

• ReadOptionsError literal union next to readOptions: { kind: "concurrency_not_positive_integer" } | { kind: "limit_not_positive_integer" } (no raw message strings; the messages carried no interpolated data).
• readOptions returns Result.Result<MirrorOptions, ReadOptionsError>; both throw new Error(...) sites become Result.fail({ kind }), the success path becomes Result.succeed({...}).
• describeReadOptionsError renders each kind to the exact message the CLI printed before, through an exhaustive switch ending in satisfies never.
• Caller main() (the only production caller, reached from import.meta.main) checks Result.isFailure, writes <message>\n to stderr and calls process.exit(1).
• Tests: the toThrow test becomes one assertFailure case per kind, plus error precedence when both flags are invalid, plus a table test that each kind renders to the original message; the two success tests narrow with assertSuccess.

Error union:

export type ReadOptionsError =
  | { readonly kind: "concurrency_not_positive_integer" }
  | { readonly kind: "limit_not_positive_integer" };

Callers: main() in scripts/mirror-vgi-bench-media.ts (1 production caller) and scripts/mirror-vgi-bench-media.test.ts.

purely mechanical: no

• Base printed a Bun uncaught-exception report to stderr (source excerpt, error: --concurrency must be a positive integer, stack trace, Bun v1.4.4 (Linux x64)) and exited 1. Head prints exactly --concurrency must be a positive integer (or the --limit message) plus a newline to stderr and exits 1. Exit status and the message text are unchanged; the stack trace and source excerpt are gone. This is the change the finding asks for.
• readOptions is exported; its return type changes from MirrorOptions to Result.Result<MirrorOptions, ReadOptionsError>. The only callers are in this file and its test.

// invariant: markers added: none.

Prod impact: none at runtime. The script runs manually (bun run mirror-vgi-bench-media) to refresh the VGI-Bench media manifest; no benchmark, solver, scorer, or dataset path imports it. Worst case: an operator passing an invalid flag sees a one-line message instead of a stack trace, still with exit code 1.

Why?

Code-scan finding sfind-bcf61ee8763443058437ea79c402a49c (Throw → Result burn-down): an out-of-range --concurrency or --limit is expected user input, not an invariant, so it belongs in the function's return type.

Show me

 main()                                         scripts/mirror-vgi-bench-media.ts
   readMediaMirrorEnv()
-  readOptions(argv) ──throws Error("--concurrency must be a positive integer")──▶ Bun uncaught-exception report, exit 1
+  readOptions(argv): Result<MirrorOptions, ReadOptionsError>
+    Result.fail({ kind: "concurrency_not_positive_integer" })
+    Result.fail({ kind: "limit_not_positive_integer" })
+  if Result.isFailure(parsed)
+    stderr ◀── describeReadOptionsError(parsed.failure) + "\n"
+    process.exit(1)
   new S3Client(env)
   fetchSourceVideos(options.revision, ...)

Proof matrix

Base worktree: git worktree add ~/proof/harness/base origin/main (c6a05aa), then git checkout 872a773 -- scripts/mirror-vgi-bench-media.test.ts. Head: 872a773. Command on both sides: bun test scripts/mirror-vgi-bench-media.test.ts.

The head test file imports describeReadOptionsError, a new export, so the unmodified head file fails to load on base (SyntaxError: Export named 'describeReadOptionsError' not found), which does not count as a base failure. The base rows below come from the same file with that one import line and the describeReadOptionsError describe block removed (scripts/base-compat.test.ts, 6 pass / 5 fail on base); every other line is identical.

Behavior	Unit	Integration	E2E
--concurrency=0 returns { kind: "concurrency_not_positive_integer" }	readOptions > fails with concurrency_not_positive_integer for --concurrency=0. Base: error: --concurrency must be a positive integer thrown before assertFailure runs. Head: pass.	n/a (pure argv parser, no boundary crossed)	CLI boundary run below
--limit=abc returns { kind: "limit_not_positive_integer" }	readOptions > fails with limit_not_positive_integer for --limit=abc. Base: error: --limit must be a positive integer thrown. Head: pass.	n/a (same)	CLI boundary run below
Concurrency is checked before limit (error precedence preserved)	readOptions > reports the first failing flag when both are invalid (--concurrency=1.5 --limit=0). Base: error: --concurrency must be a positive integer thrown. Head: pass with concurrency_not_positive_integer.	n/a	n/a (covered by the unit case; the CLI prints the first error only)
Each kind renders to the original CLI message	describeReadOptionsError > renders ... as the CLI message (2 cases). Base: function does not exist (import error, not counted). Head: pass.	n/a	CLI boundary run below shows the rendered text on stderr
Unchanged inputs: defaults and full flag set still parse	readOptions > applies defaults, readOptions > parses provided flags. Base: fail only because the return shape is now a Result (expect(received).toEqual(expected) on .success); the field values are unchanged. Head: pass.	n/a	n/a
Unchanged neighbours	extensionOf (3), candidateSources (2), hashManifest (1): pass on base and head.	n/a	n/a

CLI boundary (e2e for a script with no UI; dummy BENCH_MEDIA_S3_* env so readMediaMirrorEnv passes, no network reached because option parsing fails first):

$ bun scripts/mirror-vgi-bench-media.ts --concurrency=0   # base c6a05aa
99 |     throw new Error("--concurrency must be a positive integer");
                   ^
error: --concurrency must be a positive integer
      at readOptions (.../scripts/mirror-vgi-bench-media.ts:99:15)
      at main (.../scripts/mirror-vgi-bench-media.ts:241:19)
Bun v1.4.4 (Linux x64)
exit 1

$ bun scripts/mirror-vgi-bench-media.ts --concurrency=0   # head 872a773
--concurrency must be a positive integer
exit 1

$ bun scripts/mirror-vgi-bench-media.ts --limit=abc       # head 872a773
--limit must be a positive integer
exit 1

Head: bun test scripts/mirror-vgi-bench-media.test.ts 13 pass / 0 fail; bun run format:check, bun run check, bun run typecheck, bun run build clean. No committed e2e test is added: this repo has no tests/e2e harness and the script needs S3 credentials and network past option parsing.

Visualization

Self-contained artifact: https://openrouter.devinenterprise.com/api/presigned_proxy?token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJ1c2VyX2lkIjpudWxsLCJidWNrZXRfbmFtZSI6ImRldmluYXR0YWNobWVudHMiLCJidWNrZXRfa2V5IjoiYXR0YWNobWVudHNfcHJpdmF0ZS9jbGVyay1vcmdfMmhUSzBmbWdTSk80ZncwT041T1NwOUtFZG1iL2E1NjRhZGE3LWVjMTEtNDhmZC04NTk2LTE1ZDdhZjEzYTBlOCIsImlhdCI6MTc5MzQ4MzA4Mywib3JnX2lkIjoiY2xlcmstb3JnXzJoVEswZm1nU0pPNGZ3ME9ONU9TcDlLRWRtYiIsImZpbGVuYW1lIjoidGhyb3ctdG8tcmVzdWx0LW1pcnJvci12Z2ktYmVuY2gtbWVkaWEtMjAyNi0xMS0wMS5odG1sIn0.LD07FuQ0obCDqTXhrpocZZSUI8g2Z_Bj8LzZ8XjCvPM

Thermo-nuclear fix loop

Round 1 (head 872a773, diff +75/-12 in 2 files): no actionable finding.

• Considered collapsing the two kinds into one { kind: "flag_not_positive_integer"; flag: "concurrency" | "limit" }. Rejected: the burn-down rule asks for one kind per distinct failure, and two literal members with no payload are the smaller type.
• Considered inlining describeReadOptionsError into main(). Rejected: the exhaustive switch is the only place the message text lives, and the test pins each kind to its message through it.
• The file stays at 320 lines; no new conditionals outside the single isFailure guard at the boundary; no casts, no unknown.
• scripts/mirror-mmmu-pro-media.ts:153 carries the same --concurrency throw. Out of scope (one module per PR); it is the next module for the burn-down.

Round 2: not needed, round 1 produced no fix.

Deslop-micro pass

Fix mode, zero findings, no commit.

name	kind	existing concept	verdict
ReadOptionsError	type	<fn>Error unions with a kind discriminant (MirrorOutcome in the same file, readonly kind: unions in src/harness/core.ts)	keep
describeReadOptionsError	function	describeFailure in scripts/media-mirror.ts (same describe* verb for failure-to-text)	keep
concurrency_not_positive_integer, limit_not_positive_integer	literal	no enum for CLI option errors exists anywhere in the repo; two sites in one file	keep

Mutation testing

not measured: this repository has no test:mutation harness (the openrouter-web bun run test:mutation script does not exist here, and the vendored copy is read-only).

How to test

bun test scripts/mirror-vgi-bench-media.test.ts
BENCH_MEDIA_S3_BUCKET=b BENCH_MEDIA_S3_ENDPOINT=https://s3.example.test BENCH_MEDIA_S3_ACCESS_KEY_ID=k BENCH_MEDIA_S3_SECRET_ACCESS_KEY=s BENCH_MEDIA_PUBLIC_BASE_URL=https://media.example.test bun scripts/mirror-vgi-bench-media.ts --concurrency=0; echo "exit $?"

Expected: 13 tests pass; the script prints --concurrency must be a positive integer and exit 1.

Reviewer focus

• describeReadOptionsError keeps the two messages byte-identical to the old throw strings.
• process.exit(1) after process.stderr.write matches the old exit status; the stack trace is intentionally gone.

Checklist

• Tests cover changed behavior
• Public API or configuration changes are backward compatible, or the break is documented (readOptions return type change is documented above; no caller outside this file)
• Benchmark changes document dataset provenance and licensing (n/a, no benchmark change)
• No credentials, private results, or restricted dataset contents are included
• Documentation is updated where needed (none needed)

Link to Devin session: https://openrouter.devinenterprise.com/sessions/2dc315bbaa654730ada9ec2982d8cb74
Open in Devin Desktop: https://openrouter.devinenterprise.com/desktop/session/2dc315bbaa654730ada9ec2982d8cb74?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-bcf61ee8763443058437ea79c402a49c
Shared root cause: readOptions in packages/bench-harness/scripts/mirror-vgi-bench-media.ts (OpenRouterTeam/openrouter-web) throws new Error for an out-of-range --concurrency or --limit, which are expected user-input failures. Return a Result from readOptions; the single production caller is the script's import.meta.main block (~line 241), which should write the failure to stderr and process.exit(1) so the CLI behaves the same. Update the colocated tests to assert the failure value instead of toThrow. 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]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

[perry-v2]

✅ Approved. The change returns typed failures for invalid numeric flags and handles them at the CLI boundary; no actionable regressions found.

Scope and evidence

Reviewed both changed files (+75/-12) at 872a773 against actual base c6a05aa, with two independent correctness passes plus input/security, error-path, dependency/reuse and blast-radius checks. Examined the parser, its production caller, test helpers and affected tests. Numeric validation, default values, flag precedence and success fields are preserved; both failure kinds render the original message text. The caller unwraps success before creating the S3 client and exits 1 on parser failure.

Risk

The exported parser return type changes, and the in-repository callers are updated. Invalid-flag output intentionally loses the uncaught-exception stack/source excerpt. Environment validation still runs first, as before. The change is limited to the manual media-mirroring script and its tests; no benchmark execution path changes were found.

Verification

Static review only: no PR code, tests, installs or builds were run. Inspected tests cover defaults, supplied flags, both failure variants, error precedence and exact rendered messages. The author’s reported command results were not independently reproduced. CI was not checked.