sec (4/9): scope benchmark requirement preflight to runnable scanners

[jesse-merhi]

What changes

Benchmark requirement preflight now only considers scanners that can
actually run on the benchmark's target kind. A scanner that does not support the
target kind (e.g. a plugin-only scanner during a skill benchmark) no longer has
its required env vars demanded before the run starts.

Why it matters

Preflight fails fast when a required credential is missing — good. But it was
checking every configured scanner, including ones that would be skipped anyway
because they don't support the target kind. That meant a benchmark could abort
demanding, say, a token for a scanner that was never going to execute. Scoping
preflight to the runnable set makes the "fail before partial work" check match
the work that will actually happen.

Before / after

	Behavior
Before	preflight requires env for all configured scanners, including unrunnable ones — benchmark aborts on a credential it will never use
After	preflight requires env only for scanners that support the benchmark target kind

New helper runnableBenchmarkScanners(opts, kind) filters the scanner set via
the registry's SupportsTargetKind before requirement resolution; RunBenchmark
feeds that filtered set into preflight.

Verify

go test ./internal/runner/ -run 'RunnableBenchmarkScanners' -count=1

TestRunnableBenchmarkScannersFiltersUnsupportedTargetKind asserts a scanner
that does not support the target kind is dropped from the preflight set.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed July 25, 2026, 10:38 PM ET / July 26, 2026, 02:38 UTC.

ClawSweeper review

What this changes

The branch limits benchmark environment-variable preflight to scanners that support skill targets, so a configured scanner that would be skipped does not block the benchmark for missing unused credentials.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep this PR open for normal maintainer review. It is a fresh, clean, narrowly scoped correction to the merged user-defined-scanner work, and the author is a repository member, so conservative cleanup must not auto-close it.

Priority: P2
Reviewed head: 33a40136b4e6aace0714a4b2fcb27d1413cb3656

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A small, coherent patch with targeted regression coverage and no identified correctness or security defect.
Proof confidence	🌊 off-meta tidepool	Not applicable: This repository-member PR changes internal Go benchmark selection; the external-contributor real-behavior-proof gate does not apply, though focused test output would still strengthen merge confidence.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Not applicable	Not applicable: This repository-member PR changes internal Go benchmark selection; the external-contributor real-behavior-proof gate does not apply, though focused test output would still strengthen merge confidence.
Evidence reviewed	4 items	Target-kind filtering change: The proposed helper keeps only requested scanner IDs whose registry adapter supports the benchmark target kind, and RunBenchmark passes that filtered list to requirement validation. Focused regression coverage: The added unit test creates one skill-only and one URL-only user-defined scanner, then verifies that only the skill scanner remains eligible for a skill benchmark preflight. Feature provenance: The merged BYOS change introduced registry-backed user-defined scanners with declared target kinds; this PR is a focused follow-up on that established execution path.
Findings	None	None.
Security	None	None.

How this fits together

ClawScan benchmarks load skill targets, select configured scanner adapters, validate their declared environment requirements, then execute the runnable scanners and write raw evidence into a benchmark artifact. This change aligns the early credential check with the target-kind filtering already used to decide which scanners can run.

flowchart LR
  A[Benchmark configuration] --> B[Requested scanners]
  C[Skill benchmark targets] --> D[Runnable scanner selection]
  B --> D
  D --> E[Credential preflight]
  E --> F[Scanner execution]
  F --> G[Benchmark artifact]

Loading

Before merge

• Resolve merge risk (P1) - Before merge, confirm the focused test and the repository-required Go suite and vet checks on the current head; the supplied PR text describes the focused test but does not include captured runtime output.
• Complete next step (P2) - This is an active, clean member-authored implementation PR; the remaining action is ordinary maintainer validation and merge review, not an automated repair.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch scope	2 files affected; 38 added, 1 removed	The branch is narrowly focused on benchmark requirement selection plus one targeted unit test.
Target filtering	2 scanner kinds exercised	The regression test distinguishes a skill-capable scanner from a URL-only scanner, the behavior that caused the unnecessary credential preflight.

Merge-risk options

Maintainer options:

1. Decide the mitigation before merge
   Land the focused filtering change after the target-kind regression test and standard Go validation confirm that credential preflight still rejects every scanner that will actually execute.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land the focused filtering change after the target-kind regression test and standard Go validation confirm that credential preflight still rejects every scanner that will actually execute.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: configure a skill benchmark with both a skill-capable scanner and a URL-only scanner whose required environment variable is absent; current preflight validates the URL-only scanner even though target-kind selection would skip it.

Is this the best way to solve the issue?

Yes. Filtering the scanner IDs passed into requirement validation by the same registry target-kind capability used for execution is the narrowest maintainable repair and preserves fail-fast validation for scanners that can run.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5cfa90b129f4.

Labels

Label changes:

• add P2: This is a bounded benchmark-runner correctness fix that can block configured scans but has limited blast radius.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This repository-member PR changes internal Go benchmark selection; the external-contributor real-behavior-proof gate does not apply, though focused test output would still strengthen merge confidence.

Label justifications:

• P2: This is a bounded benchmark-runner correctness fix that can block configured scans but has limited blast radius.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This repository-member PR changes internal Go benchmark selection; the external-contributor real-behavior-proof gate does not apply, though focused test output would still strengthen merge confidence.

Evidence

What I checked:

• Target-kind filtering change: The proposed helper keeps only requested scanner IDs whose registry adapter supports the benchmark target kind, and RunBenchmark passes that filtered list to requirement validation. (internal/runner/benchmark.go:194, 33a40136b4e6)
• Focused regression coverage: The added unit test creates one skill-only and one URL-only user-defined scanner, then verifies that only the skill scanner remains eligible for a skill benchmark preflight. (internal/runner/runner_test.go:853, 33a40136b4e6)
• Feature provenance: The merged BYOS change introduced registry-backed user-defined scanners with declared target kinds; this PR is a focused follow-up on that established execution path. (internal/runner/user_defined_scanner.go, 656e558b4ba5)
• Current PR state: GitHub context reports a clean, mergeable two-file branch against current main, with the latest head updated on 2026-07-26. (33a40136b4e6)

Likely related people:

• jesse-merhi: Authored the merged user-defined scanner capability that established scanner target declarations and submitted this narrow follow-up for benchmark preflight behavior. (role: feature owner and recent area contributor; confidence: high; commits: 656e558b4ba5, 33a40136b4e6; files: internal/runner/benchmark.go, internal/runner/runner_test.go, internal/runner/user_defined_scanner.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Run and report go test -count=1 ./... and go vet ./... on the current head before merging.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.