Skip to content

[pr-review] Daily PR Code Quality Review β€” 33947957003Β #58744

Description

@github-actions

Summary

PR Author Top Issues Quality Signal
#58717 Fix static-analysis scan pipeline short-circuiting Copilot 0 🟒
#58715 Fix cacheRecoveryError %v-not-%w bug Copilot 0 🟒
#58310 Pin threat-detect artifacts to SHA-256 digests Copilot 0 🟒
#58120 repo-memory: filter disallowed files pre-upload Copilot 0 🟒
#57670 Bump sharp 0.35.3 β†’ 0.35.4 (dependabot) dependabot[bot] 0 🟒

Overall daily signal: 🟒 (all reviewed PRs ≀1 issue/PR)

Full Findings

#58717 β€” Fix static-analysis scan pipeline short-circuiting

  • Refactors the 9-scanner sequential pipeline into a shared runBatchExternalTools helper with package-level function-var indirection (runBatchActionlintOnFiles, etc.) purely for testability β€” no exported API changes lack doc comments.
  • Adds a strong regression test (TestRunBatchExternalToolsExecutesSequentialToolsWithoutEarlyAborting) that asserts every scanner still runs after an early failure and that the first error is preserved β€” good coverage of the exact bug being fixed.
  • Adds an explicit .lock.yml/.md "Assert static analysis output completeness" step with per-tool markers to catch silent no-op tool runs in CI.
  • No missing error handling observed; ctx.Err() checks preserved per-tool.
  • Minor: runBatchExternalTools/orchestration functions carry (nolint/redacted):largefunc (pre-existing convention), so oversized-function linting is already acknowledged by the codebase.

#58715 β€” Fix cacheRecoveryError's %v-not-%w bug

  • Core fix is correct: cacheRecoveryError now uses %w to wrap the causal error, verified by a new test asserting errors.Is/errors.Unwrap succeed.
  • Extends errorfwrapv and fmterrorfnoverbs linters with a new astutil.ResolveFormatString helper (has a doc comment) to detect concatenated format-string mistakes; conservatively returns ok=false for opaque (non-literal) operands to avoid false positives.
  • New linter logic has both positive and negative testdata cases (BadConcatVWrap, GoodConcatWWrap, OpaqueConcatVWrap) plus a dedicated TestResolveFormatString table test β€” solid coverage.
  • No oversized functions or missing error handling identified.

#58310 β€” Pin threat-detect artifacts to independent SHA-256 digests

  • Replaces the previously downloaded, unauthenticated checksums.txt with digests pinned inside the immutable setup action (PINNED_THREAT_DETECT_SHA256_AMD64/ARM64), closing a supply-chain gap (checksum sourced from the same untrusted release as the binary).
  • Adds strict digest-format validation (^[[:xdigit:]]{64}$) and fails closed before download when digests are missing for a non-pinned version β€” good defensive posture.
  • Test suite extended with mismatch/missing-digest/pinned-version cases (Tests 9–12), including a test that cross-checks the installer's pinned version literal against pkg/constants/version_constants.go to prevent drift.
  • Shell script: uses set -euo pipefail, quotes variables consistently; no obvious injection issues.

#58120 β€” repo-memory: filter disallowed files before validation/upload

  • Refactors a large inline loop in generateRepoMemoryArtifactUpload into several small, well-named, doc-commented helper functions (generateRepoMemorySanitizeFilenamesStep, generateRepoMemoryFilterFilesStep, generateRepoMemoryCustomValidationStep, generateRepoMemoryUploadArtifactStep), each under ~30 lines β€” good decomposition, avoids an oversized function.
  • Introduces repoMemoryUploadStepParams struct specifically to stay under the repo's parameter-count lint limit β€” noted directly in the code comment.
  • Fixes a "fail-open" gap: the new filter step's outcome now gates both the custom-validation step and the upload step, with 3 new tests (TestRepoMemoryFilterStepGatesUpload, TestRepoMemoryFilterStepEmptyBothFieldsSkipsFilter, TestRepoMemoryNoFilterStepWhenNoFilterConfigured) verifying step ordering and gating conditions.
  • No missing error handling or bare t.Log-only tests found in the diff.

#57670 β€” Bump sharp from 0.35.3 to 0.35.4 in /docs

  • Dependabot-only package-lock.json update; no application code changed. No quality issues to review (lockfile diff only).

Generated by πŸ–±οΈ Daily PR Code Quality Review Β· copilot Β· auto Β· 41.7 AIC Β· βŒ– 6.78 AIC Β· ⊞ 6.6K Β· β—·

  • expires on Sep 5, 2026, 9:49 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions