Skip to content

fix(bench): release 0.1.1 against runtime 0.90.1#506

Merged
drewstone merged 5 commits into
mainfrom
chore/agent-bench-0.1.1
Jul 9, 2026
Merged

fix(bench): release 0.1.1 against runtime 0.90.1#506
drewstone merged 5 commits into
mainfrom
chore/agent-bench-0.1.1

Conversation

@drewstone

@drewstone drewstone commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Release agent-bench 0.1.1 against the public runtime API it actually uses.

The failure was hidden by the development tsconfig alias: agent-bench source called OpenSandboxRunOptions.beforeStart, but its manifest and lockfile still installed runtime 0.89.0. A public-entrypoint compile with no source alias reproduced three errors in run-benchmarks.ts.

This change:

  • requires @tangle-network/agent-runtime ^0.90.1 and aligns agent-eval at 0.108.1
  • compiles the public entrypoint against installed packages in CI
  • discovers and runs all 16 package test files in CI and before publish
  • packs agent-bench, installs the tarball in an empty consumer, executes resolveAdapter, imports runBenchmarks, and compiles it
  • adds an agent-bench-v* OIDC release path without changing the root runtime release path

Verification

  • agent-bench frozen install
  • all 16/16 package test files passed, including the 24-shot benchmark matrix and real fixture judges
  • public-entrypoint typecheck against registry runtime 0.90.1
  • packed tarball installed, executed, and compiled in a clean consumer against runtime 0.90.1
  • runtime typecheck
  • runtime tests: 1,316 passed, 2 skipped across 134 files
  • runtime build, lint, docs check, and package-export check
  • actionlint on both changed workflows
  • clean merge-tree against origin/main

tangletools
tangletools previously approved these changes Jul 9, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 0e2050f4

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-07-09T21:39:23Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Concerns 2 (1 low, 1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 138.5s (2 bridge agents)
Total 138.5s

💰 Value — sound

Bumps agent-bench to 0.1.1 against runtime ^0.90.1 and adds a consumer-faithful typecheck + packed-tarball consumer gate (the repo's own verify:package convention) so a dev-only path alias can never again mask a manifest/lockfile drift for downstream consumers — the right, minimal, in-grain fix; s

  • What it does: Fixes a real packaging drift and adds gates so it can't recur. (1) bench/package.json:23-24 moves @tangle-network/agent-runtime from ^0.89.0 → ^0.90.1 and refreshes bench/pnpm-lock.yaml — the genuine fix, since bench/src/run-benchmarks.ts:217 uses OpenSandboxRunOptions.beforeStart, which only exists in current runtime (src/runtime/sandbox-run.ts:139, :319) and was absent from the 0.89.x the lockfi
  • Goals it achieves: Make the published @tangle-network/agent-bench artifact actually compile for downstream consumers, and make the class of bug that broke it un-shippable. Before: bench's own typecheck used the dev paths alias to local runtime source, so it green-lit against 0.90.x API while the manifest/lockfile still installed 0.89.0 — and bench had zero CI coverage (pre-PR ci.yml had only the root job). After:
  • Assessment: Good on its merits and in the codebase's grain. It names the real root cause (alias-masked version drift) and applies the lightest direct fix (a no-alias public tsconfig) plus the ground-truth pack-and-consume check — reusing the repo's already-documented verify:package / no-soft-gates convention (docs/MAINTAINING.md:59; root package.json:95; root scripts/verify-package-exports.mjs), including t
  • Better / existing approach: None — this is the right approach. I checked for an existing equivalent to reuse: root scripts/verify-package-exports.mjs (root verify:package, package.json:95) is the only pack-and-consume verifier, but it is runtime-specific — it hardcodes the runtime export map (scripts/verify-package-exports.mjs:32-40) and does a runtime named-export import() probe (:54-100) via symlink, not a TypeScript c
  • Model: opencode/kimi-for-coding/k2p7
  • Bridge attempts: 1

🎯 Usefulness — sound

A correct, in-grain fix: bumps bench's runtime dep to the version whose beforeStart API its own source already uses, and ports the established verify:package guard so the drift can never silently recur.

  • Integration: Fully wired and reachable. The dependency bump (^0.89.0 -> ^0.90.1, bench/package.json:24) resolves a real, proven break: bench/src/run-benchmarks.ts:217 assigns runOptions.beforeStart, a field added to OpenSandboxRunOptions in src/runtime/sandbox-run.ts:139 by commit 9c9c111 (0.90.x line). The dev tsconfig alias hid this; a public consumer of @tangle-network/agent-bench 0.1.0 against runtime 0.89
  • Fit with existing patterns: Idiomatic — follows the codebase's own established pattern. bench/scripts/verify-packed-consumer.mjs is a direct adaptation of the root scripts/verify-package-exports.mjs (the root's verify:package script at package.json:95), used in the root publish.yml:41. The bench variant is appropriately more thorough — it runs a real npm install of the packed tarball in a clean consumer and a real tsc compil
  • Real-world viability: Holds up off the happy path. verify-packed-consumer.mjs surfaces exec failures (dumps stdout/stderr at lines 15-19), cleans its scratch dir in a finally block (line 84-86), uses --ignore-scripts/--no-audit/--no-fund for a safe fast install, and the publish step is idempotent (skips already-published versions, publish.yml). It compiles against the registry-resolved runtime, not the dev alias, so it
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: console debug added bench/scripts/verify-packed-consumer.mjs

  • console.log(

🎯 Usefulness Audit

🟡 Packed-consumer smoke imports only the registry surface, not the beforeStart-using path [robustness] ``

bench/scripts/verify-packed-consumer.mjs:52 imports only resolveAdapter + BenchmarkAdapter and calls resolveAdapter('swe-bench'); it does not exercise runBenchmarks/OpenSandboxRunOptions, which is the path that broke. This is NOT a coverage gap today: typecheck:public compiles src/index.ts (which re-exports runBenchmarks), so the beforeStart drift is caught transitively. Noting only as a defense-in-depth option — a consumer import that also touches runBenchmarks would mirror the exact failure mo


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260709T214208Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 58a6680b

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-07-09T21:43:24Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Value Audit — sound-with-nits

Verdict sound-with-nits
Concerns 2 (1 low, 1 weak-concern)
Heuristic 0.0s
Duplication 0.0s
Interrogation 150.0s (2 bridge agents)
Total 150.0s

💰 Value — sound-with-nits

Bumps agent-bench to runtime ^0.90.1 and adds a public-entrypoint typecheck plus a packed-clean-consumer compile so the dev paths alias can never again hide a manifest/runtime drift — a coherent, in-grain fix with a gate that was empirically shown to reproduce the exact bug.

  • What it does: Three concrete deltas. (1) bench/package.json declares @tangle-network/agent-runtime ^0.90.1 (was ^0.89.0) and refreshes bench/pnpm-lock.yaml so the resolved runtime is 0.90.1 (lockfile diff confirms agent-runtime@0.89.00.90.1). (2) New bench/tsconfig.public.json (files:[src/index.ts], no paths) + typecheck:public script compiles ONLY the public entrypoint against instal
  • Goals it achieves: Eliminate the class of bug where the dev paths alias in bench/tsconfig.json:14-17 (@tangle-network/agent-runtime/loops../src/runtime/index.ts, the live monorepo source) let bench compile against in-tree runtime while its manifest still shipped ^0.89.0, so published consumers resolved a runtime whose OpenSandboxRunOptions.beforeStart shape (src/runtime/sandbox-run.ts:139) differed
  • Assessment: Good on its merits and in the codebase's grain. The fix attacks the root cause (manifest↔source skew hidden by a dev alias) rather than the symptom, and crucially the new gate was proven to catch it: the PR's public-entrypoint compile reproduced the three run-benchmarks.ts errors it exists to prevent, so this is not a no-op check. The publish workflow faithfully mirrors the existing runtime OIDC
  • Better / existing approach: none — this is the right approach. I checked for an existing equivalent before concluding: root scripts/verify-package-exports.mjs (bound to root verify:package in package.json:95) also pnpm packs, but it is runtime-specific — it hardcodes @tangle-network/agent-runtime subpaths and export-name lists (scripts/verify-package-exports.mjs:32-100) and only validates export-shape plus a runt
  • Model: opencode/kimi-for-coding/k2p7
  • Bridge attempts: 1

🎯 Usefulness — sound

A focused release fix that bumps agent-bench to 0.1.1 against runtime 0.90.1 and adds public-entrypoint + packed-consumer verification that directly prevents the dev-alias-hid-version-drift failure it targets; fully wired into CI and a new OIDC publish path.

  • Integration: Fully reachable. The new typecheck:public and verify:package scripts are invoked by a new agent-bench CI job on every push (.github/workflows/ci.yml:43-66) and gated before publish in publish.yml (verify-agent-bench at :96 needs→publish-agent-bench at :132, both keyed to agent-bench-v* tags). The root verify job is correctly scoped off bench tags (publish.yml:11 `if: startsWith(github.re
  • Fit with existing patterns: Fits the codebase's established per-package verify+publish grain precisely. The runtime root already ships scripts/verify-package-exports.mjs (pack → unpack → assert exports map + runtime-import named symbols) wired into its own verify job and v* publish stream; the new verify-packed-consumer.mjs is the appropriate sibling for a source-shipping package whose consumers TYPE-CHECK against transi
  • Real-world viability: Holds up on the realistic paths. The packed-consumer (verify-packed-consumer.mjs:28-83) does a real npm pack + file: tarball install + tsc compile of an actual import against the registry-resolved runtime, with --ignore-scripts and a tag/version lock (publish.yml:120-127) — this is the strongest available proof the published tarball compiles for a downstream consumer. It fails loud on regi
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: console debug added bench/scripts/verify-packed-consumer.mjs

  • console.log(

💰 Value Audit

🟡 packed-consumer check logs but does not assert the resolved runtime version [maintenance] ``

bench/scripts/verify-packed-consumer.mjs:78-83 reads node_modules/@tangle-network/agent-runtime/package.json and only console.logs the resolved runtime version; it never fails if the registry resolved something older than the manifest's ^0.90.1 floor. Since the entire point of the PR is proving the consumer gets the runtime the manifest declares, a one-line assertion that the resolved version satisfies the declared range (or equals an expected minimum) would make the gate fail-loud on th


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260709T214616Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 58a6680b

Review health 100/100 · Reviewer score 63/100 · Confidence 85/100 · 14 findings (2 medium, 12 low)

deepseek kimi-code aggregate
Readiness 63 71 63
Confidence 85 85 85
Correctness 63 71 63
Security 63 71 63
Testing 63 71 63
Architecture 63 71 63

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Bench test files exist but no CI/publish execution — .github/workflows/ci.yml

bench/ has 7 test files (corpus.test.mts, gate.test.mts, refine-loop.test.mts, selector.test.mts, run-benchmarks.test.mts, swe-bench-env.test.ts, tb-container-executor.test.mts) and HARNESS.md documents them as runnable (e.g. node --test --import tsx src/{selector,refine-loop}.test.mts, tsx src/gate.test.mts), but bench/package.json has no "test" script. The new CI agent-bench job runs typecheck + verify:package but skips tests entirely. The publish verify-agent-bench job also skips them. This means the bench package can be published with broken tests and CI won't catch it. Fix: add a "test" script to bench/package.json and a - name: Test step to both CI and publish verify.

🟠 MEDIUM Duplicate agent-eval and agent-interface versions in dependency tree — bench/pnpm-lock.yaml

Two versions of @tangle-network/agent-eval coexist: 0.106.3 (direct dep of bench + agent-runtime) and 0.108.1 (transitive via agent-knowledge@1.11.1→agent-eval). Two versions of @tangle-network/agent-interface coexist: 0.14.0 (direct) and 0.10.1 (via agent-eval@0.108.1). Under 0.x semver, minor bumps can include breaking changes. If agent-runtime passes a 0.106.3-typed value to agent-knowledge expecting 0.108.1 types (or vice versa), runtime type mismatches are possible. Verify that agent-knowledge's agent-eval dependency range could accept 0.106.3 to deduplicate, or confirm the two versions are compatible at the interface boundary.

🟡 LOW OIDC trusted-publisher prerequisite for @tangle-network/agent-bench is undocumented and unverifiable from the diff — .github/workflows/publish.yml

publish-agent-bench (lines 153-161) publishes with --provenance using the same tokenless OIDC flow as publish-npm, whose comment (lines 75-82) spells out the hard prerequisite: an npmjs Trusted Publisher for org tangle-network / repo agent-runtime / workflow publish.yml. OIDC trusted publishers are registered per-package, so the brand-new @tangle-network/agent-bench needs its OWN trusted-publisher entry on npmjs pointing at this repo + publish.yml before the first tag push will authenticate. That configuration lives outs

🟡 LOW agent-bench jobs omit the workflow_dispatch recovery path that the main publish jobs have — .github/workflows/publish.yml

publish.yml:12 gates the main verify job with startsWith(github.ref, 'refs/tags/v') || github.event_name == 'workflow_dispatch', and publish-npm (line 57) repeats the same escape hatch. The new verify-agent-bench (line 98) and publish-agent-bench (line 134) gate ONLY on startsWith(github.ref, 'refs/tags/agent-bench-v') with no || github.event_name == 'workflow_dispatch' clause. Effect: a manual

🟡 LOW publish-agent-bench setup-node missing cache config — .github/workflows/publish.yml

The verify-agent-bench job's setup-node step includes cache: pnpm + cache-dependency-path: bench/pnpm-lock.yaml (line 110-111), but the publish-agent-bench job's setup-node step (line 147-149) omits cache config entirely. Trivial impact since publish is infrequent, but inconsistent with its sibling jobs.

🟡 LOW bench devDependency ranges diverge from root — bench/package.json

bench declares @types/node: ^25.0.0 and typescript: ^6.0.3, while root package.json pins @types/node: ^25.9.3 and typescript: ^5.7.0 (verified). The ^25.0.0 lower bound is looser than root and the TypeScript major (6 vs 5) differs across the workspace. Both currently resolve cleanly via pnpm-lock (bench -> @types/node 25.9.5), so there is no functional break, but the skew risks future resolution drift and two TS majors in one repo. Non-blocking; consider aligning bench ranges to root (^25.9.3 / a single TS major) for consistency.

🟡 LOW @types/node major (25) ahead of declared Node engine targets (>=20) — bench/pnpm-lock.yaml

New devDependency @types/node resolves to 25.9.5 (importer lines 21-23, packages line 327), while the @tangle-network packages being typed declare engines node '>=20' (e.g. agent-runtime line 283, agent-eval line 259). Typing the public consumer against Node 25 lib types can admit APIs/globals absent on

🟡 LOW Two versions of @tangle-network/agent-eval now coexist in the bench consumer — bench/pnpm-lock.yaml

The importer still pins the direct/peer dep to 0.106.3 (lines 12-13, snapshot 567-583), but agent-runtime@0.90.1's new hard dependency on @tangle-network/agent-knowledge@1.11.1 (line 631) pulls a second copy, @tangle-network/agent-eval@0.108.1 (packages line 262, snapshot 585-601, referenced at line 617)

🟡 LOW Consumer tsconfig is a second, drifting source of truth for the public typecheck — bench/scripts/verify-packed-consumer.mjs

Lines 55-73 inline a full compilerOptions object (module/target/lib/moduleResolution/strict/noEmit/skipLibCheck/types) that conceptually duplicates bench/tsconfig.public.json used by the typecheck:public script. Because tsconfig.public.json is not listed in bench/package.json files (lines 32-39), it is not in the packed tarball, so the consumer cannot extends it and the duplication is unavoidable — but the two configs can now drift. The inline config also omits constraints the public config may ca

🟡 LOW Failure path hides the command and routes child stdout to stderr — bench/scripts/verify-packed-consumer.mjs

In the catch block (lines 15-19), error.stdout is written to process.stderr and the rethrown exec error does not include the invoked command/args. In CI this makes it ambiguous whether npm pack, npm install, or npm exec tsc failed, and child stdout is misattributed to the stderr stream. Fix: write stdout to process.stdout, and before rethrowing wrap/annotate the error with command + args.join(' ') (e.g. error.message = ${command} ${args.join(' ')} failed: ${error.message}`).

🟡 LOW No timeout on external commands — bench/scripts/verify-packed-consumer.mjs

execFileAsync is called without a timeout option. On a slow/hung npm registry, npm install or tsc could block indefinitely in CI. Add a timeout (e.g., 120_000 ms for install, 60_000 for tsc) to fail fast with a clear error.

🟡 LOW Unguarded devDependency reads can silently drop typescript/@types/node — bench/scripts/verify-packed-consumer.mjs

Lines 42-43 read manifest.devDependencies['@types/node'] and manifest.devDependencies.typescript with no existence check. If either key is ever renamed/removed in bench/package.json, the value is undefined, JSON.stringify omits the key from the generated consumer package.json, and the consumer installs without typescript. Then line 77 npm exec -- tsc -p tsconfig.json will not fail on the missing dependency — npm exec will attempt to download typescript from the registry (network) instead. The ve

🟡 LOW manifest.devDependencies accessed without null guard — bench/scripts/verify-packed-consumer.mjs

Lines 41-43 access manifest.devDependencies['@types/node'] and manifest.devDependencies.typescript without checking manifest.devDependencies exists. If devDependencies is ever removed from bench/package.json, this throws TypeError: Cannot read properties of undefined. Use manifest.devDependencies?.typescript with a fallback, or add an explicit guard with a descriptive error.

🟡 LOW runtimeManifest read lacks guard — bench/scripts/verify-packed-consumer.mjs

The script reads node_modules/@tangle-network/agent-runtime/package.json and call it a verification, but if agent-runtime fails to install as a transitive dependency, the readFile throw is an opaque ENOENT rather than a clear assertion failure. Consider wrapping with a try/catch that produces a message like 'agent-runtime not found in transitive deps' — or intentionally let it crash with a descriptive error by checking existence first.


tangletools · 2026-07-09T21:55:01Z · trace

tangletools
tangletools previously approved these changes Jul 9, 2026

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 14 non-blocking findings — 58a6680b

Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-07-09T21:55:01Z · immutable trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 53f5e8c4

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-07-09T21:57:18Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Concerns 1 (1 low)
Heuristic 0.0s
Duplication 0.0s
Interrogation 114.5s (2 bridge agents)
Total 114.5s

💰 Value — sound

Releases agent-bench 0.1.1 pinned to runtime ^0.90.1 and adds a public-entrypoint typecheck plus a clean-consumer pack/install/compile gate and an isolated OIDC release lane — a coherent, in-grain fix for a real consumer-facing breakage, with no existing equivalent to reuse.

  • What it does: Bumps @tangle-network/agent-bench 0.1.0→0.1.1 and its deps agent-runtime ^0.89.0→^0.90.1 / agent-eval ^0.106.3→^0.108.1 with a refreshed bench/pnpm-lock.yaml (bench/package.json:2,24-25); adds bench/tsconfig.public.json (no paths alias, files:[src/index.ts]) so the public entrypoint compiles against the INSTALLED registry packages instead of the dev alias in bench/tsconfig.json:14-17
  • Goals it achieves: Make the published agent-bench actually installable and compilable for real registry consumers. The dev tsconfig paths alias (@tangle-network/agent-runtime/loops → ../src/runtime/index.ts) masked that agent-bench source used OpenSandboxRunOptions.beforeStart (runtime ≥0.90 only) while its manifest still allowed ^0.89.0, so a consumer would resolve 0.89.0 and fail to compile; the fix forces
  • Assessment: Sound and in the codebase's grain. The root runtime already enforces the same publish philosophy via scripts/verify-package-exports.mjs + package.json:95 verify:package, run in CI/publish (publish.yml:41); this change extends that pattern one step further — instead of only asserting export-field shapes, it actually installs the tarball in an empty consumer and compiles against resolved deps,
  • Better / existing approach: none — this is the right approach. I checked for prior art: scripts/verify-package-exports.mjs is the nearest equivalent but does a materially different, weaker job (untar + assert exports shapes + symlink into a hand-built node_modules + dynamic-import specific symbols); the bench script's real npm install file: + consumer tsc exercises the resolver and the semver range, which is the prec
  • Model: opencode/kimi-for-coding/k2p7
  • Bridge attempts: 1

🎯 Usefulness — sound

Real latent type-drift fix (runtime 0.89→0.90.1 for beforeStart) plus regression prevention (public-entrypoint typecheck + clean-consumer packed-tarball compile + isolated agent-bench-v* OIDC publish) that mirrors the existing root-package release pattern.

  • Integration: Fully wired and reachable. The new typecheck:public, verify:package, and test scripts are invoked by both .github/workflows/ci.yml (agent-bench job, runs on every PR) and .github/workflows/publish.yml (verify-agent-bench + publish-agent-bench jobs, on agent-bench-v* tags). The tag namespaces are correctly isolated: root jobs gate on refs/tags/v* and agent-bench-v* does not start with v
  • Fit with existing patterns: Matches the established grain precisely. The root agent-runtime package already ships a verify:package script + OIDC trusted-publish path on v* tags (publish.yml publish-npm job with the same npm@11 provenance dance). This PR replicates that exact shape for the nested package: separate pnpm-lock.yaml, separate agent-bench-v* tag namespace, same pack→install→tsc verify idiom, same version-loc
  • Real-world viability: Holds up off the happy path. verify-packed-consumer.mjs uses a 120s per-command timeout, --ignore-scripts install, fail-loud if runtime isn't installed (lines 81-88), pins typescript/@types/node from the bench manifest so the consumer compiles, and cleans scratch in finally. The test script sets TSX_TSCONFIG_PATH=tsconfig.public.json so test execution ALSO resolves against installed packages
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: console debug added bench/scripts/verify-packed-consumer.mjs

  • console.log(

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260709T215939Z

@tangletools

Copy link
Copy Markdown
Contributor

❌ Needs Work — 53f5e8c4

Review health 100/100 · Reviewer score 38/100 · Confidence 85/100 · 18 findings (1 high, 2 medium, 15 low)

opencode-kimi glm deepseek aggregate
Readiness 38 71 86 38
Confidence 85 85 85 85
Correctness 38 71 86 38
Security 38 71 86 38
Testing 38 71 86 38
Architecture 38 71 86 38

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision.

Blocking

🔴 HIGH New test script silently skips 8 of 16 test files — bench/package.json

The script enumerates exactly 8 files: appworld, external-adapters, rag-benchmarks, corpus, gate, refine-loop, run-benchmarks, selector. But bench/src contains 16 test files. Present-but-omitted: src/benchmarks/aec-bench.test.mts, commit0.test.mts, dabstep.test.mts, enterpriseops-gym.test.mts, programbench.test.mts, terminal-bench.test.mts, src/swe-bench-env.test.ts, src/tb-container-executor.test.mts. Spot-check confirms the omitted benchmark tests are plain node:test unit tests against fixtures (e.g. commit0.test.mts:54 asserts 'preflight FAILS LOUD'; aec-bench.test.mts:47 asserts 'empty artifact fails closed to 0') — exactly the fail-closed/judge behavior this package sells in its description, now unrun by npm test. No test:unit/test:integration split exists to justify the curation. F

Other

🟠 MEDIUM Smoke cannot detect a dropped/renamed adapter (registry is string-typed) — bench/scripts/verify-packed-consumer.mjs

The consumer's only adapter touch is const adapter: BenchmarkAdapter = resolveAdapter('swe-bench'). Because bench/src/adapters.ts types ADAPTERS as Record<string, () => BenchmarkAdapter> (line 40) and resolveAdapter(key: string) (line 82), the key is the wide string type — so this line still compiles even if 'swe-bench' (or any key) is removed from the registry. Impact: the gate can go green while a published adapter disappears, giving false confidence about the public surface. The lever is upstream

🟠 MEDIUM Verification is non-hermetic: floats against latest registry, not the locked deps — bench/scripts/verify-packed-consumer.mjs

The consumer copies only the semver ranges ('@types/node': '^25.0.0', typescript: '^6.0.3', lines 54-57) and installs with --package-lock=false (line 79), while @tangle-network/{agent-runtime,agent-eval,sandbox} are pulled fresh from the registry (bench/package.json deps are ^0.90.1/^0.108.1/^0.9.7). Result: pass/fail depends on the registry state at run time, not on this PR's locked bench/pnpm-lock.yaml — a green run does not prove the pinned set a lockfile-consumer would get, and a red run

🟡 LOW Bench CI has no lint/format check — .github/workflows/ci.yml

The root ci job runs pnpm run lint (biome), but the new agent-bench job has no lint step. bench/package.json has no lint script at all. This is a package-level gap — if biome conventions are desired here, add a lint script to bench and a step to CI. Not a CI correctness issue.

🟡 LOW CI agent-bench job exercises the published agent-runtime, not the PR's in-repo changes — .github/workflows/ci.yml

The new ci job runs pnpm install --frozen-lockfile in bench/, and bench/package.json:25 depends on '@tangle-network/agent-runtime': '^0.90.1' resolved from the registry (there is no pnpm-workspace.yaml, so the local source is not linked). A PR that breaks agent-runtime's public API will therefore still pass this job, because it typechecks/tests against the last published version; the breakage only surfaces when bench later bumps its dependency and is released under its own agent-bench-v* tag. This appears intentional (the step is literally named 'Install published dependencies' and bench has a decoupled release train), so it is not a defect — but the verifier should be aware this job validates the published consumer contract, not in-repo agent-runtime changes. If the intent is also to ca

🟡 LOW Comment about re-running via workflow_dispatch is slightly misleading — .github/workflows/publish.yml

The comment 'Manual dispatch remains root-only. Re-run a failed bench tag workflow so tag/version locking stays intact...' implies workflow_dispatch can re-run a bench publish. In reality, verify-agent-bench and publish-agent-bench are gated on startsWith(github.ref, 'refs/tags/agent-bench-v'), and on workflow_dispatch the ref is the branch (e.g. refs/heads/main), not a tag — so manual dispatch never triggers the bench jobs. Re-runs must use the Actions UI 'Re-run failed jobs' on the original tag-push run (which preserves the tag ref). The code is correct; only the comment could confuse an operator into trying workflow_dispatch to recover a failed bench publish. Suggest rewording to 'Re-run via the Actions UI on the original tag-push run; workflow_dispatch only covers the roo

🟡 LOW No concurrency group on publish workflow — .github/workflows/publish.yml

If two agent-bench-v* tags are pushed in quick succession (or a tag is re-pushed), both publish-agent-bench jobs run concurrently. The npm view "$NAME@$VERSION" pre-check makes the actual publish idempotent (second run sees the version and skips), but a race between the npm view check and the npm publish PUT could theoretically allow a double-attempt — npm registry rejects the second as a conflict, so this is a cleanliness nit, not a correctness bug. Consider adding concurrency: { group: publish-agent-bench-${{ github.ref }}, cancel-in-progress: false } at the job or workflow level. The root publish jobs have the same gap.

🟡 LOW verify-agent-bench lacks explicit permissions block — .github/workflows/publish.yml

The job has no permissions: block, so it inherits the repo-default GITHUB_TOKEN permissions. If the repo default is permissive (read-write), the job holds more scope than it needs (it only does checkout + install + test). Add permissions: { contents: read } for least-privilege. Note: the pre-existing verify job has the same gap, so this is a repo-wide pattern issue, not introduced by this PR — but extending the pattern to a new job warrants the fix. Same nit applies to the agent-bench job in ci.yml line 44.

🟡 LOW Hardcoded test-file list in test script silently excludes new tests — bench/package.json

The test script enumerates 8 specific .test.mts files instead of using node --test 'src/**/*.test.mts' (or a glob). Any new test file added later will not be picked up by CI unless the script is also edited — a silent coverage gap. Low impact today (8 files listed), but a footgun as the suite grows. Fix: node --test --import tsx 'src/**/*.test.mts' (node 21+ supports glob), or document the convention in bench/HARNESS.md.

🟡 LOW typecheck:public never checks the test files the package now runs — bench/package.json

tsconfig.public.json sets "files": ["src/index.ts"] (read at bench/tsconfig.public.json:15), so tsc -p tsconfig.public.json typechecks only the package entry point. The 8 (or 16) .test.mts files are never typechecked, yet the new test script relies on tsx type-stripping at runtime, masking type errors. Also the test script points TSX_TSCONFIG_PATH at this entry-point-only tsconfig, which is semantically the public-surface config, not a test config. Fix: add a test tsconfig (or broaden include) and a typecheck:test step so the suite that gates publish is actually type-safe.

🟡 LOW @types/node pinned to v25, ahead of the Node 20 engine floor — bench/pnpm-lock.yaml

Resolved @types/node@25.9.5 (specifier ^25.0.0 set in package.json) corresponds to Node 25 type defs, while agent-runtime/agent-eval declare engines node>=20. This is a dev-only type-definitions package so it cannot break runtime, and it was an explicit choice in package.json (not a lockfile defect). Noting only so reviewers are aware the type surface is ahead of the runtime floor; no action required in this lockfile shot.

🟡 LOW @types/node@25 resolved but CI runs Node 22 — bench/pnpm-lock.yaml

package.json specifies ^25.0.0 which resolved to 25.9.5 in the lockfile. CI (ci.yml line 50) uses node-version: 22. @types/node@25 includes Node 25 APIs not present at Node 22 runtime. TypeScript will accept Node-25-only APIs that fail at runtime on Node 22. Mitigation: CI typechecks only the public entrypoint (src/index.ts) and also runs tests which would catch real mismatches, so practical risk is low. Consider pinning @types/node to match the CI runtime version (^22.x).

🟡 LOW 120s per-command timeout may be tight for cold-cache npm install in CI — bench/scripts/verify-packed-consumer.mjs

maxBuffer 10MB is fine, but timeout: 120_000 applies to each execFile call. npm install pulling typescript + @types/node + the packed tarball's transitive graph can exceed 2min on a cold registry. If this fires the error message is just 'npm install ... failed' without indicating it was a timeout. Consider 180-240s for the install step, or pass through error.code === 'ETIMEDOUT' in the rethrown message.

🟡 LOW Error message doesn't identify which devDep is missing — bench/scripts/verify-packed-consumer.mjs

The guard on lines 35-42 throws a single message 'requires @types/node and typescript devDependencies' regardless of whether one or both are missing/falsy. If only one is missing, the developer has to inspect package.json manually to find which. The check logic itself is correct (handles undefined, missing, and empty-string cases via lines 36-39), but the error should say which specific dependency failed — e.g., build the message from the individual failures rather than a blanket statement.

🟡 LOW Only one of three declared runtime deps is spot-checked — bench/scripts/verify-packed-consumer.mjs

bench/package.json declares @tangle-network/agent-eval, agent-runtime, and sandbox as dependencies, but the verifier only asserts node_modules/@tangle-network/agent-runtime/package.json exists. A typo or missing dep in agent-eval/sandbox wouldn't be caught here. Cheap to extend: also stat the other two manifests (or loop over Object.keys(manifest.dependencies)).

🟡 LOW Success message overstates what was verified — bench/scripts/verify-packed-consumer.mjs

Lines 81-91 only confirm that node_modules/@tangle-network/agent-runtime/package.json exists and print its version; they never assert the installed runtime version satisfies the declared ^0.90.1 range, and never check @tangle-network/agent-eval or @tangle-network/sandbox at all. The message "packed consumer verified: ... with agent-runtime@" reads as a stronger guarantee than presence-of-one-transitive-dep. Fix: assert the resolved version satisfies manifest.dependencies['@tangle-network/agent-runtime'] (e.g. via semver.satisfies) and confirm agent-eval/sandbox resolved, or soften the log to "installed" rather than "verified".

🟡 LOW finally-block rm can mask the original throw if cleanup fails — bench/scripts/verify-packed-consumer.mjs

If the body throws and the subsequent rm(scratch, {recursive:true, force:true}) also throws (e.g. EBUSY, permissions on a node_modules file), the rm error supersedes the real failure and the CI log shows a misleading cleanup error instead of the pack/install/tsc failure. Wrap rm in try/catch (or rmSync with a swallowed error) so the original error propagates.

🟡 LOW skipLibCheck:true narrows what the public typecheck proves about published deps — bench/tsconfig.public.json

The whole point of this config is to typecheck the public entry against the installed @tangle-network/agent-runtime ./loops export (the internal tsconfig.json:14-17 path-mapping is intentionally dropped). With "skipLibCheck": true, tsc will not validate the published dependency declaration files themselves, so a broken/inconsistent ./loops .d.ts in the published package would still pass typecheck:public. This is a pragmatic, near-universal default and the gap is closed by verify:package (bench/scripts/verify-packed-consumer.mjs:79-80), which does a real npm pack + clean install + tsc against the tarball. Not a blocker — flagging only so the public check's guarantee is stated accurately (it proves export-map resolution, not dependency .d.ts soundness). If you want the public


tangletools · 2026-07-09T22:09:09Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ 1 Blocking Finding — 53f5e8c4

Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 6 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-07-09T22:09:09Z · immutable trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 1acfa58d

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-07-09T22:16:01Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Concerns 1 (1 low)
Heuristic 0.0s
Duplication 0.0s
Interrogation 155.3s (2 bridge agents)
Total 155.3s

💰 Value — sound

Releases agent-bench 0.1.1 pinned to the runtime API it actually calls (^0.90.1) and adds typecheck/test/packed-consumer gates that compile against installed packages instead of the dev source-alias that hid the skew — a coherent, in-grain fix with no existing equivalent to reuse.

  • What it does: Bumps bench deps to what its source really uses (@tangle-network/agent-runtime ^0.89.0→^0.90.1, agent-eval ^0.106.3→^0.108.1; bench/package.json:24-25) and closes the verification gap that let the skew ship. The dev bench/tsconfig.json:14-17 maps @tangle-network/agent-runtime/loops → ../src/runtime/index.ts (local source), so code like OpenSandboxRunOptions.beforeStart at bench/src/run-benchmarks.
  • Goals it achieves: Make a published @tangle-network/agent-bench actually compile and run for a real registry consumer by (1) declaring the runtime/eval versions its code calls, (2) proving the public entrypoint typechecks against installed packages rather than the local-source alias, (3) proving the packed tarball installs and executes in a clean consumer that resolves declared semver ranges, and (4) giving bench it
  • Assessment: Good on its merits and squarely in the codebase's grain. The repo already treats 'verify the packed artifact' as a first-class gate (root scripts/verify-package-exports.mjs + verify:package + a CI step at ci.yml:37-38); this PR mirrors that exact convention for bench (same script name, same CI posture) and copies the root OIDC publish recipe verbatim (npm@11 pin, Trusted-Publisher comment, idemp
  • Better / existing approach: none — this is the right approach. Checked the existing scripts/verify-package-exports.mjs: it is producer-side for the RUNTIME (pnpm pack, validate the exports map at lines 32-52, symlink the unpacked dir into node_modules at line 54, spot-import named exports) and would not have caught bench's cross-package version skew, since it never installs bench nor resolves declared ranges from a registry.
  • Model: opencode/kimi-for-coding/k2p7
  • Bridge attempts: 1

🎯 Usefulness — sound

A correctly-scoped release-hardening that fixes a real version-drift bug (bench source used runtime 0.90's beforeStart while its lockfile pinned 0.89.0, hidden by a dev tsconfig paths alias) and adds the exact CI/publish gates that catch it recurring.

  • Integration: Fully wired and reachable. The three new scripts are exposed as bench package.json scripts (test, typecheck:public, verify:package) and consumed by a new agent-bench CI job (.github/workflows/ci.yml:43-71) running on every push, plus new verify-agent-bench/publish-agent-bench jobs in publish.yml:99,139 gated on agent-bench-v* tags. The tag gating cleanly separates the two relea
  • Fit with existing patterns: Fits the codebase grain. Bench is a standalone publishable package (own name @tangle-network/agent-bench, own pnpm-lock.yaml), and the root vitest.config.ts explicitly excludes bench/**, so bench was never part of the root vitest run. Using Node's built-in node --test runner and an independent OIDC release path is therefore consistent with bench being a leaf artifact, not a competing pat
  • Real-world viability: Holds up because the verification chain mirrors real consumer conditions. tsconfig.public.json drops the dev paths alias (compare bench/tsconfig.json which still maps @tangle-network/agent-runtime/loops../src/runtime/index.ts), so typecheck:public compiles against installed packages only — the alias that hid the original bug is gone. The consumer verify resolves declared ranges via
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: console debug added bench/scripts/run-package-tests.mjs

+console.log(package tests passed: ${tests.length}/${tests.length} files)


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260709T222351Z

@tangletools

Copy link
Copy Markdown
Contributor

✅ No Blockers — 1acfa58d

Review health 100/100 · Reviewer score 61/100 · Confidence 90/100 · 18 findings (4 medium, 14 low)

opencode-kimi glm deepseek aggregate
Readiness 66 61 76 61
Confidence 90 90 90 90
Correctness 66 61 76 61
Security 66 61 76 61
Testing 66 61 76 61
Architecture 66 61 76 61

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 6/6 planned shots over 9 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 6/6 planned shots over 9 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 6/6 planned shots over 9 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Dead python3 venv creation adds phantom hard dependency to Node test runner — bench/scripts/run-package-tests.mjs

Lines 41-45 create/access bench/.venv via python3 -m venv .venv but the python variable is never referenced again — tests are launched with process.execPath (node) --test. This makes python3 a hard prerequisite for pnpm test (a release gate in publish.yml:122 and ci.yml:66) for zero functional benefit. If python3 is absent from a CI image or contributor machine, the entire bench test suite fails before running any test. Confirmed no test file (.test.mts/.test.ts under src/) reads bench/.venv at runtime (grep'd: only terminal-compare.ts:39 uses .venv/bin/tb, and that is a non-test CLI script). Fix: delete [lines 41-45](https://github.com/tangle

🟠 MEDIUM Empty, unused bench/.venv makes python3 a hard dep of pnpm testbench/scripts/run-package-tests.mjs

Lines 38-43 unconditionally run python3 -m venv .venv when bench/.venv/bin/python is absent, but the runner never references that interpreter (it only runs node --test --import tsx ..., line 49), and the venv is never populated (no pip install/requirements). Benchmark sources that do read bench/.venv/bin/python (benchmarks/frames.ts:32, benchmarks/simpleqa.ts:37) require datasets/requests and fail loud in preflight; the migrated node:test suite (e.g., swe-bench-env.test.ts now uses node:test) is offline/path-

🟠 MEDIUM Hardcoded node_modules path assumes flat dependency hoisting — bench/scripts/verify-packed-consumer.mjs

The check reads node_modules/@tangle-network/agent-runtime/package.json directly, assuming npm always flattens transitive dependencies to the consumer's top-level node_modules. While npm 3+ defaults to flattening and the current single-consumer topology makes nesting unlikely, this is not a guaranteed contract. If a future agent-bench version uses bundledDependencies, or npm behavior changes, the path will be wrong and produce a false CI failure. Fix: resolve the module path programmatically — e.g., await run('node', ['-e', console.log(require.resolve('@tangle-network/agent-runtime/package.json',{paths:['${consumerDir}']}))], consumerDir) then use the resolved path in readFile. Current probability of triggering: low (single consumer, no version conflicts). Impact if triggered: conf

🟠 MEDIUM Packed-consumer verifier only proves tsx/bundler consumers, not generic node/tsc — bench/scripts/verify-packed-consumer.mjs

The generated consumer tsconfig copies publicTsconfig.compilerOptions (tsconfig.public.json: moduleResolution 'bundler', allowImportingTsExtensions true) and the script runs the consumer with tsx (line 86) and typechecks with tsc (line 85). But bench/package.json publishes raw TypeScript: main=src/index.ts, types=src/index.ts, exports['.']='./src/index.ts', exports['./benchmarks/']='./src/benchmarks/.ts'. A normal consumer using node (not tsx) resolves exports -> src/index.ts and node fails with 'Unknow

🟡 LOW agent-bench CI job always runs regardless of file changes — .github/workflows/ci.yml

The agent-bench job has no paths filter and runs on every push/PR to main, even when no files under bench/ changed. This mirrors how the root ci job works, so it is consistent, but it wastes CI minutes on unrelated changes. Add paths: ['bench/**'] (or include the root lockfile if bench deps are workspace-linked) to skip bench CI when bench files are untouched.

🟡 LOW ci.yml agent-bench job has no explicit permissions block — .github/workflows/ci.yml

The new agent-bench job inherits the repo-default GITHUB_TOKEN permissions. It only performs checkout + install + typecheck + test + verify:package (all read-only), so it should declare permissions: contents: read for least-privilege, matching the posture of the publish jobs which do set explicit permissions. Low impact (no writes performed), but defense-in-depth. Fix: add a permissions: { contents: read } block under jobs.agent-bench.

🟡 LOW Bench package ships raw TypeScript source via publish workflow (no build step) — .github/workflows/publish.yml

publish-agent-bench runs pnpm install then npm publish with no pnpm run build in between (unlike publish-npm which builds the root dist). This is consistent with bench/package.json whose main/types/exports all point at .ts files (consumers must use a TS-aware loader like tsx). The verify-agent-bench job's verify:package step validates a clean consumer can resolve the packed tarball, so this is gated. Flagging only because shipping unbuilt .ts is unusual and ties the package to tsx/ts-node consumers; not a workflow defect.

🟡 LOW Floating npm major install widens supply-chain surface — .github/workflows/publish.yml

npm install -g npm@11 resolves and installs the latest npm 11.x from the registry on every publish run with no exact version or integrity pin. This mirrors the root job (line 87) and is the documented workaround for the npm-12 provenance/sigstore bug, so it is inherited rather than introduced here, but a compromised npm 11.x release would run with id-token: write and publish to @tangle-network/agent-bench. Pin to an exact version (e.g. npm@11.5.1) or a SHA, applied to both jobs. Non-blocking; the OIDC trusted-publisher constraint still scopes the credential to this repo/workflow/ref.

🟡 LOW publish-agent-bench has no workflow_dispatch recovery path (asymmetric with root publish-npm) — .github/workflows/publish.yml

publish-npm allows github.event_name == 'workflow_dispatch' as a manual republish path, but publish-agent-bench's if: startsWith(github.ref, 'refs/tags/agent-bench-v') excludes dispatch entirely. If a bench publish fails after the tag is cut and the run cannot be cleanly re-run, the only recovery is cutting a new agent-bench-v* tag. The comment states this is intentional ('Manual dispatch remains root-only'), so this is a design note rather than a bug — but confirm the team is comfortable with tag-only recovery for @tangle-network/agent-bench.

🟡 LOW verify-agent-bench tag version check has no guard for empty tag suffix — .github/workflows/publish.yml

The tag/version lock script strips the prefix agent-bench-v from GITHUB_REF without the outer [[ ... == ... ]] guard that the root verify job uses. If a tag literally named agent-bench-v (no numeric version) is pushed, the job-level if: startsWith(...) would still match, and the strip would produce an empty TAG_VERSION, which would fail the version comparison (correct), but with a confusing error message: Tag/version mismatch: tag= package=0.1.1. The root verify job guards the inner comparison with if [[ "${GITHUB_REF:-}" == refs/tags/v* ]], which would skip the check gracefully. The bench job should either add the same inner guard or add a clearer error message for the empty-tag case.

🟡 LOW Minor version bump without CHANGELOG — bench/package.json

version goes 0.1.0→0.1.1 but the diff adds no CHANGELOG/release note; verify a release-notes entry exists elsewhere or add one before publishing. Impact: minor traceability gap, not a correctness or runtime defect.

🟡 LOW devDep typescript ^6.0.3 diverges from root ^5.7.0 — bench/package.json

bench pins typescript ^6.0.3 while the workspace root pins ^5.7.0. Pre-existing (not introduced by this diff) so out of strict scope, but it is what the new typecheck:public script will run under, so confirm a TS 6.x install actually compiles src/index.ts before relying on the new gate.

🟡 LOW agent-runtime@0.90.1 trips pnpm minimumReleaseAge policy for ~24h after publish — bench/pnpm-lock.yaml

Reproduced locally: pnpm install --frozen-lockfile in bench/ exits non-zero with ERR_PNPM_MINIMUM_RELEASE_AGE_VIOLATION — @tangle-network/agent-runtime@0.90.1 was published at 2026-07-09T21:33:20.000Z, within the minimumReleaseAge cutoff (2026-07-08T22:30:34.022Z). The packages still install (44/44 fetched), so this is a policy gate, not a resolution failure. Impact: any bench CI step that runs pnpm install --frozen-lockfile will fail red for roughly the first 24 hours after this PR merges. Root cause is intentional — 0.90.1 is the release this PR ships. Fix options: (a) no action — wait out the 24h window before re-running bench CI; (b) if bench CI must be green immediately post-merge, exempt first-party @tangle-network/* packages from minReleaseAge in .pnpmfile.cjs or pnpm config.

🟡 LOW Single 120s budget for the whole suite and for a cold lockfile-less npm install — bench/scripts/run-package-tests.mjs

Both runners cap each execFile at timeout:120_000 (run-package-tests.mjs:27, verify-packed-consumer.mjs:14). In run-package-tests the single node --test invocation (line 49) must finish all ~16 collected test files (collectTests recurses src/ and benchmarks/, lines 11-19, 45) within 120s, with no --test-timeout set so a hung test waits for the execFile kill. In verify-packed-consumer the cold consumer npm install ([line 84](https://github.com/tangle-network/agent-runtime/blob/1acfa58db0b613df5e22d4363f15ec09955

🟡 LOW Single 120s timeout covers entire 16-file node --test batch — bench/scripts/run-package-tests.mjs

The timeout: 120_000 in run() applies to the single invocation node --test --import tsx <16 files> (line 51). All 16 test files share one timeout budget. On a cold/slow CI runner with tsx transpiling each file on the fly, this could time out and produce a generic error with no per-file breakdown. The current 16 files are deterministic and fast, so this is not failing today, but it is brittle as the suite grows. Low severity — acceptable now, worth bumping to 180-240s or splitting if test count grows.

🟡 LOW Misleading comment references a non-existent 'preceding frozen install' — bench/scripts/verify-packed-consumer.mjs

The comment 'The preceding frozen install + public typecheck cover the exact bench lockfile' (lines 79-80) refers to work this script does not perform. The consumer install on line 81 uses --package-lock=false and resolves semver ranges fresh from the registry — it is explicitly NOT a frozen install. The frozen install happens in CI (pnpm install --frozen-lockfile) before this script runs, but the comment reads as if this script's own install is frozen, which misleads maintainers editing this file. Fix: rew

🟡 LOW npm exec -- tsc/tsx may network-fetch or prompt if a bin is missing — bench/scripts/verify-packed-consumer.mjs

Lines 85-86 invoke npm exec -- tsc -p tsconfig.json and npm exec -- tsx index.ts. If the requested bin is not present in the consumer's node_modules, npm exec will attempt to fetch it from the registry (and in some npm versions issue an interactive yes/no prompt), which would hang until the 120s timeout and defeat the offline-determinism intent. typescript and tsx are declared devDependencies of the generated consumer (lines 56-60) so this is unlikely, but using npm exec --no -- tsc or calling `

🟡 LOW void main() discards promise; unhandled rejection on async assertion failure — bench/src/tb-container-executor.test.mts

The test uses void main() which discards the promise. If an assertion after an await throws, the error becomes an unhandled rejection rather than a synchronous test failure. node --test does catch unhandled rejections and report them as failures, so this is functional but less clean than structuring with describe/it. Pre-existing — unchanged in this PR.


tangletools · 2026-07-09T22:34:51Z · trace

@tangletools tangletools dismissed their stale review July 9, 2026 22:34

Superseded by re-review — no blocking findings on latest commit.

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approved — 18 non-blocking findings — 1acfa58d

Full multi-shot audit completed 6/6 planned shots over 9 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 6/6 planned shots over 9 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 6/6 planned shots over 9 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-07-09T22:34:51Z · immutable trace

@drewstone drewstone merged commit 255d6d4 into main Jul 9, 2026
2 checks passed
@drewstone drewstone deleted the chore/agent-bench-0.1.1 branch July 9, 2026 22:35
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.

2 participants