Skip to content

scorecard: consolidate to single scRun() call#807

Open
justaugustus wants to merge 3 commits intomainfrom
consolidate-check-execution
Open

scorecard: consolidate to single scRun() call#807
justaugustus wants to merge 3 commits intomainfrom
consolidate-check-execution

Conversation

@justaugustus
Copy link
Copy Markdown
Member

Summary

Consolidate the Scorecard policy's check execution from N+1 scRun() calls to a single batch call, eliminating redundant check execution when SARIF upload is enabled.

Motivation

When upload.sarif: true is configured, every Scorecard check runs N+1 times — N times individually in the per-check loop (for issue text), then once more as a batch in uploadSARIF() (for SARIF + results collection). This doubles the CPU, I/O, and API cost of every scan.

Investigation of Scorecard's Run() function confirms batch execution is safe: all checks run concurrently in separate goroutines, per-check errors are captured in CheckResult.Error (not as a top-level error), and Result.Checks always contains all results.

Changes

  • Check() now calls scRun() once with all valid checks, then iterates Result.Checks for issue text generation
  • uploadSARIF() renamed to uploadSARIFResult() — accepts an existing *sc.Result instead of re-running checks
  • Unknown check names filtered before Run() and skipped (previously broke the loop, losing subsequent checks)
  • Per-check errors skipped with a warning (previously broke the loop, losing subsequent checks)
  • Archived the completed evidence-upload OpenSpec proposal to openspec/changes/archive/

Net result: -41 lines of code, simpler control flow, better error resilience.

Behavioral changes

Behavior Before After
Check execution count N+1 1
Unknown check name Breaks loop, loses subsequent Filtered, remaining execute
Per-check error Breaks loop, loses subsequent Skipped, remaining produce results

Test plan

  • All 18 existing tests pass
  • go vet clean
  • go build clean

🤖 Generated with Claude Code

@justaugustus justaugustus requested a review from a team as a code owner March 31, 2026 01:17
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 31, 2026
@justaugustus justaugustus changed the title scorecard: consolidate to single scRun() call scorecard: consolidate to single scRun() call Mar 31, 2026
justaugustus and others added 3 commits April 6, 2026 00:30
Archive the completed evidence-upload proposal to
openspec/changes/archive/2026-03-28-evidence-upload/ per OpenSpec
convention.

Add a new proposal to consolidate the N+1 Scorecard check execution
into a single sc.Run() call. Currently when upload.sarif is enabled,
checks run N times individually (per-check loop) plus once as a batch
(SARIF generation), doubling the compute and API cost.

Investigation confirms Scorecard's Run() handles per-check errors
gracefully (captured in CheckResult.Error, not as top-level error),
making consolidation safe.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Stephen Augustus <foo@auggie.dev>
Replace the per-check loop (N sequential scRun() calls) plus the
separate SARIF batch scRun() call with a single batch scRun() call.
This eliminates N redundant check executions when upload.sarif is
enabled, reducing CPU, I/O, and API cost.

Key changes:
- Check() now calls scRun() once with all valid checks, then
  iterates Result.Checks for issue text generation
- uploadSARIF() renamed to uploadSARIFResult() and accepts an
  existing *sc.Result instead of re-running checks
- Unknown check names are filtered before Run() and skipped with
  a warning (previously broke the loop, losing subsequent checks)
- Per-check errors are skipped with a warning (previously broke
  the loop, losing subsequent checks)

This is safe because Scorecard's Run() executes all checks
concurrently in separate goroutines and captures per-check errors
in CheckResult.Error rather than aborting the entire run.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Stephen Augustus <foo@auggie.dev>
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Stephen Augustus <foo@auggie.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant