Walkthrough

Three new stress benchmark files add data preparation and push scripts plus a Go benchmark that loads the archive, starts a registry, and runs concurrent validate image executions. The benchmark README also lists the new stress benchmark and its environment variables.

Changes

Stress benchmark infrastructure

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

❌ Failed checks (1 warning)

Review

Findings

Medium

• [stale-reference] benchmark/stress/stress.go:97 — The git source URL for golden-container uses the old organization name enterprise-contract (https://github.com/enterprise-contract/golden-container.git) while the rest of the codebase (simple benchmark, hack scripts) has migrated to https://github.com/conforma/golden-container. The git revision 8327c1ce7472b017b9396fe26d5d5e1ed0eb61cc also differs from the simple benchmark's 2dec8f515a64ef2f21ee3e7b1ed41da77a5c5a9a, suggesting it may reference a commit in the old repo that could become unavailable if the old repo is archived.
  Remediation: Use https://github.com/conforma/golden-container to match the existing simple benchmark pattern, and verify the revision hash exists in the conforma fork.

Low

• [edge-case] benchmark/stress/stress.go:42 — The envInt function panics on values < 1 but does not guard against unreasonably large values. For EC_STRESS_COMPONENTS, an extremely large value would cause buildSnapshot to allocate a massive slice, likely causing an OOM before the benchmark runs. Minor robustness concern since this is a developer tool.

• [incomplete-doc] benchmark/README.md — The benchmark README describes benchmarks generically but doesn't mention the new stress benchmark, its env vars (EC_STRESS_COMPONENTS, EC_STRESS_WORKERS), or its distinct purpose of simulating multi-component workloads.

Info

• [pattern-violation] benchmark/stress/stress.go:119 — The policy JSON string in the stress benchmark is well-formed JSON (no trailing commas), while the simple benchmark's policy string contains trailing commas (invalid JSON). The stress benchmark is more correct here, but the inconsistency between benchmarks is notable.

Review

Findings

Low

• [resource-leak] benchmark/stress/stress.go:68 — In setup(), if registry.Launch() fails, the temporary directory created by untar.UnTar() is never cleaned up. The panic exits without removing the temp dir. This matches the existing pattern in benchmark/simple/simple.go and the OS reclaims the directory on process exit, so practical risk is minimal.
  Remediation: Call os.RemoveAll(dir) before panicking on registry.Launch failure, or defer cleanup unconditionally.

• [missing-input-validation] benchmark/stress/stress.go:52 — envInt accepts zero and negative values for EC_STRESS_COMPONENTS and EC_STRESS_WORKERS. Setting these to zero or negative values could produce confusing benchmark results.
  Remediation: Add a check that the returned value is at least 1.

Info

• [sub-agent-failure] The style-conventions, intent-coherence, and docs-currency sub-agents did not return findings: model claude-sonnet-4-5@20250929 not available on the deployment. These are sonnet-tier dimensions; correctness (opus) completed successfully.

Review

Findings

Low

• [edge-case] benchmark/stress/stress.go:56 — envInt does not validate that the returned integer is positive. Setting EC_STRESS_COMPONENTS=0 produces a snapshot with zero components, and EC_STRESS_WORKERS=0 or a negative value is passed directly to --workers. These degenerate inputs silently produce meaningless benchmark results rather than failing fast. Consider adding a lower-bound check (e.g., if n < 1 { panic(...) }).

Info

• [design-direction] benchmark/stress/ — The stress benchmark is well-placed in the benchmark infrastructure alongside the existing simple benchmark. Both use golang.org/x/benchmarks/driver for performance measurement. If the goal evolves toward OOM reproduction/regression testing specifically, acceptance tests could complement this benchmark.
• [sub-agent-gap] The style-conventions sub-agent could not access PR branch files. Manual inspection confirms the stress benchmark follows the established patterns from benchmark/simple/: identical setup()/Closer lifecycle, same driver.Parallel(n, 1, fn) shape, consistent license headers, and proper reuse of benchmark/internal/ packages.

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.