[pr-review] Daily PR Code Quality Review — 33841572308

[github-actions]

Summary

PR	Top Issues	Signal
#58385 — Fix failing lock-drift job	0 (lock.yml only)	🟢
#58384 — Install AWF for firewall-enabled custom engine commands	0	🟢
#58383 — Require AWF v0.28.13 for GitHub CLI proxy mode	0	🟢
#58310 — Pin threat-detect artifacts to independent SHA-256 digests	0	🟢
#58120 — repo-memory: filter disallowed files before validation/upload	0	🟢

Overall signal: 🟢 — no significant code-quality issues found across today's 5 open PRs. All changed Go/JS/shell functions carry doc comments or clarifying comments, new tests include real assertions (not bare logging), and no function bodies exceed ~80 lines.

Full Findings

#58385 — Fix failing lock-drift job

• Diff is limited to regenerated .lock.yml files (no hand-written source changes) — nothing to review.

#58384 — Install AWF for firewall-enabled custom engine commands

• Small, focused change across claude_engine.go, codex_engine.go, gemini_engine.go, pi_engine.go, behavior_defined_engine.go: replaces early-return with a call to BuildNpmEngineInstallStepsWithAWF.
• New TestCustomEngineCommandFirewallInstallation table-driven test covers all 6 engines and asserts both presence of AWF install step and absence of the engine's own package install — good coverage.
• No missing error handling; functions are short; no exported functions added without doc comments.

#58383 — Require AWF v0.28.13 for GitHub CLI proxy mode

• New validateGitHubCLIProxyVersion function has a clear doc-adjacent comment via its call site and correctly returns descriptive errors including current/required versions.
• New test TestValidateGitHubCLIProxyVersion asserts both the error and success paths with require/assert, not just logging.
• Minor: the new function itself lacks a leading // doc comment (it's unexported, so not required by convention, but would improve readability given it encodes a version-gating policy).

#58310 — Pin threat-detect artifacts to independent SHA-256 digests

• Solid security-hardening of install_threat_detect_binary.sh: replaces trust-on-first-use checksums.txt download with SHA-256 digests pinned in the action itself, validated with a regex before use.
• Argument parsing was tightened (unknown flags now fail instead of warning) — appropriate given the new required flags.
• Test suite (install_threat_detect_binary_test.sh) was significantly extended (4 new cases: mismatch, missing digest, pinned-version fallback, installer/compiler version parity) — all use explicit pass/fail assertions, not bare logging.
• No Go changes in this PR, so Go-specific checks (error handling, doc comments) don't apply.

#58120 — repo-memory: filter disallowed files before validation/upload

• Clean extract-function refactor of generateRepoMemoryArtifactUpload in repo_memory.go into four smaller helpers (generateRepoMemorySanitizeFilenamesStep, generateRepoMemoryFilterFilesStep, generateRepoMemoryCustomValidationStep, generateRepoMemoryUploadArtifactStep), each under 40 lines and carrying a doc comment explaining behavior and gating logic.
• Introduces repoMemoryUploadStepParams struct to keep parameter count within the repo's lint limit — good practice.
• Similar refactor pattern in safe_outputs_config_generation.go (addStandardHandlerConfigs, addSafeJobsConfig, etc.) — extracted helpers stay well under 80 lines and are documented.
• New/updated Go tests (repo_memory_test.go, safe_outputs_config_generation_test.go, drive_memory_test.go) use require/assert throughout, no bare t.Log-only tests found.
• No missing err != nil handling identified in the reviewed hunks.

Generated by 🖱️ Daily PR Code Quality Review · copilot · auto · 44.3 AIC · ⌖ 6.5 AIC · ⊞ 6.6K · ◷

• expires on Sep 4, 2026, 9:51 PM UTC-08:00