Repository navigation
fix: reduce Super Linter fixture noise and preserve failure diagnostics - #68733
Conversation
Exclude intentional analyzer fixtures and the dependency-only workflow manifest without changing production linter checks. Preserve failed linter outcomes for bounded grouped analysis and add scope regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully!
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Design Decision Gate 🏗️Outcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
L60-74: delete: brittle source/lock text-presence mirror for unrelated reporting behavior. Keep the path-matching checks above; workflow compilation and drift checks cover generated lock consistency.
net: -15 lines possible.
Generated by ✂️ Ponytail Reviewer for #68733 · codex · gpt56 · 8.67 AIC · ⌖ 5.3 AIC · ⊞ 14.2K
Comment /ponytail to run again
| with self.subTest(path=prefix + path): | ||
| self.assertFalse(self.excluded(prefix + path)) | ||
|
|
||
| def test_generated_lock_preserves_scope_and_reports_failures(self): |
There was a problem hiding this comment.
L60-74: delete: brittle source/lock text-presence mirror for unrelated reporting behavior. Keep the path-matching checks above; workflow compilation and drift checks cover generated lock consistency.
Comment MemoryPeek at saved memory (pr-code-quality-reviewer)Note This comment is managed by comment memory.Expand the saved memory block to view or edit the persistent context for this thread.
|
There was a problem hiding this comment.
Verdict
I did not find a changed-line bug here that clearly warrants blocking the PR, but I left one maintainability comment on the new regression test.
The main risk area I saw
The workflow changes themselves look directionally sound: they preserve the failing super-linter outcome for later analysis, keep artifact retention for diagnostics, and add regression coverage around the new exclusion scope. The only issue I found worth surfacing is that part of the Python regression test now locks the workflow prompt to exact prose, which will create avoidable test churn on harmless copy edits.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 34.6 AIC · ⌖ 5.82 AIC · ⊞ 21.3K
Comment /review to run again
| self.assertIn("if-no-files-found: error", text) | ||
| self.assertNotIn("GO_CONFIG_FILE:", self.source) | ||
| self.assertNotIn("LINTER_RULES_PATH:", self.source) | ||
| self.assertIn("twenty examples total", self.source) |
There was a problem hiding this comment.
This regression test is overfitted to prompt prose, so a harmless wording cleanup will start failing make test-scripts and block unrelated workflow changes.
💡 Why this is risky and how to tighten it
Right now the assertions on "twenty examples total" and "does not establish an exploitable vulnerability" are checking editorial phrasing rather than the scope/reporting behavior this test is supposed to protect. That makes the test noisy to maintain and encourages cargo-cult prompt wording instead of letting us revise instructions safely.
A safer pattern is to keep the structural checks here (FILTER_REGEX_EXCLUDE, continue-on-error, result artifact path, missing-log failure) and drop the exact English sentence matches, or replace them with a smaller semantic anchor that is unlikely to change during prompt edits.
self.assertNotIn("[Include the full linter output here]", self.source)
# Keep behavior-focused assertions only; avoid exact prompt prose checks.That still guards the workflow contract without turning copy edits into failing tests.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs (per pr-triage) — this is a focused, well-tested bug fix with a clear regression test. Approving.
📋 Verification performed
- Ran
python3 scripts/test_super_linter_scope.pyagainst the PR commit in a clean checkout: all 3 tests pass. - Ran
./gh-aw compile .github/workflows/super-linter.mdand confirmed zero drift insuper-linter.lock.yml— the lock faithfully reflects the newFILTER_REGEX_EXCLUDE,continue-on-error: true,LINTER_OUTCOMEenv, and artifact path changes. - Confirmed
continue-on-error: trueon an external action step is an established pattern elsewhere in the codebase (checkout_step_generator.go,cache_memory.go), so this isn't a novel risk. - Confirmed the downstream
agentjob still correctly depends onneeds: [activation, super_linter], socontinue-on-erroronly prevents the job from short-circuiting, not from gating the agent step — findings will reach the reporting agent as intended. - Confirmed the root-cause diagnosis in the PR description (pinned Super Linter queues each Go file's containing directory for
GO_MODULES, so testdata fixtures get swept in) is correctly addressed at the regex-exclusion layer, not just patched around symptoms. scripts/test_super_linter_scope.pyhas solid coverage: positive/negative boundary cases for testdata exclusion, real source inclusion (including_test.gofiles and files merely namedtestdata_helpers.go), and a generated-lock assertion that catches drift between the.mdsource and.lock.yml.
Positive Highlights
- ✅ Root cause addressed at the source (file-path filtering before directory queueing), not a special-case/workaround.
- ✅ New regression test is registered in
make test-scripts, so it runs in CI rather than being a one-off manual check. - ✅
check-resultsstep now distinguishes "no log produced" (hard error) from "linter step failed but log exists" (warning + continued analysis) — this is a meaningful improvement to failure-mode clarity.
No actionable issues found in the changed lines.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 109.7 AIC · ⌖ 14.4 AIC · ⊞ 10.3K
Comment /matt to run again
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open findings
What changed in this PR
Reduces Super Linter noise by excluding intentional fixture inputs and preserving clearer failure diagnostics, while adding regression tests to prevent future scope drift.
Changes:
- Excludes
pkg/linters/*/testdata/and.github/workflows/go.modfrom GO_MODULES directory queueing viaFILTER_REGEX_EXCLUDE. - Preserves Super Linter step outcome for downstream reporting by continuing on error and emitting a result file alongside the log artifact.
- Adds Python regression tests for scope + generated lock expectations and wires them into
make test-scripts.
| File | Description |
|---|---|
| scripts/test_super_linter_scope.py | Adds regression tests for exclusion scope and for generated lock invariants (outcome/result file, bounded reporting instructions). |
| Makefile | Registers the new Python test under test-scripts. |
| .github/workflows/super-linter.md | Updates Super Linter config to reduce fixture noise, preserve failure diagnostics, and bound reporting output. |
| .github/workflows/super-linter.lock.yml | Regenerates the pinned/compiled workflow lock to reflect the updated workflow behavior. |
🧠 Review effort: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| printf 'super-linter outcome: %s\n' "$LINTER_OUTCOME" > /tmp/super-linter-result.txt | ||
| if [ ! -s "super-linter.log" ]; then | ||
| echo "::error::Super-linter did not produce a nonempty log; analysis is unavailable." | ||
| exit 1 | ||
| fi |
| path: | | ||
| /tmp/super-linter.log | ||
| /tmp/super-linter-result.txt | ||
| if-no-files-found: error |
| self.assertGreater(len(fixtures), 0) | ||
| for fixture in fixtures: | ||
| self.assertTrue(self.excluded(str(fixture.relative_to(ROOT))), fixture) | ||
| self.assertEqual(list((ROOT / ".github/workflows").rglob("*.go")), []) |
Summary
pkg/linters/*/testdata/inputs and the dependency-only.github/workflows/go.modbefore Super Linter queues GO_MODULES package directories. Keep analyzer implementations, normal tests, root module, real source, and the existing production linter checks enabled.make test-scripts.Evidence
Scheduled run 38576278267 had 2,405 diagnostic-shaped lines, including 1,016 intentional fixture lines. Exact-line deduplication leaves 1,041 nonfixture diagnostics, including four production gosec G115 conversions. These are old-run observations, not claims of exploitable vulnerabilities or results of a rerun. No production source findings were mass-fixed or suppressed; unknown custom nolint warnings remain explicitly reportable.
The pinned Super Linter implementation queues each selected Go file's containing directory, so filtering only module manifests or relying on Go's usual recursive testdata exclusion is insufficient. The new filter operates before directory queueing.
Validation
python3 scripts/test_super_linter_scope.py: three tests pass, including all existing analyzer fixture trees and negative boundary cases.make recompile: 344/344 workflows compile; only the affected generated lock is changed../gh-aw compile super-linter --validate --shellcheck --actionlint: passes with zero warnings and clean actionlint.make agent-report-progress: passes schema/source-lock freshness and full workflow drift. Impacted Go tests are skipped because no Go sources changed.Remaining limitations
The scanner-enabled compile-only dry-run emitted a private diagnostic lock but failed on unrelated diagnostic compiler output: an empty top-level env and five dangling daily-credit activation outputs; zizmor also flags existing framework secrets outside dedicated environments. Shellcheck and poutine passed. The diagnostic lock is not published and that failed gate is not claimed passed. The normal generated lock passes actionlint.
No workflow dispatch or rerun was attempted. Raw logs are not committed.